diff --git a/chrome/browser/webauthn/enclave_authenticator_browsertest.cc b/chrome/browser/webauthn/enclave_authenticator_browsertest.cc index 69e4196107271..3f5f44899d073 100644 --- a/chrome/browser/webauthn/enclave_authenticator_browsertest.cc +++ b/chrome/browser/webauthn/enclave_authenticator_browsertest.cc @@ -87,6 +87,7 @@ #include "components/trusted_vault/securebox.h" #include "components/trusted_vault/test/mock_trusted_vault_connection.h" #include "components/trusted_vault/trusted_vault_connection.h" +#include "components/trusted_vault/trusted_vault_server_constants.h" #include "components/webauthn/core/browser/passkey_model.h" #include "components/webauthn/core/browser/passkey_model_change.h" #include "content/public/browser/storage_partition.h" @@ -3307,7 +3308,8 @@ IN_PROC_BROWSER_TEST_F(EnclaveICloudRecoveryKeyTest, Enroll) { std::vector<std::unique_ptr<device::enclave::ICloudRecoveryKey>>> future; device::enclave::ICloudRecoveryKey::Retrieve( - future.GetCallback(), kICloudKeychainRecoveryKeyAccessGroup); + future.GetCallback(), trusted_vault::SecurityDomainId::kPasskeys, + kICloudKeychainRecoveryKeyAccessGroup); EXPECT_TRUE(future.Wait()); std::vector<std::unique_ptr<device::enclave::ICloudRecoveryKey>> recovery_keys = future.Take(); @@ -3329,7 +3331,8 @@ IN_PROC_BROWSER_TEST_F(EnclaveICloudRecoveryKeyTest, base::test::TestFuture<std::unique_ptr<device::enclave::ICloudRecoveryKey>> future; device::enclave::ICloudRecoveryKey::Create( - future.GetCallback(), kICloudKeychainRecoveryKeyAccessGroup); + future.GetCallback(), trusted_vault::SecurityDomainId::kPasskeys, + kICloudKeychainRecoveryKeyAccessGroup); EXPECT_TRUE(future.Wait()); std::unique_ptr<device::enclave::ICloudRecoveryKey> existing_icloud_key = future.Take(); @@ -3383,7 +3386,8 @@ IN_PROC_BROWSER_TEST_F(EnclaveICloudRecoveryKeyTest, std::vector<std::unique_ptr<device::enclave::ICloudRecoveryKey>>> list_future; device::enclave::ICloudRecoveryKey::Retrieve( - list_future.GetCallback(), kICloudKeychainRecoveryKeyAccessGroup); + list_future.GetCallback(), trusted_vault::SecurityDomainId::kPasskeys, + kICloudKeychainRecoveryKeyAccessGroup); EXPECT_TRUE(list_future.Wait()); std::vector<std::unique_ptr<device::enclave::ICloudRecoveryKey>> recovery_keys = list_future.Take(); @@ -3430,7 +3434,8 @@ IN_PROC_BROWSER_TEST_F(EnclaveICloudRecoveryKeyTest, DISABLED_Recovery) { std::vector<std::unique_ptr<device::enclave::ICloudRecoveryKey>>> future; device::enclave::ICloudRecoveryKey::Retrieve( - future.GetCallback(), kICloudKeychainRecoveryKeyAccessGroup); + future.GetCallback(), trusted_vault::SecurityDomainId::kPasskeys, + kICloudKeychainRecoveryKeyAccessGroup); EXPECT_TRUE(future.Wait()); ASSERT_EQ(future.Get().size(), 1u); } @@ -3524,7 +3529,8 @@ IN_PROC_BROWSER_TEST_F(EnclaveICloudRecoveryKeyTest, DISABLED_Recovery) { std::vector<std::unique_ptr<device::enclave::ICloudRecoveryKey>>> future; device::enclave::ICloudRecoveryKey::Retrieve( - future.GetCallback(), kICloudKeychainRecoveryKeyAccessGroup); + future.GetCallback(), trusted_vault::SecurityDomainId::kPasskeys, + kICloudKeychainRecoveryKeyAccessGroup); EXPECT_TRUE(future.Wait()); ASSERT_EQ(future.Get().size(), 1u); } diff --git a/chrome/browser/webauthn/gpm_enclave_controller.cc b/chrome/browser/webauthn/gpm_enclave_controller.cc index ca518d0d242ec..e8dd446712360 100644 --- a/chrome/browser/webauthn/gpm_enclave_controller.cc +++ b/chrome/browser/webauthn/gpm_enclave_controller.cc @@ -687,6 +687,7 @@ void GPMEnclaveController::RecoverSecurityDomain() { device::enclave::ICloudRecoveryKey::Retrieve( base::BindOnce(&GPMEnclaveController::OnICloudKeysRetrievedForRecovery, weak_ptr_factory_.GetWeakPtr()), + trusted_vault::SecurityDomainId::kPasskeys, kICloudKeychainRecoveryKeyAccessGroup); #else model_->SetStep(Step::kRecoverSecurityDomain); @@ -699,6 +700,7 @@ void GPMEnclaveController::MaybeAddICloudRecoveryKey() { device::enclave::ICloudRecoveryKey::Retrieve( base::BindOnce(&GPMEnclaveController::OnICloudKeysRetrievedForEnrollment, weak_ptr_factory_.GetWeakPtr()), + trusted_vault::SecurityDomainId::kPasskeys, kICloudKeychainRecoveryKeyAccessGroup); } @@ -730,6 +732,7 @@ void GPMEnclaveController::OnICloudKeysRetrievedForEnrollment( device::enclave::ICloudRecoveryKey::Create( base::BindOnce(&GPMEnclaveController::EnrollICloudRecoveryKey, weak_ptr_factory_.GetWeakPtr()), + trusted_vault::SecurityDomainId::kPasskeys, kICloudKeychainRecoveryKeyAccessGroup); } diff --git a/device/fido/enclave/icloud_recovery_key_mac.h b/device/fido/enclave/icloud_recovery_key_mac.h index 36614cbd069c3..6e0eba3201826 100644 --- a/device/fido/enclave/icloud_recovery_key_mac.h +++ b/device/fido/enclave/icloud_recovery_key_mac.h @@ -16,6 +16,8 @@ namespace trusted_vault { class SecureBoxKeyPair; + +enum class SecurityDomainId; } // namespace trusted_vault namespace device::enclave { @@ -36,10 +38,12 @@ class COMPONENT_EXPORT(DEVICE_FIDO) ICloudRecoveryKey { // Creates, stores in iCloud Keychain, and returns an ICloudRecoveryKey. static void Create(CreateCallback callback, + trusted_vault::SecurityDomainId security_domain_id, std::string_view keychain_access_group); // Retrieves from iCloud Keychain all ICloudRecoveryKeys. static void Retrieve(RetrieveCallback callback, + trusted_vault::SecurityDomainId security_domain_id, std::string_view keychain_access_group); // Randomly generates an ICloudRecoveryKey that is not persisted to the @@ -52,10 +56,12 @@ class COMPONENT_EXPORT(DEVICE_FIDO) ICloudRecoveryKey { private: // Like |Create| but synchronous. static std::unique_ptr<ICloudRecoveryKey> CreateAndStoreKeySlowly( + trusted_vault::SecurityDomainId security_domain_id, std::string_view keychain_access_group); // Like |Retrieve| but synchronous. static std::vector<std::unique_ptr<ICloudRecoveryKey>> RetrieveKeysSlowly( + trusted_vault::SecurityDomainId security_domain_id, std::string_view keychain_access_group); explicit ICloudRecoveryKey( diff --git a/device/fido/enclave/icloud_recovery_key_mac.mm b/device/fido/enclave/icloud_recovery_key_mac.mm index 88319b261dad4..0f6f6881e7704 100644 --- a/device/fido/enclave/icloud_recovery_key_mac.mm +++ b/device/fido/enclave/icloud_recovery_key_mac.mm @@ -26,6 +26,7 @@ #include "base/task/thread_pool.h" #include "components/device_event_log/device_event_log.h" #include "components/trusted_vault/securebox.h" +#include "components/trusted_vault/trusted_vault_server_constants.h" #include "crypto/apple_keychain_v2.h" namespace device::enclave { @@ -41,12 +42,24 @@ using base::apple::NSToCFPtrCast; constexpr char kAttrLegacyService[] = "com.google.common.folsom.cloud.private"; constexpr char kAttrHwProtectedService[] = "com.google.common.folsom.cloud.private.hw_protected"; +constexpr char kAttrChromeSyncService[] = + "com.google.common.folsom.cloud.private.chromesync"; // The value for kSecAttrType for all folsom data on the keychain. This is to // ensure only Folsom data is returned from keychain queries, even when the // access group is not set. static const uint kSecAttrTypeFolsom = 'flsm'; +std::string GetKeychainService( + trusted_vault::SecurityDomainId security_domain) { + switch (security_domain) { + case trusted_vault::SecurityDomainId::kChromeSync: + return kAttrChromeSyncService; + case trusted_vault::SecurityDomainId::kPasskeys: + return kAttrHwProtectedService; + } +} + // Returns the public key in uncompressed x9.62 format encoded in padded base64. NSString* EncodePublicKey(const trusted_vault::SecureBoxPublicKey& public_key) { return base::SysUTF8ToNSString( @@ -61,10 +74,10 @@ NSData* EncodePrivateKey( } NSMutableDictionary* GetDefaultQuery(std::string_view keychain_access_group, - std::string_view service) { + std::string_view keychain_service) { return [NSMutableDictionary dictionaryWithDictionary:@{ CFToNSPtrCast(kSecAttrSynchronizable) : @YES, - CFToNSPtrCast(kSecAttrService) : base::SysUTF8ToNSString(service), + CFToNSPtrCast(kSecAttrService) : base::SysUTF8ToNSString(keychain_service), CFToNSPtrCast(kSecClass) : CFToNSPtrCast(kSecClassGenericPassword), CFToNSPtrCast(kSecAttrType) : @(kSecAttrTypeFolsom), CFToNSPtrCast(kSecAttrAccessGroup) : @@ -76,8 +89,9 @@ NSMutableDictionary* GetDefaultQuery(std::string_view keychain_access_group, std::vector<std::unique_ptr<trusted_vault::SecureBoxKeyPair>> RetrieveKeysInternal(std::string_view keychain_access_group, - std::string_view service) { - NSDictionary* query = GetDefaultQuery(keychain_access_group, service); + std::string_view keychain_service) { + NSDictionary* query = + GetDefaultQuery(keychain_access_group, keychain_service); [query setValuesForKeysWithDictionary:@{ CFToNSPtrCast(kSecMatchLimit) : CFToNSPtrCast(kSecMatchLimitAll), CFToNSPtrCast(kSecReturnData) : @YES, @@ -123,8 +137,10 @@ ICloudRecoveryKey::ICloudRecoveryKey( ICloudRecoveryKey::~ICloudRecoveryKey() = default; // static -void ICloudRecoveryKey::Create(CreateCallback callback, - std::string_view keychain_access_group) { +void ICloudRecoveryKey::Create( + CreateCallback callback, + trusted_vault::SecurityDomainId security_domain_id, + std::string_view keychain_access_group) { // Creating a key requires disk access. Do it in a dedicated thread. scoped_refptr<base::SequencedTaskRunner> worker_task_runner = base::ThreadPool::CreateSequencedTaskRunner( @@ -134,14 +150,16 @@ void ICloudRecoveryKey::Create(CreateCallback callback, std::string keychain_access_group_copy(keychain_access_group); worker_task_runner->PostTaskAndReplyWithResult( FROM_HERE, - base::BindOnce(&CreateAndStoreKeySlowly, + base::BindOnce(&CreateAndStoreKeySlowly, security_domain_id, std::move(keychain_access_group_copy)), std::move(callback)); } // static -void ICloudRecoveryKey::Retrieve(RetrieveCallback callback, - std::string_view keychain_access_group) { +void ICloudRecoveryKey::Retrieve( + RetrieveCallback callback, + trusted_vault::SecurityDomainId security_domain_id, + std::string_view keychain_access_group) { // Retrieving keys requires disk access. Do it in a dedicated thread. scoped_refptr<base::SequencedTaskRunner> worker_task_runner = base::ThreadPool::CreateSequencedTaskRunner( @@ -151,7 +169,7 @@ void ICloudRecoveryKey::Retrieve(RetrieveCallback callback, std::string keychain_access_group_copy(keychain_access_group); worker_task_runner->PostTaskAndReplyWithResult( FROM_HERE, - base::BindOnce(&RetrieveKeysSlowly, + base::BindOnce(&RetrieveKeysSlowly, security_domain_id, std::move(keychain_access_group_copy)), std::move(callback)); } @@ -164,12 +182,13 @@ std::unique_ptr<ICloudRecoveryKey> ICloudRecoveryKey::CreateForTest() { // static std::unique_ptr<ICloudRecoveryKey> ICloudRecoveryKey::CreateAndStoreKeySlowly( + trusted_vault::SecurityDomainId security_domain_id, std::string_view keychain_access_group) { std::unique_ptr<trusted_vault::SecureBoxKeyPair> key = trusted_vault::SecureBoxKeyPair::GenerateRandom(); - NSMutableDictionary* attributes = - GetDefaultQuery(keychain_access_group, kAttrHwProtectedService); + NSMutableDictionary* attributes = GetDefaultQuery( + keychain_access_group, GetKeychainService(security_domain_id)); [attributes setValuesForKeysWithDictionary:@{ CFToNSPtrCast(kSecAttrAccount) : EncodePublicKey(key->public_key()), CFToNSPtrCast(kSecValueData) : EncodePrivateKey(key->private_key()), @@ -186,9 +205,12 @@ std::unique_ptr<ICloudRecoveryKey> ICloudRecoveryKey::CreateAndStoreKeySlowly( // static std::vector<std::unique_ptr<ICloudRecoveryKey>> -ICloudRecoveryKey::RetrieveKeysSlowly(std::string_view keychain_access_group) { +ICloudRecoveryKey::RetrieveKeysSlowly( + trusted_vault::SecurityDomainId security_domain_id, + std::string_view keychain_access_group) { std::vector<std::unique_ptr<trusted_vault::SecureBoxKeyPair>> hw_keys = - RetrieveKeysInternal(keychain_access_group, kAttrHwProtectedService); + RetrieveKeysInternal(keychain_access_group, + GetKeychainService(security_domain_id)); // Keys created before M131 use the "legacy" service tag. std::vector<std::unique_ptr<trusted_vault::SecureBoxKeyPair>> legacy_keys = RetrieveKeysInternal(keychain_access_group, kAttrLegacyService); diff --git a/device/fido/enclave/icloud_recovery_key_mac_unittest.mm b/device/fido/enclave/icloud_recovery_key_mac_unittest.mm index 4dda464fe10bb..88cb985df1145 100644 --- a/device/fido/enclave/icloud_recovery_key_mac_unittest.mm +++ b/device/fido/enclave/icloud_recovery_key_mac_unittest.mm @@ -19,6 +19,7 @@ #include "base/test/bind.h" #include "base/test/task_environment.h" #include "components/trusted_vault/securebox.h" +#include "components/trusted_vault/trusted_vault_server_constants.h" #include "crypto/apple_keychain_v2.h" #include "crypto/fake_apple_keychain_v2.h" #include "crypto/scoped_fake_apple_keychain_v2.h" @@ -35,7 +36,8 @@ constexpr uint8_t kPlaintext[]{'h', 'e', 'l', 'l', 'o'}; class ICloudRecoveryKeyTest : public testing::Test { public: - std::unique_ptr<ICloudRecoveryKey> CreateKey() { + std::unique_ptr<ICloudRecoveryKey> CreateKey( + const trusted_vault::SecurityDomainId security_domain_id) { std::unique_ptr<ICloudRecoveryKey> new_key; base::RunLoop run_loop; ICloudRecoveryKey::Create( @@ -43,7 +45,7 @@ class ICloudRecoveryKeyTest : public testing::Test { new_key = std::move(ret); run_loop.Quit(); }), - kKeychainAccessGroup); + security_domain_id, kKeychainAccessGroup); run_loop.Run(); return new_key; } @@ -54,7 +56,8 @@ class ICloudRecoveryKeyTest : public testing::Test { }; TEST_F(ICloudRecoveryKeyTest, EndToEnd) { - std::unique_ptr<ICloudRecoveryKey> key = CreateKey(); + std::unique_ptr<ICloudRecoveryKey> key = + CreateKey(trusted_vault::SecurityDomainId::kPasskeys); std::optional<std::vector<uint8_t>> encrypted = key->key()->public_key().Encrypt(base::span<uint8_t>(), kHeader, kPlaintext); @@ -69,7 +72,7 @@ TEST_F(ICloudRecoveryKeyTest, EndToEnd) { retrieved = std::move(ret.at(0)); run_loop.Quit(); }), - kKeychainAccessGroup); + trusted_vault::SecurityDomainId::kPasskeys, kKeychainAccessGroup); run_loop.Run(); std::optional<std::vector<uint8_t>> decrypted = @@ -81,11 +84,13 @@ TEST_F(ICloudRecoveryKeyTest, EndToEnd) { } TEST_F(ICloudRecoveryKeyTest, CreateAndRetrieve) { - std::unique_ptr<ICloudRecoveryKey> key1 = CreateKey(); + std::unique_ptr<ICloudRecoveryKey> key1 = + CreateKey(trusted_vault::SecurityDomainId::kPasskeys); ASSERT_TRUE(key1); ASSERT_TRUE(key1->key()); - std::unique_ptr<ICloudRecoveryKey> key2 = CreateKey(); + std::unique_ptr<ICloudRecoveryKey> key2 = + CreateKey(trusted_vault::SecurityDomainId::kPasskeys); ASSERT_TRUE(key2); ASSERT_TRUE(key2->key()); @@ -97,7 +102,7 @@ TEST_F(ICloudRecoveryKeyTest, CreateAndRetrieve) { keys = std::move(ret); run_loop.Quit(); }), - kKeychainAccessGroup); + trusted_vault::SecurityDomainId::kPasskeys, kKeychainAccessGroup); run_loop.Run(); ASSERT_TRUE(keys); @@ -119,11 +124,13 @@ TEST_F(ICloudRecoveryKeyTest, CreateAndRetrieve) { // Verify that keys are stored using the new .hw_protected kSecAttrService, but // old keys without it can still be retrieved. TEST_F(ICloudRecoveryKeyTest, RetrieveWithLegacyAttributes) { - std::unique_ptr<ICloudRecoveryKey> key1 = CreateKey(); + std::unique_ptr<ICloudRecoveryKey> key1 = + CreateKey(trusted_vault::SecurityDomainId::kPasskeys); ASSERT_TRUE(key1); ASSERT_TRUE(key1->key()); - std::unique_ptr<ICloudRecoveryKey> key2 = CreateKey(); + std::unique_ptr<ICloudRecoveryKey> key2 = + CreateKey(trusted_vault::SecurityDomainId::kPasskeys); ASSERT_TRUE(key2); ASSERT_TRUE(key2->key()); @@ -146,7 +153,7 @@ TEST_F(ICloudRecoveryKeyTest, RetrieveWithLegacyAttributes) { keys = std::move(ret); run_loop.Quit(); }), - kKeychainAccessGroup); + trusted_vault::SecurityDomainId::kPasskeys, kKeychainAccessGroup); run_loop.Run(); ASSERT_TRUE(keys); @@ -155,7 +162,8 @@ TEST_F(ICloudRecoveryKeyTest, RetrieveWithLegacyAttributes) { // Tests that keys belonging to other security domains are not retrieved. TEST_F(ICloudRecoveryKeyTest, IgnoreOtherSecurityDomains) { - std::unique_ptr<ICloudRecoveryKey> key1 = CreateKey(); + std::unique_ptr<ICloudRecoveryKey> key1 = + CreateKey(trusted_vault::SecurityDomainId::kPasskeys); ASSERT_TRUE(key1); ASSERT_TRUE(key1->key()); @@ -174,13 +182,59 @@ TEST_F(ICloudRecoveryKeyTest, IgnoreOtherSecurityDomains) { keys = std::move(ret); run_loop.Quit(); }), - kKeychainAccessGroup); + trusted_vault::SecurityDomainId::kPasskeys, kKeychainAccessGroup); run_loop.Run(); ASSERT_TRUE(keys); EXPECT_TRUE(keys->empty()); } +TEST_F(ICloudRecoveryKeyTest, MultipleSecurityDomains) { + std::unique_ptr<ICloudRecoveryKey> passkeys_key = + CreateKey(trusted_vault::SecurityDomainId::kPasskeys); + ASSERT_TRUE(passkeys_key); + ASSERT_TRUE(passkeys_key->key()); + + std::unique_ptr<ICloudRecoveryKey> chromesync_key = + CreateKey(trusted_vault::SecurityDomainId::kChromeSync); + ASSERT_TRUE(chromesync_key); + ASSERT_TRUE(chromesync_key->key()); + + { + std::optional<std::vector<std::unique_ptr<ICloudRecoveryKey>>> keys; + base::RunLoop run_loop; + ICloudRecoveryKey::Retrieve( + base::BindLambdaForTesting( + [&](std::vector<std::unique_ptr<ICloudRecoveryKey>> ret) { + keys = std::move(ret); + run_loop.Quit(); + }), + trusted_vault::SecurityDomainId::kPasskeys, kKeychainAccessGroup); + run_loop.Run(); + + ASSERT_TRUE(keys); + ASSERT_EQ(keys->size(), 1u); + EXPECT_EQ((*keys)[0]->id(), passkeys_key->id()); + } + + { + std::optional<std::vector<std::unique_ptr<ICloudRecoveryKey>>> keys; + base::RunLoop run_loop; + ICloudRecoveryKey::Retrieve( + base::BindLambdaForTesting( + [&](std::vector<std::unique_ptr<ICloudRecoveryKey>> ret) { + keys = std::move(ret); + run_loop.Quit(); + }), + trusted_vault::SecurityDomainId::kChromeSync, kKeychainAccessGroup); + run_loop.Run(); + + ASSERT_TRUE(keys); + ASSERT_EQ(keys->size(), 1u); + EXPECT_EQ((*keys)[0]->id(), chromesync_key->id()); + } +} + TEST_F(ICloudRecoveryKeyTest, CreateKeychainError) { // Force a keychain error by setting the wrong access group. std::unique_ptr<ICloudRecoveryKey> keys; @@ -190,7 +244,7 @@ TEST_F(ICloudRecoveryKeyTest, CreateKeychainError) { keys = std::move(ret); run_loop.Quit(); }), - "wrong keychain group"); + trusted_vault::SecurityDomainId::kPasskeys, "wrong keychain group"); run_loop.Run(); EXPECT_FALSE(keys); } @@ -205,7 +259,7 @@ TEST_F(ICloudRecoveryKeyTest, RetrieveKeychainError) { keys = std::move(ret); run_loop.Quit(); }), - "wrong keychain group"); + trusted_vault::SecurityDomainId::kPasskeys, "wrong keychain group"); run_loop.Run(); EXPECT_TRUE(keys->empty()); } @@ -219,13 +273,14 @@ TEST_F(ICloudRecoveryKeyTest, RetrieveEmpty) { keys = std::move(ret); run_loop.Quit(); }), - kKeychainAccessGroup); + trusted_vault::SecurityDomainId::kPasskeys, kKeychainAccessGroup); run_loop.Run(); EXPECT_TRUE(keys->empty()); } TEST_F(ICloudRecoveryKeyTest, RetrieveCorrupted) { - std::unique_ptr<ICloudRecoveryKey> key1 = CreateKey(); + std::unique_ptr<ICloudRecoveryKey> key1 = + CreateKey(trusted_vault::SecurityDomainId::kPasskeys); ASSERT_TRUE(key1); ASSERT_TRUE(key1->key()); @@ -243,7 +298,7 @@ TEST_F(ICloudRecoveryKeyTest, RetrieveCorrupted) { keys = std::move(ret); run_loop.Quit(); }), - kKeychainAccessGroup); + trusted_vault::SecurityDomainId::kPasskeys, kKeychainAccessGroup); run_loop.Run(); EXPECT_TRUE(keys->empty()); }