From 49d90ea1fbcbe0eb8d9cdcb9de2fb0179dc9ef1b Mon Sep 17 00:00:00 2001
From: Jan Wilken Dörrie <jdoerrie@chromium.org>
Date: Thu, 25 Jun 2026 02:04:00 -0700
Subject: [PATCH] [crypto] Restrict keychain queries on macOS

Ensure that keychain queries for unexportable keys are scoped to the
data protection keychain.

Bug: 520516655
Change-Id: I67fe92097b757d97549235ad5c4f2a7c6a6a6964
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7966018
Reviewed-by: Elly <ellyjones@chromium.org>
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1652247}
---

diff --git a/crypto/apple/unexportable_key_apple.mm b/crypto/apple/unexportable_key_apple.mm
index f7abf443..f8f9cd9 100644
--- a/crypto/apple/unexportable_key_apple.mm
+++ b/crypto/apple/unexportable_key_apple.mm
@@ -150,6 +150,7 @@
 FindUnexportableKeys(FindUnexportableKeysOptions options) {
   auto [access_group, application_tag_prefix, wrapped_key, lacontext] = options;
   NSMutableDictionary* query = [NSMutableDictionary dictionaryWithDictionary:@{
+    CFToNSPtrCast(kSecUseDataProtectionKeychain) : @YES,
     CFToNSPtrCast(kSecClass) : CFToNSPtrCast(kSecClassKey),
     CFToNSPtrCast(kSecAttrKeyType) :
         CFToNSPtrCast(kSecAttrKeyTypeECSECPrimeRandom),
diff --git a/crypto/apple/unexportable_key_apple_unittest.mm b/crypto/apple/unexportable_key_apple_unittest.mm
index e26809c6..769b86a 100644
--- a/crypto/apple/unexportable_key_apple_unittest.mm
+++ b/crypto/apple/unexportable_key_apple_unittest.mm
@@ -9,6 +9,8 @@
 #include <cstdint>
 #include <vector>
 
+#include "base/apple/foundation_util.h"
+#include "base/check_deref.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/time/time.h"
 #include "base/time/time_override.h"
@@ -673,6 +675,47 @@
       key_specific->GetWrappedKey()));
 }
 
+class ScopedMockKeychain : public crypto::apple::FakeKeychainV2 {
+ public:
+  explicit ScopedMockKeychain(const std::string& keychain_access_group)
+      : FakeKeychainV2(keychain_access_group) {
+    SetInstanceOverride(this);
+  }
+
+  ~ScopedMockKeychain() override { ClearInstanceOverride(); }
+
+  MOCK_METHOD(OSStatus,
+              ItemCopyMatching,
+              (CFDictionaryRef query, CFTypeRef* result),
+              (override));
+};
+
+// Setting `kSecUseDataProtectionKeychain` to `@YES` is required to explicitly
+// restrict the keychain search to the modern Data Protection keychain,
+// preventing fallback to legacy keychains where access group filtering is
+// broken. This test ensures the flag is always present in the query.
+TEST(UnexportableKeyMacFiltersTest, QueriesDataProtectionKeychain) {
+  ScopedMockKeychain mock_keychain(kTestKeychainAccessGroup);
+
+  EXPECT_CALL(mock_keychain, ItemCopyMatching)
+      .WillOnce([&mock_keychain](CFDictionaryRef query, CFTypeRef* result) {
+        auto use_data_protection =
+            base::apple::GetValueFromDictionary<CFBooleanRef>(
+                query, kSecUseDataProtectionKeychain);
+        EXPECT_TRUE(CFBooleanGetValue(use_data_protection));
+        return mock_keychain.FakeKeychainV2::ItemCopyMatching(query, result);
+      });
+
+  std::unique_ptr<UnexportableKeyProvider> provider =
+      GetUnexportableKeyProvider({
+          .keychain_access_group = kTestKeychainAccessGroup,
+          .application_tag = kTestApplicationTag,
+      });
+
+  // Trigger a query.
+  CHECK_DEREF(provider).AsStatefulUnexportableKeyProvider()->GetAllKeysSlowly();
+}
+
 }  // namespace
 
 }  // namespace crypto::apple

diff --git a/crypto/apple/unexportable_key_apple_unittest.mm b/crypto/apple/unexportable_key_apple_unittest.mm
index e26809c6..769b86a 100644
--- a/crypto/apple/unexportable_key_apple_unittest.mm
+++ b/crypto/apple/unexportable_key_apple_unittest.mm
@@ -9,6 +9,8 @@
 #include <cstdint>
 #include <vector>
 
+#include "base/apple/foundation_util.h"
+#include "base/check_deref.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/time/time.h"
 #include "base/time/time_override.h"
@@ -673,6 +675,47 @@
       key_specific->GetWrappedKey()));
 }
 
+class ScopedMockKeychain : public crypto::apple::FakeKeychainV2 {
+ public:
+  explicit ScopedMockKeychain(const std::string& keychain_access_group)
+      : FakeKeychainV2(keychain_access_group) {
+    SetInstanceOverride(this);
+  }
+
+  ~ScopedMockKeychain() override { ClearInstanceOverride(); }
+
+  MOCK_METHOD(OSStatus,
+              ItemCopyMatching,
+              (CFDictionaryRef query, CFTypeRef* result),
+              (override));
+};
+
+// Setting `kSecUseDataProtectionKeychain` to `@YES` is required to explicitly
+// restrict the keychain search to the modern Data Protection keychain,
+// preventing fallback to legacy keychains where access group filtering is
+// broken. This test ensures the flag is always present in the query.
+TEST(UnexportableKeyMacFiltersTest, QueriesDataProtectionKeychain) {
+  ScopedMockKeychain mock_keychain(kTestKeychainAccessGroup);
+
+  EXPECT_CALL(mock_keychain, ItemCopyMatching)
+      .WillOnce([&mock_keychain](CFDictionaryRef query, CFTypeRef* result) {
+        auto use_data_protection =
+            base::apple::GetValueFromDictionary<CFBooleanRef>(
+                query, kSecUseDataProtectionKeychain);
+        EXPECT_TRUE(CFBooleanGetValue(use_data_protection));
+        return mock_keychain.FakeKeychainV2::ItemCopyMatching(query, result);
+      });
+
+  std::unique_ptr<UnexportableKeyProvider> provider =
+      GetUnexportableKeyProvider({
+          .keychain_access_group = kTestKeychainAccessGroup,
+          .application_tag = kTestApplicationTag,
+      });
+
+  // Trigger a query.
+  CHECK_DEREF(provider).AsStatefulUnexportableKeyProvider()->GetAllKeysSlowly();
+}
+
 }  // namespace
 
 }  // namespace crypto::apple