From 3c848a3dcc4def1eca9b67eb61ac2359674c0c19 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 12 Feb 2024 13:45:41 +0100 Subject: [PATCH] [PM-5731] feat: implement credential assertion in client --- .../Services/Fido2AuthenticatorService.cs | 2 +- src/Core/Services/Fido2ClientService.cs | 90 ++++++- .../Fido2AuthenticatorGetAssertionParams.cs | 7 +- .../Fido2ClientAssertCredentialParams.cs | 8 +- .../Fido2ClientAssertCredentialResult.cs | 22 ++ .../Fido2ClientCreateCredentialParams.cs | 1 - .../Fido2AuthenticatorGetAssertionTests.cs | 8 +- .../Fido2ClientAssertCredentialTests.cs | 222 ++++++++++++++++++ .../Fido2ClientCreateCredentialTests.cs | 2 +- 9 files changed, 350 insertions(+), 12 deletions(-) create mode 100644 test/Core.Test/Services/Fido2ClientAssertCredentialTests.cs diff --git a/src/Core/Services/Fido2AuthenticatorService.cs b/src/Core/Services/Fido2AuthenticatorService.cs index eeacf646d..5cea087d7 100644 --- a/src/Core/Services/Fido2AuthenticatorService.cs +++ b/src/Core/Services/Fido2AuthenticatorService.cs @@ -192,7 +192,7 @@ namespace Bit.Core.Services var signature = GenerateSignature( authData: authenticatorData, - clientDataHash: assertionParams.ClientDataHash, + clientDataHash: assertionParams.Hash, privateKey: selectedFido2Credential.KeyBytes ); diff --git a/src/Core/Services/Fido2ClientService.cs b/src/Core/Services/Fido2ClientService.cs index 9c7ce6da0..bad3515ec 100644 --- a/src/Core/Services/Fido2ClientService.cs +++ b/src/Core/Services/Fido2ClientService.cs @@ -18,8 +18,7 @@ namespace Bit.Core.Services IStateService stateService, IEnvironmentService environmentService, ICryptoFunctionService cryptoFunctionService, - IFido2AuthenticatorService fido2AuthenticatorService - ) + IFido2AuthenticatorService fido2AuthenticatorService) { _stateService = stateService; _environmentService = environmentService; @@ -133,7 +132,74 @@ namespace Bit.Core.Services } } - public Task AssertCredentialAsync(Fido2ClientAssertCredentialParams assertCredentialParams) => throw new NotImplementedException(); + public async Task AssertCredentialAsync(Fido2ClientAssertCredentialParams assertCredentialParams) + { + var blockedUris = await _stateService.GetAutofillBlacklistedUrisAsync(); + var domain = CoreHelpers.GetHostname(assertCredentialParams.Origin); + if (blockedUris.Contains(domain)) + { + throw new Fido2ClientException( + Fido2ClientException.ErrorCode.UriBlockedError, + "Origin is blocked by the user"); + } + + if (!await _stateService.IsAuthenticatedAsync()) + { + throw new Fido2ClientException( + Fido2ClientException.ErrorCode.InvalidStateError, + "No user is logged in"); + } + + if (assertCredentialParams.Origin == _environmentService.GetWebVaultUrl()) + { + throw new Fido2ClientException( + Fido2ClientException.ErrorCode.NotAllowedError, + "Saving Bitwarden credentials in a Bitwarden vault is not allowed"); + } + + if (!assertCredentialParams.Origin.StartsWith("https://")) + { + throw new Fido2ClientException( + Fido2ClientException.ErrorCode.SecurityError, + "Origin is not a valid https origin"); + } + + if (!Fido2DomainUtils.IsValidRpId(assertCredentialParams.RpId, assertCredentialParams.Origin)) + { + throw new Fido2ClientException( + Fido2ClientException.ErrorCode.SecurityError, + "RP ID cannot be used with this origin"); + } + + var clientDataJSON = JsonSerializer.Serialize(new { + type = "webauthn.get", + challenge = CoreHelpers.Base64UrlEncode(assertCredentialParams.Challenge), + origin = assertCredentialParams.Origin, + crossOrigin = !assertCredentialParams.SameOriginWithAncestors, + }); + var clientDataJSONBytes = Encoding.UTF8.GetBytes(clientDataJSON); + var clientDataHash = await _cryptoFunctionService.HashAsync(clientDataJSONBytes, CryptoHashAlgorithm.Sha256); + var getAssertionParams = MapToGetAssertionParams(assertCredentialParams, clientDataHash); + + try { + var getAssertionResult = await _fido2AuthenticatorService.GetAssertionAsync(getAssertionParams); + + return new Fido2ClientAssertCredentialResult { + AuthenticatorData = getAssertionResult.AuthenticatorData, + ClientDataJSON = clientDataJSONBytes, + Id = CoreHelpers.Base64UrlEncode(getAssertionResult.SelectedCredential.Id), + RawId = getAssertionResult.SelectedCredential.Id, + Signature = getAssertionResult.Signature, + UserHandle = getAssertionResult.SelectedCredential.UserHandle + }; + } catch (InvalidStateError) { + throw new Fido2ClientException(Fido2ClientException.ErrorCode.InvalidStateError, "Unknown invalid state encountered"); + } catch (Exception) { + throw new Fido2ClientException(Fido2ClientException.ErrorCode.UnknownError, $"An unknown error occurred"); + } + + throw new NotImplementedException(); + } private Fido2AuthenticatorMakeCredentialParams MapToMakeCredentialParams( Fido2ClientCreateCredentialParams createCredentialParams, @@ -160,5 +226,23 @@ namespace Bit.Core.Services Extensions = createCredentialParams.Extensions }; } + + private Fido2AuthenticatorGetAssertionParams MapToGetAssertionParams( + Fido2ClientAssertCredentialParams assertCredentialParams, + byte[] cliendDataHash) + { + var requireUserVerification = assertCredentialParams.UserVerification == "required" || + assertCredentialParams.UserVerification == "preferred" || + assertCredentialParams.UserVerification == null; + + return new Fido2AuthenticatorGetAssertionParams { + RpId = assertCredentialParams.RpId, + Challenge = assertCredentialParams.Challenge, + AllowCredentialDescriptorList = assertCredentialParams.AllowCredentials, + RequireUserPresence = true, + RequireUserVerification = requireUserVerification, + Hash = cliendDataHash + }; + } } } diff --git a/src/Core/Utilities/Fido2/Fido2AuthenticatorGetAssertionParams.cs b/src/Core/Utilities/Fido2/Fido2AuthenticatorGetAssertionParams.cs index 4abeaa3d6..6d75680b1 100644 --- a/src/Core/Utilities/Fido2/Fido2AuthenticatorGetAssertionParams.cs +++ b/src/Core/Utilities/Fido2/Fido2AuthenticatorGetAssertionParams.cs @@ -6,7 +6,7 @@ public string RpId { get; set; } /** The hash of the serialized client data, provided by the client. */ - public byte[] ClientDataHash { get; set; } + public byte[] Hash { get; set; } public PublicKeyCredentialDescriptor[] AllowCredentialDescriptorList { get; set; } @@ -20,6 +20,11 @@ /// public bool RequireUserPresence { get; set; } + /// + /// The challenge to be signed by the authenticator. + /// + public byte[] Challenge { get; set; } + public object Extensions { get; set; } } } diff --git a/src/Core/Utilities/Fido2/Fido2ClientAssertCredentialParams.cs b/src/Core/Utilities/Fido2/Fido2ClientAssertCredentialParams.cs index f479467d5..480d2902a 100644 --- a/src/Core/Utilities/Fido2/Fido2ClientAssertCredentialParams.cs +++ b/src/Core/Utilities/Fido2/Fido2ClientAssertCredentialParams.cs @@ -11,7 +11,13 @@ namespace Bit.Core.Utilities.Fido2 public class Fido2ClientAssertCredentialParams { /// - /// S challenge that the selected authenticator signs, along with other data, when producing an authentication + /// A value which is true if and only if the caller’s environment settings object is same-origin with its ancestors. + /// It is false if caller is cross-origin. + /// + public bool SameOriginWithAncestors { get; set; } + + /// + /// The challenge that the selected authenticator signs, along with other data, when producing an authentication /// assertion. /// public required byte[] Challenge { get; set; } diff --git a/src/Core/Utilities/Fido2/Fido2ClientAssertCredentialResult.cs b/src/Core/Utilities/Fido2/Fido2ClientAssertCredentialResult.cs index 31bc9468c..a80a76d95 100644 --- a/src/Core/Utilities/Fido2/Fido2ClientAssertCredentialResult.cs +++ b/src/Core/Utilities/Fido2/Fido2ClientAssertCredentialResult.cs @@ -16,5 +16,27 @@ namespace Bit.Core.Utilities.Fido2 /// The credential identifier. /// public required byte[] RawId { get; set; } + + /// + /// The JSON-compatible serialization of client datapassed to the authenticator by the client in + /// order to generate this assertion. + /// + public required byte[] ClientDataJSON { get; set; } + + /// + /// The authenticator data returned by the authenticator. + /// + public required byte[] AuthenticatorData { get; set; } + + /// + /// The raw signature returned from the authenticator. + /// + public required byte[] Signature { get; set; } + + /// + /// The user handle returned from the authenticator, or null if the authenticator did not + /// return a user handle. + /// + public byte[]? UserHandle { get; set; } } } diff --git a/src/Core/Utilities/Fido2/Fido2ClientCreateCredentialParams.cs b/src/Core/Utilities/Fido2/Fido2ClientCreateCredentialParams.cs index 48695d463..52adbfff0 100644 --- a/src/Core/Utilities/Fido2/Fido2ClientCreateCredentialParams.cs +++ b/src/Core/Utilities/Fido2/Fido2ClientCreateCredentialParams.cs @@ -12,7 +12,6 @@ namespace Bit.Core.Utilities.Fido2 /// public required string Origin { get; set; } - // TODO: Check if we actually need this /// /// A value which is true if and only if the caller’s environment settings object is same-origin with its ancestors. /// It is false if caller is cross-origin. diff --git a/test/Core.Test/Services/Fido2AuthenticatorGetAssertionTests.cs b/test/Core.Test/Services/Fido2AuthenticatorGetAssertionTests.cs index 68eff0023..d38109f7b 100644 --- a/test/Core.Test/Services/Fido2AuthenticatorGetAssertionTests.cs +++ b/test/Core.Test/Services/Fido2AuthenticatorGetAssertionTests.cs @@ -259,7 +259,7 @@ namespace Bit.Core.Test.Services // Arrange var keyPair = GenerateKeyPair(); var rpIdHashMock = RandomBytes(32); - _params.ClientDataHash = RandomBytes(32); + _params.Hash = RandomBytes(32); _params.RequireUserVerification = true; _selectedCipher.Login.MainFido2Credential.CounterValue = 9000; _selectedCipher.Login.MainFido2Credential.KeyValue = CoreHelpers.Base64UrlEncode(keyPair.ExportPkcs8PrivateKey()); @@ -283,7 +283,7 @@ namespace Bit.Core.Test.Services Assert.Equal(rpIdHashMock, rpIdHash); Assert.Equal(new byte[] { 0b00000101 }, flags); // UP = true, UV = true Assert.Equal(new byte[] { 0, 0, 0x23, 0x29 }, counter); // 9001 in binary big-endian format - Assert.True(keyPair.VerifyData(authData.Concat(_params.ClientDataHash).ToArray(), result.Signature, HashAlgorithmName.SHA256), "Signature verification failed"); + Assert.True(keyPair.VerifyData(authData.Concat(_params.Hash).ToArray(), result.Signature, HashAlgorithmName.SHA256), "Signature verification failed"); } [Fact] @@ -357,11 +357,11 @@ namespace Bit.Core.Test.Services }; } - private Fido2AuthenticatorGetAssertionParams CreateParams(string? rpId = null, byte[]? clientDataHash = null, PublicKeyCredentialDescriptor[]? allowCredentialDescriptorList = null, bool? requireUserPresence = null, bool? requireUserVerification = null) + private Fido2AuthenticatorGetAssertionParams CreateParams(string? rpId = null, byte[]? hash = null, PublicKeyCredentialDescriptor[]? allowCredentialDescriptorList = null, bool? requireUserPresence = null, bool? requireUserVerification = null) { return new Fido2AuthenticatorGetAssertionParams { RpId = rpId ?? "bitwarden.com", - ClientDataHash = clientDataHash ?? RandomBytes(32), + Hash = hash ?? RandomBytes(32), AllowCredentialDescriptorList = allowCredentialDescriptorList ?? null, RequireUserPresence = requireUserPresence ?? true, RequireUserVerification = requireUserPresence ?? false diff --git a/test/Core.Test/Services/Fido2ClientAssertCredentialTests.cs b/test/Core.Test/Services/Fido2ClientAssertCredentialTests.cs new file mode 100644 index 000000000..04a886071 --- /dev/null +++ b/test/Core.Test/Services/Fido2ClientAssertCredentialTests.cs @@ -0,0 +1,222 @@ +using System; +using System.Text; +using System.Text.Json; +using System.Text.Json.Nodes; +using System.Threading.Tasks; +using Bit.Core.Abstractions; +using Bit.Core.Services; +using Bit.Core.Utilities; +using Bit.Core.Utilities.Fido2; +using Bit.Test.Common.AutoFixture; +using NSubstitute; +using NSubstitute.ExceptionExtensions; +using Xunit; + +namespace Bit.Core.Test.Services +{ + public class Fido2ClientAssertCredentialTests : IDisposable + { + private readonly SutProvider _sutProvider = new SutProvider().Create(); + + private Fido2ClientAssertCredentialParams _params; + + public Fido2ClientAssertCredentialTests() + { + _params = new Fido2ClientAssertCredentialParams { + Origin = "https://bitwarden.com", + Challenge = RandomBytes(32), + RpId = "bitwarden.com", + UserVerification = "required", + AllowCredentials = [ + new PublicKeyCredentialDescriptor { + Id = RandomBytes(32), + Type = "public-key" + } + ], + Timeout = 60000, + }; + + _sutProvider.GetDependency().GetAutofillBlacklistedUrisAsync().Returns([]); + _sutProvider.GetDependency().IsAuthenticatedAsync().Returns(true); + } + + public void Dispose() + { + } + + [Fact(Skip = "Not sure how to check this, or if it matters.")] + // Spec: If callerOrigin is an opaque origin, return a DOMException whose name is "NotAllowedError", and terminate this algorithm. + public Task AssertCredentialAsync_ThrowsNotAllowedError_OriginIsOpaque() => throw new NotImplementedException(); + + [Fact] + // Spec: Let effectiveDomain be the callerOrigin’s effective domain. If effective domain is not a valid domain, + // then return a DOMException whose name is "SecurityError" and terminate this algorithm. + public async Task AssertCredentialAsync_ThrowsSecurityError_OriginIsNotValidDomain() + { + // Arrange + _params.Origin = "invalid-domain-name"; + + // Act + var exception = await Assert.ThrowsAsync(() => _sutProvider.Sut.AssertCredentialAsync(_params)); + + // Assert + Assert.Equal(Fido2ClientException.ErrorCode.SecurityError, exception.Code); + } + + [Fact] + // Spec: If options.rp.id is not a registrable domain suffix of and is not equal to effectiveDomain, + // return a DOMException whose name is "SecurityError", and terminate this algorithm. + public async Task AssertCredentialAsync_ThrowsSecurityError_RpIdIsNotValidForOrigin() + { + // Arrange + _params.Origin = "https://passwordless.dev"; + _params.RpId = "bitwarden.com"; + + // Act + var exception = await Assert.ThrowsAsync(() => _sutProvider.Sut.AssertCredentialAsync(_params)); + + // Assert + Assert.Equal(Fido2ClientException.ErrorCode.SecurityError, exception.Code); + } + + [Fact] + // Spec: The origin's scheme must be https. + public async Task AssertCredentialAsync_ThrowsSecurityError_OriginIsNotHttps() + { + // Arrange + _params.Origin = "http://bitwarden.com"; + _params.RpId = "bitwarden.com"; + + // Act + var exception = await Assert.ThrowsAsync(() => _sutProvider.Sut.AssertCredentialAsync(_params)); + + // Assert + Assert.Equal(Fido2ClientException.ErrorCode.SecurityError, exception.Code); + } + + [Fact] + // Spec: If the origin's hostname is a blocked uri, then return UriBlockedError. + public async Task AssertCredentialAsync_ThrowsUriBlockedError_OriginIsBlocked() + { + // Arrange + _params.Origin = "https://sub.bitwarden.com"; + _sutProvider.GetDependency().GetAutofillBlacklistedUrisAsync().Returns([ + "sub.bitwarden.com" + ]); + + // Act + var exception = await Assert.ThrowsAsync(() => _sutProvider.Sut.AssertCredentialAsync(_params)); + + // Assert + Assert.Equal(Fido2ClientException.ErrorCode.UriBlockedError, exception.Code); + } + + [Fact] + public async Task AssertCredentialAsync_ThrowsInvalidStateError_AuthenticatorThrowsInvalidStateError() + { + // Arrange + _sutProvider.GetDependency() + .GetAssertionAsync(Arg.Any()) + .Throws(new InvalidStateError()); + + // Act + var exception = await Assert.ThrowsAsync(() => _sutProvider.Sut.AssertCredentialAsync(_params)); + + // Assert + Assert.Equal(Fido2ClientException.ErrorCode.InvalidStateError, exception.Code); + } + + [Fact] + // This keeps sensetive information form leaking + public async Task AssertCredentialAsync_ThrowsUnknownError_AuthenticatorThrowsUnknownError() + { + // Arrange + _sutProvider.GetDependency() + .GetAssertionAsync(Arg.Any()) + .Throws(new Exception("unknown error")); + + // Act + var exception = await Assert.ThrowsAsync(() => _sutProvider.Sut.AssertCredentialAsync(_params)); + + // Assert + Assert.Equal(Fido2ClientException.ErrorCode.UnknownError, exception.Code); + } + + [Fact] + public async Task AssertCredentialAsync_ThrowsInvalidStateError_UserIsLoggedOut() + { + // Arrange + _sutProvider.GetDependency().IsAuthenticatedAsync().Returns(false); + + // Act + var exception = await Assert.ThrowsAsync(() => _sutProvider.Sut.AssertCredentialAsync(_params)); + + // Assert + Assert.Equal(Fido2ClientException.ErrorCode.InvalidStateError, exception.Code); + } + + [Fact] + public async Task AssertCredentialAsync_ThrowsNotAllowedError_OriginIsBitwardenVault() + { + // Arrange + _params.Origin = "https://vault.bitwarden.com"; + _sutProvider.GetDependency().GetWebVaultUrl().Returns("https://vault.bitwarden.com"); + + // Act + var exception = await Assert.ThrowsAsync(() => _sutProvider.Sut.AssertCredentialAsync(_params)); + + // Assert + Assert.Equal(Fido2ClientException.ErrorCode.NotAllowedError, exception.Code); + } + + [Fact] + public async Task AssertCredentialAsync_ReturnsAssertion() + { + // Arrange + _params.UserVerification = "required"; + var authenticatorResult = new Fido2AuthenticatorGetAssertionResult { + AuthenticatorData = RandomBytes(32), + SelectedCredential = new Fido2AuthenticatorGetAssertionSelectedCredential { + Id = RandomBytes(16), + UserHandle = RandomBytes(32) + }, + Signature = RandomBytes(32) + }; + _sutProvider.GetDependency() + .GetAssertionAsync(Arg.Any()) + .Returns(authenticatorResult); + + // Act + var result = await _sutProvider.Sut.AssertCredentialAsync(_params); + + // Assert + await _sutProvider.GetDependency() + .Received() + .GetAssertionAsync(Arg.Is(x => + x.RpId == _params.RpId && + x.RequireUserPresence == true && + x.RequireUserVerification == true && + x.AllowCredentialDescriptorList.Length == 1 && + x.AllowCredentialDescriptorList[0].Id == _params.AllowCredentials[0].Id + )); + + Assert.Equal(authenticatorResult.SelectedCredential.Id, result.RawId); + Assert.Equal(CoreHelpers.Base64UrlEncode(authenticatorResult.SelectedCredential.Id), result.Id); + Assert.Equal(authenticatorResult.AuthenticatorData, result.AuthenticatorData); + Assert.Equal(authenticatorResult.Signature, result.Signature); + + var clientDataJSON = JsonSerializer.Deserialize(Encoding.UTF8.GetString(result.ClientDataJSON)); + Assert.Equal("webauthn.get", clientDataJSON["type"].GetValue()); + Assert.Equal(CoreHelpers.Base64UrlEncode(_params.Challenge), clientDataJSON["challenge"].GetValue()); + Assert.Equal(_params.Origin, clientDataJSON["origin"].GetValue()); + Assert.Equal(!_params.SameOriginWithAncestors, clientDataJSON["crossOrigin"].GetValue()); + } + + private byte[] RandomBytes(int length) + { + var bytes = new byte[length]; + new Random().NextBytes(bytes); + return bytes; + } + } +} diff --git a/test/Core.Test/Services/Fido2ClientCreateCredentialTests.cs b/test/Core.Test/Services/Fido2ClientCreateCredentialTests.cs index df2d5d880..b290e981d 100644 --- a/test/Core.Test/Services/Fido2ClientCreateCredentialTests.cs +++ b/test/Core.Test/Services/Fido2ClientCreateCredentialTests.cs @@ -96,7 +96,7 @@ namespace Bit.Core.Test.Services [Fact(Skip = "Not sure how to check this, or if it matters.")] // Spec: If callerOrigin is an opaque origin, return a DOMException whose name is "NotAllowedError", and terminate this algorithm. - public Task CreateCredentialAsync_ThrowsNotAllowedError_UserIdIsTooLarge() => throw new NotImplementedException(); + public Task CreateCredentialAsync_ThrowsNotAllowedError_OriginIsOpaque() => throw new NotImplementedException(); [Fact] // Spec: Let effectiveDomain be the callerOrigin’s effective domain. If effective domain is not a valid domain,