Move Nigori specifics conversion to NigoriState

This removes complexity from the bridge to a struct with much simpler
semantics, and allows future patches to compute specifics from
NigoriState instances other than the authoritative copy owned by the
bridge.

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

components/sync/BUILD.gn

@@ -649,6 +649,7 @@ source_set("unit_tests") {
     "nigori/cryptographer_impl_unittest.cc",
     "nigori/nigori_key_bag_unittest.cc",
     "nigori/nigori_model_type_processor_unittest.cc",
+    "nigori/nigori_state_unittest.cc",
     "nigori/nigori_storage_impl_unittest.cc",
     "nigori/nigori_sync_bridge_impl_unittest.cc",
     "nigori/nigori_unittest.cc",

components/sync/nigori/nigori_state.cc

@@ -4,6 +4,7 @@
 
 #include "components/sync/nigori/nigori_state.h"
 
+#include "base/base64.h"
 #include "components/sync/base/model_type.h"
 #include "components/sync/base/time.h"
 #include "components/sync/engine/sync_encryption_handler.h"
@@ -48,10 +49,123 @@ KeyDerivationParams CustomPassphraseKeyDerivationParamsFromProto(
   return KeyDerivationParams::CreateWithUnsupportedMethod();
 }
 
+// |encrypted| must not be null.
+bool EncryptKeyBag(const CryptographerImpl& cryptographer,
+                   sync_pb::EncryptedData* encrypted) {
+  DCHECK(encrypted);
+  DCHECK(cryptographer.CanEncrypt());
+
+  sync_pb::CryptographerData proto = cryptographer.ToProto();
+  DCHECK(!proto.key_bag().key().empty());
+
+  // Encrypt the bag with the default Nigori.
+  return cryptographer.Encrypt(proto.key_bag(), encrypted);
+}
+
+// Updates |specifics|'s individual datatypes encryption state based on
+// |encrypted_types|.
+void UpdateNigoriSpecificsFromEncryptedTypes(
+    ModelTypeSet encrypted_types,
+    sync_pb::NigoriSpecifics* specifics) {
+  static_assert(39 == ModelType::NUM_ENTRIES,
+                "If adding an encryptable type, update handling below.");
+  specifics->set_encrypt_bookmarks(encrypted_types.Has(BOOKMARKS));
+  specifics->set_encrypt_preferences(encrypted_types.Has(PREFERENCES));
+  specifics->set_encrypt_autofill_profile(
+      encrypted_types.Has(AUTOFILL_PROFILE));
+  specifics->set_encrypt_autofill(encrypted_types.Has(AUTOFILL));
+  specifics->set_encrypt_autofill_wallet_metadata(
+      encrypted_types.Has(AUTOFILL_WALLET_METADATA));
+  specifics->set_encrypt_themes(encrypted_types.Has(THEMES));
+  specifics->set_encrypt_typed_urls(encrypted_types.Has(TYPED_URLS));
+  specifics->set_encrypt_extensions(encrypted_types.Has(EXTENSIONS));
+  specifics->set_encrypt_search_engines(encrypted_types.Has(SEARCH_ENGINES));
+  specifics->set_encrypt_sessions(encrypted_types.Has(SESSIONS));
+  specifics->set_encrypt_apps(encrypted_types.Has(APPS));
+  specifics->set_encrypt_app_settings(encrypted_types.Has(APP_SETTINGS));
+  specifics->set_encrypt_extension_settings(
+      encrypted_types.Has(EXTENSION_SETTINGS));
+  specifics->set_encrypt_dictionary(encrypted_types.Has(DICTIONARY));
+  specifics->set_encrypt_favicon_images(encrypted_types.Has(FAVICON_IMAGES));
+  specifics->set_encrypt_favicon_tracking(
+      encrypted_types.Has(FAVICON_TRACKING));
+  specifics->set_encrypt_app_list(encrypted_types.Has(APP_LIST));
+  specifics->set_encrypt_arc_package(encrypted_types.Has(ARC_PACKAGE));
+  specifics->set_encrypt_printers(encrypted_types.Has(PRINTERS));
+  specifics->set_encrypt_reading_list(encrypted_types.Has(READING_LIST));
+  specifics->set_encrypt_mountain_shares(encrypted_types.Has(MOUNTAIN_SHARES));
+  specifics->set_encrypt_send_tab_to_self(
+      encrypted_types.Has(SEND_TAB_TO_SELF));
+  specifics->set_encrypt_web_apps(encrypted_types.Has(WEB_APPS));
+}
+
+void UpdateSpecificsFromKeyDerivationParams(
+    const KeyDerivationParams& params,
+    sync_pb::NigoriSpecifics* specifics) {
+  DCHECK_EQ(specifics->passphrase_type(),
+            sync_pb::NigoriSpecifics::CUSTOM_PASSPHRASE);
+  DCHECK_NE(params.method(), KeyDerivationMethod::UNSUPPORTED);
+  specifics->set_custom_passphrase_key_derivation_method(
+      EnumKeyDerivationMethodToProto(params.method()));
+  if (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(params.scrypt_salt(), &encoded_salt);
+    specifics->set_custom_passphrase_key_derivation_salt(encoded_salt);
+  }
+}
+
+bool EncryptKeystoreDecryptorToken(
+    const CryptographerImpl& cryptographer,
+    sync_pb::EncryptedData* keystore_decryptor_token,
+    const std::vector<std::string>& keystore_keys) {
+  DCHECK(keystore_decryptor_token);
+
+  const sync_pb::NigoriKey default_key = cryptographer.ExportDefaultKey();
+
+  std::unique_ptr<Cryptographer> keystore_cryptographer =
+      CreateCryptographerFromKeystoreKeys(keystore_keys);
+  if (!keystore_cryptographer) {
+    return false;
+  }
+
+  return keystore_cryptographer->EncryptString(default_key.SerializeAsString(),
+                                               keystore_decryptor_token);
+}
+
 }  // namespace
 
