[TrustedVault] Connection: support registration with unknown epoch

This CL adds support for authentication factor registration with unknown
epoch/last key version. If that's the case, ListSecurityDomains request
is sent first in order to detect the actual security domain epoch if it
already exists and ensure that "last" trusted vault key is actually the
last.

Afterwards, JoinSecurityDomainsRequest is sent with detected epoch or
with epoch=0 if security domain doesn't exist. In latter case, actual
epoch will be detected using JoinSecurityDomainsResponse.

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

components/sync/BUILD.gn

@@ -478,6 +478,7 @@ source_set("unit_tests") {
     "protocol/proto_value_conversions_unittest.cc",
     "trusted_vault/download_keys_response_handler_unittest.cc",
     "trusted_vault/proto_string_bytes_conversion_unittest.cc",
+    "trusted_vault/register_authentication_factor_request_unittest.cc",
     "trusted_vault/securebox_unittest.cc",
     "trusted_vault/standalone_trusted_vault_backend_unittest.cc",
     "trusted_vault/trusted_vault_access_token_fetcher_frontend_unittest.cc",

components/sync/trusted_vault/BUILD.gn

@@ -8,6 +8,8 @@ static_library("trusted_vault") {
     "download_keys_response_handler.h",
     "proto_string_bytes_conversion.cc",
     "proto_string_bytes_conversion.h",
+    "register_authentication_factor_request.cc",
+    "register_authentication_factor_request.h",
     "securebox.cc",
     "securebox.h",
     "standalone_trusted_vault_backend.cc",

components/sync/trusted_vault/register_authentication_factor_request.cc

+// Copyright 2021 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/register_authentication_factor_request.h"
+
+#include <utility>
+
+#include "base/bind.h"
+#include "base/optional.h"
+#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 "components/sync/trusted_vault/trusted_vault_crypto.h"
+#include "components/sync/trusted_vault/trusted_vault_request.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+
+namespace syncer {
+
+namespace {
+
+// TODO(crbug.com/1113598): Find a good place for kSecurityDomainName constant,
+// which is used in several files.
+const char kSecurityDomainName[] = "chromesync";
+
+std::vector<uint8_t> CreateConstantTrustedVaultKey() {
+  return std::vector<uint8_t>(16, 0);
+}
+
+// Returns pointer to sync security domain in |response|. Returns nullptr if
+// there is no sync security domain.
+const sync_pb::SecurityDomain* FindSyncSecurityDomain(
+    const sync_pb::ListSecurityDomainsResponse& response) {
+  for (const sync_pb::SecurityDomain& security_domain :
+       response.security_domains()) {
+    if (security_domain.name() == kSecurityDomainName) {
+      return &security_domain;
+    }
+  }
+  return nullptr;
+}
+
+sync_pb::SharedKey CreateMemberSharedKey(
+    const TrustedVaultKeyAndVersion& trusted_vault_key_and_version,
+    const SecureBoxPublicKey& public_key) {
+  sync_pb::SharedKey shared_key;
+  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(
+      ComputeTrustedVaultHMAC(/*key=*/trusted_vault_key_and_version.key,
+                              /*data=*/public_key.ExportToBytes()),
+      shared_key.mutable_member_proof());
+  return shared_key;
+}
+
+sync_pb::JoinSecurityDomainsRequest CreateJoinSecurityDomainsRequest(
+    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_and_version, public_key);
+
+  sync_pb::SecurityDomain security_domain;
+  security_domain.set_name(kSecurityDomainName);
+  *security_domain.add_members() = member;
+
+  sync_pb::JoinSecurityDomainsRequest result;
+  *result.add_security_domains() = security_domain;
+  return result;
+}
+
+}  // namespace
+
+RegisterAuthenticationFactorRequest::RegisterAuthenticationFactorRequest(
+    const GURL& join_security_domains_url,
+    const GURL& list_security_domains_url,
+    scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+    const CoreAccountId& account_id,
+    const SecureBoxPublicKey& authentication_factor_public_key,
+    TrustedVaultAccessTokenFetcher* access_token_fetcher)
+    : join_security_domains_url_(join_security_domains_url),
+      list_security_domains_url_(list_security_domains_url),
+      url_loader_factory_(url_loader_factory),
+      account_id_(account_id),
+      // TODO(crbug.com/1101813): Support copy or move semantic for
+      // SecureBoxPublicKey.
+      authentication_factor_public_key_(SecureBoxPublicKey::CreateByImport(
+          authentication_factor_public_key.ExportToBytes())),
+      access_token_fetcher_(access_token_fetcher) {
+  DCHECK(access_token_fetcher_);
+}
+
+RegisterAuthenticationFactorRequest::~RegisterAuthenticationFactorRequest() =
+    default;
+
+void RegisterAuthenticationFactorRequest::StartWithConstantKey(
+    TrustedVaultConnection::RegisterAuthenticationFactorCallback
+        completion_callback) {
+  DCHECK(!ongoing_request_);
+  completion_callback_ = std::move(completion_callback);
+  ongoing_request_ = std::make_unique<TrustedVaultRequest>(
+      TrustedVaultRequest::HttpMethod::kGet, list_security_domains_url_,
+      /*serialized_request_proto=*/base::nullopt);
+
+  // Using base::Unretained() is safe here, because |this| will outlive
+  // |ongoing_request_|.
+  ongoing_request_->FetchAccessTokenAndSendRequest(
+      account_id_, url_loader_factory_, access_token_fetcher_,
+      base::BindOnce(
+          &RegisterAuthenticationFactorRequest::OnListSecurityDomainsCompleted,
+          base::Unretained(this)));
+}
+
+void RegisterAuthenticationFactorRequest::
+    StartWithKnownTrustedVaultKeyAndVersion(
+        const TrustedVaultKeyAndVersion& trusted_vault_key_and_version,
+        TrustedVaultConnection::RegisterAuthenticationFactorCallback
+            completion_callback) {
+  completion_callback_ = std::move(completion_callback);
+  StartJoinSecurityDomainsRequest(trusted_vault_key_and_version);
+}
+
+void RegisterAuthenticationFactorRequest::StartJoinSecurityDomainsRequest(
+    const TrustedVaultKeyAndVersion& trusted_vault_key_and_version) {
+  DCHECK(!ongoing_request_);
+  ongoing_request_ = std::make_unique<TrustedVaultRequest>(
+      TrustedVaultRequest::HttpMethod::kPost, join_security_domains_url_,
+      /*serialized_request_proto=*/
+      CreateJoinSecurityDomainsRequest(trusted_vault_key_and_version,
+                                       *authentication_factor_public_key_)
+          .SerializeAsString());
+
+  // Using base::Unretained() is safe here, because |this| will outlive
+  // |ongoing_request_|.
+  ongoing_request_->FetchAccessTokenAndSendRequest(
+      account_id_, url_loader_factory_, access_token_fetcher_,
+      base::BindOnce(
+          &RegisterAuthenticationFactorRequest::OnJoinSecurityDomainsResponse,
+          base::Unretained(this)));
+}
+
+void RegisterAuthenticationFactorRequest::OnListSecurityDomainsCompleted(
+    TrustedVaultRequest::HttpStatus http_status,
+    const std::string& response_body) {
+  DCHECK(ongoing_request_);
+  ongoing_request_ = nullptr;
+
+  switch (http_status) {
+    case TrustedVaultRequest::HttpStatus::kSuccess:
+      break;
+    case TrustedVaultRequest::HttpStatus::kOtherError:
+    case TrustedVaultRequest::HttpStatus::kBadRequest:
+      // Don't distinguish kBadRequest here, because JoinSecurityDomains request
+      // content doesn't depend on the local state.
+      RunCallbackAndMaybeDestroySelf(TrustedVaultRequestStatus::kOtherError);
+      return;
+  }
+
+  sync_pb::ListSecurityDomainsResponse deserialized_response;
+  if (!deserialized_response.ParseFromString(response_body)) {
+    RunCallbackAndMaybeDestroySelf(TrustedVaultRequestStatus::kOtherError);
+    return;
+  }
+
+  const sync_pb::SecurityDomain* sync_security_domain =
+      FindSyncSecurityDomain(deserialized_response);
+  if (!sync_security_domain) {
+    // Sync security domain doesn't exist yet, |version| should be set to 0.
+    StartJoinSecurityDomainsRequest(TrustedVaultKeyAndVersion(
+        /*key=*/CreateConstantTrustedVaultKey(), /*version=*/0));
+    return;
+  }
+
+  const sync_pb::SecurityDomain::Member* member_with_largest_epoch = nullptr;
+  const sync_pb::SharedKey* shared_key_with_largest_epoch = nullptr;
+  int largest_epoch = 0;
+  for (const sync_pb::SecurityDomain::Member& member :
+       sync_security_domain->members()) {
+    for (const sync_pb::SharedKey& shared_key : member.keys()) {
+      if (shared_key.epoch() > largest_epoch) {
+        largest_epoch = shared_key.epoch();
+        member_with_largest_epoch = &member;
+        shared_key_with_largest_epoch = &shared_key;
+      }
+    }
+  }
+
+  if (!member_with_largest_epoch) {
+    // Security domain doesn't contain any member with a valid shared key,
+    // |version| should be set to 0.
+    StartJoinSecurityDomainsRequest(TrustedVaultKeyAndVersion(
+        /*key=*/CreateConstantTrustedVaultKey(), /*version=*/0));
+    return;
+  }
+
+  DCHECK(shared_key_with_largest_epoch);
+  if (!VerifyTrustedVaultHMAC(
+          /*key=*/CreateConstantTrustedVaultKey(),
+          /*data=*/ProtoStringToBytes(member_with_largest_epoch->public_key()),
+          /*digest=*/
+          ProtoStringToBytes(shared_key_with_largest_epoch->member_proof()))) {
+    // Latest trusted vault key isn't constant one.
+    RunCallbackAndMaybeDestroySelf(
+        TrustedVaultRequestStatus::kLocalDataObsolete);
+    return;
+  }
+
+  StartJoinSecurityDomainsRequest(TrustedVaultKeyAndVersion(
+      /*key=*/CreateConstantTrustedVaultKey(),
+      /*version=*/shared_key_with_largest_epoch->epoch()));
+}
+
+void RegisterAuthenticationFactorRequest::OnJoinSecurityDomainsResponse(
+    TrustedVaultRequest::HttpStatus http_status,
+    const std::string& response_body) {
+  switch (http_status) {
+    case TrustedVaultRequest::HttpStatus::kSuccess:
+      RunCallbackAndMaybeDestroySelf(TrustedVaultRequestStatus::kSuccess);
+      return;
+    case TrustedVaultRequest::HttpStatus::kOtherError:
+      RunCallbackAndMaybeDestroySelf(TrustedVaultRequestStatus::kOtherError);
+      return;
+    case TrustedVaultRequest::HttpStatus::kBadRequest:
+      RunCallbackAndMaybeDestroySelf(
+          TrustedVaultRequestStatus::kLocalDataObsolete);
+      return;
+  }
+  NOTREACHED();
+}
+
+void RegisterAuthenticationFactorRequest::RunCallbackAndMaybeDestroySelf(
+    TrustedVaultRequestStatus status) {
+  std::move(completion_callback_).Run(status);
+}
+
+}  // namespace syncer

components/sync/trusted_vault/register_authentication_factor_request.h

+// Copyright 2021 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_REGISTER_AUTHENTICATION_FACTOR_REQUEST_H_
+#define COMPONENTS_SYNC_TRUSTED_VAULT_REGISTER_AUTHENTICATION_FACTOR_REQUEST_H_
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "base/memory/scoped_refptr.h"
+#include "components/sync/trusted_vault/trusted_vault_connection.h"
+#include "components/sync/trusted_vault/trusted_vault_request.h"
+#include "google_apis/gaia/core_account_id.h"
+#include "url/gurl.h"
+
+namespace network {
+class SharedURLLoaderFactory;
+}  // namespace network
+
+namespace syncer {
+
+class TrustedVaultAccessTokenFetcher;
+
+// This class supports registration of the authentication factor within trusted
+// vault server. Registration may require sending two requests to the server, if
+// trusted vault key (constant key is used) is not yet know
+// (ListSecurityDomainsRequest to retrieve the server-side state and current key
+// version, then JoinSecurityDomainsRequest to actually register
+// the authentication factor). If trusted vault key is already known, only
+// JoinSecurityDomainsRequest will be issued.
+class RegisterAuthenticationFactorRequest
+    : public TrustedVaultConnection::Request {
+ public:
+  // |access_token_fetcher| must not be null and must outlive this object.
+  RegisterAuthenticationFactorRequest(
+      const GURL& join_security_domains_url,
+      const GURL& list_security_domains_url,
+      scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory,
+      const CoreAccountId& account_id,
+      const SecureBoxPublicKey& authentication_factor_public_key,
+      TrustedVaultAccessTokenFetcher* access_token_fetcher);
+  RegisterAuthenticationFactorRequest(
+      const RegisterAuthenticationFactorRequest& other) = delete;
+  RegisterAuthenticationFactorRequest& operator=(
+      RegisterAuthenticationFactorRequest& other) = delete;
+  ~RegisterAuthenticationFactorRequest() override;
+
+  void StartWithConstantKey(
+      TrustedVaultConnection::RegisterAuthenticationFactorCallback
+          completion_callback);
+  void StartWithKnownTrustedVaultKeyAndVersion(
+      const TrustedVaultKeyAndVersion& trusted_vault_key_and_version,
+      TrustedVaultConnection::RegisterAuthenticationFactorCallback
+          completion_callback);
+
+ private:
+  void StartJoinSecurityDomainsRequest(
+      const TrustedVaultKeyAndVersion& trusted_vault_key_and_version);
+  void OnListSecurityDomainsCompleted(
+      TrustedVaultRequest::HttpStatus http_status,
+      const std::string& response_body);
+  void OnJoinSecurityDomainsResponse(
+      TrustedVaultRequest::HttpStatus http_status,
+      const std::string& response_body);
+
+  void RunCallbackAndMaybeDestroySelf(TrustedVaultRequestStatus status);
+
+  const GURL join_security_domains_url_;
+  const GURL list_security_domains_url_;
+  const scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
+  const CoreAccountId account_id_;
+  const std::unique_ptr<SecureBoxPublicKey> authentication_factor_public_key_;
+  TrustedVaultAccessTokenFetcher* const access_token_fetcher_;
+
+  std::unique_ptr<TrustedVaultRequest> ongoing_request_;
+  TrustedVaultConnection::RegisterAuthenticationFactorCallback
+      completion_callback_;
+};
+
+}  // namespace syncer
+
+#endif  // COMPONENTS_SYNC_TRUSTED_VAULT_REGISTER_AUTHENTICATION_FACTOR_REQUEST_H_

components/sync/trusted_vault/register_authentication_factor_request_unittest.cc

+// Copyright 2021 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/register_authentication_factor_request.h"
+
+#include <utility>
+
+#include "base/test/mock_callback.h"
+#include "base/test/task_environment.h"
+#include "components/signin/public/identity_manager/access_token_info.h"
+#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 "components/sync/trusted_vault/trusted_vault_access_token_fetcher.h"
+#include "components/sync/trusted_vault/trusted_vault_crypto.h"
+#include "services/network/public/cpp/shared_url_loader_factory.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"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace syncer {
+
+namespace {
+
+using testing::Eq;
+using testing::Pointee;
+
+const char kAccessToken[] = "access_token";
+const char kEncodedPrivateKey[] =
+    "49e052293c29b5a50b0013eec9d030ac2ad70a42fe093be084264647cb04e16f";
+const char kJoinSecurityDomainsURL[] = "https://test.org/test/domain:join";
+const char kListSecurityDomainsURL[] = "https://test.org/test/domain:list";
+
+std::vector<uint8_t> GetConstantKey() {
+  return std::vector<uint8_t>(16, 0);
+}
+
+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);
+}
+
+GURL GetUrlWithAlternateProtoParam(const std::string& url) {
+  return GURL(url + "?alt=proto");
+}
+
+MATCHER(IsValidListSecurityDomainsRequest, "") {
+  const network::TestURLLoaderFactory::PendingRequest& pending_http_request =
+      arg;
+  const network::ResourceRequest& resource_request =
+      pending_http_request.request;
+  return resource_request.method == "GET" &&
+         resource_request.url ==
+             GetUrlWithAlternateProtoParam(kListSecurityDomainsURL);
+}
+
+MATCHER_P(IsValidJoinSecurityDomainsRequestWithSharedKey,
+          trusted_vault_key_and_version,
+          "") {
+  const network::TestURLLoaderFactory::PendingRequest& pending_http_request =
+      arg;
+  const network::ResourceRequest& resource_request =
+      pending_http_request.request;
+  if (resource_request.method != "POST" ||
+      resource_request.url !=
+          GetUrlWithAlternateProtoParam(kJoinSecurityDomainsURL)) {
+    return false;
+  }
+
+  sync_pb::JoinSecurityDomainsRequest deserialized_body;
+  deserialized_body.ParseFromString(network::GetUploadData(resource_request));
+  if (deserialized_body.security_domains_size() != 1) {
+    return false;
+  }
+
+  sync_pb::SecurityDomain security_domain =
+      deserialized_body.security_domains(0);
+  if (security_domain.name() != "chromesync" ||
+      security_domain.members_size() != 1) {
+    return false;
+  }
+
+  const std::unique_ptr<SecureBoxKeyPair> expected_member_key_pair =
+      MakeTestKeyPair();
+  sync_pb::SecurityDomain::Member member = security_domain.members(0);
+  if (ProtoStringToBytes(member.public_key()) !=
+          expected_member_key_pair->public_key().ExportToBytes() ||
+      member.keys_size() != 1) {
+    return false;
+  }
+
+  sync_pb::SharedKey shared_key = member.keys(0);
+  // |shared_key| should correspond to |trusted_vault_key_and_version| and
+  // contain valid |member_proof|.
+  return DecryptTrustedVaultWrappedKey(
+             expected_member_key_pair->private_key(),
+             /*wrapped_key=*/ProtoStringToBytes(shared_key.wrapped_key())) ==
+             trusted_vault_key_and_version.key &&
+         shared_key.epoch() == trusted_vault_key_and_version.version &&
+         VerifyTrustedVaultHMAC(
+             /*key=*/trusted_vault_key_and_version.key,
+             /*data=*/expected_member_key_pair->public_key().ExportToBytes(),
+             /*digest=*/ProtoStringToBytes(shared_key.member_proof()));
+}
+
+// TODO(crbug.com/1113598): FakeTrustedVaultAccessTokenFetcher used in three
+// tests suites already, move it to dedicated file?
+class FakeTrustedVaultAccessTokenFetcher
+    : public TrustedVaultAccessTokenFetcher {
+ public:
+  FakeTrustedVaultAccessTokenFetcher() = default;
+  ~FakeTrustedVaultAccessTokenFetcher() override = default;
+
+  void FetchAccessToken(const CoreAccountId& account_id,
+                        TokenCallback callback) override {
+    std::move(callback).Run(signin::AccessTokenInfo(
+        kAccessToken, /*expiration_time_param=*/base::Time::Now() +
+                          base::TimeDelta::FromHours(1),
+        /*id_token=*/std::string()));
+  }
+};
+
+// TODO(crbug.com/1113598): factor out some helper functions from
+// download_keys_response_handler.cc and add coverage for more cases when
+// ListSecurityDomainsRequest is sent. Add coverage for various error cases.
+class RegisterAuthenticationFactorRequestTest : public testing::Test {
+ public:
+  RegisterAuthenticationFactorRequestTest()
+      : request_(
+            GURL(kJoinSecurityDomainsURL),
+            GURL(kListSecurityDomainsURL),
+            /*url_loader_factory=*/
+            base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
+                &test_url_loader_factory_),
+            CoreAccountId(),
+            /*authentication_factor_public_key=*/
+            MakeTestKeyPair()->public_key(),
+            &access_token_fetcher_) {}
+  ~RegisterAuthenticationFactorRequestTest() override = default;
+
+  RegisterAuthenticationFactorRequest* request() { return &request_; }
+
+  bool RespondToJoinSecurityDomainsRequest() {
+    return test_url_loader_factory_.SimulateResponseForPendingRequest(
+        GetUrlWithAlternateProtoParam(kJoinSecurityDomainsURL).spec(),
+        /*content=*/std::string(), net::HttpStatusCode::HTTP_OK);
+  }
+
+  bool RespondToListSecurityDomainsRequest(
+      const sync_pb::ListSecurityDomainsResponse& response) {
+    return test_url_loader_factory_.SimulateResponseForPendingRequest(
+        GetUrlWithAlternateProtoParam(kListSecurityDomainsURL).spec(),
+        /*content=*/response.SerializeAsString(), net::HttpStatusCode::HTTP_OK);
+  }
+
+  network::TestURLLoaderFactory::PendingRequest* GetPendingHTTPRequest() {
+    return test_url_loader_factory_.GetPendingRequest(/*index=*/0);
+  }
+
+ private:
+  base::test::TaskEnvironment task_environment_;
+  network::TestURLLoaderFactory test_url_loader_factory_;
+  FakeTrustedVaultAccessTokenFetcher access_token_fetcher_;
+
+  RegisterAuthenticationFactorRequest request_;
+};
+
+TEST_F(RegisterAuthenticationFactorRequestTest,
+       ShouldRegisterWithKnownTrustedVaultKeyAndVersion) {
+  const TrustedVaultKeyAndVersion kVaultKeyAndVersion = {
+      /*key=*/std::vector<uint8_t>{1, 2, 3, 4}, /*version=*/1234};
+  base::MockCallback<
+      TrustedVaultConnection::RegisterAuthenticationFactorCallback>
+      callback;
+
+  request()->StartWithKnownTrustedVaultKeyAndVersion(kVaultKeyAndVersion,
+                                                     callback.Get());
+  EXPECT_THAT(GetPendingHTTPRequest(),
+              Pointee(IsValidJoinSecurityDomainsRequestWithSharedKey(
+                  kVaultKeyAndVersion)));
+
+  EXPECT_CALL(callback, Run(TrustedVaultRequestStatus::kSuccess));
+  EXPECT_TRUE(RespondToJoinSecurityDomainsRequest());
+}
+
+TEST_F(RegisterAuthenticationFactorRequestTest,
+       ShouldRegisterWithConstantKeyWhenSecurityDomainNotExists) {
+  base::MockCallback<
+      TrustedVaultConnection::RegisterAuthenticationFactorCallback>
+      callback;
+  request()->StartWithConstantKey(callback.Get());
+  EXPECT_THAT(GetPendingHTTPRequest(),
+              Pointee(IsValidListSecurityDomainsRequest()));
+
+  EXPECT_TRUE(RespondToListSecurityDomainsRequest(
+      sync_pb::ListSecurityDomainsResponse()));
+  EXPECT_THAT(
+      GetPendingHTTPRequest(),
+      Pointee(IsValidJoinSecurityDomainsRequestWithSharedKey(
+          TrustedVaultKeyAndVersion(/*key=*/GetConstantKey(), /*version=*/0))));
+
+  EXPECT_CALL(callback, Run(TrustedVaultRequestStatus::kSuccess));
+  EXPECT_TRUE(RespondToJoinSecurityDomainsRequest());
+}
+
+}  // namespace
+}  // namespace syncer

