From 5d5d11336909bdc3dbbe7c51cda0fb9a60c65029 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Mon, 29 Jan 2024 14:43:14 +0100 Subject: [PATCH] [PM-5731] feat: implement signing --- .../Abstractions/ICryptoFunctionService.cs | 1 - .../Models/Domain/CryptoEcdsaAlgorithm.cs | 6 ---- .../Models/Domain/CryptoSignEcdsaOptions.cs | 13 -------- src/Core/Models/Domain/ICryptoSignOptions.cs | 6 ---- .../Services/Fido2AuthenticatorService.cs | 32 +++++++++--------- src/Core/Services/PclCryptoFunctionService.cs | 10 ------ .../Fido2AuthenticatorGetAssertionTests.cs | 33 ++++++++++--------- .../Fido2AuthenticatorMakeCredentialTests.cs | 2 +- 8 files changed, 35 insertions(+), 68 deletions(-) delete mode 100644 src/Core/Models/Domain/CryptoEcdsaAlgorithm.cs delete mode 100644 src/Core/Models/Domain/CryptoSignEcdsaOptions.cs delete mode 100644 src/Core/Models/Domain/ICryptoSignOptions.cs diff --git a/src/Core/Abstractions/ICryptoFunctionService.cs b/src/Core/Abstractions/ICryptoFunctionService.cs index 90fb33ad4..630dc1b96 100644 --- a/src/Core/Abstractions/ICryptoFunctionService.cs +++ b/src/Core/Abstractions/ICryptoFunctionService.cs @@ -21,7 +21,6 @@ namespace Bit.Core.Abstractions Task HkdfAsync(byte[] ikm, byte[] salt, byte[] info, int outputByteSize, HkdfAlgorithm algorithm); Task HkdfExpandAsync(byte[] prk, string info, int outputByteSize, HkdfAlgorithm algorithm); Task HkdfExpandAsync(byte[] prk, byte[] info, int outputByteSize, HkdfAlgorithm algorithm); - Task SignAsync(byte[] data, byte[] privateKey, ICryptoSignOptions options); Task HashAsync(string value, CryptoHashAlgorithm algorithm); Task HashAsync(byte[] value, CryptoHashAlgorithm algorithm); Task HmacAsync(byte[] value, byte[] key, CryptoHashAlgorithm algorithm); diff --git a/src/Core/Models/Domain/CryptoEcdsaAlgorithm.cs b/src/Core/Models/Domain/CryptoEcdsaAlgorithm.cs deleted file mode 100644 index fb813495b..000000000 --- a/src/Core/Models/Domain/CryptoEcdsaAlgorithm.cs +++ /dev/null @@ -1,6 +0,0 @@ -namespace Bit.Core.Models.Domain -{ - public enum CryptoEcdsaAlgorithm : byte { - P256Sha256 = 0, - } -} diff --git a/src/Core/Models/Domain/CryptoSignEcdsaOptions.cs b/src/Core/Models/Domain/CryptoSignEcdsaOptions.cs deleted file mode 100644 index eee718ff6..000000000 --- a/src/Core/Models/Domain/CryptoSignEcdsaOptions.cs +++ /dev/null @@ -1,13 +0,0 @@ -namespace Bit.Core.Models.Domain -{ - public struct CryptoSignEcdsaOptions : ICryptoSignOptions - { - public enum DsaSignatureFormat : byte { - IeeeP1363FixedFieldConcatenation = 0, - Rfc3279DerSequence = 1 - } - - public CryptoEcdsaAlgorithm Algorithm { get; set; } - public DsaSignatureFormat SignatureFormat { get; set; } - } -} diff --git a/src/Core/Models/Domain/ICryptoSignOptions.cs b/src/Core/Models/Domain/ICryptoSignOptions.cs deleted file mode 100644 index 154a95e0b..000000000 --- a/src/Core/Models/Domain/ICryptoSignOptions.cs +++ /dev/null @@ -1,6 +0,0 @@ -namespace Bit.Core.Models.Domain -{ - public interface ICryptoSignOptions - { - } -} diff --git a/src/Core/Services/Fido2AuthenticatorService.cs b/src/Core/Services/Fido2AuthenticatorService.cs index 887c45bbf..646001a2e 100644 --- a/src/Core/Services/Fido2AuthenticatorService.cs +++ b/src/Core/Services/Fido2AuthenticatorService.cs @@ -5,6 +5,7 @@ using Bit.Core.Models.Domain; using Bit.Core.Utilities.Fido2; using Bit.Core.Utilities; using System.Formats.Cbor; +using System.Security.Cryptography; namespace Bit.Core.Services { @@ -184,7 +185,7 @@ namespace Bit.Core.Services counter: selectedFido2Credential.CounterValue ); - var signature = await GenerateSignature( + var signature = GenerateSignature( authData: authenticatorData, clientDataHash: assertionParams.Hash, privateKey: selectedFido2Credential.KeyBytes @@ -286,8 +287,8 @@ namespace Bit.Core.Services // TODO: Move this to a separate service private (PublicKey publicKey, byte[] privateKey) GenerateKeyPair() { - var dsa = System.Security.Cryptography.ECDsa.Create(); - dsa.GenerateKey(System.Security.Cryptography.ECCurve.NamedCurves.nistP256); + var dsa = ECDsa.Create(); + dsa.GenerateKey(ECCurve.NamedCurves.nistP256); var privateKey = dsa.ExportPkcs8PrivateKey(); return (new PublicKey(dsa), privateKey); @@ -400,20 +401,19 @@ namespace Bit.Core.Services return attestationObject.Encode(); } - private async Task GenerateSignature( - byte[] authData, - byte[] clientDataHash, - byte[] privateKey - ) + // TODO: Move this to a separate service + private byte[] GenerateSignature(byte[] authData, byte[] clientDataHash, byte[] privateKey) { var sigBase = authData.Concat(clientDataHash).ToArray(); - var signature = await _cryptoFunctionService.SignAsync(sigBase, privateKey, new CryptoSignEcdsaOptions - { - Algorithm = CryptoEcdsaAlgorithm.P256Sha256, - SignatureFormat = CryptoSignEcdsaOptions.DsaSignatureFormat.Rfc3279DerSequence - }); + var dsa = ECDsa.Create(); + dsa.ImportPkcs8PrivateKey(privateKey, out var bytesRead); - return signature; + if (bytesRead == 0) + { + throw new Exception("Failed to import private key"); + } + + return dsa.SignData(sigBase, HashAlgorithmName.SHA256); } private string GuidToStandardFormat(byte[] bytes) @@ -428,9 +428,9 @@ namespace Bit.Core.Services private class PublicKey { - private readonly System.Security.Cryptography.ECDsa _dsa; + private readonly ECDsa _dsa; - public PublicKey(System.Security.Cryptography.ECDsa dsa) { + public PublicKey(ECDsa dsa) { _dsa = dsa; } diff --git a/src/Core/Services/PclCryptoFunctionService.cs b/src/Core/Services/PclCryptoFunctionService.cs index 3a55db662..4254fa5d5 100644 --- a/src/Core/Services/PclCryptoFunctionService.cs +++ b/src/Core/Services/PclCryptoFunctionService.cs @@ -122,16 +122,6 @@ namespace Bit.Core.Services return okm.Take(outputByteSize).ToArray(); } - public Task SignAsync(byte[] data, byte[] privateKey, ICryptoSignOptions options) - { - throw new NotSupportedException(); - - // Not supported on iOS and Android - // var provider = AsymmetricKeyAlgorithmProvider.OpenAlgorithm(AsymmetricAlgorithm.EcdsaP256Sha256); - // var cryptoKey = provider.ImportKeyPair(privateKey, CryptographicPrivateKeyBlobType.Pkcs8RawPrivateKeyInfo); - // return Task.FromResult(CryptographicEngine.Sign(cryptoKey, data)); - } - public Task HashAsync(string value, CryptoHashAlgorithm algorithm) { return HashAsync(Encoding.UTF8.GetBytes(value), algorithm); diff --git a/test/Core.Test/Services/Fido2AuthenticatorGetAssertionTests.cs b/test/Core.Test/Services/Fido2AuthenticatorGetAssertionTests.cs index e560d2168..6b2190d17 100644 --- a/test/Core.Test/Services/Fido2AuthenticatorGetAssertionTests.cs +++ b/test/Core.Test/Services/Fido2AuthenticatorGetAssertionTests.cs @@ -16,6 +16,7 @@ using Xunit; using Bit.Core.Utilities; using System.Collections.Generic; using System.Linq; +using System.Security.Cryptography; namespace Bit.Core.Test.Services { @@ -290,7 +291,6 @@ namespace Bit.Core.Test.Services [Theory] [InlineCustomAutoData(new[] { typeof(SutProviderCustomization) })] - // Spec: Increment the credential associated signature counter public async Task GetAssertionAsync_ReturnsAssertion(SutProvider sutProvider, Fido2AuthenticatorGetAssertionParams aParams) { // Common Arrange var cipherView = CreateCipherView(null, "bitwarden.com", true); @@ -303,34 +303,28 @@ namespace Bit.Core.Test.Services }); // Arrange + var keyPair = GenerateKeyPair(); var rpIdHashMock = RandomBytes(32); + aParams.Hash = RandomBytes(32); sutProvider.GetDependency().HashAsync(aParams.RpId, CryptoHashAlgorithm.Sha256).Returns(rpIdHashMock); cipherView.Login.MainFido2Credential.CounterValue = 9000; - var signatureMock = RandomBytes(32); - sutProvider.GetDependency().SignAsync( - Arg.Any(), - Arg.Any(), - new CryptoSignEcdsaOptions { - Algorithm = CryptoEcdsaAlgorithm.P256Sha256, - SignatureFormat = CryptoSignEcdsaOptions.DsaSignatureFormat.Rfc3279DerSequence - } - ).Returns(signatureMock); + cipherView.Login.MainFido2Credential.KeyValue = CoreHelpers.Base64UrlEncode(keyPair.ExportPkcs8PrivateKey()); // Act var result = await sutProvider.Sut.GetAssertionAsync(aParams); // Assert - var encAuthData = result.AuthenticatorData; - var rpIdHash = encAuthData.Take(32); - var flags = encAuthData.Skip(32).Take(1); - var counter = encAuthData.Skip(33).Take(4); + var authData = result.AuthenticatorData; + var rpIdHash = authData.Take(32); + var flags = authData.Skip(32).Take(1); + var counter = authData.Skip(33).Take(4); Assert.Equal(Guid.Parse(cipherView.Login.MainFido2Credential.CredentialId).ToByteArray(), result.SelectedCredential.Id); Assert.Equal(CoreHelpers.Base64UrlDecode(cipherView.Login.MainFido2Credential.UserHandle), result.SelectedCredential.UserHandle); 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.Equal(signatureMock, result.Signature); + Assert.True(keyPair.VerifyData(authData.Concat(aParams.Hash).ToArray(), result.Signature, HashAlgorithmName.SHA256), "Signature verification failed"); } [Theory] @@ -364,6 +358,14 @@ namespace Bit.Core.Test.Services return bytes; } + private ECDsa GenerateKeyPair() + { + var dsa = ECDsa.Create(); + dsa.GenerateKey(ECCurve.NamedCurves.nistP256); + + return dsa; + } + #nullable enable private CipherView CreateCipherView(string? credentialId, string? rpId, bool? discoverable) { @@ -378,6 +380,7 @@ namespace Bit.Core.Test.Services RpId = rpId ?? "bitwarden.com", Discoverable = discoverable.HasValue ? discoverable.ToString() : "true", UserHandleValue = RandomBytes(32), + KeyValue = "MIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgO4wC7AlY4eJP7uedRUJGYsAIJAd6gN1Vp7uJh6xXAp6hRANCAARGvr56F_t27DEG1Tzl-qJRhrTUtC7jOEbasAEEZcE3TiMqoWCan0sxKDPylhRYk-1qyrBC_feN1UtGWH57sROa" } } } diff --git a/test/Core.Test/Services/Fido2AuthenticatorMakeCredentialTests.cs b/test/Core.Test/Services/Fido2AuthenticatorMakeCredentialTests.cs index 61854c3e7..8ce718208 100644 --- a/test/Core.Test/Services/Fido2AuthenticatorMakeCredentialTests.cs +++ b/test/Core.Test/Services/Fido2AuthenticatorMakeCredentialTests.cs @@ -456,7 +456,7 @@ namespace Bit.Core.Test.Services CredentialId = credentialId ?? Guid.NewGuid().ToString(), RpId = rpId ?? "bitwarden.com", Discoverable = discoverable.HasValue ? discoverable.ToString() : "true", - UserHandleValue = RandomBytes(32), + UserHandleValue = RandomBytes(32) } } : null }