[Sync:USS] Support decryption for explicit passphrase

After receiving the Nigori with explicit passphrase type bridge
requests passphrase via OnPassphraseRequired() observer's method.
Calling OnPassphraseRequired() should make UI ask for passphrase and
calling SetDecryptionPassprase() once it is provided. UI part is
responsible for verifying passphrase by using pending keys exposed by
OnPassphraseRequired().

This CL doesn't contain logic for SCRYPT key derivation method and
triggering Nigori rewrite in case we have additional encryption keys
locally.

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

components/sync/engine/sync_encryption_handler.h

@@ -150,14 +150,15 @@ class SyncEncryptionHandler {
   // Notifies observers of the result of the operation via OnPassphraseAccepted
   // or OnPassphraseRequired, updates the nigori node, and does re-encryption as
   // appropriate. If an explicit password has been set previously, we drop
-  // subsequent requests to set a passphrase.
+  // subsequent requests to set a passphrase. |passphrase| shouldn't be empty.
   virtual void SetEncryptionPassphrase(const std::string& passphrase) = 0;
 
   // Provides a passphrase for decrypting the user's existing sync data.
   // Notifies observers of the result of the operation via OnPassphraseAccepted
   // or OnPassphraseRequired, updates the nigori node, and does re-encryption as
   // appropriate if there is a previously cached encryption passphrase. It is an
-  // error to call this when we don't have pending keys.
+  // error to call this when we don't have pending keys. |passphrase| shouldn't
+  // be empty.
   virtual void SetDecryptionPassphrase(const std::string& passphrase) = 0;
 
   // Enables encryption of all datatypes.

components/sync/nigori/nigori_sync_bridge_impl.cc

@@ -253,6 +253,45 @@ void NigoriSyncBridgeImpl::SetEncryptionPassphrase(
 void NigoriSyncBridgeImpl::SetDecryptionPassphrase(
     const std::string& passphrase) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  // |passphrase| should be a valid one already (verified by the UI part, using
+  // pending keys exposed by OnPassphraseRequired()).
+  DCHECK(!passphrase.empty());
+  DCHECK(cryptographer_.has_pending_keys());
+  // has_pending_keys() should mean it's an explicit passphrase user.
+  DCHECK(passphrase_type_ == NigoriSpecifics::FROZEN_IMPLICIT_PASSPHRASE ||
+         passphrase_type_ == NigoriSpecifics::CUSTOM_PASSPHRASE);
+
+  KeyParams key_params = {KeyDerivationParams::CreateForPbkdf2(), passphrase};
+  // The line below should set given |passphrase| as default key and cause
+  // decryption of pending keys.
+  if (!cryptographer_.AddKey(key_params)) {
+    processor_->ReportError(ModelError(
+        FROM_HERE, "Failed to add decryption passphrase to cryptographer."));
+    return;
+  }
+  if (cryptographer_.has_pending_keys()) {
+    // TODO(crbug.com/922900): old implementation assumes that pending keys
+    // encryption key may change in between of OnPassphraseRequired() and
+    // SetDecryptionPassphrase() calls, verify whether it's really possible.
+    // Hypothetical cases are transition from FROZEN_IMPLICIT_PASSPHRASE to
+    // CUSTOM_PASSPHRASE and changing of passphrase due to conflict resolution.
+    processor_->ReportError(ModelError(
+        FROM_HERE,
+        "Failed to decrypt pending keys with provided explicit passphrase."));
+    return;
+  }
+  for (auto& observer : observers_) {
+    observer.OnCryptographerStateChanged(&cryptographer_);
+  }
+  for (auto& observer : observers_) {
+    observer.OnPassphraseAccepted();
+  }
+  // TODO(crbug.com/922900): persist |passphrase| in corresponding storage.
+  // TODO(crbug.com/922900): support SCRYPT key derivation method and
+  // corresponding migration code.
+  // TODO(crbug.com/922900): we may need to rewrite encryption_keybag in Nigori
+  // node in case we have some keys in |cryptographer_| which is not stored in
+  // encryption_keybag yet.
   NOTIMPLEMENTED();
 }
 
@@ -437,6 +476,20 @@ base::Optional<ModelError> NigoriSyncBridgeImpl::UpdateLocalState(
   for (auto& observer : observers_) {
     observer.OnCryptographerStateChanged(&cryptographer_);
   }
+  if (cryptographer_.has_pending_keys()) {
+    // Update with keystore Nigori shouldn't reach this point, since it should
+    // report model error if it has pending keys.
+    DCHECK(passphrase_type_ == NigoriSpecifics::CUSTOM_PASSPHRASE ||
+           passphrase_type_ == NigoriSpecifics::FROZEN_IMPLICIT_PASSPHRASE);
+    for (auto& observer : observers_) {
+      // TODO(crbug.com/922900): pass correct key_derivation_params once SCRYPT
+      // support is added.
+      observer.OnPassphraseRequired(
+          /*reason=*/REASON_DECRYPTION,
+          /*key_derivation_params=*/KeyDerivationParams::CreateForPbkdf2(),
+          /*pending_keys=*/cryptographer_.GetPendingKeys());
+    }
+  }
   return base::nullopt;
 }
 
@@ -469,11 +522,24 @@ NigoriSyncBridgeImpl::UpdateCryptographerFromKeystoreNigori(
 }
 
 void NigoriSyncBridgeImpl::UpdateCryptographerFromExplicitPassphraseNigori(
-    const sync_pb::EncryptedData& keybag) {
+    const sync_pb::EncryptedData& encryption_keybag) {
   // TODO(crbug.com/922900): support the case when client knows passphrase.
   NOTIMPLEMENTED();
-  DCHECK(!keybag.blob().empty());
-  cryptographer_.SetPendingKeys(keybag);
+  DCHECK(!encryption_keybag.blob().empty());
+  if (!cryptographer_.CanDecrypt(encryption_keybag)) {
+    // 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
+  // |encryption_keybag| since some other client can write old keys to
+  // |encryption_keybag| and could encrypt some data with them.
+  // TODO(crbug.com/922900): find and document at least one real case
+  // corresponding to the sentence above.
+  // TODO(crbug.com/922900): we may also need to rewrite Nigori with keys
+  // currently stored in cryptographer, in case it doesn't have them already.
+  cryptographer_.InstallKeys(encryption_keybag);
 }
 
 std::unique_ptr<EntityData> NigoriSyncBridgeImpl::GetData() {

components/sync/nigori/nigori_sync_bridge_impl_unittest.cc

@@ -77,6 +77,12 @@ MATCHER_P(CanDecryptWith, key_params, "") {
   return decrypted == unencrypted;
 }
 
+MATCHER_P(EncryptedDataEq, expected, "") {
+  const sync_pb::EncryptedData& given = arg;
+  return given.key_name() == expected.key_name() &&
+         given.blob() == expected.blob();
+}
+
 KeyParams Pbkdf2KeyParams(std::string key) {
   return {KeyDerivationParams::CreateForPbkdf2(), std::move(key)};
 }
@@ -125,7 +131,8 @@ class MockObserver : public SyncEncryptionHandler::Observer {
 class NigoriSyncBridgeImplTest : public testing::Test {
  protected:
   NigoriSyncBridgeImplTest() {
-    auto processor = std::make_unique<MockNigoriLocalChangeProcessor>();
+    auto processor =
+        std::make_unique<testing::NiceMock<MockNigoriLocalChangeProcessor>>();
     processor_ = processor.get();
     bridge_ = std::make_unique<NigoriSyncBridgeImpl>(std::move(processor),
                                                      &encryptor_);
@@ -174,12 +181,24 @@ class NigoriSyncBridgeImplTest : public testing::Test {
     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& key_params) {
+      const KeyParams& passphrase_key_params,
+      const base::Optional<KeyParams>& old_key_params = base::nullopt) {
     sync_pb::NigoriSpecifics specifics;
 
     Cryptographer cryptographer(&encryptor_);
-    cryptographer.AddKey(key_params);
+    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_time(TimeToProtoTime(base::Time::Now()));
@@ -194,7 +213,7 @@ class NigoriSyncBridgeImplTest : public testing::Test {
   FakeEncryptor encryptor_;
   std::unique_ptr<NigoriSyncBridgeImpl> bridge_;
   // Ownership transferred to |bridge_|.
-  MockNigoriLocalChangeProcessor* processor_;
+  testing::NiceMock<MockNigoriLocalChangeProcessor>* processor_;
   testing::NiceMock<MockObserver> observer_;
 };
 
@@ -401,6 +420,39 @@ TEST_F(NigoriSyncBridgeImplTest, ShouldFailOnUnknownPassprase) {
               Ne(base::nullopt));
 }
 
+// Tests decryption logic for explicit passphrase. In order to check that we're
+// able to decrypt the data encrypted with old key (i.e. keystore keys or old
+// GAIA passphrase) we add one extra key to the encryption keybag.
+TEST_F(NigoriSyncBridgeImplTest,
+       ShouldDecryptWithCustomPassphraseAndUpdateDefaultKey) {
+  const KeyParams kOldKeyParams = Pbkdf2KeyParams("old_key");
+  const KeyParams kPassphraseKeyParams = Pbkdf2KeyParams("passphrase");
+  EntityData entity_data;
+  *entity_data.specifics.mutable_nigori() =
+      BuildCustomPassphraseNigoriSpecifics(kPassphraseKeyParams, kOldKeyParams);
+
+  ASSERT_TRUE(bridge()->SetKeystoreKeys({"keystore_key"}));
+
+  EXPECT_CALL(
+      *observer(),
+      OnPassphraseRequired(
+          /*reason=*/REASON_DECRYPTION,
+          /*key_derivation_params=*/KeyDerivationParams::CreateForPbkdf2(),
+          /*pending_keys=*/
+          EncryptedDataEq(entity_data.specifics.nigori().encryption_keybag())));
+  ASSERT_THAT(bridge()->MergeSyncData(std::move(entity_data)),
+              Eq(base::nullopt));
+
+  EXPECT_CALL(*observer(), OnPassphraseAccepted());
+  EXPECT_CALL(*observer(), OnCryptographerStateChanged(NotNull()));
+  bridge()->SetDecryptionPassphrase(kPassphraseKeyParams.password);
+
+  const Cryptographer& cryptographer = bridge()->GetCryptographerForTesting();
+  EXPECT_THAT(cryptographer, CanDecryptWith(kOldKeyParams));
+  EXPECT_THAT(cryptographer, CanDecryptWith(kPassphraseKeyParams));
+  EXPECT_THAT(cryptographer, HasDefaultKeyDerivedFrom(kPassphraseKeyParams));
+}
+
 }  // namespace
 
 }  // namespace syncer