From 41161864dbfdd8f5c2773320393b84a7ff5580bc Mon Sep 17 00:00:00 2001 From: Federico Maccaroni Date: Fri, 16 Feb 2024 19:30:33 -0300 Subject: [PATCH] PM-5154 [Passkeys iOS] Fix Credential ID handling on bytes and string formats. Fix Discoverable to be lowercase on set so it doesn't break parsing on clients. Added UserDisplayName on Fido2 entities. Extracted the Guid Standard/Raw format helpers to a extensions class. --- src/Core/Models/Api/Fido2CredentialApi.cs | 5 +- src/Core/Models/Data/Fido2CredentialData.cs | 2 + src/Core/Models/Domain/Fido2Credential.cs | 8 ++-- src/Core/Models/View/Fido2CredentialView.cs | 3 +- .../Services/Fido2AuthenticatorService.cs | 48 ++++++++----------- .../Fido2/PublicKeyCredentialDescriptor.cs | 1 - src/Core/Utilities/GuidExtensions.cs | 15 ++++++ ...edentialProviderViewController.Passkeys.cs | 21 +++----- .../CredentialProviderViewController.cs | 2 +- src/iOS.Core/Utilities/ASHelpers.cs | 3 +- 10 files changed, 56 insertions(+), 52 deletions(-) create mode 100644 src/Core/Utilities/GuidExtensions.cs diff --git a/src/Core/Models/Api/Fido2CredentialApi.cs b/src/Core/Models/Api/Fido2CredentialApi.cs index 7953e06a1..672a7ec47 100644 --- a/src/Core/Models/Api/Fido2CredentialApi.cs +++ b/src/Core/Models/Api/Fido2CredentialApi.cs @@ -1,5 +1,4 @@ -using System; -using Bit.Core.Models.Domain; +using Bit.Core.Models.Domain; namespace Bit.Core.Models.Api { @@ -21,6 +20,7 @@ namespace Bit.Core.Models.Api RpName = fido2Key.RpName?.EncryptedString; UserHandle = fido2Key.UserHandle?.EncryptedString; UserName = fido2Key.UserName?.EncryptedString; + UserDisplayName = fido2Key.UserDisplayName?.EncryptedString; Counter = fido2Key.Counter?.EncryptedString; CreationDate = fido2Key.CreationDate; } @@ -35,6 +35,7 @@ namespace Bit.Core.Models.Api public string RpName { get; set; } public string UserHandle { get; set; } public string UserName { get; set; } + public string UserDisplayName { get; set; } public string Counter { get; set; } public DateTime CreationDate { get; set; } } diff --git a/src/Core/Models/Data/Fido2CredentialData.cs b/src/Core/Models/Data/Fido2CredentialData.cs index 846df59f4..103d03cbf 100644 --- a/src/Core/Models/Data/Fido2CredentialData.cs +++ b/src/Core/Models/Data/Fido2CredentialData.cs @@ -19,6 +19,7 @@ namespace Bit.Core.Models.Data RpName = apiData.RpName; UserHandle = apiData.UserHandle; UserName = apiData.UserName; + UserDisplayName = apiData.UserDisplayName; Counter = apiData.Counter; CreationDate = apiData.CreationDate; } @@ -33,6 +34,7 @@ namespace Bit.Core.Models.Data public string RpName { get; set; } public string UserHandle { get; set; } public string UserName { get; set; } + public string UserDisplayName { get; set; } public string Counter { get; set; } public DateTime CreationDate { get; set; } } diff --git a/src/Core/Models/Domain/Fido2Credential.cs b/src/Core/Models/Domain/Fido2Credential.cs index 7c6928204..313ff2c8f 100644 --- a/src/Core/Models/Domain/Fido2Credential.cs +++ b/src/Core/Models/Domain/Fido2Credential.cs @@ -1,8 +1,4 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Threading.Tasks; -using Bit.Core.Models.Data; +using Bit.Core.Models.Data; using Bit.Core.Models.View; namespace Bit.Core.Models.Domain @@ -21,6 +17,7 @@ namespace Bit.Core.Models.Domain nameof(RpName), nameof(UserHandle), nameof(UserName), + nameof(UserDisplayName), nameof(Counter) }; @@ -48,6 +45,7 @@ namespace Bit.Core.Models.Domain public EncString RpName { get; set; } public EncString UserHandle { get; set; } public EncString UserName { get; set; } + public EncString UserDisplayName { get; set; } public EncString Counter { get; set; } public DateTime CreationDate { get; set; } diff --git a/src/Core/Models/View/Fido2CredentialView.cs b/src/Core/Models/View/Fido2CredentialView.cs index f76a7486e..8d3a1296b 100644 --- a/src/Core/Models/View/Fido2CredentialView.cs +++ b/src/Core/Models/View/Fido2CredentialView.cs @@ -26,6 +26,7 @@ namespace Bit.Core.Models.View public string RpName { get; set; } public string UserHandle { get; set; } public string UserName { get; set; } + public string UserDisplayName { get; set; } public string Counter { get; set; } public DateTime CreationDate { get; set; } @@ -50,7 +51,7 @@ namespace Bit.Core.Models.View [JsonIgnore] public bool DiscoverableValue { get => bool.TryParse(Discoverable, out var discoverable) && discoverable; - set => Discoverable = value.ToString(); + set => Discoverable = value.ToString().ToLower(); // must be lowercase so it can be parsed in the current version of clients } [JsonIgnore] diff --git a/src/Core/Services/Fido2AuthenticatorService.cs b/src/Core/Services/Fido2AuthenticatorService.cs index e3abea4b5..a5fbf6124 100644 --- a/src/Core/Services/Fido2AuthenticatorService.cs +++ b/src/Core/Services/Fido2AuthenticatorService.cs @@ -79,6 +79,8 @@ namespace Bit.Core.Services var keyPair = GenerateKeyPair(); var fido2Credential = CreateCredentialView(makeCredentialParams, keyPair.privateKey); + ClipLogger.Log($"[Fido2Authenticator] IsDiscoverable {fido2Credential.Discoverable} - {fido2Credential.DiscoverableValue}"); + var encrypted = await _cipherService.GetAsync(cipherId); var cipher = await encrypted.DecryptAsync(); @@ -95,18 +97,20 @@ namespace Bit.Core.Services await _cipherService.SaveWithServerAsync(reencrypted); credentialId = fido2Credential.CredentialId; + ClipLogger.Log($"[Fido2Authenticator] IsDiscoverable {cipher.Login.MainFido2Credential.Discoverable} - {cipher.Login.MainFido2Credential.DiscoverableValue}"); + var authData = await GenerateAuthDataAsync( rpId: makeCredentialParams.RpEntity.Id, counter: fido2Credential.CounterValue, userPresence: true, userVerification: userVerified, - credentialId: GuidToRawFormat(credentialId), + credentialId: credentialId.GuidToRawFormat(), publicKey: keyPair.publicKey ); return new Fido2AuthenticatorMakeCredentialResult { - CredentialId = GuidToRawFormat(credentialId), + CredentialId = credentialId.GuidToRawFormat(), AttestationObject = EncodeAttestationObject(authData), AuthData = authData, PublicKey = keyPair.publicKey.ExportDer(), @@ -197,6 +201,9 @@ namespace Bit.Core.Services throw new NotAllowedError(); } + // TODO: Remove this hardcoding + userVerified = true; + if (!userVerified && (assertionParams.RequireUserVerification || selectedCipher.Reprompt != CipherRepromptType.None)) { // _logService.Info( // "[Fido2Authenticator] Aborting because user verification was unsuccessful." @@ -221,6 +228,9 @@ namespace Bit.Core.Services var encrypted = await _cipherService.EncryptAsync(selectedCipher); await _cipherService.SaveWithServerAsync(encrypted); + ClipLogger.Log($"[Fido2Authenticator] Selected fido2 cred RPID {selectedFido2Credential.RpId}"); + ClipLogger.Log($"[Fido2Authenticator] param RpId {assertionParams.RpId}"); + var authenticatorData = await GenerateAuthDataAsync( rpId: selectedFido2Credential.RpId, userPresence: true, @@ -245,7 +255,7 @@ namespace Bit.Core.Services { SelectedCredential = new Fido2AuthenticatorGetAssertionSelectedCredential { - Id = GuidToRawFormat(selectedCredentialId), + Id = selectedCredentialId.GuidToRawFormat(), UserHandle = selectedFido2Credential.UserHandleValue }, AuthenticatorData = authenticatorData, @@ -265,7 +275,7 @@ namespace Bit.Core.Services { var credentials = (await FindCredentialsByRpAsync(rpId)).Select(cipher => new Fido2AuthenticatorDiscoverableCredentialMetadata { Type = "public-key", - Id = GuidToRawFormat(cipher.Login.MainFido2Credential.CredentialId), + Id = cipher.Login.MainFido2Credential.CredentialId.GuidToRawFormat(), RpId = cipher.Login.MainFido2Credential.RpId, UserHandle = cipher.Login.MainFido2Credential.UserHandleValue, UserName = cipher.Login.MainFido2Credential.UserName @@ -290,7 +300,7 @@ namespace Bit.Core.Services { try { - ids.Add(GuidToStandardFormat(credential.Id)); + ids.Add(credential.Id.GuidToStandardFormat()); } catch {} } @@ -320,15 +330,8 @@ namespace Bit.Core.Services { try { - if (credential.IdStr != null) - { - ClipLogger.Log($"[Fido2Authenticator] FindCredentialsByIdAsync -> Adding credID: {credential.IdStr}"); - ids.Add(credential.IdStr); - continue; - } - ClipLogger.Log($"[Fido2Authenticator] FindCredentialsByIdAsync -> Converting Guid byte length: {credential.Id.Length}"); - ids.Add(GuidToStandardFormat(credential.Id)); + ids.Add(credential.Id.GuidToStandardFormat()); } catch(Exception ex) { @@ -384,16 +387,16 @@ namespace Bit.Core.Services { return new Fido2CredentialView { CredentialId = Guid.NewGuid().ToString(), - KeyType = Bit.Core.Constants.DefaultFido2CredentialType, - KeyAlgorithm = Bit.Core.Constants.DefaultFido2CredentialAlgorithm, - KeyCurve = Bit.Core.Constants.DefaultFido2CredentialCurve, + KeyType = Constants.DefaultFido2CredentialType, + KeyAlgorithm = Constants.DefaultFido2CredentialAlgorithm, + KeyCurve = Constants.DefaultFido2CredentialCurve, KeyValue = CoreHelpers.Base64UrlEncode(privateKey), RpId = makeCredentialsParams.RpEntity.Id, UserHandle = CoreHelpers.Base64UrlEncode(makeCredentialsParams.UserEntity.Id), UserName = makeCredentialsParams.UserEntity.Name, CounterValue = 0, RpName = makeCredentialsParams.RpEntity.Name, - // UserDisplayName = makeCredentialsParams.UserEntity.DisplayName, + UserDisplayName = makeCredentialsParams.UserEntity.DisplayName, DiscoverableValue = makeCredentialsParams.RequireResidentKey, CreationDate = DateTime.Now }; @@ -414,6 +417,7 @@ namespace Bit.Core.Services var rpIdHash = await _cryptoFunctionService.HashAsync(rpId, CryptoHashAlgorithm.Sha256); authData.AddRange(rpIdHash); + ClipLogger.Log($"[Fido2Authenticator] GenerateAuthDataAsync -> rpIdHash: {rpIdHash}"); ClipLogger.Log($"[Fido2Authenticator] GenerateAuthDataAsync -> ad: {isAttestation} - uv: {userVerification} - up: {userPresence}"); @@ -520,16 +524,6 @@ namespace Bit.Core.Services return dsa.SignData(sigBase, HashAlgorithmName.SHA256, DSASignatureFormat.Rfc3279DerSequence); } - private string GuidToStandardFormat(byte[] bytes) - { - return new Guid(bytes).ToString(); - } - - private byte[] GuidToRawFormat(string guid) - { - return Guid.Parse(guid).ToByteArray(); - } - private class PublicKey { private readonly ECDsa _dsa; diff --git a/src/Core/Utilities/Fido2/PublicKeyCredentialDescriptor.cs b/src/Core/Utilities/Fido2/PublicKeyCredentialDescriptor.cs index 557695a85..16fa9e698 100644 --- a/src/Core/Utilities/Fido2/PublicKeyCredentialDescriptor.cs +++ b/src/Core/Utilities/Fido2/PublicKeyCredentialDescriptor.cs @@ -2,7 +2,6 @@ namespace Bit.Core.Utilities.Fido2 { public class PublicKeyCredentialDescriptor { public byte[] Id { get; set; } - public string IdStr { get; set; } public string[] Transports { get; set; } public string Type { get; set; } } diff --git a/src/Core/Utilities/GuidExtensions.cs b/src/Core/Utilities/GuidExtensions.cs new file mode 100644 index 000000000..8e7574ef6 --- /dev/null +++ b/src/Core/Utilities/GuidExtensions.cs @@ -0,0 +1,15 @@ +namespace Bit.Core.Utilities +{ + public static class GuidExtensions + { + public static string GuidToStandardFormat(this byte[] bytes) + { + return new Guid(bytes).ToString(); + } + + public static byte[] GuidToRawFormat(this string guid) + { + return Guid.Parse(guid).ToByteArray(); + } + } +} diff --git a/src/iOS.Autofill/CredentialProviderViewController.Passkeys.cs b/src/iOS.Autofill/CredentialProviderViewController.Passkeys.cs index 5999c68c4..b51827c07 100644 --- a/src/iOS.Autofill/CredentialProviderViewController.Passkeys.cs +++ b/src/iOS.Autofill/CredentialProviderViewController.Passkeys.cs @@ -14,7 +14,6 @@ using Foundation; using Microsoft.Maui.ApplicationModel; using ObjCRuntime; using UIKit; -using Vision; namespace Bit.iOS.Autofill { @@ -110,10 +109,13 @@ namespace Bit.iOS.Autofill UserEntity = new PublicKeyCredentialUserEntity { Id = credIdentity.UserHandle.ToArray(), - Name = credIdentity.UserName + Name = credIdentity.UserName, + DisplayName = credIdentity.UserName } }); + await ASHelpers.ReplaceAllIdentitiesAsync(); + ClipLogger.Log($"PIFPR Completing"); ClipLogger.Log($"PIFPR Completing - RpId: {credIdentity.RelyingPartyIdentifier}"); ClipLogger.Log($"PIFPR Completing - CDH: {passkeyRegistrationRequest.ClientDataHash.GetBase64EncodedString(NSDataBase64EncodingOptions.None)}"); @@ -127,15 +129,6 @@ namespace Bit.iOS.Autofill NSData.FromArray(result.AttestationObject))); ClipLogger.Log($"CompleteRegistrationRequestAsync: {expired}"); - - //else if (await IsLocked()) - //{ - // PerformSegue("lockPasswordSegue", this); - //} - //else - //{ - // PerformSegue("loginListSegue", this); - //} } private PublicKeyCredentialParameters[] GetCredTypesAndPubKeyAlgs(NSNumber[] supportedAlgorithms) @@ -204,14 +197,14 @@ namespace Bit.iOS.Autofill var fido2AssertionResult = await Fido2AuthService.GetAssertionAsync(new Bit.Core.Utilities.Fido2.Fido2AuthenticatorGetAssertionParams { RpId = rpId, - Hash = _context.PasskeyCredentialRequest.ClientDataHash.ToByteArray(), + Hash = _context.PasskeyCredentialRequest.ClientDataHash.ToArray(), RequireUserVerification = _context.PasskeyCredentialRequest.UserVerificationPreference == "required", RequireUserPresence = false, AllowCredentialDescriptorList = new Bit.Core.Utilities.Fido2.PublicKeyCredentialDescriptor[] { new Bit.Core.Utilities.Fido2.PublicKeyCredentialDescriptor { - IdStr = credentialIdData.ToString() + Id = credentialIdData.ToArray() } } }); @@ -227,7 +220,7 @@ namespace Bit.iOS.Autofill ClipLogger.Log("selectedUserHandleData:" + selectedUserHandleData); var selectedCredentialIdData = fido2AssertionResult.SelectedCredential != null - ? new Guid(fido2AssertionResult.SelectedCredential.Id).ToString() + ? NSData.FromArray(fido2AssertionResult.SelectedCredential.Id) : credentialIdData; ClipLogger.Log("selectedCredentialIdData:" + selectedCredentialIdData); diff --git a/src/iOS.Autofill/CredentialProviderViewController.cs b/src/iOS.Autofill/CredentialProviderViewController.cs index bb2e49a72..c5e98f167 100644 --- a/src/iOS.Autofill/CredentialProviderViewController.cs +++ b/src/iOS.Autofill/CredentialProviderViewController.cs @@ -509,7 +509,7 @@ namespace Bit.iOS.Autofill try { ClipLogger.Log("ProvideCredentialAsync"); - + if (_context.IsPasskey && UIDevice.CurrentDevice.CheckSystemVersion(17, 0)) { if (_context.PasskeyCredentialIdentity is null) diff --git a/src/iOS.Core/Utilities/ASHelpers.cs b/src/iOS.Core/Utilities/ASHelpers.cs index 0f2a0fdf0..22fcf7e7a 100644 --- a/src/iOS.Core/Utilities/ASHelpers.cs +++ b/src/iOS.Core/Utilities/ASHelpers.cs @@ -3,6 +3,7 @@ using Bit.Core.Abstractions; using Bit.Core.Enums; using Bit.Core.Models.View; using Bit.Core.Utilities; +using Foundation; using UIKit; namespace Bit.iOS.Core.Utilities @@ -146,7 +147,7 @@ namespace Bit.iOS.Core.Utilities return new ASPasskeyCredentialIdentity(cipher.Login.MainFido2Credential.RpId, cipher.Login.MainFido2Credential.UserName, - cipher.Login.MainFido2Credential.CredentialId, + NSData.FromArray(cipher.Login.MainFido2Credential.CredentialId.GuidToRawFormat()), cipher.Login.MainFido2Credential.UserHandle, cipher.Id); }