Revert "Remove DirectoryCryptographer usages from encryption_helper"

This reverts commit dc9b6d5. Reason for revert: sync_integration_tests failing on https://ci.chromium.org/p/chromium/builders/ci/linux-chromeos-rel/35758, https://ci.chromium.org/p/chromium/builders/ci/Linux%20TSan%20Tests/52223 and https://ci.chromium.org/p/chrome/builders/ci/linux-chromeos-chrome/3930 Original change's description: > Remove DirectoryCryptographer usages from encryption_helper > > DirectoryCryptographer usages are replaced with CryptographerImpl. > KeyParams moved to encryption_helper.h. > > Bug: 1061045 > Change-Id: I4674ea9b0d5c162b672269622d8361bdfdf25279 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2100933 > Commit-Queue: Maksim Moskvitin <mmoskvitin@google.com> > Commit-Queue: Mikel Astiz <mastiz@chromium.org> > Auto-Submit: Maksim Moskvitin <mmoskvitin@google.com> > Reviewed-by: Mikel Astiz <mastiz@chromium.org> > Cr-Commit-Position: refs/heads/master@{#749754} TBR=mastiz@chromium.org,mmoskvitin@google.com Change-Id: I306c38645aa2988b03500382b28d59c5e3812a4e No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 1061045 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2101313 Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org> Commit-Queue: Oksana Zhuravlova <oksamyt@chromium.org> Cr-Commit-Position: refs/heads/master@{#749836}
Denger-Network · Mar 12, 2020 · 248779c · 248779c
1 parent 6085a1a
commit 248779c
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 48 deletions.
diff --git a/chrome/browser/sync/test/integration/encryption_helper.cc b/chrome/browser/sync/test/integration/encryption_helper.cc
@@ -13,8 +13,6 @@
 #include "components/sync/driver/profile_sync_service.h"
 #include "components/sync/driver/sync_client.h"
 #include "components/sync/engine/sync_engine_switches.h"
-#include "components/sync/nigori/cryptographer_impl.h"
-#include "components/sync/nigori/nigori_key_bag.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace encryption_helper {
@@ -35,46 +33,36 @@ std::unique_ptr<syncer::Cryptographer>
 InitCustomPassphraseCryptographerFromNigori(
     const sync_pb::NigoriSpecifics& nigori,
     const std::string& passphrase) {
-  std::unique_ptr<syncer::CryptographerImpl> cryptographer;
+  auto cryptographer = std::make_unique<syncer::DirectoryCryptographer>();
   sync_pb::EncryptedData keybag = nigori.encryption_keybag();
+  cryptographer->SetPendingKeys(keybag);
 
   std::string decoded_salt;
   switch (syncer::ProtoKeyDerivationMethodToEnum(
       nigori.custom_passphrase_key_derivation_method())) {
     case syncer::KeyDerivationMethod::PBKDF2_HMAC_SHA1_1003:
-      cryptographer =
-          syncer::CryptographerImpl::FromSingleKeyForTesting(passphrase);
+      EXPECT_TRUE(cryptographer->DecryptPendingKeys(
+          {syncer::KeyDerivationParams::CreateForPbkdf2(), passphrase}));
       break;
     case syncer::KeyDerivationMethod::SCRYPT_8192_8_11:
       EXPECT_TRUE(base::Base64Decode(
           nigori.custom_passphrase_key_derivation_salt(), &decoded_salt));
-      cryptographer = syncer::CryptographerImpl::FromSingleKeyForTesting(
-          passphrase,
-          syncer::KeyDerivationParams::CreateForScrypt(decoded_salt));
+      EXPECT_TRUE(cryptographer->DecryptPendingKeys(
+          {syncer::KeyDerivationParams::CreateForScrypt(decoded_salt),
+           passphrase}));
       break;
     case syncer::KeyDerivationMethod::UNSUPPORTED:
       // This test cannot pass since we wouldn't know how to decrypt data
       // encrypted using an unsupported method.
       ADD_FAILURE() << "Unsupported key derivation method encountered: "
                     << nigori.custom_passphrase_key_derivation_method();
-      return syncer::CryptographerImpl::CreateEmpty();
   }
 
-  std::string decrypted_keys_str;
-  EXPECT_TRUE(cryptographer->DecryptToString(nigori.encryption_keybag(),
-                                             &decrypted_keys_str));
-
-  sync_pb::NigoriKeyBag decrypted_keys;
-  EXPECT_TRUE(decrypted_keys.ParseFromString(decrypted_keys_str));
-
-  syncer::NigoriKeyBag key_bag =
-      syncer::NigoriKeyBag::CreateFromProto(decrypted_keys);
-
-  cryptographer->EmplaceKeysFrom(key_bag);
   return cryptographer;
 }
 
