[Sync:USS] Load explicit passphrase key from prefs

In order to allow silent migration from Directory to USS implementation
of Nigori, we need to support loading explicit passphrase key from
preferences (bootstrap_token). We just pass it to constructor of
NigoriSyncBridgeImpl and use on-demand (once we receive explicit
passphrase Nigori from the server), to avoid potentially wrong Nigori
states due to early initialization of Cryptographer.

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

components/sync/driver/glue/sync_engine_backend.cc

@@ -349,7 +349,8 @@ void SyncEngineBackend::DoInitialize(SyncEngine::InitParams params) {
         NIGORI, std::make_unique<ForwardingModelTypeControllerDelegate>(
                     nigori_processor->GetControllerDelegate().get()));
     sync_encryption_handler_ = std::make_unique<NigoriSyncBridgeImpl>(
-        std::move(nigori_processor), &encryptor_);
+        std::move(nigori_processor), &encryptor_,
+        params.restored_key_for_bootstrapping);
   } else {
     sync_encryption_handler_ = std::make_unique<SyncEncryptionHandlerImpl>(
         &user_share_, &encryptor_, params.restored_key_for_bootstrapping,

components/sync/nigori/nigori_sync_bridge_impl.cc

@@ -361,13 +361,52 @@ std::string PackExplicitPassphraseKey(const Encryptor& encryptor,
   return encoded_key;
 }
 
+// Unpacks explicit passphrase keys. Returns serialized sync_pb::NigoriKey if
+// successful. If |packed_key| is empty or decoding/decryption errors occur.
+// Should be aligned with Directory implementation (
+// Cryptographer::UnpackBootstrapToken()) unless it is removed.
+std::string UnpackExplicitPassphraseKey(const Encryptor& encryptor,
+                                        const std::string& packed_key) {
+  if (packed_key.empty()) {
+    return std::string();
+  }
+
+  std::string decoded_key;
+  if (!base::Base64Decode(packed_key, &decoded_key)) {
+    DLOG(ERROR) << "Failed to decode explicit passphrase key.";
+    return std::string();
+  }
+
+  std::string decrypted_key;
+  if (!encryptor.DecryptString(decoded_key, &decrypted_key)) {
+    DLOG(ERROR) << "Failed to decrypt expliciti passphrase key.";
+    return std::string();
+  }
+  return decrypted_key;
+}
+
+bool CanDecryptWithSerializedNigoriKey(
+    const std::string& serialized_key,
+    const sync_pb::EncryptedData& encrypted_data) {
+  if (serialized_key.empty()) {
+    return false;
+  }
+  Cryptographer cryptographer;
+  cryptographer.ImportNigoriKey(serialized_key);
+  return cryptographer.CanDecrypt(encrypted_data);
+}
+
 }  // namespace
 
 NigoriSyncBridgeImpl::NigoriSyncBridgeImpl(
     std::unique_ptr<NigoriLocalChangeProcessor> processor,
-    const Encryptor* encryptor)
+    const Encryptor* encryptor,
+    const std::string& packed_explicit_passphrase_key)
     : encryptor_(encryptor),
       processor_(std::move(processor)),
