0

Make ICloudRecoveryKey support arbitrary security domains.

This will allow the Keychain client implementation in
components/trusted_vault to reuse ICloudRecoveryKey. That will help to
support the iCloud Keychain as recovery factor on MacOS for the
chromesync security domain.

Bug: b:398160323
Change-Id: Ie07fc409d68e4efa780e38fc28fe1625e59919f9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6382150
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: Nina Satragno <nsatragno@chromium.org>
Commit-Queue: Thomas Thrainer <thomasth@google.com>
Cr-Commit-Position: refs/heads/main@{#1439389}
This commit is contained in:
Thomas Thrainer 2025-03-28 07:23:15 -07:00 committed by Chromium LUCI CQ
parent 730a98cfbc
commit 648be12d12
5 changed files with 128 additions and 36 deletions

@ -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);
}

@ -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);
}

@ -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(

@ -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);

@ -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());
}