Fix: GetKeyLocations returning non-existing user token

Add a special case for public_slot==system_slot in GetKeyLocations.

After the system slot is ready, SystemTokenCertDbInitializer creates an
NSSCertDatabase instance to be used as a wrapper for the system slot.
SystemTokenCertDbInitializer initializes the public slot of the created
NSSCertDatabase to be the same as the system slot just as a dummy value
because NSSCertDatabase expects public_slot to not be nullptr. This
introduced a problem while trying to retrieve key locations on login
screen, because on login screen, the NSSCertDatabase instance
initialized by SystemTokenCertDbInitializer is used in
PlatformKeysService.

Bug: 1150973
Change-Id: I58a7f968bd688ad118eb58431dad6c74a2cf4513
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2550643
Commit-Queue: Omar Morsi <omorsi@google.com>
Reviewed-by: Alexander Hendrich <hendrich@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829779}

chrome/browser/chromeos/platform_keys/platform_keys_service_browsertest.cc

@@ -619,6 +619,22 @@ IN_PROC_BROWSER_TEST_P(PlatformKeysServicePerTokenBrowserTest,
   EXPECT_FALSE(is_key_on_token_waiter.on_slot().value());
 }
 
+IN_PROC_BROWSER_TEST_P(PlatformKeysServicePerTokenBrowserTest,
+                       GetKeyLocations) {
+  const TokenId token_id = GetParam().token_id;
+
+  const std::string public_key = GenerateKeyPair(token_id);
+
+  test_util::GetKeyLocationsExecutionWaiter get_key_locations_waiter;
+  platform_keys_service()->GetKeyLocations(
+      public_key, get_key_locations_waiter.GetCallback());
+  get_key_locations_waiter.Wait();
+
+  EXPECT_EQ(get_key_locations_waiter.status(), Status::kSuccess);
+  ASSERT_EQ(get_key_locations_waiter.key_locations().size(), 1U);
+  EXPECT_EQ(get_key_locations_waiter.key_locations()[0], token_id);
+}
+
 INSTANTIATE_TEST_SUITE_P(
     AllSupportedProfilesAndTokensTypes,
     PlatformKeysServicePerTokenBrowserTest,

chrome/browser/chromeos/platform_keys/platform_keys_service_nss.cc

@@ -1309,7 +1309,11 @@ void GetKeyLocationsWithDB(std::unique_ptr<GetKeyLocationsState> state,
     if (rsa_key)
       token_ids.push_back(TokenId::kUser);
   }
-  if (token_ids.empty() && cert_db->GetPublicSlot().get()) {
+
+  // The "system" NSSCertDatabaseChromeOS instance reuses its "system slot" as
+  // "public slot", but that doesn't mean it's a user-specific slot.
+  if (token_ids.empty() && cert_db->GetPublicSlot().get() &&
+      cert_db->GetPublicSlot().get() != cert_db->GetSystemSlot().get()) {
     crypto::ScopedSECKEYPrivateKey rsa_key =
         crypto::FindNSSKeyFromPublicKeyInfoInSlot(
             public_key_vector, cert_db->GetPublicSlot().get());

chrome/browser/chromeos/platform_keys/platform_keys_service_test_util.cc

@@ -41,6 +41,9 @@ GetAllKeysExecutionWaiter::~GetAllKeysExecutionWaiter() = default;
 IsKeyOnTokenExecutionWaiter::IsKeyOnTokenExecutionWaiter() = default;
 IsKeyOnTokenExecutionWaiter::~IsKeyOnTokenExecutionWaiter() = default;
 
+GetKeyLocationsExecutionWaiter::GetKeyLocationsExecutionWaiter() = default;
+GetKeyLocationsExecutionWaiter::~GetKeyLocationsExecutionWaiter() = default;
+
 }  // namespace test_util
 }  // namespace platform_keys
 }  // namespace chromeos

chrome/browser/chromeos/platform_keys/platform_keys_service_test_util.h

@@ -183,6 +183,17 @@ class IsKeyOnTokenExecutionWaiter
   }
 };
 
+class GetKeyLocationsExecutionWaiter
+    : public ExecutionWaiter<const std::vector<TokenId>&> {
+ public:
+  GetKeyLocationsExecutionWaiter();
+  ~GetKeyLocationsExecutionWaiter();
+
+  const std::vector<TokenId>& key_locations() const {
+    return std::get<0>(result_callback_args());
+  }
+};
+
 }  // namespace test_util
 }  // namespace platform_keys
 }  // namespace chromeos