Introduce new sync Cryptographer implementation

The new class is a thin layer on top of NigoriKeyBag and introduces the
notion of a default encryption key, central in the Cryptographer API.

It has several advantages compared to the legacy DirectoryCryptographer:
1. Very small API, which in particular excludes the historic quirks and
   design decisions at the sync protocol level and in terms of storage
   in prefs (i.e. bootstrap token).

2. Separates the selection of the default encryption key to an explicit
   function call, which is also the only function that changes the
   default key, which is a privacy-sensitive operation.

3. Does not have the notion of pending keys, which we intend to remove
   from the Cryptographer API, as per existing TODOs.

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

components/sync/BUILD.gn

@@ -311,6 +311,8 @@ jumbo_static_library("rest_of_sync") {
     "model_impl/syncable_service_based_bridge.h",
     "nigori/cryptographer.cc",
     "nigori/cryptographer.h",
+    "nigori/cryptographer_impl.cc",
+    "nigori/cryptographer_impl.h",
     "nigori/forwarding_model_type_processor.cc",
     "nigori/forwarding_model_type_processor.h",
     "nigori/keystore_keys_handler.h",
@@ -642,6 +644,7 @@ source_set("unit_tests") {
     "model_impl/model_type_store_impl_unittest.cc",
     "model_impl/processor_entity_unittest.cc",
     "model_impl/syncable_service_based_bridge_unittest.cc",
+    "nigori/cryptographer_impl_unittest.cc",
     "nigori/nigori_key_bag_unittest.cc",
     "nigori/nigori_model_type_processor_unittest.cc",
     "nigori/nigori_storage_impl_unittest.cc",

components/sync/nigori/cryptographer_impl.cc

@@ -0,0 +1,128 @@
+// 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/cryptographer_impl.h"
+
+#include <utility>
+
+#include "base/logging.h"
+#include "base/memory/ptr_util.h"
+
+namespace syncer {
+
+// static
+std::unique_ptr<CryptographerImpl> CryptographerImpl::CreateEmpty() {
+  return base::WrapUnique(
+      new CryptographerImpl(NigoriKeyBag::CreateEmpty(),
+                            /*default_encryption_key_name=*/std::string()));
+}
+
+// static
+std::unique_ptr<CryptographerImpl> CryptographerImpl::FromSingleKeyForTesting(
+    const std::string& passphrase,
+    const KeyDerivationParams& derivation_params) {
+  std::unique_ptr<CryptographerImpl> cryptographer = CreateEmpty();
+  std::string key_name =
+      cryptographer->EmplaceKey(passphrase, derivation_params);
+  cryptographer->SelectDefaultEncryptionKey(key_name);
+  return cryptographer;
+}
+
+// static
+std::unique_ptr<CryptographerImpl> CryptographerImpl::FromProto(
+    const sync_pb::CryptographerData& proto) {
+  NigoriKeyBag key_bag = NigoriKeyBag::CreateFromProto(proto.key_bag());
+  std::string default_encryption_key_name = proto.default_key_name();
+  if (!default_encryption_key_name.empty() &&
+      !key_bag.HasKey(default_encryption_key_name)) {
+    // A default key name was specified but not present among keys.
+    return nullptr;
+  }
+
+  return base::WrapUnique(new CryptographerImpl(
+      std::move(key_bag), std::move(default_encryption_key_name)));
+}
+
+CryptographerImpl::CryptographerImpl(NigoriKeyBag key_bag,
+                                     std::string default_encryption_key_name)
+    : key_bag_(std::move(key_bag)),
+      default_encryption_key_name_(std::move(default_encryption_key_name)) {
+  DCHECK(default_encryption_key_name_.empty() ||
+         key_bag_.HasKey(default_encryption_key_name_));
+}
+
+CryptographerImpl::~CryptographerImpl() = default;
+
+sync_pb::CryptographerData CryptographerImpl::ToProto() const {
+  sync_pb::CryptographerData proto;
+  *proto.mutable_key_bag() = key_bag_.ToProto();
+  proto.set_default_key_name(default_encryption_key_name_);
+  return proto;
+}
+
+std::string CryptographerImpl::EmplaceKey(
+    const std::string& passphrase,
+    const KeyDerivationParams& derivation_params) {
+  return key_bag_.AddKey(
+      Nigori::CreateByDerivation(derivation_params, passphrase));
+}
+
+void CryptographerImpl::EmplaceKeysFrom(const NigoriKeyBag& key_bag) {
+  key_bag_.AddAllUnknownKeysFrom(key_bag);
+}
+
+void CryptographerImpl::SelectDefaultEncryptionKey(
+    const std::string& key_name) {
+  DCHECK(!key_name.empty());
+  DCHECK(key_bag_.HasKey(key_name));
+  default_encryption_key_name_ = key_name;
+}
+
+void CryptographerImpl::ClearDefaultEncryptionKey() {
+  default_encryption_key_name_.clear();
+}
+
+std::unique_ptr<Cryptographer> CryptographerImpl::Clone() const {
+  return base::WrapUnique(
+      new CryptographerImpl(key_bag_.Clone(), default_encryption_key_name_));
+}
+
+bool CryptographerImpl::CanEncrypt() const {
+  return !default_encryption_key_name_.empty();
+}
+
+bool CryptographerImpl::CanDecrypt(
+    const sync_pb::EncryptedData& encrypted) const {
+  return key_bag_.HasKey(encrypted.key_name());
+}
+
+std::string CryptographerImpl::GetDefaultEncryptionKeyName() const {
+  return default_encryption_key_name_;
+}
+
+bool CryptographerImpl::EncryptString(const std::string& decrypted,
+                                      sync_pb::EncryptedData* encrypted) const {
+  DCHECK(encrypted);
+  encrypted->Clear();
+
+  if (!CanEncrypt()) {
+    return false;
+  }
+
+  return key_bag_.EncryptWithKey(default_encryption_key_name_, decrypted,
+                                 encrypted);
+}
+
+bool CryptographerImpl::DecryptToString(const sync_pb::EncryptedData& encrypted,
+                                        std::string* decrypted) const {
+  DCHECK(decrypted);
+  return key_bag_.Decrypt(encrypted, decrypted);
+}
+
+bool CryptographerImpl::has_pending_keys() const {
+  // TODO(crbug.com/967417): Move this hack away.
+  return !CanEncrypt() && key_bag_.size() != 0;
+}
+
+}  // namespace syncer

components/sync/nigori/cryptographer_impl.h

@@ -0,0 +1,96 @@
+// 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.
+
+#ifndef COMPONENTS_SYNC_NIGORI_CRYPTOGRAPHER_IMPL_H_
+#define COMPONENTS_SYNC_NIGORI_CRYPTOGRAPHER_IMPL_H_
+
+#include <memory>
+#include <string>
+
+#include "base/macros.h"
+#include "components/sync/nigori/cryptographer.h"
+#include "components/sync/nigori/nigori.h"
+#include "components/sync/nigori/nigori_key_bag.h"
+#include "components/sync/protocol/nigori_local_data.pb.h"
+
+namespace sync_pb {
+class CryptographerData;
+}  // namespace sync_pb
+
+namespace syncer {
+
+// This class manages the Nigori objects used to encrypt and decrypt sensitive
+// sync data (eg. passwords). Each Nigori object knows how to handle data
+// protected with a particular encryption key.
+//
+// The implementation consists of a keybag representing all known keys and
+// optionally the notion of a default encryption key which, if it exists, is
+// present in the keybag.
+class CryptographerImpl : public Cryptographer {
+ public:
+  // Factory methods.
+  static std::unique_ptr<CryptographerImpl> CreateEmpty();
+  static std::unique_ptr<CryptographerImpl> FromSingleKeyForTesting(
+      const std::string& passphrase,
+      const KeyDerivationParams& derivation_params =
+          KeyDerivationParams::CreateForPbkdf2());
+  // Returns null in case of error (e.g. default key not present in keybag).
+  static std::unique_ptr<CryptographerImpl> FromProto(
+      const sync_pb::CryptographerData& proto);
+
+  ~CryptographerImpl() override;
+
+  // Serialization.
+  sync_pb::CryptographerData ToProto() const;
+
+  // Creates and registers a new key after deriving Nigori keys. Returns the
+  // name of the key, or an empty string in case of error. Note that emplacing
+  // an already-known key is not considered an error (just a no-op).
+  //
+  // Does NOT set or change the default encryption key.
+  std::string EmplaceKey(const std::string& passphrase,
+                         const KeyDerivationParams& derivation_params);
+
+  // Adds all keys from |key_bag| that weren't previously known.
+  //
+  // Does NOT set or change the default encryption key.
+  void EmplaceKeysFrom(const NigoriKeyBag& key_bag);
+
+  // Sets or changes the default encryption key, which causes CanEncrypt() to
+  // return true. |key_name| must not be empty and must represent a known key.
+  void SelectDefaultEncryptionKey(const std::string& key_name);
+
+  // Clears the default encryption key, which causes CanEncrypt() to return
+  // false.
+  void ClearDefaultEncryptionKey();
+
+  // Cryptographer overrides.
+  std::unique_ptr<Cryptographer> Clone() const override;
+  bool CanEncrypt() const override;
+  bool CanDecrypt(const sync_pb::EncryptedData& encrypted) const override;
+  std::string GetDefaultEncryptionKeyName() const override;
+  bool EncryptString(const std::string& decrypted,
+                     sync_pb::EncryptedData* encrypted) const override;
+  bool DecryptToString(const sync_pb::EncryptedData& encrypted,
+                       std::string* decrypted) const override;
+  bool has_pending_keys() const override;
+
+ private:
+  CryptographerImpl(NigoriKeyBag key_bag,
+                    std::string default_encryption_key_name);
+
+  // The actual keys we know about.
+  NigoriKeyBag key_bag_;
+
+  // The key name associated with the default encryption key. If non-empty, it
+  // must correspond to a key within |key_bag_|. May be empty even if |key_bag_|
+  // is not.
+  std::string default_encryption_key_name_;
+
+  DISALLOW_ASSIGN(CryptographerImpl);
+};
+
+}  // namespace syncer
+
+#endif  // COMPONENTS_SYNC_NIGORI_CRYPTOGRAPHER_IMPL_H_

components/sync/nigori/cryptographer_impl_unittest.cc

@@ -0,0 +1,135 @@
+// 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/cryptographer_impl.h"
+
+#include "components/sync/protocol/nigori_local_data.pb.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace syncer {
+namespace {
+
+using testing::Eq;
+using testing::Ne;
+using testing::NotNull;
+
+}  // namespace
+
+TEST(CryptographerImplTest, ShouldCreateEmpty) {
+  std::unique_ptr<CryptographerImpl> cryptographer =
+      CryptographerImpl::CreateEmpty();
+  ASSERT_THAT(cryptographer, NotNull());
+
+  EXPECT_FALSE(cryptographer->CanEncrypt());
+
+  sync_pb::EncryptedData encrypted;
+  encrypted.set_key_name("foo");
+  encrypted.set_blob("bar");
+
+  EXPECT_FALSE(cryptographer->CanDecrypt(encrypted));
+
+  std::string output;
+  EXPECT_FALSE(cryptographer->DecryptToString(encrypted, &output));
+}
+
+TEST(CryptographerImplTest, ShouldEmplaceKey) {
+  std::unique_ptr<CryptographerImpl> cryptographer =
+      CryptographerImpl::CreateEmpty();
+  ASSERT_THAT(cryptographer, NotNull());
+  ASSERT_FALSE(cryptographer->CanEncrypt());
+
+  const std::string key_name = cryptographer->EmplaceKey(
+      "password1", KeyDerivationParams::CreateForPbkdf2());
+  EXPECT_THAT(key_name, Ne(std::string()));
+
+  sync_pb::EncryptedData encrypted;
+  encrypted.set_key_name(key_name);
+  encrypted.set_blob("fakeblob");
+
+  EXPECT_TRUE(cryptographer->CanDecrypt(encrypted));
+  EXPECT_FALSE(cryptographer->CanEncrypt());
+}
+
+TEST(CryptographerImplTest, ShouldEmplaceExistingKey) {
+  std::unique_ptr<CryptographerImpl> cryptographer =
+      CryptographerImpl::CreateEmpty();
+  ASSERT_THAT(cryptographer, NotNull());
+
+  const std::string key_name = cryptographer->EmplaceKey(
+      "password1", KeyDerivationParams::CreateForPbkdf2());
+  ASSERT_THAT(key_name, Ne(std::string()));
+  EXPECT_THAT(cryptographer->EmplaceKey("password1",
+                                        KeyDerivationParams::CreateForPbkdf2()),
+              Eq(key_name));
+}
+
+TEST(CryptographerImplTest, ShouldEmplaceSecondKey) {
+  std::unique_ptr<CryptographerImpl> cryptographer =
+      CryptographerImpl::CreateEmpty();
+  ASSERT_THAT(cryptographer, NotNull());
+
+  const std::string key_name1 = cryptographer->EmplaceKey(
+      "password1", KeyDerivationParams::CreateForPbkdf2());
+  const std::string key_name2 = cryptographer->EmplaceKey(
+      "password2", KeyDerivationParams::CreateForPbkdf2());
+
+  EXPECT_THAT(key_name1, Ne(std::string()));
+  EXPECT_THAT(key_name2, Ne(std::string()));
+  EXPECT_THAT(key_name1, Ne(key_name2));
+}
+
+TEST(CryptographerImplTest, ShouldSelectDefaultEncryptionKey) {
+  std::unique_ptr<CryptographerImpl> cryptographer =
+      CryptographerImpl::CreateEmpty();
+  ASSERT_THAT(cryptographer, NotNull());
+  ASSERT_FALSE(cryptographer->CanEncrypt());
+
+  const std::string key_name = cryptographer->EmplaceKey(
+      "password1", KeyDerivationParams::CreateForPbkdf2());
+  ASSERT_THAT(key_name, Ne(std::string()));
+
+  cryptographer->SelectDefaultEncryptionKey(key_name);
+  ASSERT_TRUE(cryptographer->CanEncrypt());
+
+  sync_pb::EncryptedData encrypted;
+  EXPECT_TRUE(cryptographer->EncryptString("foo", &encrypted));
+  EXPECT_THAT(encrypted.key_name(), Eq(key_name));
+}
+
+TEST(CryptographerImplTest, ShouldSerializeToAndFromProto) {
+  const std::string kText1 = "foo";
+  const std::string kText2 = "bar";
+
+  std::unique_ptr<CryptographerImpl> original_cryptographer =
+      CryptographerImpl::CreateEmpty();
+  ASSERT_THAT(original_cryptographer, NotNull());
+
+  const std::string key_name1 = original_cryptographer->EmplaceKey(
+      "password1", KeyDerivationParams::CreateForPbkdf2());
+  const std::string key_name2 = original_cryptographer->EmplaceKey(
+      "password2", KeyDerivationParams::CreateForPbkdf2());
+
+  original_cryptographer->SelectDefaultEncryptionKey(key_name1);
+  sync_pb::EncryptedData encrypted1;
+  EXPECT_TRUE(original_cryptographer->EncryptString(kText1, &encrypted1));
+
+  original_cryptographer->SelectDefaultEncryptionKey(key_name2);
+  sync_pb::EncryptedData encrypted2;
+  EXPECT_TRUE(original_cryptographer->EncryptString(kText2, &encrypted2));
+
+  // Restore a new cryptographer from proto.
+  std::unique_ptr<CryptographerImpl> restored_cryptographer =
+      CryptographerImpl::FromProto(original_cryptographer->ToProto());
+  ASSERT_THAT(restored_cryptographer, NotNull());
+  EXPECT_TRUE(restored_cryptographer->CanEncrypt());
+
+  std::string decrypted;
+  EXPECT_TRUE(restored_cryptographer->DecryptToString(encrypted1, &decrypted));
+  EXPECT_THAT(decrypted, Eq(kText1));
+  EXPECT_TRUE(restored_cryptographer->DecryptToString(encrypted2, &decrypted));
+  EXPECT_THAT(decrypted, Eq(kText2));
+}
+
+}  // namespace syncer