components/sync/trusted_vault/standalone_trusted_vault_backend_unittest.cc

@@ -42,12 +42,23 @@ MATCHER_P(KeyMaterialEq, expected, "") {
   return key_material_as_bytes == expected;
 }
 
+// TODO(crbug.com/1102340): leave only Optional* version of this matcher once
+// TrustedVaultConnection::DownloadKeys() accept optional key.
 MATCHER_P2(TrustedVaultKeyAndVersionEq, expected_key, expected_version, "") {
   const TrustedVaultKeyAndVersion& key_and_version = arg;
   return key_and_version.key == expected_key &&
          key_and_version.version == expected_version;
 }
 
+MATCHER_P2(OptionalTrustedVaultKeyAndVersionEq,
+           expected_key,
+           expected_version,
+           "") {
+  const base::Optional<TrustedVaultKeyAndVersion>& key_and_version = arg;
+  return key_and_version.has_value() && key_and_version->key == expected_key &&
+         key_and_version->version == expected_version;
+}
+
 base::FilePath CreateUniqueTempDir(base::ScopedTempDir* temp_dir) {
   EXPECT_TRUE(temp_dir->CreateUniqueTempDir());
   return temp_dir->GetPath();
@@ -64,14 +75,14 @@ class MockTrustedVaultConnection : public TrustedVaultConnection {
  public:
   MockTrustedVaultConnection() = default;
   ~MockTrustedVaultConnection() override = default;
-  MOCK_METHOD(
-      std::unique_ptr<Request>,
-      RegisterAuthenticationFactor,
-      (const CoreAccountInfo& account_info,
-       const TrustedVaultKeyAndVersion& last_trusted_vault_key_and_version,
-       const SecureBoxPublicKey& authentication_factor_public_key,
-       RegisterAuthenticationFactorCallback callback),
-      (override));
+  MOCK_METHOD(std::unique_ptr<Request>,
+              RegisterAuthenticationFactor,
+              (const CoreAccountInfo& account_info,
+               const base::Optional<TrustedVaultKeyAndVersion>&
+                   last_trusted_vault_key_and_version,
+               const SecureBoxPublicKey& authentication_factor_public_key,
+               RegisterAuthenticationFactorCallback callback),
+              (override));
   MOCK_METHOD(
       std::unique_ptr<Request>,
       DownloadKeys,
@@ -132,11 +143,12 @@ class StandaloneTrustedVaultBackendTest : public testing::Test {
     EXPECT_CALL(*connection_,
                 RegisterAuthenticationFactor(
                     Eq(account_info),
-                    TrustedVaultKeyAndVersionEq(vault_keys.back(),
-                                                last_vault_key_version),
+                    OptionalTrustedVaultKeyAndVersionEq(vault_keys.back(),
+                                                        last_vault_key_version),
                     _, _))
         .WillOnce(
-            [&](const CoreAccountInfo&, const TrustedVaultKeyAndVersion&,
+            [&](const CoreAccountInfo&,
+                const base::Optional<TrustedVaultKeyAndVersion>&,
                 const SecureBoxPublicKey& device_public_key,
                 TrustedVaultConnection::RegisterAuthenticationFactorCallback
                     callback) {
@@ -320,12 +332,13 @@ TEST_F(StandaloneTrustedVaultBackendTest, ShouldRegisterDevice) {
   TrustedVaultConnection::RegisterAuthenticationFactorCallback
       device_registration_callback;
   std::vector<uint8_t> serialized_public_device_key;
-  EXPECT_CALL(
-      *connection(),
-      RegisterAuthenticationFactor(
-          Eq(account_info),
-          TrustedVaultKeyAndVersionEq(kVaultKey, kLastKeyVersion), _, _))
-      .WillOnce([&](const CoreAccountInfo&, const TrustedVaultKeyAndVersion&,
+  EXPECT_CALL(*connection(),
+              RegisterAuthenticationFactor(Eq(account_info),
+                                           OptionalTrustedVaultKeyAndVersionEq(
+                                               kVaultKey, kLastKeyVersion),
+                                           _, _))
+      .WillOnce([&](const CoreAccountInfo&,
+                    const base::Optional<TrustedVaultKeyAndVersion>&,
                     const SecureBoxPublicKey& device_public_key,
                     TrustedVaultConnection::RegisterAuthenticationFactorCallback
                         callback) {
@@ -368,7 +381,8 @@ TEST_F(StandaloneTrustedVaultBackendTest,
       device_registration_callback;
   ON_CALL(*connection(), RegisterAuthenticationFactor(_, _, _, _))
       .WillByDefault(
-          [&](const CoreAccountInfo&, const TrustedVaultKeyAndVersion&,
+          [&](const CoreAccountInfo&,
+              const base::Optional<TrustedVaultKeyAndVersion>&,
               const SecureBoxPublicKey&,
               TrustedVaultConnection::RegisterAuthenticationFactorCallback
                   callback) {
@@ -423,7 +437,8 @@ TEST_F(StandaloneTrustedVaultBackendTest,
       device_registration_callback;
   ON_CALL(*connection(), RegisterAuthenticationFactor(_, _, _, _))
       .WillByDefault(
-          [&](const CoreAccountInfo&, const TrustedVaultKeyAndVersion&,
+          [&](const CoreAccountInfo&,
+              const base::Optional<TrustedVaultKeyAndVersion>&,
               const SecureBoxPublicKey&,
               TrustedVaultConnection::RegisterAuthenticationFactorCallback
                   callback) {

components/sync/trusted_vault/trusted_vault_connection.h

@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "base/callback.h"
+#include "base/optional.h"
 
 struct CoreAccountInfo;
 
@@ -66,12 +67,15 @@ class TrustedVaultConnection {
 
   // Asynchronously attempts to register the authentication factor on the
   // trusted vault server to allow further vault server API calls using this
-  // authentication factor. Calls |callback| upon completion, unless the
-  // returned object is destroyed earlier. Caller should hold returned request
-  // object until |callback| call or until request needs to be cancelled.
+  // authentication factor. If |last_trusted_vault_key_and_version| is
+  // base::nullopt, registration attempt with constant key will be made. Calls
+  // |callback| upon completion, unless the returned object is destroyed
+  // earlier. Caller should hold returned request object until |callback| call
+  // or until request needs to be cancelled.
   virtual std::unique_ptr<Request> RegisterAuthenticationFactor(
       const CoreAccountInfo& account_info,
-      const TrustedVaultKeyAndVersion& last_trusted_vault_key_and_version,
+      const base::Optional<TrustedVaultKeyAndVersion>&
+          last_trusted_vault_key_and_version,
       const SecureBoxPublicKey& authentication_factor_public_key,
       RegisterAuthenticationFactorCallback callback) WARN_UNUSED_RESULT = 0;
 

components/sync/trusted_vault/trusted_vault_connection_impl.cc

@@ -11,6 +11,7 @@
 #include "components/sync/protocol/vault.pb.h"
 #include "components/sync/trusted_vault/download_keys_response_handler.h"
 #include "components/sync/trusted_vault/proto_string_bytes_conversion.h"
+#include "components/sync/trusted_vault/register_authentication_factor_request.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"
@@ -23,61 +24,6 @@ namespace {
 
 const char kJoinSecurityDomainsURLPath[] = "/domain:join";
 const char kListSecurityDomainsURLPathAndQuery[] = "/domain:list?view=1";
-const char kSecurityDomainName[] = "chromesync";
-
-void ProcessRegisterDeviceResponse(
-    TrustedVaultConnection::RegisterAuthenticationFactorCallback callback,
-    TrustedVaultRequest::HttpStatus http_status,
-    const std::string& response_body) {
-  TrustedVaultRequestStatus registration_status;
-  switch (http_status) {
-    case TrustedVaultRequest::HttpStatus::kSuccess:
-      registration_status = TrustedVaultRequestStatus::kSuccess;
-      break;
-    case TrustedVaultRequest::HttpStatus::kOtherError:
-      registration_status = TrustedVaultRequestStatus::kOtherError;
-      break;
-    case TrustedVaultRequest::HttpStatus::kBadRequest:
-      // Bad request response indicates that client data is outdated (e.g.
-      // locally available trusted vault key is not the recent one).
-      registration_status = TrustedVaultRequestStatus::kLocalDataObsolete;
-  }
-  std::move(callback).Run(registration_status);
-}
-
-sync_pb::SharedKey CreateMemberSharedKey(
-    const TrustedVaultKeyAndVersion& trusted_vault_key_and_version,
-    const SecureBoxPublicKey& public_key) {
-  sync_pb::SharedKey shared_key;
-  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(
-      ComputeTrustedVaultHMAC(/*key=*/trusted_vault_key_and_version.key,
-                              /*data=*/public_key.ExportToBytes()),
-      shared_key.mutable_member_proof());
-  return shared_key;
-}
-
-sync_pb::JoinSecurityDomainsRequest CreateJoinSecurityDomainsRequest(
-    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_and_version, public_key);
-
-  sync_pb::SecurityDomain security_domain;
-  security_domain.set_name(kSecurityDomainName);
-  *security_domain.add_members() = member;
-
-  sync_pb::JoinSecurityDomainsRequest result;
-  *result.add_security_domains() = security_domain;
-  return result;
-}
 
 void ProcessDownloadKeysResponse(
     std::unique_ptr<DownloadKeysResponseHandler> response_handler,
@@ -108,20 +54,24 @@ TrustedVaultConnectionImpl::~TrustedVaultConnectionImpl() = default;
 std::unique_ptr<TrustedVaultConnection::Request>
 TrustedVaultConnectionImpl::RegisterAuthenticationFactor(
     const CoreAccountInfo& account_info,
-    const TrustedVaultKeyAndVersion& last_trusted_vault_key_and_version,
+    const base::Optional<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_and_version,
-                                       public_key)
-          .SerializeAsString());
-  request->FetchAccessTokenAndSendRequest(
-      account_info.account_id, GetOrCreateURLLoaderFactory(),
-      access_token_fetcher_.get(),
-      base::BindOnce(ProcessRegisterDeviceResponse, std::move(callback)));
+  auto request = std::make_unique<RegisterAuthenticationFactorRequest>(
+      /*join_security_domains_url=*/GURL(trusted_vault_service_url_.spec() +
+                                         kJoinSecurityDomainsURLPath),
+      /*list_security_domains_url=*/
+      GURL(trusted_vault_service_url_.spec() +
+           kListSecurityDomainsURLPathAndQuery),
+      GetOrCreateURLLoaderFactory(), account_info.account_id, public_key,
+      access_token_fetcher_.get());
+  if (last_trusted_vault_key_and_version.has_value()) {
+    request->StartWithKnownTrustedVaultKeyAndVersion(
+        *last_trusted_vault_key_and_version, std::move(callback));
+  } else {
+    request->StartWithConstantKey(std::move(callback));
+  }
   return request;
 }
 

components/sync/trusted_vault/trusted_vault_connection_impl.h

@@ -38,7 +38,8 @@ class TrustedVaultConnectionImpl : public TrustedVaultConnection {
 
   std::unique_ptr<Request> RegisterAuthenticationFactor(
       const CoreAccountInfo& account_info,
-      const TrustedVaultKeyAndVersion& last_trusted_vault_key_and_version,
+      const base::Optional<TrustedVaultKeyAndVersion>&
+          last_trusted_vault_key_and_version,
       const SecureBoxPublicKey& authentication_factor_public_key,
       RegisterAuthenticationFactorCallback callback) override;
 

components/sync/trusted_vault/trusted_vault_connection_impl_unittest.cc

@@ -75,6 +75,9 @@ class FakeTrustedVaultAccessTokenFetcher
   const base::Optional<std::string> access_token_;
 };
 
+// TODO(crbug.com/1113598): revisit this tests suite and determine what actually
+// should be tested on the Connection level and what should be done on lower
+// levels (DownloadKeysResponseHandler and RegisterAuthenticationFactorRequest).
 class TrustedVaultConnectionImplTest : public testing::Test {
  public:
   TrustedVaultConnectionImplTest()