[TrustedVault] Support basic registration of authentication factor

This CL adds partial implementation of TrustedVaultConnectionImpl to
support basic scenario of authentication factor registion, when there
is known and pre-existing trusted vault key.

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

components/sync/BUILD.gn

@@ -479,6 +479,7 @@ source_set("unit_tests") {
     "trusted_vault/securebox_unittest.cc",
     "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_request_unittest.cc",
   ]
 

components/sync/base/sync_base_switches.cc

@@ -9,6 +9,9 @@ namespace switches {
 // Overrides the default server used for profile sync.
 const char kSyncServiceURL[] = "sync-url";
 
+// Specifies the vault server used for trusted vault passphrase.
+const char kTrustedVaultServiceURL[] = "trusted-vault-service-url";
+
 const base::Feature kSyncNigoriRemoveMetadataOnCacheGuidMismatch{
     "SyncNigoriRemoveMetadataOnCacheGuidMismatch",
     base::FEATURE_ENABLED_BY_DEFAULT};

components/sync/base/sync_base_switches.h

@@ -11,6 +11,7 @@
 namespace switches {
 
 extern const char kSyncServiceURL[];
+extern const char kTrustedVaultServiceURL[];
 
 extern const base::Feature kSyncNigoriRemoveMetadataOnCacheGuidMismatch;
 extern const base::Feature kSyncForceDisableScryptForCustomPassphrase;

components/sync/protocol/protocol_sources.gni

@@ -62,6 +62,7 @@ sync_protocol_bases = [
   "user_consent_specifics",
   "user_consent_types",
   "user_event_specifics",
+  "vault",
   "web_app_specifics",
   "wifi_configuration_specifics",
 ]

components/sync/protocol/vault.proto

+// 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.
+
+syntax = "proto2";
+
+option optimize_for = LITE_RUNTIME;
+
+option java_package = "sync_pb";
+package sync_pb;
+
+message SharedKey {
+  optional int32 epoch = 1;
+  optional bytes wrapped_key = 2;
+  optional bytes member_proof = 3;
+}
+
+message SecurityDomain {
+  optional string name = 1;
+
+  message Member {
+    optional bytes public_key = 1;
+    repeated SharedKey keys = 2;
+  }
+
+  repeated Member members = 2;
+}
+
+message JoinSecurityDomainsRequest {
+  repeated SecurityDomain security_domains = 1;
+}

components/sync/trusted_vault/standalone_trusted_vault_backend.cc

@@ -13,7 +13,6 @@
 #include "base/logging.h"
 #include "base/sequence_checker.h"
 #include "components/os_crypt/os_crypt.h"
-#include "components/sync/driver/sync_driver_switches.h"
 #include "components/sync/trusted_vault/securebox.h"
 
 namespace syncer {
@@ -79,8 +78,7 @@ void StandaloneTrustedVaultBackend::FetchKeys(
   // |primary_account_| is set before FetchKeys() call and this may cause
   // redundant sync error in the UI (for key retrieval), especially during the
   // browser startup. Try to find a way to avoid this issue.
-  if (!base::FeatureList::IsEnabled(switches::kFollowTrustedVaultKeyRotation) ||
-      !primary_account_.has_value() ||
+  if (!connection_ || !primary_account_.has_value() ||
       primary_account_->gaia != account_info.gaia || !per_user_vault ||
       !per_user_vault->keys_are_stale() ||
       !per_user_vault->local_device_registration_info().device_registered()) {
@@ -96,7 +94,7 @@ void StandaloneTrustedVaultBackend::FetchKeys(
   // |account_info|.
   // 3. Concurrent FetchKeys() calls aren't supported, so there is no keys
   // download for |account_info|.
-  DCHECK(!weak_factory_for_connection_.HasWeakPtrs());
+  DCHECK(!ongoing_connection_request_);
 
   std::unique_ptr<SecureBoxKeyPair> key_pair =
       SecureBoxKeyPair::CreateByPrivateKeyImport(base::as_bytes(
@@ -122,12 +120,14 @@ void StandaloneTrustedVaultBackend::FetchKeys(
                              .at(per_user_vault->vault_key_size() - 1)
                              .key_material();
   std::vector<uint8_t> last_key_bytes(last_key.begin(), last_key.end());
-  connection_->DownloadKeys(
+  // |this| outlives |connection_| and |ongoing_connection_request_|, so it's
+  // safe to use base::Unretained() here.
+  ongoing_connection_request_ = connection_->DownloadKeys(
       *primary_account_, last_key_bytes,
       per_user_vault->last_vault_key_version(), std::move(key_pair),
       base::BindOnce(&StandaloneTrustedVaultBackend::OnKeysDownloaded,
-                     weak_factory_for_connection_.GetWeakPtr(),
-                     account_info.gaia));
+                     base::Unretained(this), account_info.gaia));
+  DCHECK(ongoing_connection_request_);
 }
 
 void StandaloneTrustedVaultBackend::StoreKeys(
@@ -229,7 +229,8 @@ void StandaloneTrustedVaultBackend::SetRecoverabilityDegradedForTesting() {
 
 void StandaloneTrustedVaultBackend::MaybeRegisterDevice(
     const std::string& gaia_id) {
-  if (!base::FeatureList::IsEnabled(switches::kFollowTrustedVaultKeyRotation)) {
+  if (!connection_) {
+    // Feature disabled.
     return;
   }
   if (!primary_account_.has_value() || primary_account_->gaia != gaia_id) {
@@ -281,11 +282,14 @@ void StandaloneTrustedVaultBackend::MaybeRegisterDevice(
   // Cancel existing callbacks passed to |connection_| to ensure there is only
   // one ongoing request.
   AbandonConnectionRequest();
-  connection_->RegisterAuthenticationFactor(
+  // |this| outlives |connection_| and |ongoing_connection_request_|, so it's
+  // safe to use base::Unretained() here.
+  ongoing_connection_request_ = connection_->RegisterAuthenticationFactor(
       *primary_account_, last_key_bytes,
       per_user_vault->last_vault_key_version(), key_pair->public_key(),
       base::BindOnce(&StandaloneTrustedVaultBackend::OnDeviceRegistered,
-                     weak_factory_for_connection_.GetWeakPtr(), gaia_id));
+                     base::Unretained(this), gaia_id));
+  DCHECK(ongoing_connection_request_);
 }
 
 void StandaloneTrustedVaultBackend::OnDeviceRegistered(
@@ -295,6 +299,12 @@ void StandaloneTrustedVaultBackend::OnDeviceRegistered(
   // cancelled.
   DCHECK(primary_account_ && primary_account_->gaia == gaia_id);
 
+  // This method should be called only as a result of
+  // |ongoing_connection_request_| completion/failure, verify this condition
+  // and destroy |ongoing_connection_request_| as it's not needed anymore.
+  DCHECK(ongoing_connection_request_);
+  ongoing_connection_request_ = nullptr;
+
   sync_pb::LocalTrustedVaultPerUser* per_user_vault = FindUserVault(gaia_id);
   DCHECK(per_user_vault);
 
@@ -324,6 +334,12 @@ void StandaloneTrustedVaultBackend::OnKeysDownloaded(
   DCHECK(!ongoing_fetch_keys_callback_.is_null());
   DCHECK(ongoing_fetch_keys_gaia_id_ == gaia_id);
 
+  // This method should be called only as a result of
+  // |ongoing_connection_request_| completion/failure, verify this condition
+  // and destroy |ongoing_connection_request_| as it's not needed anymore.
+  DCHECK(ongoing_connection_request_);
+  ongoing_connection_request_ = nullptr;
+
   sync_pb::LocalTrustedVaultPerUser* per_user_vault = FindUserVault(gaia_id);
   DCHECK(per_user_vault);
 
@@ -355,7 +371,7 @@ void StandaloneTrustedVaultBackend::OnKeysDownloaded(
 }
 
 void StandaloneTrustedVaultBackend::AbandonConnectionRequest() {
-  weak_factory_for_connection_.InvalidateWeakPtrs();
+  ongoing_connection_request_ = nullptr;
   FulfillOngoingFetchKeys();
 }
 

components/sync/trusted_vault/standalone_trusted_vault_backend.h

@@ -42,6 +42,9 @@ class StandaloneTrustedVaultBackend
     virtual void NotifyRecoverabilityDegradedChanged() = 0;
   };
 
+  // |connection| can be null, in this case functionality that involves
+  // interaction with vault service (such as device registration, keys
+  // downloading, etc.) will be disabled.
   StandaloneTrustedVaultBackend(
       const base::FilePath& file_path,
       std::unique_ptr<Delegate> delegate,
@@ -128,7 +131,11 @@ class StandaloneTrustedVaultBackend
 
   const std::unique_ptr<Delegate> delegate_;
 
-  // Used for communication with trusted vault server.
+  // Used for communication with trusted vault server. Can be null, in this case
+  // functionality that involves interaction with vault service (such as device
+  // registration, keys downloading, etc.) will be disabled.
+  // TODO(crbug.com/1113598): clean up logic around nullable |connection_|, once
+  // kFollowTrustedVaultKeyRotation feature flag is removed.
   const std::unique_ptr<TrustedVaultConnection> connection_;
 
   sync_pb::LocalTrustedVault data_;
@@ -143,11 +150,10 @@ class StandaloneTrustedVaultBackend
   // Account used in last FetchKeys() call.
   base::Optional<std::string> ongoing_fetch_keys_gaia_id_;
 
-  bool is_recoverability_degraded_for_testing_ = false;
+  // Destroying this will cancel the ongoing request.
+  std::unique_ptr<TrustedVaultConnection::Request> ongoing_connection_request_;
 
-  // Used for cancellation of callbacks passed to |connection_|.
-  base::WeakPtrFactory<StandaloneTrustedVaultBackend>
-      weak_factory_for_connection_{this};
+  bool is_recoverability_degraded_for_testing_ = false;
 };
 
 }  // namespace syncer

components/sync/trusted_vault/standalone_trusted_vault_backend_unittest.cc

@@ -56,21 +56,21 @@ class MockTrustedVaultConnection : public TrustedVaultConnection {
  public:
   MockTrustedVaultConnection() = default;
   ~MockTrustedVaultConnection() override = default;
-  MOCK_METHOD(void,
+  MOCK_METHOD(std::unique_ptr<Request>,
               RegisterAuthenticationFactor,
-              (const CoreAccountInfo&,
-               const std::vector<uint8_t>&,
-               int,
-               const SecureBoxPublicKey&,
-               RegisterAuthenticationFactorCallback),
+              (const CoreAccountInfo& account_info,
+               const std::vector<uint8_t>& last_trusted_vault_key,
+               int last_trusted_vault_key_version,
+               const SecureBoxPublicKey& authentication_factor_public_key,
+               RegisterAuthenticationFactorCallback callback),
               (override));
-  MOCK_METHOD(void,
+  MOCK_METHOD(std::unique_ptr<Request>,
               DownloadKeys,
-              (const CoreAccountInfo&,
-               const std::vector<uint8_t>&,
-               int,
-               std::unique_ptr<SecureBoxKeyPair>,
-               DownloadKeysCallback),
+              (const CoreAccountInfo& account_info,
+               const std::vector<uint8_t>& last_trusted_vault_key,
+               int last_trusted_vault_key_version,
+               std::unique_ptr<SecureBoxKeyPair> device_key_pair,
+               DownloadKeysCallback callback),
               (override));
 };
 
@@ -126,6 +126,11 @@ class StandaloneTrustedVaultBackendTest : public testing::Test {
                 TrustedVaultConnection::RegisterAuthenticationFactorCallback
                     callback) {
               device_registration_callback = std::move(callback);
+              // Note: TrustedVaultConnection::Request doesn't support
+              // cancellation, so these tests don't cover the contract that
+              // caller should store Request object until it's completed or need
+              // to be cancelled.
+              return std::make_unique<TrustedVaultConnection::Request>();
             });
     // Setting the primary account will trigger device registration.
     backend()->SetPrimaryAccount(account_info);
@@ -308,6 +313,7 @@ TEST_F(StandaloneTrustedVaultBackendTest, ShouldRegisterDevice) {
                         callback) {
         serialized_public_device_key = device_public_key.ExportToBytes();
         device_registration_callback = std::move(callback);
+        return std::make_unique<TrustedVaultConnection::Request>();
       });
 
   // Setting the primary account will trigger device registration.
@@ -382,6 +388,7 @@ TEST_F(StandaloneTrustedVaultBackendTest, ShouldDownloadKeys) {
                     TrustedVaultConnection::DownloadKeysCallback callback) {
         device_key_pair = std::move(key_pair);
         download_keys_callback = std::move(callback);
+        return std::make_unique<TrustedVaultConnection::Request>();
       });
 
   // FetchKeys() should trigger keys downloading.

components/sync/trusted_vault/standalone_trusted_vault_client.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/bind_helpers.h"
+#include "base/command_line.h"
 #include "base/memory/ref_counted.h"
 #include "base/sequenced_task_runner.h"
 #include "base/task/task_traits.h"
@@ -14,6 +15,8 @@
 #include "base/task_runner_util.h"
 #include "components/signin/public/identity_manager/account_info.h"
 #include "components/sync/base/bind_to_task_runner.h"
+#include "components/sync/base/sync_base_switches.h"
+#include "components/sync/driver/sync_driver_switches.h"
 #include "components/sync/engine/sync_engine_switches.h"
 #include "components/sync/trusted_vault/standalone_trusted_vault_backend.h"
 #include "components/sync/trusted_vault/trusted_vault_access_token_fetcher_impl.h"
@@ -28,6 +31,17 @@ constexpr base::TaskTraits kBackendTaskTraits = {
     base::MayBlock(), base::TaskPriority::USER_VISIBLE,
     base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN};
 
+GURL ExtractTrustedVaultServiceURLFromCommandLine() {
+  std::string string_url =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kTrustedVaultServiceURL);
+  if (string_url.empty()) {
+    // Command line switch is not specified or is not a valid ASCII string.
+    return GURL();
+  }
+  return GURL(string_url);
+}
+
 class PrimaryAccountObserver : public signin::IdentityManager::Observer {
  public:
   PrimaryAccountObserver(
@@ -143,16 +157,24 @@ StandaloneTrustedVaultClient::StandaloneTrustedVaultClient(
           switches::kSyncSupportTrustedVaultPassphrase)) {
     return;
   }
+
+  std::unique_ptr<TrustedVaultConnection> connection;
+  GURL trusted_vault_service_gurl =
+      ExtractTrustedVaultServiceURLFromCommandLine();
+  if (trusted_vault_service_gurl.is_valid()) {
+    connection = std::make_unique<TrustedVaultConnectionImpl>(
+        trusted_vault_service_gurl, url_loader_factory->Clone(),
+        std::make_unique<TrustedVaultAccessTokenFetcherImpl>(
+            access_token_fetcher_frontend_.GetWeakPtr()));
+  }
+
   backend_ = base::MakeRefCounted<StandaloneTrustedVaultBackend>(
       file_path,
       std::make_unique<
           BackendDelegate>(BindToCurrentSequence(base::BindRepeating(
           &StandaloneTrustedVaultClient::NotifyRecoverabilityDegradedChanged,
           weak_ptr_factory_.GetWeakPtr()))),
-      std::make_unique<TrustedVaultConnectionImpl>(
-          url_loader_factory->Clone(),
-          std::make_unique<TrustedVaultAccessTokenFetcherImpl>(
-              access_token_fetcher_frontend_.GetWeakPtr())));
+      std::move(connection));
   backend_task_runner_->PostTask(
       FROM_HERE,
       base::BindOnce(&StandaloneTrustedVaultBackend::ReadDataFromDisk,

components/sync/trusted_vault/trusted_vault_connection.h

@@ -38,6 +38,16 @@ class TrustedVaultConnection {
                               const std::vector<std::vector<uint8_t>>& /*keys*/,
                               int /*last_key_version*/)>;
 
+  // Used to control ongoing request lifetime, destroying Request object causes
+  // request cancellation.
+  class Request {
+   public:
+    Request() = default;
+    Request(const Request& other) = delete;
+    Request& operator=(const Request& other) = delete;
+    virtual ~Request() = default;
+  };
+
   TrustedVaultConnection() = default;
   TrustedVaultConnection(const TrustedVaultConnection& other) = delete;
   TrustedVaultConnection& operator=(const TrustedVaultConnection& other) =
@@ -46,21 +56,25 @@ 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.
-  virtual void RegisterAuthenticationFactor(
+  // 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.
+  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 SecureBoxPublicKey& authentication_factor_public_key,
-      RegisterAuthenticationFactorCallback callback) = 0;
+      RegisterAuthenticationFactorCallback callback) WARN_UNUSED_RESULT = 0;
 
   // Asynchronously attempts to download new vault keys from the trusted vault
-  // server.
-  virtual void DownloadKeys(const CoreAccountInfo& account_info,
+  // server. Caller should hold returned request object until |callback| call
+  // 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,
       std::unique_ptr<SecureBoxKeyPair> device_key_pair,
-                            DownloadKeysCallback callback) = 0;
+      DownloadKeysCallback callback) WARN_UNUSED_RESULT = 0;
 };
 
 }  // namespace syncer

components/sync/trusted_vault/trusted_vault_connection_impl.cc

@@ -6,36 +6,151 @@
 
 #include <utility>
 
+#include "base/containers/span.h"
+#include "components/signin/public/identity_manager/account_info.h"
+#include "components/sync/protocol/vault.pb.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_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 kSecurityDomainName[] = "chromesync";
+
+// Helper function for filling protobuf bytes field: protobuf represent them as
+// std::string, while in code std::vector<uint8_t> or base::span<uint8_t> is
+// more common.
+void AssignBytesToProtoString(base::span<const uint8_t> bytes,
+                              std::string* bytes_proto_field) {
+  *bytes_proto_field = std::string(bytes.begin(), bytes.end());
+}
+
+void ProcessRegisterDeviceResponse(
+    TrustedVaultConnection::RegisterAuthenticationFactorCallback callback,
+    bool success,
+    const std::string& response_body) {
+  // TODO(crbug.com/1113598): distinguish kLocalDataObsolete and kOtherError
+  // (based on http error code?).
+  std::move(callback).Run(success ? TrustedVaultRequestStatus::kSuccess
+                                  : TrustedVaultRequestStatus::kOtherError);
+}
+
+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());
+  return shared_key;
+}
+
+sync_pb::JoinSecurityDomainsRequest CreateJoinSecurityDomainsRequest(
+    int last_trusted_vault_key_version,
+    const std::vector<uint8_t>& last_trusted_vault_key,
+    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);
+
+  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
+
 TrustedVaultConnectionImpl::TrustedVaultConnectionImpl(
-    std::unique_ptr<network::PendingSharedURLLoaderFactory> url_loader_factory,
+    const GURL& trusted_vault_service_url,
+    std::unique_ptr<network::PendingSharedURLLoaderFactory>
+        pending_url_loader_factory,
     std::unique_ptr<TrustedVaultAccessTokenFetcher> access_token_fetcher)
-    : pending_url_loader_factory_(std::move(url_loader_factory)),
-      access_token_fetcher_(std::move(access_token_fetcher)) {}
+    : pending_url_loader_factory_(std::move(pending_url_loader_factory)),
+      access_token_fetcher_(std::move(access_token_fetcher)),
+      trusted_vault_service_url_(trusted_vault_service_url) {
+  DCHECK(trusted_vault_service_url_.is_valid());
+}
 
 TrustedVaultConnectionImpl::~TrustedVaultConnectionImpl() = default;
 
-void TrustedVaultConnectionImpl::RegisterAuthenticationFactor(
+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 SecureBoxPublicKey& authentication_factor_public_key,
+    const SecureBoxPublicKey& public_key,
     RegisterAuthenticationFactorCallback callback) {
-  NOTIMPLEMENTED();
+  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)
+          .SerializeAsString());
+  request->FetchAccessTokenAndSendRequest(
+      account_info.account_id, GetOrCreateURLLoaderFactory(),
+      access_token_fetcher_.get(),
+      base::BindOnce(ProcessRegisterDeviceResponse, std::move(callback)));
+  return request;
 }
 
-void TrustedVaultConnectionImpl::DownloadKeys(
+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,
     std::unique_ptr<SecureBoxKeyPair> device_key_pair,
     DownloadKeysCallback callback) {
   NOTIMPLEMENTED();
+  // TODO(crbug.com/1113598): implement logic.
+  return std::make_unique<Request>();
+}
+
+scoped_refptr<network::SharedURLLoaderFactory>
+TrustedVaultConnectionImpl::GetOrCreateURLLoaderFactory() {
+  if (!url_loader_factory_) {
+    url_loader_factory_ = network::SharedURLLoaderFactory::Create(
+        std::move(pending_url_loader_factory_));
+  }
+  return url_loader_factory_;
 }
 
 }  // namespace syncer

components/sync/trusted_vault/trusted_vault_connection_impl.h

@@ -9,11 +9,14 @@
 #include <string>
 #include <vector>
 
+#include "base/memory/scoped_refptr.h"
 #include "components/sync/trusted_vault/trusted_vault_access_token_fetcher.h"
 #include "components/sync/trusted_vault/trusted_vault_connection.h"
+#include "url/gurl.h"
 
 namespace network {
 class PendingSharedURLLoaderFactory;
+class SharedURLLoaderFactory;
 }  // namespace network
 
 namespace syncer {
@@ -23,6 +26,7 @@ namespace syncer {
 class TrustedVaultConnectionImpl : public TrustedVaultConnection {
  public:
   TrustedVaultConnectionImpl(
+      const GURL& trusted_vault_service_url,
       std::unique_ptr<network::PendingSharedURLLoaderFactory>
           pending_url_loader_factory,
       std::unique_ptr<TrustedVaultAccessTokenFetcher> access_token_fetcher);
@@ -32,23 +36,33 @@ class TrustedVaultConnectionImpl : public TrustedVaultConnection {
       const TrustedVaultConnectionImpl& other) = delete;
   ~TrustedVaultConnectionImpl() override;
 
-  void RegisterAuthenticationFactor(
+  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 SecureBoxPublicKey& authentication_factor_public_key,
       RegisterAuthenticationFactorCallback callback) override;
 
-  void DownloadKeys(const CoreAccountInfo& account_info,
+  std::unique_ptr<Request> DownloadKeys(
+      const CoreAccountInfo& account_info,
       const std::vector<uint8_t>& last_trusted_vault_key,
       int last_trusted_vault_key_version,
       std::unique_ptr<SecureBoxKeyPair> device_key_pair,
       DownloadKeysCallback callback) override;
 
  private:
-  const std::unique_ptr<network::PendingSharedURLLoaderFactory>
+  // SharedURLLoaderFactory is created lazily, because it needs to be done on
+  // the backend sequence, while this class ctor is called on UI thread.
+  scoped_refptr<network::SharedURLLoaderFactory> GetOrCreateURLLoaderFactory();
+
+  std::unique_ptr<network::PendingSharedURLLoaderFactory>
       pending_url_loader_factory_;
   const std::unique_ptr<TrustedVaultAccessTokenFetcher> access_token_fetcher_;
+
+  // Instantiated upon first need using |pending_url_loader_factory_|.
+  scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
+
+  GURL trusted_vault_service_url_;
 };
 
 }  // namespace syncer

components/sync/trusted_vault/trusted_vault_connection_impl_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_connection_impl.h"
+
+#include <utility>
+
+#include "base/memory/scoped_refptr.h"
+#include "base/run_loop.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/test/mock_callback.h"
+#include "base/test/task_environment.h"
+#include "components/signin/public/identity_manager/access_token_info.h"
+#include "components/signin/public/identity_manager/account_info.h"
+#include "components/sync/protocol/vault.pb.h"
+#include "components/sync/trusted_vault/securebox.h"
+#include "components/sync/trusted_vault/trusted_vault_access_token_fetcher.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::_;
+using testing::Eq;
+using testing::IsNull;
+using testing::Ne;
+using testing::NotNull;
+using testing::SizeIs;
+
+const char kAccessToken[] = "access_token";
+const char kEncodedPrivateKey[] =
+    "49e052293c29b5a50b0013eec9d030ac2ad70a42fe093be084264647cb04e16f";
+const char kTestURL[] = "https://test.com/test";
+const char kTestJoinSecurityDomainsURL[] =
+    "https://test.com/test/domain:join?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;
+  bool success = base::HexStringToBytes(kEncodedPrivateKey, &private_key_bytes);
+  DCHECK(success);
+  return SecureBoxKeyPair::CreateByPrivateKeyImport(private_key_bytes);
+}
+
+class FakeTrustedVaultAccessTokenFetcher
+    : public TrustedVaultAccessTokenFetcher {
+ public:
+  explicit FakeTrustedVaultAccessTokenFetcher(
+      const base::Optional<std::string>& access_token)
+      : access_token_(access_token) {}
+  ~FakeTrustedVaultAccessTokenFetcher() override = default;
+
+  void FetchAccessToken(const CoreAccountId& account_id,
+                        TokenCallback callback) override {
+    base::Optional<signin::AccessTokenInfo> access_token_info;
+    if (access_token_) {
+      access_token_info = signin::AccessTokenInfo(
+          *access_token_, /*expiration_time_param=*/base::Time::Now() +
+                              base::TimeDelta::FromHours(1),
+          /*id_token=*/std::string());
+    }
+    std::move(callback).Run(access_token_info);
+  }
+
+ private:
+  const base::Optional<std::string> access_token_;
+};
+
+class TrustedVaultConnectionImplTest : public testing::Test {
+ public:
+  TrustedVaultConnectionImplTest()
+      : connection_(
+            GURL(kTestURL),
+            base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
+                &test_url_loader_factory_)
+                ->Clone(),
+            std::make_unique<FakeTrustedVaultAccessTokenFetcher>(
+                kAccessToken)) {}
+
+  ~TrustedVaultConnectionImplTest() override = default;
+
+  TrustedVaultConnectionImpl* connection() { return &connection_; }
+
+  // Allows overloading of FakeTrustedVaultAccessTokenFetcher behavior, doesn't
+  // overwrite connection().
+  std::unique_ptr<TrustedVaultConnectionImpl> CreateConnectionWithAccessToken(
+      base::Optional<std::string> access_token) {
+    return std::make_unique<TrustedVaultConnectionImpl>(
+        GURL(kTestURL),
+        base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
+            &test_url_loader_factory_)
+            ->Clone(),
+        std::make_unique<FakeTrustedVaultAccessTokenFetcher>(access_token));
+  }
+
+  network::TestURLLoaderFactory::PendingRequest* GetPendingHTTPRequest() {
+    // Allow request to reach |test_url_loader_factory_|.
+    base::RunLoop().RunUntilIdle();
+    return test_url_loader_factory_.GetPendingRequest(/*index=*/0);
+  }
+
+  bool RespondToJoinSecurityDomainsRequest(
+      net::HttpStatusCode response_http_code) {
+    // Allow request to reach |test_url_loader_factory_|.
+    base::RunLoop().RunUntilIdle();
+    return test_url_loader_factory_.SimulateResponseForPendingRequest(
+        kTestJoinSecurityDomainsURL,
+        /*content=*/std::string(), response_http_code);
+  }
+
+  const std::vector<uint8_t> kTrustedVaultKey = {1, 2, 3, 4};
+
+ private:
+  base::test::TaskEnvironment task_environment_;
+
+  network::TestURLLoaderFactory test_url_loader_factory_;
+
+  TrustedVaultConnectionImpl connection_;
+};
+
+TEST_F(TrustedVaultConnectionImplTest, ShouldSendJoinSecurityDomainsRequest) {
+  const int kLastKeyVersion = 1234;
+
+  std::unique_ptr<SecureBoxKeyPair> key_pair = MakeTestKeyPair();
+  ASSERT_THAT(key_pair, NotNull());
+
+  std::unique_ptr<TrustedVaultConnection::Request> request =
+      connection()->RegisterAuthenticationFactor(
+          /*account_info=*/CoreAccountInfo(), kTrustedVaultKey, kLastKeyVersion,
+          key_pair->public_key(),
+          TrustedVaultConnection::RegisterAuthenticationFactorCallback());
+  EXPECT_THAT(request, NotNull());
+
+  network::TestURLLoaderFactory::PendingRequest* pending_http_request =
+      GetPendingHTTPRequest();
+  ASSERT_THAT(pending_http_request, NotNull());
+
+  const network::ResourceRequest& resource_request =
+      pending_http_request->request;
+  EXPECT_THAT(resource_request.method, Eq("POST"));
+  EXPECT_THAT(resource_request.url,
+              Eq(GURL(std::string(kTestURL) + "/domain:join?alt=proto")));
+
+  sync_pb::JoinSecurityDomainsRequest deserialized_body;
+  deserialized_body.ParseFromString(network::GetUploadData(resource_request));
+
+  ASSERT_THAT(deserialized_body.security_domains(), SizeIs(1));
+  const sync_pb::SecurityDomain& security_domain =
+      deserialized_body.security_domains(0);
+  EXPECT_THAT(security_domain.name(), Eq("chromesync"));
+
+  ASSERT_THAT(security_domain.members(), SizeIs(1));
+  const sync_pb::SecurityDomain::Member& member = security_domain.members(0);
+  const std::vector<uint8_t> member_public_key_bytes(
+      member.public_key().begin(), member.public_key().end());
+  EXPECT_THAT(member_public_key_bytes,
+              Eq(key_pair->public_key().ExportToBytes()));
+
+  ASSERT_THAT(member.keys(), SizeIs(1));
+  const sync_pb::SharedKey& shared_key = member.keys(0);
+  EXPECT_THAT(shared_key.epoch(), Eq(kLastKeyVersion));
+
+  const std::vector<uint8_t> wrapped_key_bytes(shared_key.wrapped_key().begin(),
+                                               shared_key.wrapped_key().end());
+  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=*/wrapped_key_bytes);
+  ASSERT_THAT(decrypted_trusted_vault_key, Ne(base::nullopt));
+  EXPECT_THAT(*decrypted_trusted_vault_key, Eq(kTrustedVaultKey));
+
+  // HMAC_SHA256 result using |key_pair| public key as message and
+  // kTrustedVaultKey as a secret key.
+  const std::string kHexEncodedExpectedMemberProof =
+      "247b19e1a835dff90405b349413d51a8a9c3c10035772cf5d60e9403053e67fd";
+  std::string expected_member_proof;
+  ASSERT_TRUE(base::HexStringToString(kHexEncodedExpectedMemberProof,
+                                      &expected_member_proof));
+  EXPECT_THAT(shared_key.member_proof(), Eq(expected_member_proof));
+}
+
+TEST_F(TrustedVaultConnectionImplTest,
+       ShouldHandleSuccessfulJoinSecurityDomainsRequest) {
+  std::unique_ptr<SecureBoxKeyPair> key_pair = MakeTestKeyPair();
+  ASSERT_THAT(key_pair, NotNull());
+
+  base::MockCallback<
+      TrustedVaultConnection::RegisterAuthenticationFactorCallback>
+      callback;
+
+  std::unique_ptr<TrustedVaultConnection::Request> request =
+      connection()->RegisterAuthenticationFactor(
+          /*account_info=*/CoreAccountInfo(), kTrustedVaultKey,
+          /*last_trusted_vault_key_version=*/1, key_pair->public_key(),
+          callback.Get());
+  ASSERT_THAT(request, NotNull());
+
+  EXPECT_CALL(callback, Run(Eq(TrustedVaultRequestStatus::kSuccess)));
+  EXPECT_TRUE(RespondToJoinSecurityDomainsRequest(net::HTTP_OK));
+}
+
+TEST_F(TrustedVaultConnectionImplTest,
+       ShouldHandleFailedJoinSecurityDomainsRequest) {
+  std::unique_ptr<SecureBoxKeyPair> key_pair = MakeTestKeyPair();
+  ASSERT_THAT(key_pair, NotNull());
+
+  base::MockCallback<
+      TrustedVaultConnection::RegisterAuthenticationFactorCallback>
+      callback;
+
+  std::unique_ptr<TrustedVaultConnection::Request> request =
+      connection()->RegisterAuthenticationFactor(
+          /*account_info=*/CoreAccountInfo(), kTrustedVaultKey,
+          /*last_trusted_vault_key_version=*/1, key_pair->public_key(),
+          callback.Get());
+  ASSERT_THAT(request, NotNull());
+
+  EXPECT_CALL(callback, Run(Eq(TrustedVaultRequestStatus::kOtherError)));
+  EXPECT_TRUE(
+      RespondToJoinSecurityDomainsRequest(net::HTTP_INTERNAL_SERVER_ERROR));
+}
+
+TEST_F(
+    TrustedVaultConnectionImplTest,
+    ShouldHandleAccessTokenFetchingFailureWhenRegisteringAuthenticationFactor) {
+  std::unique_ptr<TrustedVaultConnectionImpl> connection =
+      CreateConnectionWithAccessToken(
+          /*access_token=*/base::nullopt);
+
+  std::unique_ptr<SecureBoxKeyPair> key_pair = MakeTestKeyPair();
+  ASSERT_THAT(key_pair, NotNull());
+
+  base::MockCallback<
+      TrustedVaultConnection::RegisterAuthenticationFactorCallback>
+      callback;
+
+  // |callback| is called immediately after RegisterAuthenticationFactor(),
+  // because there is no access token.
+  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());
+  ASSERT_THAT(request, NotNull());
+
+  // No requests should be sent to the network.
+  EXPECT_THAT(GetPendingHTTPRequest(), IsNull());
+}
+
+TEST_F(TrustedVaultConnectionImplTest, ShouldCancelJoinSecurityDomainsRequest) {
+  std::unique_ptr<SecureBoxKeyPair> key_pair = MakeTestKeyPair();
+  ASSERT_THAT(key_pair, NotNull());
+
+  base::MockCallback<
+      TrustedVaultConnection::RegisterAuthenticationFactorCallback>
+      callback;
+
+  std::unique_ptr<TrustedVaultConnection::Request> request =
+      connection()->RegisterAuthenticationFactor(
+          /*account_info=*/CoreAccountInfo(), kTrustedVaultKey,
+          /*last_trusted_vault_key_version=*/1, key_pair->public_key(),
+          callback.Get());
+  ASSERT_THAT(request, NotNull());
+
+  EXPECT_CALL(callback, Run(_)).Times(0);
+  request.reset();
+  // Returned value isn't checked here, because the request can be cancelled
+  // before reaching TestURLLoaderFactory.
+  RespondToJoinSecurityDomainsRequest(net::HTTP_OK);
+}
+
+}  // namespace
+
+}  // namespace syncer

components/sync/trusted_vault/trusted_vault_request.h

@@ -11,6 +11,7 @@
 #include "base/callback.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
+#include "components/sync/trusted_vault/trusted_vault_connection.h"
 #include "url/gurl.h"
 
 struct CoreAccountId;
@@ -29,7 +30,7 @@ namespace syncer {
 class TrustedVaultAccessTokenFetcher;
 
 // Allows calling VaultService API using proto-over-http.
-class TrustedVaultRequest {
+class TrustedVaultRequest : public TrustedVaultConnection::Request {
  public:
   using CompletionCallback =
       base::OnceCallback<void(bool success, const std::string& response_body)>;
@@ -45,7 +46,7 @@ class TrustedVaultRequest {
       const base::Optional<std::string>& serialized_request_proto);
   TrustedVaultRequest(const TrustedVaultRequest& other) = delete;
   TrustedVaultRequest& operator=(const TrustedVaultRequest& other) = delete;
-  ~TrustedVaultRequest();
+  ~TrustedVaultRequest() override;
 
   // Attempts to fetch access token and sends the request if fetch was
   // successful or populates error into ResultCallback otherwise. Should be