PlatformKeys: Replace std::string with std::vector<uint8_t> (8)

Replace some std::string's with std::vector<uint8_t>'s where the stored data is actually binary. (Part 8) Bug: b/192071491 Test: Compilation, *PlatformKey* browser and unit tests Change-Id: If22fe76c1b6f2a233767a5e0afa791b2e3e8e5cf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4208958 Reviewed-by: Pavol Marko <pmarko@chromium.org> Reviewed-by: Roland Bock <rbock@google.com> Commit-Queue: Michael Ershov <miersh@google.com> Cr-Commit-Position: refs/heads/main@{#1100355}
chromium · Feb 2, 2023 · 23bd6da · 23bd6da
1 parent ad7635d
commit 23bd6da
Show file tree

Hide file tree

Showing 15 changed files with 117 additions and 136 deletions.
diff --git a/chrome/browser/ash/cert_provisioning/cert_provisioning_worker_dynamic.cc b/chrome/browser/ash/cert_provisioning/cert_provisioning_worker_dynamic.cc
@@ -381,7 +381,7 @@ void CertProvisioningWorkerDynamic::GenerateRegularKey() {
 }
 
 void CertProvisioningWorkerDynamic::OnGenerateRegularKeyDone(
-    const std::string& public_key_spki_der,
+    std::vector<uint8_t> public_key_spki_der,
     chromeos::platform_keys::Status status) {
   if (status != chromeos::platform_keys::Status::kSuccess ||
       public_key_spki_der.empty()) {
@@ -394,7 +394,7 @@ void CertProvisioningWorkerDynamic::OnGenerateRegularKeyDone(
   }
 
   key_location_ = KeyLocation::kPkcs11Token;
-  public_key_ = StrToBytes(public_key_spki_der);
+  public_key_ = std::move(public_key_spki_der);
   MarkRegularKey();
 }
 

diff --git a/chrome/browser/ash/cert_provisioning/cert_provisioning_worker_dynamic.h b/chrome/browser/ash/cert_provisioning/cert_provisioning_worker_dynamic.h
@@ -64,7 +64,7 @@ class CertProvisioningWorkerDynamic : public CertProvisioningWorker {
   void GenerateKey();
 
   void GenerateRegularKey();
-  void OnGenerateRegularKeyDone(const std::string& public_key_spki_der,
+  void OnGenerateRegularKeyDone(std::vector<uint8_t> public_key_spki_der,
                                 chromeos::platform_keys::Status status);
 
   void GenerateKeyForVa();

diff --git a/chrome/browser/ash/cert_provisioning/cert_provisioning_worker_dynamic_unittest.cc b/chrome/browser/ash/cert_provisioning/cert_provisioning_worker_dynamic_unittest.cc
@@ -899,7 +899,7 @@ TEST_F(CertProvisioningWorkerDynamicTest, NoVaSuccess) {
                                /*sw_backed=*/false,
                                /*callback=*/_))
         .Times(1)
-        .WillOnce(RunOnceCallback<3>(GetPublicKey(), Status::kSuccess));
+        .WillOnce(RunOnceCallback<3>(GetPublicKeyBin(), Status::kSuccess));
 
     EXPECT_CALL(*key_permissions_manager_,
                 AllowKeyForUsage(/*callback=*/_, KeyUsage::kCorporate,
@@ -1033,7 +1033,7 @@ TEST_F(CertProvisioningWorkerDynamicTest, NoVaTooManyTwoProofsOfPossession) {
                                /*sw_backed=*/false,
                                /*callback=*/_))
         .Times(1)
-        .WillOnce(RunOnceCallback<3>(GetPublicKey(), Status::kSuccess));
+        .WillOnce(RunOnceCallback<3>(GetPublicKeyBin(), Status::kSuccess));
 
     EXPECT_CALL(*key_permissions_manager_,
                 AllowKeyForUsage(/*callback=*/_, KeyUsage::kCorporate,
@@ -1634,7 +1634,7 @@ TEST_F(CertProvisioningWorkerDynamicTest, RetryUploadProofOfPossession) {
                                /*sw_backed=*/false,
                                /*callback=*/_))
         .Times(1)
-        .WillOnce(RunOnceCallback<3>(GetPublicKey(), Status::kSuccess));
+        .WillOnce(RunOnceCallback<3>(GetPublicKeyBin(), Status::kSuccess));
 
     EXPECT_CALL(*key_permissions_manager_,
                 AllowKeyForUsage(/*callback=*/_, KeyUsage::kCorporate,

diff --git a/chrome/browser/ash/cert_provisioning/cert_provisioning_worker_static.cc b/chrome/browser/ash/cert_provisioning/cert_provisioning_worker_static.cc
@@ -383,7 +383,7 @@ void CertProvisioningWorkerStatic::GenerateRegularKey() {
 }
 
 void CertProvisioningWorkerStatic::OnGenerateRegularKeyDone(
-    const std::string& public_key_spki_der,
+    std::vector<uint8_t> public_key_spki_der,
     chromeos::platform_keys::Status status) {
   if (status != chromeos::platform_keys::Status::kSuccess ||
       public_key_spki_der.empty()) {
@@ -394,7 +394,7 @@ void CertProvisioningWorkerStatic::OnGenerateRegularKeyDone(
     return;
   }
 
-  public_key_ = StrToBytes(public_key_spki_der);
+  public_key_ = std::move(public_key_spki_der);
   UpdateState(FROM_HERE, CertProvisioningWorkerState::kKeypairGenerated);
   DoStep();
 }

diff --git a/chrome/browser/ash/cert_provisioning/cert_provisioning_worker_static.h b/chrome/browser/ash/cert_provisioning/cert_provisioning_worker_static.h
@@ -63,7 +63,7 @@ class CertProvisioningWorkerStatic : public CertProvisioningWorker {
   void GenerateKey();
 
   void GenerateRegularKey();
-  void OnGenerateRegularKeyDone(const std::string& public_key_spki_der,
+  void OnGenerateRegularKeyDone(std::vector<uint8_t> public_key_spki_der,
                                 chromeos::platform_keys::Status status);
 
   void GenerateKeyForVa();

diff --git a/chrome/browser/ash/cert_provisioning/cert_provisioning_worker_static_unittest.cc b/chrome/browser/ash/cert_provisioning/cert_provisioning_worker_static_unittest.cc
@@ -649,7 +649,7 @@ TEST_F(CertProvisioningWorkerStaticTest, NoVaSuccess) {
                                /*sw_backed=*/false,
                                /*callback=*/_))
         .Times(1)
-        .WillOnce(RunOnceCallback<3>(GetPublicKey(), Status::kSuccess));
+        .WillOnce(RunOnceCallback<3>(GetPublicKeyBin(), Status::kSuccess));
 
     EXPECT_START_CSR_OK_WITHOUT_VA(
         StartCsr(Eq(std::ref(provisioning_process)), /*callback=*/_),

diff --git a/chrome/browser/ash/crosapi/keystore_service_ash.cc b/chrome/browser/ash/crosapi/keystore_service_ash.cc
@@ -683,13 +683,12 @@ void KeystoreServiceAsh::GenerateKey(
 // static
 void KeystoreServiceAsh::DidGenerateKey(
     GenerateKeyCallback callback,
-    const std::string& public_key,
+    std::vector<uint8_t> public_key,
     chromeos::platform_keys::Status status) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   crosapi::mojom::KeystoreBinaryResultPtr result_ptr;
   if (status == chromeos::platform_keys::Status::kSuccess) {
-    result_ptr = mojom::KeystoreBinaryResult::NewBlob(
-        std::vector<uint8_t>(public_key.begin(), public_key.end()));
+    result_ptr = mojom::KeystoreBinaryResult::NewBlob(std::move(public_key));
   } else {
     result_ptr = mojom::KeystoreBinaryResult::NewError(
         chromeos::platform_keys::StatusToKeystoreError(status));

diff --git a/chrome/browser/ash/crosapi/keystore_service_ash.h b/chrome/browser/ash/crosapi/keystore_service_ash.h
@@ -180,7 +180,7 @@ class KeystoreServiceAsh : public mojom::KeystoreService, public KeyedService {
   static void DidRemoveCertificate(RemoveCertificateCallback callback,
                                    chromeos::platform_keys::Status status);
   static void DidGenerateKey(GenerateKeyCallback callback,
-                             const std::string& public_key,
+                             std::vector<uint8_t> public_key,
                              chromeos::platform_keys::Status status);
   static void DidRemoveKey(RemoveKeyCallback callback,
                            chromeos::platform_keys::Status status);

diff --git a/chrome/browser/ash/crosapi/keystore_service_ash_unittest.cc b/chrome/browser/ash/crosapi/keystore_service_ash_unittest.cc
@@ -244,7 +244,7 @@ TEST_F(KeystoreServiceAshTest, UserKeystoreRsaAlgoGenerateKeySuccess) {
       platform_keys_service_,
       GenerateRSAKey(TokenId::kUser, modulus_length, /*sw_backed=*/false,
                      /*callback=*/_))
-      .WillOnce(RunOnceCallback<3>(GetPublicKeyStr(), Status::kSuccess));
+      .WillOnce(RunOnceCallback<3>(GetPublicKeyBin(), Status::kSuccess));
   CallbackObserver<mojom::KeystoreBinaryResultPtr> observer;
   keystore_service_.GenerateKey(
       mojom::KeystoreType::kUser,
@@ -260,7 +260,7 @@ TEST_F(KeystoreServiceAshTest, DeviceKeystoreEcAlgoGenerateKeySuccess) {
 
   EXPECT_CALL(platform_keys_service_,
               GenerateECKey(TokenId::kSystem, named_curve, /*callback=*/_))
-      .WillOnce(RunOnceCallback<2>(GetPublicKeyStr(), Status::kSuccess));
+      .WillOnce(RunOnceCallback<2>(GetPublicKeyBin(), Status::kSuccess));
 
   CallbackObserver<mojom::KeystoreBinaryResultPtr> observer;
   keystore_service_.GenerateKey(mojom::KeystoreType::kDevice,
@@ -273,7 +273,8 @@ TEST_F(KeystoreServiceAshTest, DeviceKeystoreEcAlgoGenerateKeySuccess) {
 
 TEST_F(KeystoreServiceAshTest, UserKeystoreUnsupportedEcCurveGenerateKeyFail) {
   EXPECT_CALL(platform_keys_service_, GenerateECKey)
-      .WillOnce(RunOnceCallback<2>("", Status::kErrorInternal));
+      .WillOnce(
+          RunOnceCallback<2>(std::vector<uint8_t>(), Status::kErrorInternal));
 
   CallbackObserver<mojom::KeystoreBinaryResultPtr> observer;
   keystore_service_.GenerateKey(mojom::KeystoreType::kUser,

diff --git a/chrome/browser/ash/platform_keys/key_permissions/key_permissions_manager_browsertest.cc b/chrome/browser/ash/platform_keys/key_permissions/key_permissions_manager_browsertest.cc
@@ -97,14 +97,15 @@ class KeyPermissionsManagerBrowserTestBase
   virtual KeyPermissionsManager* GetKeyPermissionsManager() = 0;
 
   std::vector<uint8_t> GenerateKey() {
-    test_util::GenerateKeyExecutionWaiter generate_key_waiter;
+    base::test::TestFuture<std::vector<uint8_t>,
+                           chromeos::platform_keys::Status>
+        generate_key_waiter;
     GetPlatformKeysService()->GenerateRSAKey(GetToken(),
                                              /*modulus_length_bits=*/2048,
                                              /*sw_backed=*/false,
                                              generate_key_waiter.GetCallback());
     EXPECT_TRUE(generate_key_waiter.Wait());
-    const std::string& pub_key = generate_key_waiter.public_key_spki_der();
-    return std::vector<uint8_t>(pub_key.begin(), pub_key.end());
+    return std::get<std::vector<uint8_t>>(generate_key_waiter.Take());
   }
 
   // Returns all keys on the token.

diff --git a/chrome/browser/ash/platform_keys/platform_keys_service.h b/chrome/browser/ash/platform_keys/platform_keys_service.h
@@ -26,11 +26,10 @@ class NSSCertDatabase;
 class ClientCertStore;
 }  // namespace net
 
-namespace ash {
-namespace platform_keys {
+namespace ash::platform_keys {
 
 using GenerateKeyCallback =
-    base::OnceCallback<void(const std::string& public_key_spki_der,
+    base::OnceCallback<void(std::vector<uint8_t> public_key_spki_der,
                             chromeos::platform_keys::Status status)>;
 
 using SignCallback =
@@ -411,7 +410,6 @@ class PlatformKeysServiceImpl final : public PlatformKeysService {
   base::WeakPtrFactory<PlatformKeysServiceImpl> weak_factory_{this};
 };
 
-}  // namespace platform_keys
-}  // namespace ash
+}  // namespace ash::platform_keys
 
 #endif  // CHROME_BROWSER_ASH_PLATFORM_KEYS_PLATFORM_KEYS_SERVICE_H_