[TrustedVault] Factor out crypto functions

This CL moves trusted vault crypto functions to a dedicated header,
because they were used in multiple files and it leads to code and
constants duplicates.

Bug: 1113598
Change-Id: I5450a1ec0f9836471c5e5d3669e7cd2173527d29
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2565510Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Maksim Moskvitin <mmoskvitin@google.com>
Cr-Commit-Position: refs/heads/master@{#832309}

components/sync/BUILD.gn

@@ -482,6 +482,7 @@ source_set("unit_tests") {
     "trusted_vault/standalone_trusted_vault_backend_unittest.cc",
     "trusted_vault/trusted_vault_access_token_fetcher_frontend_unittest.cc",
     "trusted_vault/trusted_vault_connection_impl_unittest.cc",
+    "trusted_vault/trusted_vault_crypto_unittest.cc",
     "trusted_vault/trusted_vault_request_unittest.cc",
   ]
 

components/sync/trusted_vault/BUILD.gn

@@ -22,6 +22,8 @@ static_library("trusted_vault") {
     "trusted_vault_connection.h",
     "trusted_vault_connection_impl.cc",
     "trusted_vault_connection_impl.h",
+    "trusted_vault_crypto.cc",
+    "trusted_vault_crypto.h",
     "trusted_vault_request.cc",
     "trusted_vault_request.h",
   ]

components/sync/trusted_vault/download_keys_response_handler.cc

@@ -10,17 +10,12 @@
 #include "components/sync/protocol/vault.pb.h"
 #include "components/sync/trusted_vault/proto_string_bytes_conversion.h"
 #include "components/sync/trusted_vault/securebox.h"
-#include "crypto/hmac.h"
+#include "components/sync/trusted_vault/trusted_vault_crypto.h"
 
 namespace syncer {
 
 namespace {
 
-// TODO(crbug.com/1113598): move these constants to a dedicated header, since
-// they are used in multiple places already.
-const size_t kKeyProofLength = 32;
-const uint8_t kWrappedKeyHeader[] = {'V', '1', ' ', 's', 'h', 'a', 'r',
-                                     'e', 'd', '_', 'k', 'e', 'y'};
 const char kSecurityDomainName[] = "chromesync";
 
 struct ExtractedSharedKey {
@@ -73,9 +68,8 @@ std::vector<ExtractedSharedKey> ExtractAndSortSharedKeys(
   std::vector<ExtractedSharedKey> result;
   for (const sync_pb::SharedKey& shared_key : member.keys()) {
     base::Optional<std::vector<uint8_t>> decrypted_key =
-        member_private_key.Decrypt(
-            base::span<const uint8_t>(), kWrappedKeyHeader,
-            /*encrypted_payload=*/ProtoStringToBytes(shared_key.wrapped_key()));
+        DecryptTrustedVaultWrappedKey(
+            member_private_key, ProtoStringToBytes(shared_key.wrapped_key()));
     if (!decrypted_key.has_value()) {
       // Decryption failed.
       return std::vector<ExtractedSharedKey>();
@@ -115,14 +109,12 @@ bool IsValidKeyChain(const std::vector<ExtractedSharedKey>& key_chain,
       return false;
     }
 
-    crypto::HMAC hmac(crypto::HMAC::SHA256);
-    CHECK(hmac.Init(last_valid_key));
-
-    std::vector<uint8_t> digest_bytes(kKeyProofLength);
-    if (!hmac.Verify(next_key.trusted_vault_key, next_key.key_proof)) {
+    if (!VerifyTrustedVaultHMAC(last_valid_key, next_key.trusted_vault_key,
+                                next_key.key_proof)) {
       // |key_proof| isn't valid.
       return false;
     }
+
     last_valid_key_version = next_key.version;
     last_valid_key = next_key.trusted_vault_key;
   }

components/sync/trusted_vault/download_keys_response_handler_unittest.cc

@@ -10,7 +10,7 @@
 #include "components/sync/protocol/vault.pb.h"
 #include "components/sync/trusted_vault/proto_string_bytes_conversion.h"
 #include "components/sync/trusted_vault/securebox.h"
-#include "crypto/hmac.h"
+#include "components/sync/trusted_vault/trusted_vault_crypto.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -22,11 +22,8 @@ using testing::ElementsAre;
 using testing::Eq;
 using testing::IsEmpty;
 
-const size_t kKeyProofLength = 32;
 const char kEncodedPrivateKey[] =
     "49e052293c29b5a50b0013eec9d030ac2ad70a42fe093be084264647cb04e16f";
-const uint8_t kWrappedKeyHeader[] = {'V', '1', ' ', 's', 'h', 'a', 'r',
-                                     'e', 'd', '_', 'k', 'e', 'y'};
 const char kSecurityDomainName[] = "chromesync";
 
 std::unique_ptr<SecureBoxKeyPair> MakeTestKeyPair() {
@@ -53,19 +50,13 @@ void FillSecurityDomainMember(
     sync_pb::SharedKey* shared_key = member->add_keys();
     shared_key->set_epoch(trusted_vault_keys_versions[i]);
     AssignBytesToProtoString(
-        public_key.Encrypt(
-            /*shared_secret=*/base::span<const uint8_t>(), kWrappedKeyHeader,
-            /*payload=*/trusted_vault_keys[i]),
+        ComputeTrustedVaultWrappedKey(public_key, trusted_vault_keys[i]),
         shared_key->mutable_wrapped_key());
 
     if (!signing_keys[i].empty()) {
-      crypto::HMAC hmac(crypto::HMAC::SHA256);
-      CHECK(hmac.Init(signing_keys[i]));
-
-      std::vector<uint8_t> key_proof_bytes(kKeyProofLength);
-      CHECK(hmac.Sign(trusted_vault_keys[i], key_proof_bytes));
-      AssignBytesToProtoString(key_proof_bytes,
-                               shared_key->mutable_key_proof());
+      AssignBytesToProtoString(
+          ComputeTrustedVaultHMAC(signing_keys[i], trusted_vault_keys[i]),
+          shared_key->mutable_key_proof());
     }
   }
 }

components/sync/trusted_vault/trusted_vault_connection_impl.cc

@@ -13,17 +13,14 @@
 #include "components/sync/trusted_vault/proto_string_bytes_conversion.h"
 #include "components/sync/trusted_vault/securebox.h"
 #include "components/sync/trusted_vault/trusted_vault_access_token_fetcher.h"
+#include "components/sync/trusted_vault/trusted_vault_crypto.h"
 #include "components/sync/trusted_vault/trusted_vault_request.h"
-#include "crypto/hmac.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
 namespace syncer {
 
 namespace {
 
-const size_t kMemberProofLength = 32;
-const uint8_t kWrappedKeyHeader[] = {'V', '1', ' ', 's', 'h', 'a', 'r',
-                                     'e', 'd', '_', 'k', 'e', 'y'};
 const char kJoinSecurityDomainsURLPath[] = "/domain:join";
 const char kListSecurityDomainsURLPathAndQuery[] = "/domain:list?view=1";
 const char kSecurityDomainName[] = "chromesync";
@@ -48,36 +45,19 @@ void ProcessRegisterDeviceResponse(
   std::move(callback).Run(registration_status);
 }
 
-std::vector<uint8_t> ComputeWrappedKey(
-    const SecureBoxPublicKey& public_key,
-    const std::vector<uint8_t>& trusted_vault_key) {
-  return public_key.Encrypt(
-      /*shared_secret=*/base::span<const uint8_t>(), kWrappedKeyHeader,
-      /*payload=*/trusted_vault_key);
-}
-
-std::vector<uint8_t> ComputeMemberProof(
-    const SecureBoxPublicKey& public_key,
-    const std::vector<uint8_t>& trusted_vault_key) {
-  crypto::HMAC hmac(crypto::HMAC::SHA256);
-  CHECK(hmac.Init(trusted_vault_key));
-
-  std::vector<uint8_t> digest_bytes(kMemberProofLength);
-  CHECK(hmac.Sign(public_key.ExportToBytes(), digest_bytes));
-
-  return digest_bytes;
-}
-
 sync_pb::SharedKey CreateMemberSharedKey(
     int trusted_vault_key_version,
     const std::vector<uint8_t>& trusted_vault_key,
     const SecureBoxPublicKey& public_key) {
   sync_pb::SharedKey shared_key;
   shared_key.set_epoch(trusted_vault_key_version);
-  AssignBytesToProtoString(ComputeWrappedKey(public_key, trusted_vault_key),
-                           shared_key.mutable_wrapped_key());
-  AssignBytesToProtoString(ComputeMemberProof(public_key, trusted_vault_key),
-                           shared_key.mutable_member_proof());
+  AssignBytesToProtoString(
+      ComputeTrustedVaultWrappedKey(public_key, trusted_vault_key),
+      shared_key.mutable_wrapped_key());
+  AssignBytesToProtoString(
+      ComputeTrustedVaultHMAC(/*key=*/trusted_vault_key,
+                              /*data=*/public_key.ExportToBytes()),
+      shared_key.mutable_member_proof());
   return shared_key;
 }
 

components/sync/trusted_vault/trusted_vault_connection_impl_unittest.cc

@@ -17,6 +17,7 @@
 #include "components/sync/trusted_vault/proto_string_bytes_conversion.h"
 #include "components/sync/trusted_vault/securebox.h"
 #include "components/sync/trusted_vault/trusted_vault_access_token_fetcher.h"
+#include "components/sync/trusted_vault/trusted_vault_crypto.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
 #include "services/network/test/test_url_loader_factory.h"
 #include "services/network/test/test_utils.h"
@@ -42,8 +43,6 @@ const char kTestJoinSecurityDomainsURL[] =
     "https://test.com/test/domain:join?alt=proto";
 const char kTestListSecurityDomainsURL[] =
     "https://test.com/test/domain:list?view=1&alt=proto";
-const uint8_t kWrappedKeyHeader[] = {'V', '1', ' ', 's', 'h', 'a', 'r',
-                                     'e', 'd', '_', 'k', 'e', 'y'};
 
 std::unique_ptr<SecureBoxKeyPair> MakeTestKeyPair() {
   std::vector<uint8_t> private_key_bytes;
@@ -178,9 +177,9 @@ TEST_F(TrustedVaultConnectionImplTest, ShouldSendJoinSecurityDomainsRequest) {
   EXPECT_THAT(shared_key.epoch(), Eq(kLastKeyVersion));
 
   base::Optional<std::vector<uint8_t>> decrypted_trusted_vault_key =
-      key_pair->private_key().Decrypt(
-          /*shared_key=*/base::span<const uint8_t>(), kWrappedKeyHeader,
-          /*encrypted_payload=*/ProtoStringToBytes(shared_key.wrapped_key()));
+      DecryptTrustedVaultWrappedKey(
+          key_pair->private_key(),
+          /*wrapped_key=*/ProtoStringToBytes(shared_key.wrapped_key()));
   ASSERT_THAT(decrypted_trusted_vault_key, Ne(base::nullopt));
   EXPECT_THAT(*decrypted_trusted_vault_key, Eq(kTrustedVaultKey));
 

components/sync/trusted_vault/trusted_vault_crypto.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/sync/trusted_vault/trusted_vault_crypto.h"
+
+#include "components/sync/trusted_vault/securebox.h"
+#include "crypto/hmac.h"
+
+namespace syncer {
+
+namespace {
+
+const size_t kHMACDigestLength = 32;
+const uint8_t kWrappedKeyHeader[] = {'V', '1', ' ', 's', 'h', 'a', 'r',
+                                     'e', 'd', '_', 'k', 'e', 'y'};
+
+}  // namespace
+
+base::Optional<std::vector<uint8_t>> DecryptTrustedVaultWrappedKey(
+    const SecureBoxPrivateKey& private_key,
+    base::span<const uint8_t> wrapped_key) {
+  return private_key.Decrypt(
+      /*shared_secret=*/base::span<const uint8_t>(), kWrappedKeyHeader,
+      /*encrypted_payload=*/wrapped_key);
+}
+
+std::vector<uint8_t> ComputeTrustedVaultWrappedKey(
+    const SecureBoxPublicKey& public_key,
+    base::span<const uint8_t> trusted_vault_key) {
+  return public_key.Encrypt(
+      /*shared_secret=*/base::span<const uint8_t>(), kWrappedKeyHeader,
+      /*payload=*/trusted_vault_key);
+}
+
+std::vector<uint8_t> ComputeTrustedVaultHMAC(base::span<const uint8_t> key,
+                                             base::span<const uint8_t> data) {
+  crypto::HMAC hmac(crypto::HMAC::SHA256);
+  CHECK(hmac.Init(key));
+
+  std::vector<uint8_t> digest(kHMACDigestLength);
+  CHECK(hmac.Sign(data, digest));
+  return digest;
+}
+
+bool VerifyTrustedVaultHMAC(base::span<const uint8_t> key,
+                            base::span<const uint8_t> data,
+                            base::span<const uint8_t> digest) {
+  crypto::HMAC hmac(crypto::HMAC::SHA256);
+  CHECK(hmac.Init(key));
+  return hmac.Verify(data, digest);
+}
+
+}  // namespace syncer

components/sync/trusted_vault/trusted_vault_crypto.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_SYNC_TRUSTED_VAULT_TRUSTED_VAULT_CRYPTO_H_
+#define COMPONENTS_SYNC_TRUSTED_VAULT_TRUSTED_VAULT_CRYPTO_H_
+
+#include <vector>
+
+#include "base/containers/span.h"
+#include "base/optional.h"
+
+namespace syncer {
+
+class SecureBoxPrivateKey;
+class SecureBoxPublicKey;
+
+// Decrypts |wrapped_key| using securebox. Returns decrypted key if successful
+// and base::nullopt otherwise.
+base::Optional<std::vector<uint8_t>> DecryptTrustedVaultWrappedKey(
+    const SecureBoxPrivateKey& private_key,
+    base::span<const uint8_t> wrapped_key);
+
+// Encrypts |trusted_vault_key| using securebox.
+std::vector<uint8_t> ComputeTrustedVaultWrappedKey(
+    const SecureBoxPublicKey& public_key,
+    base::span<const uint8_t> trusted_vault_key);
+
+// Computes HMAC digest using SHA-256.
+std::vector<uint8_t> ComputeTrustedVaultHMAC(base::span<const uint8_t> key,
+                                             base::span<const uint8_t> data);
+
+// Returns true if |digest| is a valid HMAC SHA-256 digest of |data| and |key|.
+bool VerifyTrustedVaultHMAC(base::span<const uint8_t> key,
+                            base::span<const uint8_t> data,
+                            base::span<const uint8_t> digest);
+
+}  // namespace syncer
+
+#endif  // COMPONENTS_SYNC_TRUSTED_VAULT_TRUSTED_VAULT_CRYPTO_H_

components/sync/trusted_vault/trusted_vault_crypto_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/sync/trusted_vault/trusted_vault_crypto.h"
+
+#include <memory>
+
+#include "base/strings/string_number_conversions.h"
+#include "components/sync/trusted_vault/securebox.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace syncer {
+
+namespace {
+
+using testing::Eq;
+using testing::Ne;
+
+const char kEncodedPrivateKey[] =
+    "49e052293c29b5a50b0013eec9d030ac2ad70a42fe093be084264647cb04e16f";
+
+std::unique_ptr<SecureBoxKeyPair> MakeTestKeyPair() {
+  std::vector<uint8_t> private_key_bytes;
+  bool success = base::HexStringToBytes(kEncodedPrivateKey, &private_key_bytes);
+  DCHECK(success);
+  return SecureBoxKeyPair::CreateByPrivateKeyImport(private_key_bytes);
+}
+
+TEST(TrustedVaultCrypto, ShouldHandleDecryptionFailure) {
+  EXPECT_THAT(DecryptTrustedVaultWrappedKey(
+                  MakeTestKeyPair()->private_key(),
+                  /*wrapped_key=*/std::vector<uint8_t>{1, 2, 3, 4}),
+              Eq(base::nullopt));
+}
+
+TEST(TrustedVaultCrypto, ShouldEncryptAndDecryptWrappedKey) {
+  const std::vector<uint8_t> trusted_vault_key = {1, 2, 3, 4};
+  const std::unique_ptr<SecureBoxKeyPair> key_pair = MakeTestKeyPair();
+  base::Optional<std::vector<uint8_t>> decrypted_trusted_vault_key =
+      DecryptTrustedVaultWrappedKey(
+          key_pair->private_key(),
+          /*wrapped_key=*/ComputeTrustedVaultWrappedKey(key_pair->public_key(),
+                                                        trusted_vault_key));
+  ASSERT_THAT(decrypted_trusted_vault_key, Ne(base::nullopt));
+  EXPECT_THAT(*decrypted_trusted_vault_key, Eq(trusted_vault_key));
+}
+
+TEST(TrustedVaultCrypto, ShouldComputeAndVerifyHMAC) {
+  const std::vector<uint8_t> key = {1, 2, 3, 4};
+  const std::vector<uint8_t> data = {1, 2, 3, 5};
+  EXPECT_TRUE(
+      VerifyTrustedVaultHMAC(key, data,
+                             /*digest=*/ComputeTrustedVaultHMAC(key, data)));
+}
+
+TEST(TrustedVaultCrypto, ShouldDetectIncorrectHMAC) {
+  const std::vector<uint8_t> correct_key = {1, 2, 3, 4};
+  const std::vector<uint8_t> incorrect_key = {1, 2, 3, 5};
+  const std::vector<uint8_t> data = {1, 2, 3, 6};
+  EXPECT_FALSE(VerifyTrustedVaultHMAC(
+      correct_key, data,
+      /*digest=*/ComputeTrustedVaultHMAC(incorrect_key, data)));
+}
+
+}  // namespace
+
+}  // namespace syncer