[TrustedVault] Add TrustedVaultKeyAndVersion struct

Trusted vault key is often passed as argument together with its
version. Moreover, there are plans to pass them as optional parameters.

This CL introduces a new struct that stores both vault key and its
version and updates some APIs to use it.

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

components/sync/trusted_vault/BUILD.gn

@@ -19,6 +19,7 @@ static_library("trusted_vault") {
     "trusted_vault_access_token_fetcher_frontend.h",
     "trusted_vault_access_token_fetcher_impl.cc",
     "trusted_vault_access_token_fetcher_impl.h",
+    "trusted_vault_connection.cc",
     "trusted_vault_connection.h",
     "trusted_vault_connection_impl.cc",
     "trusted_vault_connection_impl.h",

components/sync/trusted_vault/download_keys_response_handler.cc

@@ -87,19 +87,21 @@ std::vector<ExtractedSharedKey> ExtractAndSortSharedKeys(
 }
 
 // Validates |key_proof| starting from the key next to
-// |last_known_trusted_vault_key|, returns false if validation fails or |keys|
-// doesn't have a key next to |last_known_trusted_vault_key|.
-bool IsValidKeyChain(const std::vector<ExtractedSharedKey>& key_chain,
-                     const std::vector<uint8_t>& last_known_trusted_vault_key,
-                     const int last_known_trusted_vault_key_version) {
+// last known trusted vault key, returns false if validation fails or |keys|
+// doesn't have a key next to last known trusted vault key.
+bool IsValidKeyChain(
+    const std::vector<ExtractedSharedKey>& key_chain,
+    const TrustedVaultKeyAndVersion& last_known_trusted_vault_key_and_version) {
   DCHECK(!key_chain.empty());
-  if (key_chain.back().version <= last_known_trusted_vault_key_version) {
+  if (key_chain.back().version <=
+      last_known_trusted_vault_key_and_version.version) {
     // |keys| doesn't contain any new key. Note: this may mean that key rotation
     // happened, but state corresponding to the current member wasn't updated.
     return false;
   }
-  int last_valid_key_version = last_known_trusted_vault_key_version;
-  std::vector<uint8_t> last_valid_key = last_known_trusted_vault_key;
+  int last_valid_key_version = last_known_trusted_vault_key_and_version.version;
+  std::vector<uint8_t> last_valid_key =
+      last_known_trusted_vault_key_and_version.key;
   for (const ExtractedSharedKey& next_key : key_chain) {
     if (next_key.version <= last_valid_key_version) {
       continue;
@@ -144,11 +146,9 @@ DownloadKeysResponseHandler::ProcessedResponse::operator=(
 DownloadKeysResponseHandler::ProcessedResponse::~ProcessedResponse() = default;
 
 DownloadKeysResponseHandler::DownloadKeysResponseHandler(
-    const std::vector<uint8_t>& last_trusted_vault_key,
-    int last_trusted_vault_key_version,
+    const TrustedVaultKeyAndVersion& last_trusted_vault_key_and_version,
     std::unique_ptr<SecureBoxKeyPair> device_key_pair)
-    : last_trusted_vault_key_(last_trusted_vault_key),
-      last_trusted_vault_key_version_(last_trusted_vault_key_version),
+    : last_trusted_vault_key_and_version_(last_trusted_vault_key_and_version),
       device_key_pair_(std::move(device_key_pair)) {
   DCHECK(device_key_pair_);
 }
@@ -187,18 +187,17 @@ DownloadKeysResponseHandler::ProcessResponse(
   std::vector<ExtractedSharedKey> extracted_keys = ExtractAndSortSharedKeys(
       *current_member, device_key_pair_->private_key());
   if (extracted_keys.empty() ||
-      !IsValidKeyChain(extracted_keys, last_trusted_vault_key_,
-                       last_trusted_vault_key_version_)) {
+      !IsValidKeyChain(extracted_keys, last_trusted_vault_key_and_version_)) {
     return ProcessedResponse(
         /*status=*/TrustedVaultRequestStatus::kLocalDataObsolete);
   }
 
   std::vector<std::vector<uint8_t>> new_keys;
   for (const ExtractedSharedKey& key : extracted_keys) {
-    if (key.version >= last_trusted_vault_key_version_) {
+    if (key.version >= last_trusted_vault_key_and_version_.version) {
       // Don't include previous keys into the result, because they weren't
-      // validated using |last_trusted_vault_key_| and client should be already
-      // aware of them.
+      // validated using |last_trusted_vault_key_and_version| and client should
+      // be already aware of them.
       new_keys.push_back(key.trusted_vault_key);
     }
   }

components/sync/trusted_vault/download_keys_response_handler.h

@@ -38,20 +38,19 @@ class DownloadKeysResponseHandler {
     // isn't valid serialized ListSecurityDomainsResponse proto.
     TrustedVaultRequestStatus status;
 
-    // Contains new keys and potentially |last_trusted_vault_key| if it wasn't
-    // removed server-side. Doesn't contain keys that predate
-    // |last_trusted_vault_key|, because it's impossible to validate them and
-    // the client should be aware of them already.
+    // Contains new keys and potentially previously known trusted vault key if
+    // it wasn't removed server-side. Doesn't contain keys that predate
+    // last known trusted vault key, because it's impossible to validate them
+    // and the client should be aware of them already.
     std::vector<std::vector<uint8_t>> keys;
     int last_key_version;
   };
 
   // |device_key_pair| must not be null. This class doesn't make an assumption
-  // of |last_trusted_vault_key| being non-empty or
-  // |last_trusted_vault_key_version| being non-negative.
+  // of |last_trusted_vault_key_and_version.key| being non-empty or
+  // |last_trusted_vault_key_version_and_version.version| being non-negative.
   DownloadKeysResponseHandler(
-      const std::vector<uint8_t>& last_trusted_vault_key,
-      int last_trusted_vault_key_version,
+      const TrustedVaultKeyAndVersion& last_trusted_vault_key_and_version,
       std::unique_ptr<SecureBoxKeyPair> device_key_pair);
   DownloadKeysResponseHandler(const DownloadKeysResponseHandler& other) =
       delete;
@@ -63,8 +62,7 @@ class DownloadKeysResponseHandler {
                                     const std::string& response_body) const;
 
  private:
-  const std::vector<uint8_t> last_trusted_vault_key_;
-  const int last_trusted_vault_key_version_;
+  const TrustedVaultKeyAndVersion last_trusted_vault_key_and_version_;
   const std::unique_ptr<SecureBoxKeyPair> device_key_pair_;
 };
 

components/sync/trusted_vault/download_keys_response_handler_unittest.cc

@@ -33,6 +33,8 @@ std::unique_ptr<SecureBoxKeyPair> MakeTestKeyPair() {
   return SecureBoxKeyPair::CreateByPrivateKeyImport(private_key_bytes);
 }
 
+// TODO(crbug.com/1113598): consider using TrustedVaultKeyAndVersion instead of
+// |trusted_vault_key| and |trusted_vault_keys_versions|.
 void FillSecurityDomainMember(
     const SecureBoxPublicKey& public_key,
     const std::vector<std::vector<uint8_t>>& trusted_vault_keys,
@@ -77,8 +79,8 @@ std::string CreateListSecurityDomainsResponseWithSingleSyncMember(
 class DownloadKeysResponseHandlerTest : public testing::Test {
  public:
   DownloadKeysResponseHandlerTest()
-      : handler_(kKnownTrustedVaultKey,
-                 kKnownTrustedVaultKeyVersion,
+      : handler_(TrustedVaultKeyAndVersion(kKnownTrustedVaultKey,
+                                           kKnownTrustedVaultKeyVersion),
                  MakeTestKeyPair()) {}
 
   ~DownloadKeysResponseHandlerTest() override = default;

components/sync/trusted_vault/standalone_trusted_vault_backend.cc

@@ -51,6 +51,16 @@ void WriteToDisk(const sync_pb::LocalTrustedVault& data,
   }
 }
 
+base::Optional<TrustedVaultKeyAndVersion> GetLastTrustedVaultKeyAndVersion(
+    const sync_pb::LocalTrustedVaultPerUser& per_user_vault) {
+  if (per_user_vault.vault_key_size() != 0) {
+    return TrustedVaultKeyAndVersion(
+        ProtoStringToBytes(per_user_vault.vault_key().rbegin()->key_material()),
+        per_user_vault.last_vault_key_version());
+  }
+  return base::nullopt;
+}
+
 }  // namespace
 
 StandaloneTrustedVaultBackend::StandaloneTrustedVaultBackend(
@@ -115,21 +125,20 @@ void StandaloneTrustedVaultBackend::FetchKeys(
     FulfillOngoingFetchKeys();
     return;
   }
-  if (per_user_vault->vault_key_size() == 0) {
-    // Corrupted state: device is registered, but there is no vault keys.
-    // TODO(crbug.com/1094326): restore from this state (just mark device as not
-    // registered?).
+  base::Optional<TrustedVaultKeyAndVersion> last_trusted_vault_key_and_version =
+      GetLastTrustedVaultKeyAndVersion(*per_user_vault);
+  if (!last_trusted_vault_key_and_version.has_value()) {
+    // TODO(crbug.com/1094326): properly support this state (constant key case).
     FulfillOngoingFetchKeys();
+    NOTIMPLEMENTED();
     return;
   }
 
   // |this| outlives |connection_| and |ongoing_connection_request_|, so it's
   // safe to use base::Unretained() here.
   ongoing_connection_request_ = connection_->DownloadKeys(
-      *primary_account_,
-      /*last_trusted_vault_key=*/
-      ProtoStringToBytes(per_user_vault->vault_key().rbegin()->key_material()),
-      per_user_vault->last_vault_key_version(), std::move(key_pair),
+      *primary_account_, *last_trusted_vault_key_and_version,
+      std::move(key_pair),
       base::BindOnce(&StandaloneTrustedVaultBackend::OnKeysDownloaded,
                      base::Unretained(this), account_info.gaia));
   DCHECK(ongoing_connection_request_);
@@ -250,9 +259,19 @@ void StandaloneTrustedVaultBackend::MaybeRegisterDevice(
     return;
   }
   sync_pb::LocalTrustedVaultPerUser* per_user_vault = FindUserVault(gaia_id);
-  if (!per_user_vault || per_user_vault->vault_key_size() == 0 ||
+  if (!per_user_vault) {
+    // TODO(crbug.com/1102340): make non-null |per_user_vault| a precondition
+    // for this function?
+    return;
+  }
+  base::Optional<TrustedVaultKeyAndVersion> last_trusted_vault_key_and_version =
+      GetLastTrustedVaultKeyAndVersion(*per_user_vault);
+  if (!last_trusted_vault_key_and_version.has_value() ||
       per_user_vault->keys_are_stale()) {
     // Fresh vault key is required to register the device.
+    // TODO(crbug.com/1102340): relax this condition to support device
+    // registration without real trusted vault key.
+    NOTIMPLEMENTED();
     return;
   }
   if (per_user_vault->local_device_registration_info().device_registered()) {
@@ -294,10 +313,8 @@ void StandaloneTrustedVaultBackend::MaybeRegisterDevice(
   // |this| outlives |connection_| and |ongoing_connection_request_|, so it's
   // safe to use base::Unretained() here.
   ongoing_connection_request_ = connection_->RegisterAuthenticationFactor(
-      *primary_account_,
-      /*last_trusted_vault_key=*/
-      ProtoStringToBytes(per_user_vault->vault_key().rbegin()->key_material()),
-      per_user_vault->last_vault_key_version(), key_pair->public_key(),
+      *primary_account_, *last_trusted_vault_key_and_version,
+      key_pair->public_key(),
       base::BindOnce(&StandaloneTrustedVaultBackend::OnDeviceRegistered,
                      base::Unretained(this), gaia_id));
   DCHECK(ongoing_connection_request_);

components/sync/trusted_vault/trusted_vault_connection.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_connection.h"
+
+namespace syncer {
+
+TrustedVaultKeyAndVersion::TrustedVaultKeyAndVersion(
+    const std::vector<uint8_t>& key,
+    int version)
+    : key(key), version(version) {}
+
+TrustedVaultKeyAndVersion::TrustedVaultKeyAndVersion(
+    const TrustedVaultKeyAndVersion& other) = default;
+
+TrustedVaultKeyAndVersion& TrustedVaultKeyAndVersion::operator=(
+    const TrustedVaultKeyAndVersion& other) = default;
+
+TrustedVaultKeyAndVersion::~TrustedVaultKeyAndVersion() = default;
+
+}  // namespace syncer

components/sync/trusted_vault/trusted_vault_connection.h

@@ -27,6 +27,16 @@ enum class TrustedVaultRequestStatus {
   kOtherError
 };
 
+struct TrustedVaultKeyAndVersion {
+  TrustedVaultKeyAndVersion(const std::vector<uint8_t>& key, int version);
+  TrustedVaultKeyAndVersion(const TrustedVaultKeyAndVersion& other);
+  TrustedVaultKeyAndVersion& operator=(const TrustedVaultKeyAndVersion& other);
+  ~TrustedVaultKeyAndVersion();
+
+  std::vector<uint8_t> key;
+  int version;
+};
+
 // Supports interaction with vault service, all methods must called on trusted
 // vault backend sequence.
 class TrustedVaultConnection {
@@ -61,8 +71,7 @@ class TrustedVaultConnection {
   // object until |callback| call or until request needs to be cancelled.
   virtual std::unique_ptr<Request> RegisterAuthenticationFactor(
       const CoreAccountInfo& account_info,
-      const std::vector<uint8_t>& last_trusted_vault_key,
-      int last_trusted_vault_key_version,
+      const TrustedVaultKeyAndVersion& last_trusted_vault_key_and_version,
       const SecureBoxPublicKey& authentication_factor_public_key,
       RegisterAuthenticationFactorCallback callback) WARN_UNUSED_RESULT = 0;
 
@@ -71,8 +80,7 @@ class TrustedVaultConnection {
   // or until request needs to be cancelled.
   virtual std::unique_ptr<Request> DownloadKeys(
       const CoreAccountInfo& account_info,
-      const std::vector<uint8_t>& last_trusted_vault_key,
-      int last_trusted_vault_key_version,
+      const TrustedVaultKeyAndVersion& last_trusted_vault_key_and_version,
       std::unique_ptr<SecureBoxKeyPair> device_key_pair,
       DownloadKeysCallback callback) WARN_UNUSED_RESULT = 0;
 };

components/sync/trusted_vault/trusted_vault_connection_impl.cc

@@ -46,31 +46,29 @@ void ProcessRegisterDeviceResponse(
 }
 
 sync_pb::SharedKey CreateMemberSharedKey(
-    int trusted_vault_key_version,
-    const std::vector<uint8_t>& trusted_vault_key,
+    const TrustedVaultKeyAndVersion& trusted_vault_key_and_version,
     const SecureBoxPublicKey& public_key) {
   sync_pb::SharedKey shared_key;
-  shared_key.set_epoch(trusted_vault_key_version);
+  shared_key.set_epoch(trusted_vault_key_and_version.version);
+  AssignBytesToProtoString(ComputeTrustedVaultWrappedKey(
+                               public_key, trusted_vault_key_and_version.key),
+                           shared_key.mutable_wrapped_key());
   AssignBytesToProtoString(
-      ComputeTrustedVaultWrappedKey(public_key, trusted_vault_key),
-      shared_key.mutable_wrapped_key());
-  AssignBytesToProtoString(
-      ComputeTrustedVaultHMAC(/*key=*/trusted_vault_key,
+      ComputeTrustedVaultHMAC(/*key=*/trusted_vault_key_and_version.key,
                               /*data=*/public_key.ExportToBytes()),
       shared_key.mutable_member_proof());
   return shared_key;
 }
 
 sync_pb::JoinSecurityDomainsRequest CreateJoinSecurityDomainsRequest(
-    int last_trusted_vault_key_version,
-    const std::vector<uint8_t>& last_trusted_vault_key,
+    const TrustedVaultKeyAndVersion& last_trusted_vault_key_and_version,
     const SecureBoxPublicKey& public_key) {
   sync_pb::SecurityDomain::Member member;
   const std::vector<uint8_t> public_key_bytes = public_key.ExportToBytes();
   AssignBytesToProtoString(public_key.ExportToBytes(),
                            member.mutable_public_key());
-  *member.add_keys() = CreateMemberSharedKey(
-      last_trusted_vault_key_version, last_trusted_vault_key, public_key);
+  *member.add_keys() =
+      CreateMemberSharedKey(last_trusted_vault_key_and_version, public_key);
 
   sync_pb::SecurityDomain security_domain;
   security_domain.set_name(kSecurityDomainName);
@@ -110,16 +108,15 @@ TrustedVaultConnectionImpl::~TrustedVaultConnectionImpl() = default;
 std::unique_ptr<TrustedVaultConnection::Request>
 TrustedVaultConnectionImpl::RegisterAuthenticationFactor(
     const CoreAccountInfo& account_info,
-    const std::vector<uint8_t>& last_trusted_vault_key,
-    int last_trusted_vault_key_version,
+    const TrustedVaultKeyAndVersion& last_trusted_vault_key_and_version,
     const SecureBoxPublicKey& public_key,
     RegisterAuthenticationFactorCallback callback) {
   auto request = std::make_unique<TrustedVaultRequest>(
       TrustedVaultRequest::HttpMethod::kPost,
       GURL(trusted_vault_service_url_.spec() + kJoinSecurityDomainsURLPath),
       /*serialized_request_proto=*/
-      CreateJoinSecurityDomainsRequest(last_trusted_vault_key_version,
-                                       last_trusted_vault_key, public_key)
+      CreateJoinSecurityDomainsRequest(last_trusted_vault_key_and_version,
+                                       public_key)
           .SerializeAsString());
   request->FetchAccessTokenAndSendRequest(
       account_info.account_id, GetOrCreateURLLoaderFactory(),
@@ -131,8 +128,7 @@ TrustedVaultConnectionImpl::RegisterAuthenticationFactor(
 std::unique_ptr<TrustedVaultConnection::Request>
 TrustedVaultConnectionImpl::DownloadKeys(
     const CoreAccountInfo& account_info,
-    const std::vector<uint8_t>& last_trusted_vault_key,
-    int last_trusted_vault_key_version,
+    const TrustedVaultKeyAndVersion& last_trusted_vault_key_and_version,
     std::unique_ptr<SecureBoxKeyPair> device_key_pair,
     DownloadKeysCallback callback) {
   auto request = std::make_unique<TrustedVaultRequest>(
@@ -144,12 +140,12 @@ TrustedVaultConnectionImpl::DownloadKeys(
   request->FetchAccessTokenAndSendRequest(
       account_info.account_id, GetOrCreateURLLoaderFactory(),
       access_token_fetcher_.get(),
-      base::BindOnce(ProcessDownloadKeysResponse,
-                     /*response_processor=*/
-                     std::make_unique<DownloadKeysResponseHandler>(
-                         last_trusted_vault_key, last_trusted_vault_key_version,
-                         std::move(device_key_pair)),
-                     std::move(callback)));
+      base::BindOnce(
+          ProcessDownloadKeysResponse,
+          /*response_processor=*/
+          std::make_unique<DownloadKeysResponseHandler>(
+              last_trusted_vault_key_and_version, std::move(device_key_pair)),
+          std::move(callback)));
 
   return request;
 }

components/sync/trusted_vault/trusted_vault_connection_impl.h

@@ -38,15 +38,13 @@ class TrustedVaultConnectionImpl : public TrustedVaultConnection {
 
   std::unique_ptr<Request> RegisterAuthenticationFactor(
       const CoreAccountInfo& account_info,
-      const std::vector<uint8_t>& last_trusted_vault_key,
-      int last_trusted_vault_key_version,
+      const TrustedVaultKeyAndVersion& last_trusted_vault_key_and_version,
       const SecureBoxPublicKey& authentication_factor_public_key,
       RegisterAuthenticationFactorCallback callback) override;
 
   std::unique_ptr<Request> DownloadKeys(
       const CoreAccountInfo& account_info,
-      const std::vector<uint8_t>& last_trusted_vault_key,
-      int last_trusted_vault_key_version,
+      const TrustedVaultKeyAndVersion& last_trusted_vault_key_and_version,
       std::unique_ptr<SecureBoxKeyPair> device_key_pair,
       DownloadKeysCallback callback) override;
 

components/sync/trusted_vault/trusted_vault_connection_impl_unittest.cc

@@ -144,7 +144,8 @@ TEST_F(TrustedVaultConnectionImplTest, ShouldSendJoinSecurityDomainsRequest) {
 
   std::unique_ptr<TrustedVaultConnection::Request> request =
       connection()->RegisterAuthenticationFactor(
-          /*account_info=*/CoreAccountInfo(), kTrustedVaultKey, kLastKeyVersion,
+          /*account_info=*/CoreAccountInfo(),
+          TrustedVaultKeyAndVersion(kTrustedVaultKey, kLastKeyVersion),
           key_pair->public_key(),
           TrustedVaultConnection::RegisterAuthenticationFactorCallback());
   EXPECT_THAT(request, NotNull());
@@ -204,9 +205,9 @@ TEST_F(TrustedVaultConnectionImplTest,
 
   std::unique_ptr<TrustedVaultConnection::Request> request =
       connection()->RegisterAuthenticationFactor(
-          /*account_info=*/CoreAccountInfo(), kTrustedVaultKey,
-          /*last_trusted_vault_key_version=*/1, key_pair->public_key(),
-          callback.Get());
+          /*account_info=*/CoreAccountInfo(),
+          TrustedVaultKeyAndVersion(kTrustedVaultKey, /*version=*/1),
+          key_pair->public_key(), callback.Get());
   ASSERT_THAT(request, NotNull());
 
   EXPECT_CALL(callback, Run(Eq(TrustedVaultRequestStatus::kSuccess)));
@@ -224,9 +225,9 @@ TEST_F(TrustedVaultConnectionImplTest,
 
   std::unique_ptr<TrustedVaultConnection::Request> request =
       connection()->RegisterAuthenticationFactor(
-          /*account_info=*/CoreAccountInfo(), kTrustedVaultKey,
-          /*last_trusted_vault_key_version=*/1, key_pair->public_key(),
-          callback.Get());
+          /*account_info=*/CoreAccountInfo(),
+          TrustedVaultKeyAndVersion(kTrustedVaultKey, /*version=*/1),
+          key_pair->public_key(), callback.Get());
   ASSERT_THAT(request, NotNull());
 
   EXPECT_CALL(callback, Run(Eq(TrustedVaultRequestStatus::kOtherError)));
@@ -245,13 +246,13 @@ TEST_F(TrustedVaultConnectionImplTest,
 
   std::unique_ptr<TrustedVaultConnection::Request> request =
       connection()->RegisterAuthenticationFactor(
-          /*account_info=*/CoreAccountInfo(), kTrustedVaultKey,
-          /*last_trusted_vault_key_version=*/1, key_pair->public_key(),
-          callback.Get());
+          /*account_info=*/CoreAccountInfo(),
+          TrustedVaultKeyAndVersion(kTrustedVaultKey, /*version=*/1),
+          key_pair->public_key(), callback.Get());
   ASSERT_THAT(request, NotNull());
 
   // In particular, HTTP_BAD_REQUEST indicates that
-  // |last_trusted_vault_key_version| is not actually the last on the server
+  // |last_trusted_vault_key_and_version| is not actually the last on the server
   // side.
   EXPECT_CALL(callback, Run(Eq(TrustedVaultRequestStatus::kLocalDataObsolete)));
   EXPECT_TRUE(RespondToJoinSecurityDomainsRequest(net::HTTP_BAD_REQUEST));
@@ -276,9 +277,9 @@ TEST_F(
   EXPECT_CALL(callback, Run(Eq(TrustedVaultRequestStatus::kOtherError)));
   std::unique_ptr<TrustedVaultConnection::Request> request =
       connection->RegisterAuthenticationFactor(
-          /*account_info=*/CoreAccountInfo(), kTrustedVaultKey,
-          /*last_trusted_vault_key_version=*/1, key_pair->public_key(),
-          callback.Get());
+          /*account_info=*/CoreAccountInfo(),
+          TrustedVaultKeyAndVersion(kTrustedVaultKey, /*version=*/1),
+          key_pair->public_key(), callback.Get());
   ASSERT_THAT(request, NotNull());
 
   // No requests should be sent to the network.
@@ -295,9 +296,9 @@ TEST_F(TrustedVaultConnectionImplTest, ShouldCancelJoinSecurityDomainsRequest) {
 
   std::unique_ptr<TrustedVaultConnection::Request> request =
       connection()->RegisterAuthenticationFactor(
-          /*account_info=*/CoreAccountInfo(), kTrustedVaultKey,
-          /*last_trusted_vault_key_version=*/1, key_pair->public_key(),
-          callback.Get());
+          /*account_info=*/CoreAccountInfo(),
+          TrustedVaultKeyAndVersion(kTrustedVaultKey, /*version=*/1),
+          key_pair->public_key(), callback.Get());
   ASSERT_THAT(request, NotNull());
 
   EXPECT_CALL(callback, Run).Times(0);
@@ -311,8 +312,8 @@ TEST_F(TrustedVaultConnectionImplTest, ShouldSendListSecurityDomainsRequest) {
   std::unique_ptr<TrustedVaultConnection::Request> request =
       connection()->DownloadKeys(
           /*account_info=*/CoreAccountInfo(),
-          /*last_trusted_vault_key=*/std::vector<uint8_t>(),
-          /*last_trusted_vault_key_version=*/0,
+          TrustedVaultKeyAndVersion(/*key=*/std::vector<uint8_t>(),
+                                    /*version=*/0),
           /*device_key_pair=*/MakeTestKeyPair(), base::DoNothing());
   EXPECT_THAT(request, NotNull());
 
@@ -336,8 +337,8 @@ TEST_F(TrustedVaultConnectionImplTest,
   std::unique_ptr<TrustedVaultConnection::Request> request =
       connection()->DownloadKeys(
           /*account_info=*/CoreAccountInfo(),
-          /*last_trusted_vault_key=*/std::vector<uint8_t>(),
-          /*last_trusted_vault_key_version=*/0,
+          TrustedVaultKeyAndVersion(/*key=*/std::vector<uint8_t>(),
+                                    /*version=*/0),
           /*device_key_pair=*/MakeTestKeyPair(), callback.Get());
   ASSERT_THAT(request, NotNull());
 
@@ -360,8 +361,9 @@ TEST_F(TrustedVaultConnectionImplTest,
   std::unique_ptr<TrustedVaultConnection::Request> request =
       connection->DownloadKeys(
           /*account_info=*/CoreAccountInfo(),
-          /*last_trusted_vault_key=*/std::vector<uint8_t>(),
-          /*last_trusted_vault_key_version=*/0,
+          TrustedVaultKeyAndVersion(
+              /*key=*/std::vector<uint8_t>(),
+              /*version=*/0),
           /*device_key_pair=*/MakeTestKeyPair(), callback.Get());
   ASSERT_THAT(request, NotNull());
 
@@ -375,8 +377,9 @@ TEST_F(TrustedVaultConnectionImplTest, ShouldCancelListSecurityDomainsRequest) {
   std::unique_ptr<TrustedVaultConnection::Request> request =
       connection()->DownloadKeys(
           /*account_info=*/CoreAccountInfo(),
-          /*last_trusted_vault_key=*/std::vector<uint8_t>(),
-          /*last_trusted_vault_key_version=*/0,
+          TrustedVaultKeyAndVersion(
+              /*key=*/std::vector<uint8_t>(),
+              /*version=*/0),
           /*device_key_pair=*/MakeTestKeyPair(), callback.Get());
   ASSERT_THAT(request, NotNull());