-sync_pb::NigoriSpecifics CreateCustomPassphraseNigori(const KeyParams& params) {
+sync_pb::NigoriSpecifics CreateCustomPassphraseNigori(
+    const syncer::KeyParams& params) {
   syncer::KeyDerivationMethod method = params.derivation_params.method();
 
   sync_pb::NigoriSpecifics nigori;
@@ -112,26 +100,27 @@ sync_pb::NigoriSpecifics CreateCustomPassphraseNigori(const KeyParams& params) {
   // keybag using a key derived from that passphrase). However, in some migrated
   // states, the keybag might also additionally contain an old, pre-migration
   // key.
-  auto cryptographer = syncer::CryptographerImpl::FromSingleKeyForTesting(
-      params.password, params.derivation_params);
-  sync_pb::CryptographerData proto = cryptographer->ToProto();
-  DCHECK(cryptographer->Encrypt(proto.key_bag(),
-                                nigori.mutable_encryption_keybag()));
+  syncer::DirectoryCryptographer cryptographer;
+  bool add_key_result = cryptographer.AddKey(params);
+  DCHECK(add_key_result);
+  bool get_keys_result =
+      cryptographer.GetKeys(nigori.mutable_encryption_keybag());
+  DCHECK(get_keys_result);
 
   return nigori;
 }
 
 sync_pb::EntitySpecifics GetEncryptedBookmarkEntitySpecifics(
     const sync_pb::BookmarkSpecifics& bookmark_specifics,
-    const KeyParams& key_params) {
+    const syncer::KeyParams& key_params) {
   sync_pb::EntitySpecifics new_specifics;
 
   sync_pb::EntitySpecifics wrapped_entity_specifics;
   *wrapped_entity_specifics.mutable_bookmark() = bookmark_specifics;
-  auto cryptographer = syncer::CryptographerImpl::FromSingleKeyForTesting(
-      key_params.password, key_params.derivation_params);
-
-  bool encrypt_result = cryptographer->Encrypt(
+  syncer::DirectoryCryptographer cryptographer;
+  bool add_key_result = cryptographer.AddKey(key_params);
+  DCHECK(add_key_result);
+  bool encrypt_result = cryptographer.Encrypt(
       wrapped_entity_specifics, new_specifics.mutable_encrypted());
   DCHECK(encrypt_result);
 

diff --git a/chrome/browser/sync/test/integration/encryption_helper.h b/chrome/browser/sync/test/integration/encryption_helper.h
@@ -12,21 +12,12 @@
 #include "chrome/browser/sync/test/integration/single_client_status_change_checker.h"
 #include "chrome/browser/sync/test/integration/status_change_checker.h"
 #include "components/sync/driver/trusted_vault_client.h"
-#include "components/sync/nigori/nigori.h"
 #include "components/sync/protocol/nigori_specifics.pb.h"
+#include "components/sync/syncable/directory_cryptographer.h"
 #include "components/sync/test/fake_server/fake_server.h"
 
-namespace syncer {
-class Cryptographer;
-}  // namespace syncer
-
 namespace encryption_helper {
 
-struct KeyParams {
-  syncer::KeyDerivationParams derivation_params;
-  std::string password;
-};
-
 // Given a |fake_server|, fetches its Nigori node and writes it to the
 // proto pointed to by |nigori|. Returns false if the server does not contain
 // exactly one Nigori node.
@@ -51,11 +42,12 @@ InitCustomPassphraseCryptographerFromNigori(
 // provided BookmarkSpecifics and encrypted using the given |key_params|.
 sync_pb::EntitySpecifics GetEncryptedBookmarkEntitySpecifics(
     const sync_pb::BookmarkSpecifics& specifics,
-    const KeyParams& key_params);
+    const syncer::KeyParams& key_params);
 
 // Creates a NigoriSpecifics that describes encryption using a custom passphrase
 // with the given key parameters.
-sync_pb::NigoriSpecifics CreateCustomPassphraseNigori(const KeyParams& params);
+sync_pb::NigoriSpecifics CreateCustomPassphraseNigori(
+    const syncer::KeyParams& params);
 
 }  // namespace encryption_helper
 

diff --git a/chrome/browser/sync/test/integration/single_client_custom_passphrase_sync_test.cc b/chrome/browser/sync/test/integration/single_client_custom_passphrase_sync_test.cc
@@ -11,7 +11,7 @@
 #include "components/sync/driver/profile_sync_service.h"
 #include "components/sync/driver/sync_driver_switches.h"
 #include "components/sync/engine/sync_engine_switches.h"
-#include "components/sync/nigori/cryptographer_impl.h"
+#include "components/sync/nigori/cryptographer.h"
 #include "content/public/test/test_launcher.h"
 #include "crypto/ec_private_key.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -26,14 +26,14 @@ using encryption_helper::CreateCustomPassphraseNigori;
 using encryption_helper::GetEncryptedBookmarkEntitySpecifics;
 using encryption_helper::GetServerNigori;
 using encryption_helper::InitCustomPassphraseCryptographerFromNigori;
-using encryption_helper::KeyParams;
 using encryption_helper::SetNigoriInFakeServer;
 using fake_server::FakeServer;
 using sync_pb::EncryptedData;
 using sync_pb::NigoriSpecifics;
 using sync_pb::SyncEntity;
 using syncer::Cryptographer;
 using syncer::KeyDerivationParams;
+using syncer::KeyParams;
 using syncer::LoopbackServerEntity;
 using syncer::ModelType;
 using syncer::ModelTypeSet;
@@ -107,8 +107,7 @@ class SingleClientCustomPassphraseSyncTest : public SyncTest {
       const std::vector<ServerBookmarksEqualityChecker::ExpectedBookmark>&
           expected_bookmarks,
       const KeyParams& key_params) {
-    auto cryptographer = syncer::CryptographerImpl::FromSingleKeyForTesting(
-        key_params.password, key_params.derivation_params);
+    auto cryptographer = CreateCryptographerWithKeyParams(key_params);
     return ServerBookmarksEqualityChecker(GetSyncService(), GetFakeServer(),
                                           expected_bookmarks,
                                           cryptographer.get())
@@ -161,6 +160,16 @@ class SingleClientCustomPassphraseSyncTest : public SyncTest {
     return InitCustomPassphraseCryptographerFromNigori(nigori, passphrase);
   }
 
+  // A cryptographer initialized with the given KeyParams has not "seen" the
+  // server-side Nigori, and so any data decryptable by such a cryptographer
+  // does not depend on external info.
+  std::unique_ptr<Cryptographer> CreateCryptographerWithKeyParams(
+      const KeyParams& key_params) {
+    auto cryptographer = std::make_unique<syncer::DirectoryCryptographer>();
+    cryptographer->AddKey(key_params);
+    return cryptographer;
+  }
+
   void InjectEncryptedServerBookmark(const std::string& title,
                                      const GURL& url,
                                      const KeyParams& key_params) {

diff --git a/chrome/browser/sync/test/integration/single_client_nigori_sync_test.cc b/chrome/browser/sync/test/integration/single_client_nigori_sync_test.cc
@@ -37,11 +37,15 @@
 namespace {
 
 using encryption_helper::GetServerNigori;
-using encryption_helper::KeyParams;
 using encryption_helper::SetNigoriInFakeServer;
 using testing::NotNull;
 using testing::SizeIs;
 
+struct KeyParams {
+  syncer::KeyDerivationParams derivation_params;
+  std::string password;
+};
+
 MATCHER_P(IsDataEncryptedWith, key_params, "") {
   const sync_pb::EncryptedData& encrypted_data = arg;
   std::unique_ptr<syncer::Nigori> nigori = syncer::Nigori::CreateByDerivation(