Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Api/Auth/Controllers/WebAuthnController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ private async Task ValidateIfUserCanUsePasskeyLogin(Guid userId)
public async Task UpdateCredential([FromBody] WebAuthnLoginCredentialUpdateRequestModel model)
{
var tokenable = _assertionOptionsDataProtector.Unprotect(model.Token);
if (!tokenable.TokenIsValid(WebAuthnLoginAssertionOptionsScope.UpdateKeySet))
if (!tokenable.TokenIsValid(WebAuthnLoginAssertionOptionsScope.UpdateKeySet) || tokenable.Options == null)
{
throw new BadRequestException("The token associated with your request is invalid or has expired. A valid token is required to continue.");
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
// FIXME: Update this file to be null safe and then delete the line below
#nullable disable

using System.Text.Json.Serialization;
using System.Text.Json.Serialization;
using Bit.Core.Auth.Enums;
using Bit.Core.Tokens;
using Fido2NetLib;
Expand All @@ -14,19 +11,19 @@ public class WebAuthnLoginAssertionOptionsTokenable : ExpiringTokenable
// - 6 Minutes for Attestation (max webauthn timeout)
// - 6 Minutes for PRF Assertion (max webauthn timeout)
// - 5 minutes for user to complete the process (name their passkey, etc)
private static readonly TimeSpan _tokenLifetime = TimeSpan.FromMinutes(17);
public static readonly TimeSpan TokenLifetime = TimeSpan.FromMinutes(17);
public const string ClearTextPrefix = "BWWebAuthnLoginAssertionOptions_";
public const string DataProtectorPurpose = "WebAuthnLoginAssertionOptionsDataProtector";
public const string TokenIdentifier = "WebAuthnLoginAssertionOptionsToken";

public string Identifier { get; set; } = TokenIdentifier;
public AssertionOptions Options { get; set; }
public AssertionOptions? Options { get; set; }
public WebAuthnLoginAssertionOptionsScope Scope { get; set; }

[JsonConstructor]
public WebAuthnLoginAssertionOptionsTokenable()
{
ExpirationDate = DateTime.UtcNow.Add(_tokenLifetime);
ExpirationDate = DateTime.UtcNow.Add(TokenLifetime);
}

public WebAuthnLoginAssertionOptionsTokenable(WebAuthnLoginAssertionOptionsScope scope, AssertionOptions options) : this()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ private static void AddWebAuthnLoginCommands(this IServiceCollection services)
services.AddScoped<ICreateWebAuthnLoginCredentialCommand, CreateWebAuthnLoginCredentialCommand>();
services.AddScoped<IGetWebAuthnLoginCredentialAssertionOptionsCommand, GetWebAuthnLoginCredentialAssertionOptionsCommand>();
services.AddScoped<IAssertWebAuthnLoginCredentialCommand, AssertWebAuthnLoginCredentialCommand>();
services.AddScoped<IWebAuthnChallengeCacheProvider, WebAuthnChallengeCacheProvider>();
}

private static void AddTwoFactorCommandsQueries(this IServiceCollection services)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
namespace Bit.Core.Auth.UserFeatures.WebAuthnLogin;

/// <summary>
/// Enforces single-use semantics for WebAuthn assertion challenges per the W3C WebAuthn
/// specification (§13.4.3). Tracks used challenges so that each challenge can only be
/// validated once.
/// </summary>
public interface IWebAuthnChallengeCacheProvider
{
/// <summary>
/// Attempts to mark a challenge as used. Returns <c>true</c> if this is the first use
/// (challenge was not in cache and has now been saved). Returns <c>false</c> if the
/// challenge was already marked as used (found in cache).
/// </summary>
/// <param name="challenge">The challenge bytes from <see cref="Fido2NetLib.AssertionOptions.Challenge"/>.</param>
Task<bool> TryMarkChallengeAsUsedAsync(byte[] challenge);
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,27 @@ internal class AssertWebAuthnLoginCredentialCommand : IAssertWebAuthnLoginCreden
private readonly IFido2 _fido2;
private readonly IWebAuthnCredentialRepository _webAuthnCredentialRepository;
private readonly IUserRepository _userRepository;
private readonly IWebAuthnChallengeCacheProvider _webAuthnChallengeCache;

public AssertWebAuthnLoginCredentialCommand(IFido2 fido2, IWebAuthnCredentialRepository webAuthnCredentialRepository, IUserRepository userRepository)
public AssertWebAuthnLoginCredentialCommand(
IFido2 fido2,
IWebAuthnCredentialRepository webAuthnCredentialRepository,
IUserRepository userRepository,
IWebAuthnChallengeCacheProvider webAuthnChallengeCache)
{
_fido2 = fido2;
_webAuthnCredentialRepository = webAuthnCredentialRepository;
_userRepository = userRepository;
_webAuthnChallengeCache = webAuthnChallengeCache;
}

public async Task<(User, WebAuthnCredential)> AssertWebAuthnLoginCredential(AssertionOptions options, AuthenticatorAssertionRawResponse assertionResponse)
{
if (!await _webAuthnChallengeCache.TryMarkChallengeAsUsedAsync(options.Challenge))
{
throw new BadRequestException("Invalid credential.");
}

if (!GuidUtilities.TryParseBytes(assertionResponse.Response.UserHandle, out var userId))
{
throw new BadRequestException("Invalid credential.");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
using Bit.Core.Auth.Models.Business.Tokenables;
using Bit.Core.Utilities;
using Microsoft.Extensions.Caching.Distributed;
using Microsoft.Extensions.DependencyInjection;

namespace Bit.Core.Auth.UserFeatures.WebAuthnLogin.Implementations;

internal class WebAuthnChallengeCacheProvider(
[FromKeyedServices("persistent")] IDistributedCache distributedCache) : IWebAuthnChallengeCacheProvider
{
private const string _cacheKeyPrefix = "WebAuthnAssertion_";
private static readonly DistributedCacheEntryOptions _cacheOptions = new()
{
AbsoluteExpirationRelativeToNow = WebAuthnLoginAssertionOptionsTokenable.TokenLifetime
};

private readonly IDistributedCache _distributedCache = distributedCache;

public async Task<bool> TryMarkChallengeAsUsedAsync(byte[] challenge)
{
var cacheKey = BuildCacheKey(challenge);
var cached = await _distributedCache.GetAsync(cacheKey);
if (cached != null)
{
return false;
}

await _distributedCache.SetAsync(cacheKey, [1], _cacheOptions);
return true;
}

private static string BuildCacheKey(byte[] challenge)
=> $"{_cacheKeyPrefix}{CoreHelpers.Base64UrlEncode(challenge)}";
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
// FIXME: Update this file to be null safe and then delete the line below
#nullable disable
Comment thread
ike-kottlowski marked this conversation as resolved.

using System.Security.Claims;
using System.Security.Claims;
using System.Text.Json;
using Bit.Core;
using Bit.Core.AdminConsole.OrganizationFeatures.Policies;
Expand Down Expand Up @@ -101,7 +98,7 @@ public async Task ValidateAsync(ExtensionGrantValidationContext context)
token.TokenIsValid(WebAuthnLoginAssertionOptionsScope.Authentication);
var deviceResponse = JsonSerializer.Deserialize<AuthenticatorAssertionRawResponse>(rawDeviceResponse);

if (!verified)
if (!verified || deviceResponse == null || token.Options == null)
{
context.Result = new GrantValidationResult(TokenRequestErrors.InvalidRequest);
return;
Expand Down
43 changes: 43 additions & 0 deletions test/Api.Test/Auth/Controllers/WebAuthnControllerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,25 @@ public async Task AssertionOptions_UserVerificationSuccess_ReturnsAssertionOptio
Assert.NotNull(result);
Assert.IsType<WebAuthnLoginAssertionOptionsResponseModel>(result);
}

[Theory, BitAutoData]
public async Task AssertionOptions_Success_ProtectsTokenWithUpdateKeySetScope(SecretVerificationRequestModel requestModel, User user, SutProvider<WebAuthnController> sutProvider)
{
// Arrange
sutProvider.GetDependency<IUserService>().GetUserByPrincipalAsync(default).ReturnsForAnyArgs(user);
sutProvider.GetDependency<IUserService>().VerifySecretAsync(user, requestModel.Secret).Returns(true);
sutProvider.GetDependency<IDataProtectorTokenFactory<WebAuthnLoginAssertionOptionsTokenable>>()
.Protect(Arg.Any<WebAuthnLoginAssertionOptionsTokenable>()).Returns("token");

// Act
await sutProvider.Sut.AssertionOptions(requestModel);

// Assert
sutProvider.GetDependency<IDataProtectorTokenFactory<WebAuthnLoginAssertionOptionsTokenable>>()
.Received(1)
.Protect(Arg.Is<WebAuthnLoginAssertionOptionsTokenable>(t =>
t.Scope == Core.Auth.Enums.WebAuthnLoginAssertionOptionsScope.UpdateKeySet));
}
#endregion

[Theory, BitAutoData]
Expand Down Expand Up @@ -419,6 +438,30 @@ public async Task Put_TokenVerificationFailed_ThrowsBadRequestException(Assertio
Assert.Equal(expectedMessage, exception.Message);
}

[Theory, BitAutoData]
public async Task Put_TokenWithNullOptions_ThrowsBadRequestException(WebAuthnLoginCredentialUpdateRequestModel requestModel, SutProvider<WebAuthnController> sutProvider)
{
// Arrange - tokenable deserialized with correct scope but Options == null
var expectedMessage = "The token associated with your request is invalid or has expired. A valid token is required to continue.";
var token = new WebAuthnLoginAssertionOptionsTokenable
{
Scope = Core.Auth.Enums.WebAuthnLoginAssertionOptionsScope.UpdateKeySet,
Options = null,
};
sutProvider.GetDependency<IDataProtectorTokenFactory<WebAuthnLoginAssertionOptionsTokenable>>()
.Unprotect(requestModel.Token)
.Returns(token);

// Act
var exception = await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.UpdateCredential(requestModel));

// Assert
Assert.Equal(expectedMessage, exception.Message);
await sutProvider.GetDependency<IAssertWebAuthnLoginCredentialCommand>()
.DidNotReceive()
.AssertWebAuthnLoginCredential(Arg.Any<AssertionOptions>(), Arg.Any<AuthenticatorAssertionRawResponse>());
}

[Theory, BitAutoData]
public async Task Put_CredentialNotFound_ThrowsBadRequestException(AssertionOptions assertionOptions, WebAuthnLoginCredentialUpdateRequestModel requestModel, SutProvider<WebAuthnController> sutProvider)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System.Text;
using Bit.Core.Auth.Entities;
using Bit.Core.Auth.Repositories;
using Bit.Core.Auth.UserFeatures.WebAuthnLogin;
using Bit.Core.Auth.UserFeatures.WebAuthnLogin.Implementations;
using Bit.Core.Entities;
using Bit.Core.Exceptions;
Expand All @@ -19,10 +20,27 @@ namespace Bit.Core.Test.Auth.UserFeatures.WebAuthnLogin;
[SutProviderCustomize]
public class AssertWebAuthnLoginCredentialCommandTests
{
[Theory, BitAutoData]
internal async Task ChallengeNotCacheable_ThrowsBadRequestException(SutProvider<AssertWebAuthnLoginCredentialCommand> sutProvider, AssertionOptions options, AuthenticatorAssertionRawResponse response)
{
// Arrange
sutProvider.GetDependency<IWebAuthnChallengeCacheProvider>()
.TryMarkChallengeAsUsedAsync(options.Challenge)
.Returns(false);

// Act
var result = async () => await sutProvider.Sut.AssertWebAuthnLoginCredential(options, response);

// Assert
await Assert.ThrowsAsync<BadRequestException>(result);
}

[Theory, BitAutoData]
internal async Task InvalidUserHandle_ThrowsBadRequestException(SutProvider<AssertWebAuthnLoginCredentialCommand> sutProvider, AssertionOptions options, AuthenticatorAssertionRawResponse response)
{
// Arrange
sutProvider.GetDependency<IWebAuthnChallengeCacheProvider>()
.TryMarkChallengeAsUsedAsync(options.Challenge).Returns(true);
response.Response.UserHandle = Encoding.UTF8.GetBytes("invalid-user-handle");

// Act
Expand All @@ -36,6 +54,8 @@ internal async Task InvalidUserHandle_ThrowsBadRequestException(SutProvider<Asse
internal async Task UserNotFound_ThrowsBadRequestException(SutProvider<AssertWebAuthnLoginCredentialCommand> sutProvider, User user, AssertionOptions options, AuthenticatorAssertionRawResponse response)
{
// Arrange
sutProvider.GetDependency<IWebAuthnChallengeCacheProvider>()
.TryMarkChallengeAsUsedAsync(options.Challenge).Returns(true);
response.Response.UserHandle = user.Id.ToByteArray();
sutProvider.GetDependency<IUserRepository>().GetByIdAsync(user.Id).ReturnsNull();

Expand All @@ -50,6 +70,8 @@ internal async Task UserNotFound_ThrowsBadRequestException(SutProvider<AssertWeb
internal async Task NoMatchingCredentialExists_ThrowsBadRequestException(SutProvider<AssertWebAuthnLoginCredentialCommand> sutProvider, User user, AssertionOptions options, AuthenticatorAssertionRawResponse response)
{
// Arrange
sutProvider.GetDependency<IWebAuthnChallengeCacheProvider>()
.TryMarkChallengeAsUsedAsync(options.Challenge).Returns(true);
response.Response.UserHandle = user.Id.ToByteArray();
sutProvider.GetDependency<IUserRepository>().GetByIdAsync(user.Id).Returns(user);
sutProvider.GetDependency<IWebAuthnCredentialRepository>().GetManyByUserIdAsync(user.Id).Returns(new WebAuthnCredential[] { });
Expand All @@ -65,6 +87,8 @@ internal async Task NoMatchingCredentialExists_ThrowsBadRequestException(SutProv
internal async Task AssertionFails_ThrowsBadRequestException(SutProvider<AssertWebAuthnLoginCredentialCommand> sutProvider, User user, AssertionOptions options, AuthenticatorAssertionRawResponse response, WebAuthnCredential credential, AssertionVerificationResult assertionResult)
{
// Arrange
sutProvider.GetDependency<IWebAuthnChallengeCacheProvider>()
.TryMarkChallengeAsUsedAsync(options.Challenge).Returns(true);
var credentialId = Guid.NewGuid().ToByteArray();
credential.CredentialId = CoreHelpers.Base64UrlEncode(credentialId);
response.Id = credentialId;
Expand All @@ -86,6 +110,8 @@ internal async Task AssertionFails_ThrowsBadRequestException(SutProvider<AssertW
internal async Task AssertionSucceeds_ReturnsUserAndCredential(SutProvider<AssertWebAuthnLoginCredentialCommand> sutProvider, User user, AssertionOptions options, AuthenticatorAssertionRawResponse response, WebAuthnCredential credential, AssertionVerificationResult assertionResult)
{
// Arrange
sutProvider.GetDependency<IWebAuthnChallengeCacheProvider>()
.TryMarkChallengeAsUsedAsync(options.Challenge).Returns(true);
var credentialId = Guid.NewGuid().ToByteArray();
credential.CredentialId = CoreHelpers.Base64UrlEncode(credentialId);
response.Id = credentialId;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
using Bit.Core.Auth.UserFeatures.WebAuthnLogin.Implementations;
using Bit.Core.Utilities;
using Bit.Test.Common.AutoFixture;
using Bit.Test.Common.AutoFixture.Attributes;
using Microsoft.Extensions.Caching.Distributed;
using NSubstitute;
using Xunit;

namespace Bit.Core.Test.Auth.UserFeatures.WebAuthnLogin;

[SutProviderCustomize]
public class WebAuthnChallengeCacheProviderTests
{
[Theory, BitAutoData]
internal async Task TryMarkChallengeAsUsedAsync_FirstUse_SavesAndReturnsTrue(
SutProvider<WebAuthnChallengeCacheProvider> sutProvider)
{
// Arrange
var challenge = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8 };
var expectedKey = $"WebAuthnAssertion_{CoreHelpers.Base64UrlEncode(challenge)}";

sutProvider.GetDependency<IDistributedCache>()
.GetAsync(expectedKey, Arg.Any<CancellationToken>())
.Returns((byte[])null);

// Act
var result = await sutProvider.Sut.TryMarkChallengeAsUsedAsync(challenge);

// Assert
Assert.True(result);
await sutProvider.GetDependency<IDistributedCache>()
.Received(1)
.SetAsync(
expectedKey,
Arg.Is<byte[]>(b => b.Length == 1 && b[0] == 1),
Arg.Is<DistributedCacheEntryOptions>(o =>
o.AbsoluteExpirationRelativeToNow == TimeSpan.FromMinutes(17)),
Arg.Any<CancellationToken>());
}

[Theory, BitAutoData]
internal async Task TryMarkChallengeAsUsedAsync_AlreadyUsed_ReturnsFalseAndDoesNotSave(
SutProvider<WebAuthnChallengeCacheProvider> sutProvider)
{
// Arrange
var challenge = new byte[] { 1, 2, 3, 4, 5, 6, 7, 8 };
var expectedKey = $"WebAuthnAssertion_{CoreHelpers.Base64UrlEncode(challenge)}";

sutProvider.GetDependency<IDistributedCache>()
.GetAsync(expectedKey, Arg.Any<CancellationToken>())
.Returns(new byte[] { 1 });

// Act
var result = await sutProvider.Sut.TryMarkChallengeAsUsedAsync(challenge);

// Assert
Assert.False(result);
await sutProvider.GetDependency<IDistributedCache>()
.DidNotReceive()
.SetAsync(
Arg.Any<string>(),
Arg.Any<byte[]>(),
Arg.Any<DistributedCacheEntryOptions>(),
Arg.Any<CancellationToken>());
}
}
Loading
Loading