+std::unique_ptr<CryptographerImpl> CreateCryptographerFromKeystoreKeys(
+    const std::vector<std::string>& keystore_keys) {
+  std::unique_ptr<CryptographerImpl> cryptographer =
+      CryptographerImpl::CreateEmpty();
+
+  if (keystore_keys.empty()) {
+    return cryptographer;
+  }
+
+  std::string last_key_name;
+  for (const std::string& key : keystore_keys) {
+    last_key_name =
+        cryptographer->EmplaceKey(key, KeyDerivationParams::CreateForPbkdf2());
+    // TODO(crbug.com/922900): possible behavioral change. Old implementation
+    // fails only if we failed to add current keystore key. Failing to add any
+    // of these keys doesn't seem valid. This line seems to be a good candidate
+    // for UMA, as it's not a normal situation, if we fail to add any key.
+    if (last_key_name.empty()) {
+      return nullptr;
+    }
+  }
+
+  DCHECK(!last_key_name.empty());
+  cryptographer->SelectDefaultEncryptionKey(last_key_name);
+
+  return cryptographer;
+}
+
 // static
-NigoriState NigoriState::CreateFromProto(const sync_pb::NigoriModel& proto) {
+NigoriState NigoriState::CreateFromLocalProto(
+    const sync_pb::NigoriModel& proto) {
   NigoriState state;
 
   state.cryptographer =
@@ -93,7 +207,7 @@ NigoriState::~NigoriState() = default;
 
 NigoriState& NigoriState::operator=(NigoriState&& other) = default;
 
-sync_pb::NigoriModel NigoriState::ToProto() const {
+sync_pb::NigoriModel NigoriState::ToLocalProto() const {
   sync_pb::NigoriModel proto;
   *proto.mutable_cryptographer_data() = cryptographer->ToProto();
   if (pending_keys.has_value()) {
@@ -138,4 +252,54 @@ sync_pb::NigoriModel NigoriState::ToProto() const {
   return proto;
 }
 
+sync_pb::NigoriSpecifics NigoriState::ToSpecificsProto() const {
+  sync_pb::NigoriSpecifics specifics;
+  if (cryptographer->CanEncrypt()) {
+    EncryptKeyBag(*cryptographer, specifics.mutable_encryption_keybag());
+  } else {
+    DCHECK(pending_keys.has_value());
+    // This case is reachable only from processor's GetAllNodesForDebugging(),
+    // since currently commit is never issued while bridge has |pending_keys_|.
+    // Note: with complete support of TRUSTED_VAULT mode, commit might be
+    // issued in this case as well.
+    *specifics.mutable_encryption_keybag() = *pending_keys;
+  }
+  specifics.set_keybag_is_frozen(true);
+  specifics.set_encrypt_everything(encrypt_everything);
+  if (encrypt_everything) {
+    UpdateNigoriSpecificsFromEncryptedTypes(EncryptableUserTypes(), &specifics);
+  }
+  specifics.set_passphrase_type(passphrase_type);
+  if (passphrase_type == sync_pb::NigoriSpecifics::CUSTOM_PASSPHRASE) {
+    DCHECK(custom_passphrase_key_derivation_params);
+    UpdateSpecificsFromKeyDerivationParams(
+        *custom_passphrase_key_derivation_params, &specifics);
+  }
+  if (passphrase_type == sync_pb::NigoriSpecifics::KEYSTORE_PASSPHRASE) {
+    // TODO(crbug.com/922900): it seems possible to have corrupted
+    // |pending_keystore_decryptor_token| and an ability to recover it in case
+    // |pending_keys| isn't set and |keystore_keys| contains some keys.
+    if (pending_keystore_decryptor_token.has_value()) {
+      *specifics.mutable_keystore_decryptor_token() =
+          *pending_keystore_decryptor_token;
+    } else {
+      DCHECK(!keystore_keys.empty());
+      EncryptKeystoreDecryptorToken(
+          *cryptographer, specifics.mutable_keystore_decryptor_token(),
+          keystore_keys);
+    }
+  }
+  if (!keystore_migration_time.is_null()) {
+    specifics.set_keystore_migration_time(
+        TimeToProtoTime(keystore_migration_time));
+  }
+  if (!custom_passphrase_time.is_null()) {
+    specifics.set_custom_passphrase_time(
+        TimeToProtoTime(custom_passphrase_time));
+  }
+  // TODO(crbug.com/922900): add other fields support.
+  NOTIMPLEMENTED();
+  return specifics;
+}
+
 }  // namespace syncer

components/sync/nigori/nigori_state.h

@@ -24,6 +24,13 @@ namespace syncer {
 
 class CryptographerImpl;
 
+// Creates a cryptographer that can decrypt all |keystore_keys| and uses the
+// last one as default encryption key. May return null in case of error.
+// TODO(crbug.com/922900): consider maintaining cached version of this
+// cryptographer in NigoriState to avoid repeated creations.
+std::unique_ptr<CryptographerImpl> CreateCryptographerFromKeystoreKeys(
+    const std::vector<std::string>& keystore_keys);
+
 struct NigoriState {
   static constexpr sync_pb::NigoriSpecifics::PassphraseType
       kInitialPassphraseType = sync_pb::NigoriSpecifics::UNKNOWN;
@@ -31,7 +38,7 @@ struct NigoriState {
   static constexpr bool kInitialEncryptEverything = false;
 
   // Deserialization from proto.
-  static NigoriState CreateFromProto(const sync_pb::NigoriModel& proto);
+  static NigoriState CreateFromLocalProto(const sync_pb::NigoriModel& proto);
 
   NigoriState();
   NigoriState(NigoriState&& other);
@@ -39,8 +46,11 @@ struct NigoriState {
 
   NigoriState& operator=(NigoriState&& other);
 
-  // Serialization to proto.
-  sync_pb::NigoriModel ToProto() const;
+  // Serialization to proto as persisted on local disk.
+  sync_pb::NigoriModel ToLocalProto() const;
+
+  // Serialization to proto as sent to the sync server.
+  sync_pb::NigoriSpecifics ToSpecificsProto() const;
 
   std::unique_ptr<CryptographerImpl> cryptographer;
 

components/sync/nigori/nigori_state_unittest.cc

+// Copyright 2019 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/nigori/nigori_state.h"
+
+#include "components/sync/base/time.h"
+#include "components/sync/nigori/cryptographer_impl.h"
+#include "components/sync/nigori/nigori.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace syncer {
+
+namespace {
+
+using sync_pb::NigoriSpecifics;
+using testing::Eq;
+using testing::Ne;
+
+TEST(NigoriStateTest, ShouldConvertCustomPassphraseStateToSpecifics) {
+  const base::Time now = base::Time::Now();
+  const std::string kKey = "key1";
+
+  NigoriState state;
+  state.passphrase_type = NigoriSpecifics::CUSTOM_PASSPHRASE;
+  state.encrypt_everything = true;
+  state.custom_passphrase_time = now;
+  state.custom_passphrase_key_derivation_params =
+      KeyDerivationParams::CreateForPbkdf2();
+
+  const std::string key_name = state.cryptographer->EmplaceKey(
+      kKey, KeyDerivationParams::CreateForPbkdf2());
+  ASSERT_THAT(key_name, Ne(""));
+
+  state.cryptographer->SelectDefaultEncryptionKey(key_name);
+
+  NigoriSpecifics specifics = state.ToSpecificsProto();
+  EXPECT_THAT(specifics.passphrase_type(),
+              Eq(NigoriSpecifics::CUSTOM_PASSPHRASE));
+  EXPECT_TRUE(specifics.keybag_is_frozen());
+  EXPECT_THAT(specifics.encryption_keybag().key_name(), Eq(key_name));
+  EXPECT_THAT(specifics.custom_passphrase_key_derivation_method(),
+              Eq(NigoriSpecifics::PBKDF2_HMAC_SHA1_1003));
+  EXPECT_FALSE(specifics.has_keystore_decryptor_token());
+  EXPECT_FALSE(specifics.has_keystore_migration_time());
+  EXPECT_THAT(specifics.custom_passphrase_time(), Eq(TimeToProtoTime(now)));
+}
+
+TEST(NigoriStateTest, ShouldConvertKeystoreStateToSpecifics) {
+  // Note that in practice having a NigoriState with two keystore keys and yet
+  // a default encryption key that is neither of them is not realistic. However,
+  // it serves this test well to verify that a) which key is used to encrypt the
+  // keybag and b) which key is used to encrypt the keystore decryptor token.
+  const base::Time now = base::Time::Now();
+  const std::string kKeystoreKey1 = "keystorekey1";
+  const std::string kKeystoreKey2 = "keystorekey2";
+  const std::string kDefaultEncryptionKey = "defaultkey";
+
+  NigoriState state;
+  state.keystore_keys = {kKeystoreKey1, kKeystoreKey2};
+  state.passphrase_type = NigoriSpecifics::KEYSTORE_PASSPHRASE;
+  state.keystore_migration_time = now;
+  state.cryptographer = CryptographerImpl::CreateEmpty();
+  state.cryptographer->EmplaceKey(kKeystoreKey1,
+                                  KeyDerivationParams::CreateForPbkdf2());
+  const std::string last_keystore_key_name = state.cryptographer->EmplaceKey(
+      kKeystoreKey2, KeyDerivationParams::CreateForPbkdf2());
+  const std::string default_encryption_key_name =
+      state.cryptographer->EmplaceKey(kDefaultEncryptionKey,
+                                      KeyDerivationParams::CreateForPbkdf2());
+  state.cryptographer->SelectDefaultEncryptionKey(default_encryption_key_name);
+
+  ASSERT_THAT(last_keystore_key_name, Ne(""));
+  ASSERT_THAT(default_encryption_key_name, Ne(""));
+  ASSERT_THAT(default_encryption_key_name, Ne(last_keystore_key_name));
+
+  NigoriSpecifics specifics = state.ToSpecificsProto();
+  EXPECT_THAT(specifics.passphrase_type(),
+              NigoriSpecifics::KEYSTORE_PASSPHRASE);
+  EXPECT_TRUE(specifics.keybag_is_frozen());
+  EXPECT_THAT(specifics.encryption_keybag().key_name(),
+              Eq(default_encryption_key_name));
+  EXPECT_TRUE(specifics.has_keystore_decryptor_token());
+  EXPECT_THAT(specifics.keystore_decryptor_token().key_name(),
+              Eq(last_keystore_key_name));
+  EXPECT_FALSE(specifics.has_custom_passphrase_time());
+  EXPECT_FALSE(specifics.has_custom_passphrase_key_derivation_method());
+  EXPECT_THAT(specifics.keystore_migration_time(), Eq(TimeToProtoTime(now)));
+}
+
+}  // namespace
+
+}  // namespace syncer