[Sync] Remove Encryptor

Encryptor was used only to override OSCrypt in tests, this CL achieves
the same with OSCryptMocker.

Bug: 1223759
Change-Id: I453fd46f3102e9cdb5957626585dbd1b2378176c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2988028
Auto-Submit: Maksim Moskvitin <mmoskvitin@google.com>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#896102}

components/sync/BUILD.gn

@@ -131,7 +131,6 @@ source_set("unit_tests") {
     "base/protobuf_unittest.cc",
     "base/sync_prefs_unittest.cc",
     "base/sync_util_unittest.cc",
-    "base/system_encryptor_unittest.cc",
     "base/unique_position_unittest.cc",
     "base/weak_handle_unittest.cc",
     "driver/backend_migrator_unittest.cc",

components/sync/base/BUILD.gn

@@ -17,7 +17,6 @@ static_library("base") {
     "client_tag_hash.h",
     "data_type_histogram.cc",
     "data_type_histogram.h",
-    "encryptor.h",
     "enum_set.h",
     "extensions_activity.cc",
     "extensions_activity.h",
@@ -58,8 +57,6 @@ static_library("base") {
     "sync_util.h",
     "syncer_error.cc",
     "syncer_error.h",
-    "system_encryptor.cc",
-    "system_encryptor.h",
     "time.cc",
     "time.h",
     "unique_position.cc",
@@ -79,7 +76,6 @@ static_library("base") {
   deps = [
     "//base:i18n",
     "//build:chromeos_buildflags",
-    "//components/os_crypt",
     "//components/pref_registry",
     "//components/prefs",
     "//components/version_info",
@@ -108,8 +104,6 @@ static_library("test_support") {
     "../test/mock_invalidation_tracker.h",
     "../test/trackable_mock_invalidation.cc",
     "../test/trackable_mock_invalidation.h",
-    "fake_encryptor.cc",
-    "fake_encryptor.h",
     "model_type_test_util.cc",
     "model_type_test_util.h",
   ]

components/sync/base/DEPS

@@ -1,7 +1,6 @@
 include_rules = [
   "+ash/constants",
   "+components/invalidation",
-  "+components/os_crypt",
   "+components/pref_registry",
   "+components/prefs",
   "+components/sync/protocol",

components/sync/driver/DEPS

@@ -3,7 +3,6 @@ include_rules = [
   "+components/invalidation",
   "+components/keyed_service/core",
   "+components/metrics",
-  "+components/os_crypt",
   "+components/policy",
   "+components/pref_registry",
   "+components/prefs",

components/sync/driver/glue/sync_engine_backend.cc

@@ -214,8 +214,8 @@ void SyncEngineBackend::DoInitialize(
   sync_encryption_handler_ = std::make_unique<NigoriSyncBridgeImpl>(
       std::move(nigori_processor),
       std::make_unique<NigoriStorageImpl>(
-          sync_data_folder_.Append(kNigoriStorageFilename), &encryptor_),
-      &encryptor_, base::BindRepeating(&Nigori::GenerateScryptSalt),
+          sync_data_folder_.Append(kNigoriStorageFilename)),
+      base::BindRepeating(&Nigori::GenerateScryptSalt),
       params.encryption_bootstrap_token,
       restored_local_transport_data.keystore_encryption_bootstrap_token);
 

components/sync/driver/glue/sync_engine_backend.h

@@ -17,7 +17,6 @@
 #include "components/invalidation/public/invalidation.h"
 #include "components/invalidation/public/invalidator_state.h"
 #include "components/invalidation/public/topic_invalidation_map.h"
-#include "components/sync/base/system_encryptor.h"
 #include "components/sync/engine/cancelation_signal.h"
 #include "components/sync/engine/model_type_configurer.h"
 #include "components/sync/engine/shutdown_reason.h"
@@ -187,9 +186,6 @@ class SyncEngineBackend : public base::RefCountedThreadSafe<SyncEngineBackend>,
   // Our parent SyncEngineImpl.
   WeakHandle<SyncEngineImpl> host_;
 
-  // Our encryptor, which uses Chrome's encryption functions.
-  SystemEncryptor encryptor_;
-
   // Should outlive |sync_manager_|.
   std::unique_ptr<SyncEncryptionHandler> sync_encryption_handler_;
 

components/sync/nigori/BUILD.gn

@@ -28,6 +28,8 @@ static_library("nigori") {
     "pending_local_nigori_commit.h",
   ]
 
+  deps = [ "//components/os_crypt" ]
+
   public_deps = [
     "//base",
     "//components/sync/base",

components/sync/nigori/DEPS

@@ -1,4 +1,5 @@
 include_rules = [
+  "+components/os_crypt",
   "+components/sync/base",
   "+components/sync/engine",
   "+components/sync/model",

components/sync/nigori/nigori_storage_impl.cc

@@ -9,15 +9,12 @@
 #include "base/files/file_util.h"
 #include "base/files/important_file_writer.h"
 #include "base/logging.h"
-#include "components/sync/base/encryptor.h"
+#include "components/os_crypt/os_crypt.h"
 
 namespace syncer {
 
-NigoriStorageImpl::NigoriStorageImpl(const base::FilePath& path,
-                                     const Encryptor* encryptor)
-    : path_(path), encryptor_(encryptor) {
-  DCHECK(encryptor_);
-}
+NigoriStorageImpl::NigoriStorageImpl(const base::FilePath& path)
+    : path_(path) {}
 
 NigoriStorageImpl::~NigoriStorageImpl() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -33,7 +30,7 @@ void NigoriStorageImpl::StoreData(const sync_pb::NigoriLocalData& data) {
   }
 
   std::string encrypted_data;
-  if (!encryptor_->EncryptString(serialized_data, &encrypted_data)) {
+  if (!OSCrypt::EncryptString(serialized_data, &encrypted_data)) {
     DLOG(ERROR) << "Failed to encrypt NigoriLocalData.";
     return;
   }
@@ -56,7 +53,7 @@ absl::optional<sync_pb::NigoriLocalData> NigoriStorageImpl::RestoreData() {
   }
 
   std::string serialized_data;
-  if (!encryptor_->DecryptString(encrypted_data, &serialized_data)) {
+  if (!OSCrypt::DecryptString(encrypted_data, &serialized_data)) {
     DLOG(ERROR) << "Failed to decrypt NigoriLocalData.";
     return absl::nullopt;
   }

components/sync/nigori/nigori_storage_impl.h

@@ -11,12 +11,10 @@
 
 namespace syncer {
 
-class Encryptor;
-
 class NigoriStorageImpl : public NigoriStorage {
  public:
   // |encryptor| must be not null and must outlive this object.
-  NigoriStorageImpl(const base::FilePath& path, const Encryptor* encryptor);
+  explicit NigoriStorageImpl(const base::FilePath& path);
   ~NigoriStorageImpl() override;
 
   // NigoriStorage implementation.
@@ -27,7 +25,6 @@ class NigoriStorageImpl : public NigoriStorage {
 
  private:
   base::FilePath path_;
-  const Encryptor* const encryptor_;
 
   SEQUENCE_CHECKER(sequence_checker_);
   DISALLOW_COPY_AND_ASSIGN(NigoriStorageImpl);

components/sync/nigori/nigori_storage_impl_unittest.cc

@@ -6,7 +6,7 @@
 
 #include "base/files/file_util.h"
 #include "base/files/scoped_temp_dir.h"
-#include "components/sync/base/fake_encryptor.h"
+#include "components/os_crypt/os_crypt_mocker.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace syncer {
@@ -26,41 +26,43 @@ class NigoriStorageImplTest : public testing::Test {
   NigoriStorageImplTest() = default;
   ~NigoriStorageImplTest() override = default;
 
-  void SetUp() override { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); }
+  void SetUp() override {
+    ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+    OSCryptMocker::SetUp();
+  }
 
-  const Encryptor* encryptor() { return &encryptor_; }
+  void TearDown() override { OSCryptMocker::TearDown(); }
 
   base::FilePath GetFilePath() {
     return temp_dir_.GetPath().Append(
         base::FilePath(FILE_PATH_LITERAL("some_file")));
   }
 
  private:
-  FakeEncryptor encryptor_;
   base::ScopedTempDir temp_dir_;
 };
 
 TEST_F(NigoriStorageImplTest, ShouldBeAbleToRestoreAfterWrite) {
-  NigoriStorageImpl writer_storage(GetFilePath(), encryptor());
+  NigoriStorageImpl writer_storage(GetFilePath());
   sync_pb::NigoriLocalData write_data = MakeSomeNigoriLocalData();
   writer_storage.StoreData(write_data);
 
   // Use different NigoriStorageImpl when reading to avoid dependency on its
   // state and emulate browser restart.
-  NigoriStorageImpl reader_storage(GetFilePath(), encryptor());
+  NigoriStorageImpl reader_storage(GetFilePath());
   absl::optional<sync_pb::NigoriLocalData> read_data =
       reader_storage.RestoreData();
   EXPECT_NE(read_data, absl::nullopt);
   EXPECT_EQ(read_data->SerializeAsString(), write_data.SerializeAsString());
 }
 
 TEST_F(NigoriStorageImplTest, ShouldReturnNulloptWhenFileNotExists) {
-  NigoriStorageImpl storage(GetFilePath(), encryptor());
+  NigoriStorageImpl storage(GetFilePath());
   EXPECT_EQ(storage.RestoreData(), absl::nullopt);
 }
 
 TEST_F(NigoriStorageImplTest, ShouldRemoveFile) {
-  NigoriStorageImpl storage(GetFilePath(), encryptor());
+  NigoriStorageImpl storage(GetFilePath());
   sync_pb::NigoriLocalData data = MakeSomeNigoriLocalData();
   storage.StoreData(data);
   ASSERT_TRUE(base::PathExists(GetFilePath()));