+      serialized_explicit_passphrase_key_(
+          UnpackExplicitPassphraseKey(*encryptor,
+                                      packed_explicit_passphrase_key)),
       passphrase_type_(NigoriSpecifics::UNKNOWN),
       encrypt_everything_(false) {
   DCHECK(encryptor);
@@ -752,9 +791,18 @@ void NigoriSyncBridgeImpl::UpdateCryptographerFromExplicitPassphraseNigori(
   NOTIMPLEMENTED();
   DCHECK(!encryption_keybag.blob().empty());
   if (!cryptographer_.CanDecrypt(encryption_keybag)) {
-    // This will lead to OnPassphraseRequired() call later.
-    cryptographer_.SetPendingKeys(encryption_keybag);
-    return;
+    // Historically, prior to USS, key derived from explicit passphrase was
+    // stored in prefs and effectively we do migration here.
+    if (CanDecryptWithSerializedNigoriKey(serialized_explicit_passphrase_key_,
+                                          encryption_keybag)) {
+      // ImportNigoriKey() will set default key from
+      // |serialized_explicit_passphrase_key_|.
+      cryptographer_.ImportNigoriKey(serialized_explicit_passphrase_key_);
+    } else {
+      // This will lead to OnPassphraseRequired() call later.
+      cryptographer_.SetPendingKeys(encryption_keybag);
+      return;
+    }
   }
   // |cryptographer_| can already have explicit passphrase, in that case it
   // should be able to decrypt |encryption_keybag|. We need to take keys from
@@ -823,6 +871,12 @@ const Cryptographer& NigoriSyncBridgeImpl::GetCryptographerForTesting() const {
   return cryptographer_;
 }
 
+std::string NigoriSyncBridgeImpl::PackExplicitPassphraseKeyForTesting(
+    const Encryptor& encryptor,
+    const Cryptographer& cryptographer) {
+  return PackExplicitPassphraseKey(encryptor, cryptographer);
+}
+
 base::Time NigoriSyncBridgeImpl::GetExplicitPassphraseTime() const {
   switch (passphrase_type_) {
     case NigoriSpecifics::IMPLICIT_PASSPHRASE:

components/sync/nigori/nigori_sync_bridge_impl.h

@@ -40,7 +40,8 @@ class NigoriSyncBridgeImpl : public KeystoreKeysHandler,
  public:
   // |encryptor| must be not null and must outlive this object.
   NigoriSyncBridgeImpl(std::unique_ptr<NigoriLocalChangeProcessor> processor,
-                       const Encryptor* encryptor);
+                       const Encryptor* encryptor,
+                       const std::string& packed_explicit_passphrase_key);
   ~NigoriSyncBridgeImpl() override;
 
   // SyncEncryptionHandler implementation.
@@ -74,6 +75,9 @@ class NigoriSyncBridgeImpl : public KeystoreKeysHandler,
   // tests and decide whether this method should be a part of
   // SyncEncryptionHandler interface.
   const Cryptographer& GetCryptographerForTesting() const;
+  static std::string PackExplicitPassphraseKeyForTesting(
+      const Encryptor& encryptor,
+      const Cryptographer& cryptographer);
 
  private:
   base::Optional<ModelError> UpdateLocalState(
@@ -102,6 +106,11 @@ class NigoriSyncBridgeImpl : public KeystoreKeysHandler,
 
   const std::unique_ptr<NigoriLocalChangeProcessor> processor_;
 
+  // Stores serialized sync_pb::NigoriKey derived from explicit passphrase and
+  // loaded from the prefs. Empty if prefs doesn't contain this key or in case
+  // of decryption/decoding errors.
+  const std::string serialized_explicit_passphrase_key_;
+
   // Base64 encoded keystore keys. The last element is the current keystore
   // key. These keys are not a part of Nigori node and are persisted
   // separately. Should be encrypted with OSCrypt before persisting.

components/sync/nigori/nigori_sync_bridge_impl_unittest.cc

@@ -114,6 +114,88 @@ KeyParams ScryptKeyParams(const std::string& key) {
   return {KeyDerivationParams::CreateForScrypt("some_constant_salt"), key};
 }
 
+std::string PackKeyAsExplicitPassphrase(const KeyParams& key_params,
+                                        const Encryptor& encryptor) {
+  Cryptographer cryptographer;
+  cryptographer.AddKey(key_params);
+  return NigoriSyncBridgeImpl::PackExplicitPassphraseKeyForTesting(
+      encryptor, cryptographer);
+}
+
+// Builds NigoriSpecifics with following fields:
+// 1. encryption_keybag contains all keys derived from |keybag_keys_params|
+// and encrypted with a key derived from |keybag_decryptor_params|.
+// keystore_decryptor_token is always saved in encryption_keybag, even if it
+// is not derived from any params in |keybag_keys_params|.
+// 2. keystore_decryptor_token contains the key derived from
+// |keybag_decryptor_params| and encrypted with a key derived from
+// |keystore_key_params|.
+// 3. passphrase_type is KEYSTORE_PASSHPRASE.
+// 4. Other fields are default.
+sync_pb::NigoriSpecifics BuildKeystoreNigoriSpecifics(
+    const std::vector<KeyParams>& keybag_keys_params,
+    const KeyParams& keystore_decryptor_params,
+    const KeyParams& keystore_key_params) {
+  sync_pb::NigoriSpecifics specifics;
+
+  Cryptographer cryptographer;
+  cryptographer.AddKey(keystore_decryptor_params);
+  for (const KeyParams& key_params : keybag_keys_params) {
+    cryptographer.AddNonDefaultKey(key_params);
+  }
+  EXPECT_TRUE(cryptographer.GetKeys(specifics.mutable_encryption_keybag()));
+
+  std::string serialized_keystore_decryptor =
+      cryptographer.GetDefaultNigoriKeyData();
+  Cryptographer keystore_cryptographer;
+  keystore_cryptographer.AddKey(keystore_key_params);
+  EXPECT_TRUE(keystore_cryptographer.EncryptString(
+      serialized_keystore_decryptor,
+      specifics.mutable_keystore_decryptor_token()));
+
+  specifics.set_passphrase_type(sync_pb::NigoriSpecifics::KEYSTORE_PASSPHRASE);
+  return specifics;
+}
+
+// Builds NigoriSpecifics with following fields:
+// 1. encryption_keybag contains keys derived from |passphrase_key_params|
+// and |*old_key_params| (if |old_key_params| isn't nullopt). Encrypted with
+// key derived from |passphrase_key_params|.
+// 2. custom_passphrase_time is current time.
+// 3. passphrase_type is CUSTOM_PASSPHRASE.
+// 4. encrypt_everything is true.
+// 5. Other fields are default.
+sync_pb::NigoriSpecifics BuildCustomPassphraseNigoriSpecifics(
+    const KeyParams& passphrase_key_params,
+    const base::Optional<KeyParams>& old_key_params = base::nullopt) {
+  sync_pb::NigoriSpecifics specifics;
+
+  Cryptographer cryptographer;
+  cryptographer.AddKey(passphrase_key_params);
+  if (old_key_params) {
+    cryptographer.AddNonDefaultKey(*old_key_params);
+  }
+  EXPECT_TRUE(cryptographer.GetKeys(specifics.mutable_encryption_keybag()));
+
+  specifics.set_custom_passphrase_key_derivation_method(
+      EnumKeyDerivationMethodToProto(
+          passphrase_key_params.derivation_params.method()));
+  if (passphrase_key_params.derivation_params.method() ==
+      KeyDerivationMethod::SCRYPT_8192_8_11) {
+    // Persist the salt used for key derivation in Nigori if we're using
+    // scrypt.
+    std::string encoded_salt;
+    base::Base64Encode(passphrase_key_params.derivation_params.scrypt_salt(),
+                       &encoded_salt);
+    specifics.set_custom_passphrase_key_derivation_salt(encoded_salt);
+  }
+  specifics.set_custom_passphrase_time(TimeToProtoTime(base::Time::Now()));
+  specifics.set_passphrase_type(sync_pb::NigoriSpecifics::CUSTOM_PASSPHRASE);
+  specifics.set_encrypt_everything(true);
+
+  return specifics;
+}
+
 class MockNigoriLocalChangeProcessor : public NigoriLocalChangeProcessor {
  public:
   MockNigoriLocalChangeProcessor() = default;
@@ -153,8 +235,9 @@ class NigoriSyncBridgeImplTest : public testing::Test {
     auto processor =
         std::make_unique<testing::NiceMock<MockNigoriLocalChangeProcessor>>();
     processor_ = processor.get();
-    bridge_ = std::make_unique<NigoriSyncBridgeImpl>(std::move(processor),
-                                                     &encryptor_);
+    bridge_ = std::make_unique<NigoriSyncBridgeImpl>(
+        std::move(processor), &encryptor_,
+        /*packed_explicit_passphrase_key=*/std::string());
     bridge_->AddObserver(&observer_);
   }
 
@@ -164,81 +247,6 @@ class NigoriSyncBridgeImplTest : public testing::Test {
   MockNigoriLocalChangeProcessor* processor() { return processor_; }
   MockObserver* observer() { return &observer_; }
 
-  // Builds NigoriSpecifics with following fields:
-  // 1. encryption_keybag contains all keys derived from |keybag_keys_params|
-  // and encrypted with a key derived from |keybag_decryptor_params|.
-  // keystore_decryptor_token is always saved in encryption_keybag, even if it
-  // is not derived from any params in |keybag_keys_params|.
-  // 2. keystore_decryptor_token contains the key derived from
-  // |keybag_decryptor_params| and encrypted with a key derived from
-  // |keystore_key_params|.
-  // 3. passphrase_type is KEYSTORE_PASSHPRASE.
-  // 4. Other fields are default.
-  sync_pb::NigoriSpecifics BuildKeystoreNigoriSpecifics(
-      const std::vector<KeyParams>& keybag_keys_params,
-      const KeyParams& keystore_decryptor_params,
-      const KeyParams& keystore_key_params) {
-    sync_pb::NigoriSpecifics specifics;
-
-    Cryptographer cryptographer;
-    cryptographer.AddKey(keystore_decryptor_params);
-    for (const KeyParams& key_params : keybag_keys_params) {
-      cryptographer.AddNonDefaultKey(key_params);
-    }
-    EXPECT_TRUE(cryptographer.GetKeys(specifics.mutable_encryption_keybag()));
-
-    std::string serialized_keystore_decryptor =
-        cryptographer.GetDefaultNigoriKeyData();
-    Cryptographer keystore_cryptographer;
-    keystore_cryptographer.AddKey(keystore_key_params);
-    EXPECT_TRUE(keystore_cryptographer.EncryptString(
-        serialized_keystore_decryptor,
-        specifics.mutable_keystore_decryptor_token()));
-
-    specifics.set_passphrase_type(
-        sync_pb::NigoriSpecifics::KEYSTORE_PASSPHRASE);
-    return specifics;
-  }
-
-  // Builds NigoriSpecifics with following fields:
-  // 1. encryption_keybag contains keys derived from |passphrase_key_params|
-  // and |*old_key_params| (if |old_key_params| isn't nullopt). Encrypted with
-  // key derived from |passphrase_key_params|.
-  // 2. custom_passphrase_time is current time.
-  // 3. passphrase_type is CUSTOM_PASSPHRASE.
-  // 4. encrypt_everything is true.
-  // 5. Other fields are default.
-  sync_pb::NigoriSpecifics BuildCustomPassphraseNigoriSpecifics(
-      const KeyParams& passphrase_key_params,
-      const base::Optional<KeyParams>& old_key_params = base::nullopt) {
-    sync_pb::NigoriSpecifics specifics;
-
-    Cryptographer cryptographer;
-    cryptographer.AddKey(passphrase_key_params);
-    if (old_key_params) {
-      cryptographer.AddNonDefaultKey(*old_key_params);
-    }
-    EXPECT_TRUE(cryptographer.GetKeys(specifics.mutable_encryption_keybag()));
-
-    specifics.set_custom_passphrase_key_derivation_method(
-        EnumKeyDerivationMethodToProto(
-            passphrase_key_params.derivation_params.method()));
-    if (passphrase_key_params.derivation_params.method() ==
-        KeyDerivationMethod::SCRYPT_8192_8_11) {
-      // Persist the salt used for key derivation in Nigori if we're using
-      // scrypt.
-      std::string encoded_salt;
-      base::Base64Encode(passphrase_key_params.derivation_params.scrypt_salt(),
-                         &encoded_salt);
-      specifics.set_custom_passphrase_key_derivation_salt(encoded_salt);
-    }
-    specifics.set_custom_passphrase_time(TimeToProtoTime(base::Time::Now()));
-    specifics.set_passphrase_type(sync_pb::NigoriSpecifics::CUSTOM_PASSPHRASE);
-    specifics.set_encrypt_everything(true);
-
-    return specifics;
-  }
-
  private:
   const FakeEncryptor encryptor_;
   std::unique_ptr<NigoriSyncBridgeImpl> bridge_;
@@ -558,7 +566,7 @@ TEST_F(NigoriSyncBridgeImplTest,
 TEST_F(NigoriSyncBridgeImplTest, ShouldNotAllowCustomPassphraseChange) {
   EntityData entity_data;
   *entity_data.specifics.mutable_nigori() =
-      BuildCustomPassphraseNigoriSpecifics(Pbkdf2KeyParams("passphrase"), {});
+      BuildCustomPassphraseNigoriSpecifics(Pbkdf2KeyParams("passphrase"));
   ASSERT_TRUE(bridge()->SetKeystoreKeys({"keystore_key"}));
   ASSERT_THAT(bridge()->MergeSyncData(std::move(entity_data)),
               Eq(base::nullopt));
@@ -567,6 +575,36 @@ TEST_F(NigoriSyncBridgeImplTest, ShouldNotAllowCustomPassphraseChange) {
   bridge()->SetEncryptionPassphrase("new_passphrase");
 }
 
+// Tests that we can use packed explicit passphrase key passed to bridge to
+// decrypt custom passphrase NigoriSpecifics.
+TEST(NigoriSyncBridgeImplTestWithPackedExplicitPassphrase,
+     ShouldDecryptWithExplicitPassphraseFromPrefs) {
+  const KeyParams kKeyParams = Pbkdf2KeyParams("passphrase");
+
+  const FakeEncryptor encryptor;
+  auto processor =
+      std::make_unique<testing::NiceMock<MockNigoriLocalChangeProcessor>>();
+  auto bridge = std::make_unique<NigoriSyncBridgeImpl>(
+      std::move(processor), &encryptor,
+      PackKeyAsExplicitPassphrase(kKeyParams, encryptor));
+  testing::NiceMock<MockObserver> observer;
+  bridge->AddObserver(&observer);
+
+  EntityData entity_data;
+  *entity_data.specifics.mutable_nigori() =
+      BuildCustomPassphraseNigoriSpecifics(kKeyParams);
+  ASSERT_TRUE(bridge->SetKeystoreKeys({"keystore_key"}));
+
+  EXPECT_CALL(observer, OnCryptographerStateChanged(NotNull()));
+  EXPECT_CALL(observer, OnPassphraseRequired(_, _, _)).Times(0);
+  ASSERT_THAT(bridge->MergeSyncData(std::move(entity_data)), Eq(base::nullopt));
+
+  const Cryptographer& cryptographer = bridge->GetCryptographerForTesting();
+  EXPECT_THAT(cryptographer, CanDecryptWith(kKeyParams));
+  EXPECT_THAT(cryptographer, HasDefaultKeyDerivedFrom(kKeyParams));
+  bridge->RemoveObserver(&observer);
+}
+
 }  // namespace
 
 }  // namespace syncer