[Passwords] Return Optional<string> from Encryption Utils

This change modifies encryption utils used for Password Check to return
base::Optional<std::string> instead of std::string. Now these methods
return a nullopt instead of an empty string in case there was an error
in the used Cipher, forcing callers to explicitly handle error cases.

Bug: 1157004
Change-Id: I21bea839020cd89e097fcf2a36efabe2adbed91a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2642144
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: Friedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845597}

components/password_manager/core/browser/leak_detection/authenticated_leak_check_unittest.cc

@@ -263,11 +263,11 @@ TEST_F(AuthenticatedLeakCheckTest, ParseResponse_DecryptionError) {
   std::string key_server;
   // Append trash bytes to force a decryption error.
   response->reencrypted_lookup_hash =
-      CipherReEncrypt(payload_and_callback.payload, &key_server) +
+      *CipherReEncrypt(payload_and_callback.payload, &key_server) +
       "trash_bytes";
   response->encrypted_leak_match_prefixes.push_back(
-      crypto::SHA256HashString(CipherEncryptWithKey(
-          ScryptHashUsernameAndPassword("another_username", kPassword),
+      crypto::SHA256HashString(*CipherEncryptWithKey(
+          *ScryptHashUsernameAndPassword("another_username", kPassword),
           key_server)));
 
   EXPECT_CALL(delegate(), OnLeakDetectionDone(false, GURL(kExampleCom),
@@ -297,10 +297,10 @@ TEST_F(AuthenticatedLeakCheckTest, ParseResponse_NoLeak) {
   auto response = std::make_unique<SingleLookupResponse>();
   std::string key_server;
   response->reencrypted_lookup_hash =
-      CipherReEncrypt(payload_and_callback.payload, &key_server);
+      *CipherReEncrypt(payload_and_callback.payload, &key_server);
   response->encrypted_leak_match_prefixes.push_back(
-      crypto::SHA256HashString(CipherEncryptWithKey(
-          ScryptHashUsernameAndPassword("another_username", kPassword),
+      crypto::SHA256HashString(*CipherEncryptWithKey(
+          *ScryptHashUsernameAndPassword("another_username", kPassword),
           key_server)));
 
   EXPECT_CALL(delegate(), OnLeakDetectionDone(false, GURL(kExampleCom),
@@ -337,10 +337,10 @@ TEST_F(AuthenticatedLeakCheckTest, ParseResponse_Leak) {
   auto response = std::make_unique<SingleLookupResponse>();
   std::string key_server;
   response->reencrypted_lookup_hash =
-      CipherReEncrypt(payload_and_callback.payload, &key_server);
+      *CipherReEncrypt(payload_and_callback.payload, &key_server);
   response->encrypted_leak_match_prefixes.push_back(
-      crypto::SHA256HashString(CipherEncryptWithKey(
-          ScryptHashUsernameAndPassword(canonicalized_username, kPassword),
+      crypto::SHA256HashString(*CipherEncryptWithKey(
+          *ScryptHashUsernameAndPassword(canonicalized_username, kPassword),
           key_server)));
 
   EXPECT_CALL(delegate(), OnLeakDetectionDone(true, GURL(kExampleCom),

components/password_manager/core/browser/leak_detection/bulk_leak_check_impl.cc

@@ -76,7 +76,7 @@ BulkLeakCheckImpl::BulkLeakCheckImpl(
     : delegate_(delegate),
       identity_manager_(identity_manager),
       url_loader_factory_(std::move(url_loader_factory)),
-      encryption_key_(CreateNewKey()),
+      encryption_key_(CreateNewKey().value_or("")),
       payload_task_runner_(base::ThreadPool::CreateSequencedTaskRunner(
           {base::TaskPriority::USER_VISIBLE,
            base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN})) {

components/password_manager/core/browser/leak_detection/bulk_leak_check_impl_unittest.cc

@@ -263,11 +263,11 @@ TEST_F(BulkLeakCheckTest, CheckCredentialsDecryptionError) {
   std::string key_server;
   // Append trash bytes to force a decryption error.
   response->reencrypted_lookup_hash =
-      CipherReEncrypt(payload_and_callback.payload, &key_server) +
+      *CipherReEncrypt(payload_and_callback.payload, &key_server) +
       "trash_bytes";
   response->encrypted_leak_match_prefixes.push_back(
-      crypto::SHA256HashString(CipherEncryptWithKey(
-          ScryptHashUsernameAndPassword("another_username", kTestPassword),
+      crypto::SHA256HashString(*CipherEncryptWithKey(
+          *ScryptHashUsernameAndPassword("another_username", kTestPassword),
           key_server)));
 
   EXPECT_CALL(delegate(), OnError(LeakDetectionError::kHashingFailure));
@@ -292,10 +292,10 @@ TEST_F(BulkLeakCheckTest, CheckCredentialsNotLeaked) {
   auto response = std::make_unique<SingleLookupResponse>();
   std::string key_server;
   response->reencrypted_lookup_hash =
-      CipherReEncrypt(payload_and_callback.payload, &key_server);
+      *CipherReEncrypt(payload_and_callback.payload, &key_server);
   response->encrypted_leak_match_prefixes.push_back(
-      crypto::SHA256HashString(CipherEncryptWithKey(
-          ScryptHashUsernameAndPassword("another_username", kTestPassword),
+      crypto::SHA256HashString(*CipherEncryptWithKey(
+          *ScryptHashUsernameAndPassword("another_username", kTestPassword),
           key_server)));
 
   EXPECT_EQ(1u, bulk_check().GetPendingChecksCount());
@@ -325,10 +325,10 @@ TEST_F(BulkLeakCheckTest, CheckCredentialsLeaked) {
   auto response = std::make_unique<SingleLookupResponse>();
   std::string key_server;
   response->reencrypted_lookup_hash =
-      CipherReEncrypt(payload_and_callback.payload, &key_server);
+      *CipherReEncrypt(payload_and_callback.payload, &key_server);
   response->encrypted_leak_match_prefixes.push_back(
-      crypto::SHA256HashString(CipherEncryptWithKey(
-          ScryptHashUsernameAndPassword("abc", kTestPassword), key_server)));
+      crypto::SHA256HashString(*CipherEncryptWithKey(
+          *ScryptHashUsernameAndPassword("abc", kTestPassword), key_server)));
 
   EXPECT_EQ(1u, bulk_check().GetPendingChecksCount());
   leaked_credential = TestCredential("abc");

components/password_manager/core/browser/leak_detection/encryption_utils.cc

@@ -5,7 +5,9 @@
 #include "components/password_manager/core/browser/leak_detection/encryption_utils.h"
 
 #include <climits>
+#include <utility>
 
+#include "base/optional.h"
 #include "base/stl_util.h"
 #include "base/strings/strcat.h"
 #include "base/strings/string_piece.h"
@@ -83,7 +85,7 @@ std::string BucketizeUsername(base::StringPiece canonicalized_username) {
   return prefix;
 }
 
-std::string ScryptHashUsernameAndPassword(
+base::Optional<std::string> ScryptHashUsernameAndPassword(
     base::StringPiece canonicalized_username,
     base::StringPiece password) {
   // Constant salt added to the password hash on top of canonicalized_username.
@@ -118,68 +120,74 @@ std::string ScryptHashUsernameAndPassword(
                      reinterpret_cast<const uint8_t*>(salt.data()), salt.size(),
                      kScryptCost, kScryptBlockSize, kScryptParallelization,
                      kScryptMaxMemory, key_data, kHashKeyLength);
-  return scrypt_ok == 1 ? std::move(result) : std::string();
+  return scrypt_ok == 1 ? base::make_optional(std::move(result))
+                        : base::nullopt;
 }
 
-std::string CipherEncrypt(const std::string& plaintext, std::string* key) {
+base::Optional<std::string> CipherEncrypt(const std::string& plaintext,
+                                          std::string* key) {
   using ::private_join_and_compute::ECCommutativeCipher;
   auto cipher = ECCommutativeCipher::CreateWithNewKey(
       NID_X9_62_prime256v1, ECCommutativeCipher::SHA256);
   if (cipher.ok()) {
-    *key = cipher.ValueOrDie()->GetPrivateKeyBytes();
     auto result = cipher.ValueOrDie()->Encrypt(plaintext);
-    if (result.ok())
-      return result.ValueOrDie();
+    if (result.ok()) {
+      *key = cipher.ValueOrDie()->GetPrivateKeyBytes();
+      return std::move(result).ValueOrDie();
+    }
   }
-  return std::string();
+  return base::nullopt;
 }
 
-std::string CipherEncryptWithKey(const std::string& plaintext,
-                                 const std::string& key) {
+base::Optional<std::string> CipherEncryptWithKey(const std::string& plaintext,
+                                                 const std::string& key) {
   using ::private_join_and_compute::ECCommutativeCipher;
   auto cipher = ECCommutativeCipher::CreateFromKey(NID_X9_62_prime256v1, key,
                                                    ECCommutativeCipher::SHA256);
   if (cipher.ok()) {
     auto result = cipher.ValueOrDie()->Encrypt(plaintext);
     if (result.ok())
-      return result.ValueOrDie();
+      return std::move(result).ValueOrDie();
   }
-  return std::string();
+  return base::nullopt;
 }
 
-std::string CipherReEncrypt(const std::string& already_encrypted,
-                            std::string* key) {
+base::Optional<std::string> CipherReEncrypt(
+    const std::string& already_encrypted,
+    std::string* key) {
   using ::private_join_and_compute::ECCommutativeCipher;
   auto cipher = ECCommutativeCipher::CreateWithNewKey(
       NID_X9_62_prime256v1, ECCommutativeCipher::SHA256);
   if (cipher.ok()) {
-    *key = cipher.ValueOrDie()->GetPrivateKeyBytes();
     auto result = cipher.ValueOrDie()->ReEncrypt(already_encrypted);
-    if (result.ok())
-      return result.ValueOrDie();
+    if (result.ok()) {
+      *key = cipher.ValueOrDie()->GetPrivateKeyBytes();
+      return std::move(result).ValueOrDie();
+    }
   }
-  return std::string();
+  return base::nullopt;
 }
 
-std::string CipherDecrypt(const std::string& ciphertext,
-                          const std::string& key) {
+base::Optional<std::string> CipherDecrypt(const std::string& ciphertext,
+                                          const std::string& key) {
   using ::private_join_and_compute::ECCommutativeCipher;
   auto cipher = ECCommutativeCipher::CreateFromKey(NID_X9_62_prime256v1, key,
                                                    ECCommutativeCipher::SHA256);
   if (cipher.ok()) {
     auto result = cipher.ValueOrDie()->Decrypt(ciphertext);
     if (result.ok())
-      return result.ValueOrDie();
+      return std::move(result).ValueOrDie();
   }
-  return std::string();
+  return base::nullopt;
 }
 
-std::string CreateNewKey() {
+base::Optional<std::string> CreateNewKey() {
   using ::private_join_and_compute::ECCommutativeCipher;
   auto cipher = ECCommutativeCipher::CreateWithNewKey(
       NID_X9_62_prime256v1, ECCommutativeCipher::SHA256);
-  return cipher.ok() ? cipher.ValueOrDie()->GetPrivateKeyBytes()
-                     : std::string();
+  if (cipher.ok())
+    return cipher.ValueOrDie()->GetPrivateKeyBytes();
+  return base::nullopt;
 }
 
 }  // namespace password_manager

components/password_manager/core/browser/leak_detection/encryption_utils.h

@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/optional.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_piece_forward.h"
 
@@ -31,7 +32,8 @@ std::string BucketizeUsername(base::StringPiece canonicalized_username);
 
 // Produces the username/password pair hash using scrypt algorithm.
 // |canonicalized_username| and |password| are UTF-8 strings.
-std::string ScryptHashUsernameAndPassword(
+// Returns nullopt in case of encryption failure.
+base::Optional<std::string> ScryptHashUsernameAndPassword(
     base::StringPiece canonicalized_username,
     base::StringPiece password);
 
@@ -39,26 +41,35 @@ std::string ScryptHashUsernameAndPassword(
 
 // Encrypts |plaintext| with a new key. The key is returned via |key|.
 // Internally the function does some hashing first and then encrypts the result.
-std::string CipherEncrypt(const std::string& plaintext, std::string* key);
+// In case of an encryption failure this returns nullopt and does not modify
+// |key|.
+base::Optional<std::string> CipherEncrypt(const std::string& plaintext,
+                                          std::string* key);
 
 // Encrypts |plaintext| with the existing key.
-std::string CipherEncryptWithKey(const std::string& plaintext,
-                                 const std::string& key);
+// Returns nullopt in case of encryption failure.
+base::Optional<std::string> CipherEncryptWithKey(const std::string& plaintext,
+                                                 const std::string& key);
 
 // |already_encrypted| is an already encrypted string (output of CipherEncrypt).
 // Encrypts it again with a new key. The key is returned in |key|.
 // The function is different from CipherEncrypt() as it doesn't apply hashing on
 // the input.
-std::string CipherReEncrypt(const std::string& already_encrypted,
-                            std::string* key);
+// In case of an encryption failure this returns nullopt and does not modify
+// |key|.
+base::Optional<std::string> CipherReEncrypt(
+    const std::string& already_encrypted,
+    std::string* key);
 
 // Decrypts |ciphertext| using |key|. The result isn't the original string but a
 // hash of it.
-std::string CipherDecrypt(const std::string& ciphertext,
-                          const std::string& key);
+// Returns nullopt in case of decryption failure.
+base::Optional<std::string> CipherDecrypt(const std::string& ciphertext,
+                                          const std::string& key);
 
-// Returns a new key suitable for the encryption functions above.
-std::string CreateNewKey();
+// Returns a new key suitable for the encryption functions above, or nullopt if
+// the operation failed.
+base::Optional<std::string> CreateNewKey();
 
 }  // namespace password_manager
 

components/password_manager/core/browser/leak_detection/encryption_utils_unittest.cc

@@ -77,21 +77,21 @@ TEST(EncryptionUtils, ScryptHashUsernameAndPassword) {
                                 -56,  -82, -38, 31,   114,  61,  -7,  103,
                                 76,   91,  52,  -52,  47,   -22, 107, 77,
                                 118,  123, -14, -125, -123, 85,  115, -3};
-  std::string result = ScryptHashUsernameAndPassword("user", "password123");
+  std::string result = *ScryptHashUsernameAndPassword("user", "password123");
   EXPECT_THAT(result, ElementsAreArray(kExpected));
 }
 
 TEST(EncryptionUtils, EncryptAndDecrypt) {
   constexpr char kRandomString[] = "very_secret";
   std::string key;
-  std::string cipher = CipherEncrypt(kRandomString, &key);
+  std::string cipher = *CipherEncrypt(kRandomString, &key);
   SCOPED_TRACE(testing::Message()
                << "key=" << testing::PrintToString(StringAsArray(key)));
 
   EXPECT_NE(kRandomString, cipher);
   EXPECT_NE(std::string(), key);
   EXPECT_THAT(CalculateECCurveHash(kRandomString),
-              ElementsAreArray(CipherDecrypt(cipher, key)));
+              ElementsAreArray(*CipherDecrypt(cipher, key)));
 }
 
 TEST(EncryptionUtils, EncryptAndDecryptWithPredefinedKey) {
@@ -109,10 +109,10 @@ TEST(EncryptionUtils, EncryptAndDecryptWithPredefinedKey) {
                              -112, -64, 58,  -64, 76,   -35, -117, -23, -100,
                              25,   63,  37,  114, 74,   88};
 
-  std::string cipher = CipherEncryptWithKey(kRandomString, kKey);
+  std::string cipher = *CipherEncryptWithKey(kRandomString, kKey);
   EXPECT_THAT(cipher, ElementsAreArray(kEncrypted));
   EXPECT_THAT(CalculateECCurveHash(kRandomString),
-              ElementsAreArray(CipherDecrypt(cipher, kKey)));
+              ElementsAreArray(*CipherDecrypt(cipher, kKey)));
 }
 
 TEST(EncryptionUtils, CipherIsCommutative) {
@@ -120,31 +120,31 @@ TEST(EncryptionUtils, CipherIsCommutative) {
 
   // Client encrypts the string.
   std::string key_client;
-  std::string cipher = CipherEncrypt(kRandomString, &key_client);
+  std::string cipher = *CipherEncrypt(kRandomString, &key_client);
   SCOPED_TRACE(testing::Message()
                << "key_client="
                << testing::PrintToString(StringAsArray(key_client)));
 
   // Server encrypts the result.
   std::string key_server;
-  cipher = CipherReEncrypt(cipher, &key_server);
+  cipher = *CipherReEncrypt(cipher, &key_server);
   SCOPED_TRACE(testing::Message()
                << "key_server="
                << testing::PrintToString(StringAsArray(key_server)));
 
-  EXPECT_THAT(CipherEncryptWithKey(kRandomString, key_server),
-              ElementsAreArray(CipherDecrypt(cipher, key_client)));
+  EXPECT_THAT(*CipherEncryptWithKey(kRandomString, key_server),
+              ElementsAreArray(*CipherDecrypt(cipher, key_client)));
 }
 
 TEST(EncryptionUtils, CreateNewKey) {
-  const std::string key = CreateNewKey();
+  const std::string key = *CreateNewKey();
   SCOPED_TRACE(testing::Message()
                << "key=" << testing::PrintToString(StringAsArray(key)));
   constexpr char kRandomString[] = "very_secret";
 
-  std::string cipher = CipherEncryptWithKey(kRandomString, key);
+  std::string cipher = *CipherEncryptWithKey(kRandomString, key);
   EXPECT_THAT(CalculateECCurveHash(kRandomString),
-              ElementsAreArray(CipherDecrypt(cipher, key)));
+              ElementsAreArray(*CipherDecrypt(cipher, key)));
 }
 
 }  // namespace password_manager

components/password_manager/core/browser/leak_detection/fuzzer/cipher_encrypt_fuzzer.cc

@@ -14,7 +14,7 @@ namespace password_manager {
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
   std::string payload(reinterpret_cast<const char*>(data), size);
   std::string key;
-  std::string cipher = password_manager::CipherEncrypt(payload, &key);
+  std::string cipher = *password_manager::CipherEncrypt(payload, &key);
   return 0;
 }
 

components/password_manager/core/browser/leak_detection/fuzzer/cypher_decrypt_fuzzer.cc

@@ -32,7 +32,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
     return 0;
 
   std::string payload(reinterpret_cast<const char*>(data), size);
-  std::string result = password_manager::CipherDecrypt(payload, key);
+  std::string result = *password_manager::CipherDecrypt(payload, key);
   return 0;
 }
 

components/password_manager/core/browser/leak_detection/fuzzer/cypher_encrypt_with_key_fuzzer.cc

@@ -32,7 +32,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
     return 0;
 
   std::string payload(reinterpret_cast<const char*>(data), size);
-  std::string result = password_manager::CipherEncryptWithKey(payload, key);
+  std::string result = *password_manager::CipherEncryptWithKey(payload, key);
   return 0;
 }
 

components/password_manager/core/browser/leak_detection/fuzzer/cypher_reencrypt_fuzzer.cc

@@ -9,7 +9,7 @@ namespace password_manager {
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
   std::string payload(reinterpret_cast<const char*>(data), size);
   std::string key;
-  std::string result = password_manager::CipherEncrypt(payload, &key);
+  std::string result = *password_manager::CipherEncrypt(payload, &key);
   return 0;
 }
 

components/password_manager/core/browser/leak_detection/leak_detection_request_utils.cc

@@ -8,6 +8,7 @@
 
 #include "base/containers/span.h"
 #include "base/debug/dump_without_crashing.h"
+#include "base/optional.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece_forward.h"
 #include "base/strings/string_util.h"
@@ -49,7 +50,8 @@ LookupSingleLeakPayload ProduceHashes(base::StringPiece username,
   LookupSingleLeakPayload payload;
   payload.username_hash_prefix = BucketizeUsername(canonicalized_username);
   payload.encrypted_payload =
-      ScryptHashUsernameAndPassword(canonicalized_username, password);
+      ScryptHashUsernameAndPassword(canonicalized_username, password)
+          .value_or("");
   if (payload.encrypted_payload.empty())
     return LookupSingleLeakPayload();
   return payload;
@@ -64,7 +66,8 @@ LookupSingleLeakData PrepareLookupSingleLeakData(base::StringPiece username,
   if (data.payload.encrypted_payload.empty())
     return LookupSingleLeakData();
   data.payload.encrypted_payload =
-      CipherEncrypt(data.payload.encrypted_payload, &data.encryption_key);
+      CipherEncrypt(data.payload.encrypted_payload, &data.encryption_key)
+          .value_or("");
   return data.payload.encrypted_payload.empty() ? LookupSingleLeakData()
                                                 : std::move(data);
 }
@@ -79,7 +82,8 @@ LookupSingleLeakPayload PrepareLookupSingleLeakDataWithKey(
   if (payload.encrypted_payload.empty())
     return LookupSingleLeakPayload();
   payload.encrypted_payload =
-      CipherEncryptWithKey(payload.encrypted_payload, encryption_key);
+      CipherEncryptWithKey(payload.encrypted_payload, encryption_key)
+          .value_or("");
   return payload.encrypted_payload.empty() ? LookupSingleLeakPayload()
                                            : std::move(payload);
 }
@@ -90,9 +94,9 @@ LookupSingleLeakPayload PrepareLookupSingleLeakDataWithKey(
 AnalyzeResponseResult CheckIfCredentialWasLeaked(
     std::unique_ptr<SingleLookupResponse> response,
     const std::string& encryption_key) {
-  std::string decrypted_username_password =
+  base::Optional<std::string> decrypted_username_password =
       CipherDecrypt(response->reencrypted_lookup_hash, encryption_key);
-  if (decrypted_username_password.empty()) {
+  if (!decrypted_username_password) {
     DLOG(ERROR) << "Can't decrypt data="
                 << base::HexEncode(base::as_bytes(
                        base::make_span(response->reencrypted_lookup_hash)));
@@ -100,7 +104,7 @@ AnalyzeResponseResult CheckIfCredentialWasLeaked(
   }
 
   std::string hash_username_password =
-      crypto::SHA256HashString(decrypted_username_password);
+      crypto::SHA256HashString(*decrypted_username_password);
 
   const ptrdiff_t matched_prefixes =
       std::count_if(response->encrypted_leak_match_prefixes.begin(),

components/password_manager/core/browser/leak_detection/leak_detection_request_utils_unittest.cc

@@ -93,10 +93,10 @@ TEST(LeakDetectionRequestUtils, AnalyzeResponseResult_NoLeak) {
   auto response = std::make_unique<SingleLookupResponse>();
   std::string key_client;
   std::string encrypted_username_password =
-      CipherEncrypt(kUsernamePasswordHash, &key_client);
+      *CipherEncrypt(kUsernamePasswordHash, &key_client);
   std::string key_server;
   response->reencrypted_lookup_hash =
-      CipherReEncrypt(encrypted_username_password, &key_server);
+      *CipherReEncrypt(encrypted_username_password, &key_server);
   SCOPED_TRACE(testing::Message()
                << "key_client="
                << testing::PrintToString(StringToArray(key_client))
@@ -104,7 +104,7 @@ TEST(LeakDetectionRequestUtils, AnalyzeResponseResult_NoLeak) {
                << testing::PrintToString(StringToArray(key_server)));
 
   response->encrypted_leak_match_prefixes.push_back(crypto::SHA256HashString(
-      CipherEncryptWithKey("unrelated_trash", key_server)));
+      *CipherEncryptWithKey("unrelated_trash", key_server)));
 
   base::MockCallback<SingleLeakResponseAnalysisCallback> callback;
   AnalyzeResponse(std::move(response), key_client, callback.Get());
@@ -119,10 +119,10 @@ TEST(LeakDetectionRequestUtils, AnalyzeResponseResult_Leak) {
   auto response = std::make_unique<SingleLookupResponse>();
   std::string key_client;
   std::string encrypted_username_password =
-      CipherEncrypt(kUsernamePasswordHash, &key_client);
+      *CipherEncrypt(kUsernamePasswordHash, &key_client);
   std::string key_server;
   response->reencrypted_lookup_hash =
-      CipherReEncrypt(encrypted_username_password, &key_server);
+      *CipherReEncrypt(encrypted_username_password, &key_server);
   SCOPED_TRACE(testing::Message()
                << "key_client="
                << testing::PrintToString(StringToArray(key_client))
@@ -133,10 +133,10 @@ TEST(LeakDetectionRequestUtils, AnalyzeResponseResult_Leak) {
   // The server can pick any value.
   constexpr int kPrefixLength = 30;
   response->encrypted_leak_match_prefixes.push_back(crypto::SHA256HashString(
-      CipherEncryptWithKey("unrelated_trash", key_server)));
+      *CipherEncryptWithKey("unrelated_trash", key_server)));
   response->encrypted_leak_match_prefixes.push_back(
       crypto::SHA256HashString(
-          CipherEncryptWithKey(kUsernamePasswordHash, key_server))
+          *CipherEncryptWithKey(kUsernamePasswordHash, key_server))
           .substr(0, kPrefixLength));
 
   base::MockCallback<SingleLeakResponseAnalysisCallback> callback;