Skip to content

Commit

Permalink
PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
JamesNK committed Apr 2, 2024
1 parent 70af53b commit 06d5f2f
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 28 deletions.
2 changes: 1 addition & 1 deletion src/Aspire.Dashboard/Components/Pages/Login.razor.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export async function validateToken(token) {
try {
var url = `/api/validate-token?token=${encodeURIComponent(token)}`;
var url = `/api/validatetoken?token=${encodeURIComponent(token)}`;
var response = await fetch(url, { method: 'POST' });
return response.text();
} catch (ex) {
Expand Down
11 changes: 6 additions & 5 deletions src/Aspire.Dashboard/DashboardWebApplication.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
using Aspire.Dashboard.Otlp.Grpc;
using Aspire.Dashboard.Otlp.Storage;
using Aspire.Hosting;
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Authentication.Certificate;
using Microsoft.AspNetCore.Authentication.Cookies;
using Microsoft.AspNetCore.Authentication.OpenIdConnect;
Expand Down Expand Up @@ -235,16 +234,18 @@ public DashboardWebApplication(Action<WebApplicationBuilder>? configureBuilder =

if (dashboardOptions.Frontend.AuthMode == FrontendAuthMode.BrowserToken)
{
_app.MapPost("/api/validate-token", async (string token, HttpContext httpContext, IOptionsMonitor<DashboardOptions> dashboardOptions) =>
_app.MapPost("/api/validatetoken", async (string token, HttpContext httpContext, IOptionsMonitor<DashboardOptions> dashboardOptions) =>
{
return await ValidateTokenMiddleware.TryAuthenticateAsync(token, httpContext, dashboardOptions).ConfigureAwait(false);
});

#if DEBUG
// Temporary for testing.
_app.MapGet("/sign-out", async (HttpContext httpContext) =>
// Available in local debug for testing.
_app.MapGet("/api/signout", async (HttpContext httpContext) =>
{
await httpContext.SignOutAsync(CookieAuthenticationDefaults.AuthenticationScheme).ConfigureAwait(false);
await Microsoft.AspNetCore.Authentication.AuthenticationHttpContextExtensions.SignOutAsync(
httpContext,
CookieAuthenticationDefaults.AuthenticationScheme).ConfigureAwait(false);
httpContext.Response.Redirect("/");
});
#endif
Expand Down
29 changes: 8 additions & 21 deletions src/Aspire.Dashboard/Utils/CompareHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Buffers;
using System.Diagnostics;
using System.Security.Cryptography;
using System.Text;

Expand All @@ -16,24 +17,19 @@ public static bool CompareKey(byte[] expectedKeyBytes, string requestKey)

var requestByteCount = Encoding.UTF8.GetByteCount(requestKey);

// Key will never match if lengths are different. But still do all the comparison work to avoid timing attacks.
var lengthsEqual = expectedKeyBytes.Length == requestByteCount;

var requestSpanLength = Math.Max(requestByteCount, expectedKeyBytes.Length);
// A rented array could have previous data. However, we're trimming it to the exact byte count we need.
// That means all used bytes are overwritten by the following Encoding.GetBytes call.
byte[]? requestPooled = null;
var requestBytesSpan = (requestSpanLength <= StackAllocThreshold ?
var requestBytesSpan = (requestByteCount <= StackAllocThreshold ?
stackalloc byte[StackAllocThreshold] :
(requestPooled = RentClearedArray(requestSpanLength))).Slice(0, requestSpanLength);
(requestPooled = ArrayPool<byte>.Shared.Rent(requestByteCount))).Slice(0, requestByteCount);

try
{
// Always succeeds because the byte span is always as big or bigger than required.
Encoding.UTF8.GetBytes(requestKey, requestBytesSpan);

// Trim request bytes to the same length as expected bytes. Need to be the same size for fixed time comparison.
var equals = CryptographicOperations.FixedTimeEquals(expectedKeyBytes, requestBytesSpan.Slice(0, expectedKeyBytes.Length));
var encodedByteCount = Encoding.UTF8.GetBytes(requestKey, requestBytesSpan);
Debug.Assert(encodedByteCount == requestBytesSpan.Length, "Should match because span was previously trimmed to byte count value.");

return equals && lengthsEqual;
return CryptographicOperations.FixedTimeEquals(expectedKeyBytes, requestBytesSpan);
}
finally
{
Expand All @@ -42,14 +38,5 @@ public static bool CompareKey(byte[] expectedKeyBytes, string requestKey)
ArrayPool<byte>.Shared.Return(requestPooled);
}
}

static byte[] RentClearedArray(int byteCount)
{
// UTF8 bytes are copied into the array but remaining bytes are untouched.
// Because all bytes in the array are compared, clear the array to avoid comparing previous data.
var array = ArrayPool<byte>.Shared.Rent(byteCount);
Array.Clear(array);
return array;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public async Task Post_ValidateTokenApi_AvailableBasedOnOptions(FrontendAuthMode
using var client = new HttpClient { BaseAddress = new Uri($"http://{app.FrontendEndPointAccessor().EndPoint}") };

// Act
var response = await client.PostAsync("/api/validate-token?token=" + requestToken, content: null);
var response = await client.PostAsync("/api/validatetoken?token=" + requestToken, content: null);

// Assert
Assert.Equal(statusCode, response.StatusCode);
Expand Down

0 comments on commit 06d5f2f

Please sign in to comment.