Store cached version of keystore cryptographer

For decryption and encryption of |keystore_decryptor_token| we need a
special version of cryptographer, which contains only keystore keys and
uses last keystore key as the default encryption key. To avoid heavy
creation of keystore cryptographer and simplify some future error
handling (crypto errors, which don't allow to create keystore
cryptographer), we now maintain its cached version inside NigoriState.

It's implemented as KeystoreKeysCryptographer, which wraps
CryptographerImpl and provides interface specific for encryption and
decryption of |keystore_decryptor_token|. Keystore keys now stored
inside KeystoreKeysCryptographer.

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

components/sync/BUILD.gn

@@ -314,6 +314,8 @@ jumbo_static_library("rest_of_sync") {
     "nigori/cryptographer_impl.h",
     "nigori/forwarding_model_type_processor.cc",
     "nigori/forwarding_model_type_processor.h",
+    "nigori/keystore_keys_cryptographer.cc",
+    "nigori/keystore_keys_cryptographer.h",
     "nigori/keystore_keys_handler.h",
     "nigori/nigori.cc",
     "nigori/nigori.h",

components/sync/nigori/keystore_keys_cryptographer.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/keystore_keys_cryptographer.h"
+
+#include <utility>
+
+#include "base/memory/ptr_util.h"
+#include "components/sync/nigori/cryptographer_impl.h"
+#include "components/sync/protocol/encryption.pb.h"
+#include "components/sync/protocol/nigori_specifics.pb.h"
+
+namespace syncer {
+
+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;
+}
+
+}  // namespace
+
+// static
+std::unique_ptr<KeystoreKeysCryptographer>
+KeystoreKeysCryptographer::CreateEmpty() {
+  return base::WrapUnique(new KeystoreKeysCryptographer(
+      CryptographerImpl::CreateEmpty(),
+      /*keystore_keys=*/std::vector<std::string>()));
+}
+
+// static
+std::unique_ptr<KeystoreKeysCryptographer>
+KeystoreKeysCryptographer::FromKeystoreKeys(
+    const std::vector<std::string>& keystore_keys) {
+  std::unique_ptr<CryptographerImpl> cryptographer =
+      CreateCryptographerFromKeystoreKeys(keystore_keys);
+  if (!cryptographer) {
+    return nullptr;
+  }
+  return base::WrapUnique(
+      new KeystoreKeysCryptographer(std::move(cryptographer), keystore_keys));
+}
+
+KeystoreKeysCryptographer::KeystoreKeysCryptographer(
+    std::unique_ptr<CryptographerImpl> cryptographer,
+    const std::vector<std::string>& keystore_keys)
+    : cryptographer_(std::move(cryptographer)), keystore_keys_(keystore_keys) {
+  DCHECK(cryptographer_);
+}
+
+KeystoreKeysCryptographer::~KeystoreKeysCryptographer() = default;
+
+bool KeystoreKeysCryptographer::IsEmpty() const {
+  return keystore_keys_.empty();
+}
+
+std::unique_ptr<KeystoreKeysCryptographer> KeystoreKeysCryptographer::Clone()
+    const {
+  return base::WrapUnique(new KeystoreKeysCryptographer(
+      cryptographer_->CloneImpl(), keystore_keys_));
+}
+
+std::unique_ptr<CryptographerImpl>
+KeystoreKeysCryptographer::ToCryptographerImpl() const {
+  return cryptographer_->CloneImpl();
+}
+
+bool KeystoreKeysCryptographer::EncryptKeystoreDecryptorToken(
+    const sync_pb::NigoriKey& keystore_decryptor_key,
+    sync_pb::EncryptedData* keystore_decryptor_token) const {
+  DCHECK(keystore_decryptor_token);
+  if (IsEmpty()) {
+    return false;
+  }
+  return cryptographer_->EncryptString(
+      keystore_decryptor_key.SerializeAsString(), keystore_decryptor_token);
+}
+
+bool KeystoreKeysCryptographer::DecryptKeystoreDecryptorToken(
+    const sync_pb::EncryptedData& keystore_decryptor_token,
+    sync_pb::NigoriKey* keystore_decryptor_key) const {
+  return cryptographer_->Decrypt(keystore_decryptor_token,
+                                 keystore_decryptor_key);
+}
+
+}  // namespace syncer

components/sync/nigori/keystore_keys_cryptographer.h

+// 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_KEYSTORE_KEYS_CRYPTOGRAPHER_H_
+#define COMPONENTS_SYNC_NIGORI_KEYSTORE_KEYS_CRYPTOGRAPHER_H_
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "base/macros.h"
+
+namespace sync_pb {
+
+class EncryptedData;
+class NigoriKey;
+
+}  // namespace sync_pb
+
+namespace syncer {
+
+class CryptographerImpl;
+
+// Wrapper of CryptographerImpl, which contains only keystore keys and uses the
+// last one as the default encryption key.
+class KeystoreKeysCryptographer {
+ public:
+  // Factory methods.
+  static std::unique_ptr<KeystoreKeysCryptographer> CreateEmpty();
+  // Returns null if crypto error occurs.
+  static std::unique_ptr<KeystoreKeysCryptographer> FromKeystoreKeys(
+      const std::vector<std::string>& keystore_keys);
+
+  ~KeystoreKeysCryptographer();
+
+  const std::vector<std::string>& keystore_keys() const {
+    return keystore_keys_;
+  }
+
+  bool IsEmpty() const;
+
+  std::unique_ptr<KeystoreKeysCryptographer> Clone() const;
+
+  // Returns CryptographerImpl, which contains all keystore keys and uses the
+  // last one as the default encryption key.
+  std::unique_ptr<CryptographerImpl> ToCryptographerImpl() const;
+
+  // Encrypts |keystore_decryptor_key| into |keystore_decryptor_token|.
+  // |keystore_decryptor_token| must be not null. Returns false if there is no
+  // keystore keys or crypto error occurs.
+  bool EncryptKeystoreDecryptorToken(
+      const sync_pb::NigoriKey& keystore_decryptor_key,
+      sync_pb::EncryptedData* keystore_decryptor_token) const;
+
+  // Decrypts |keystore_decryptor_token| into |keystore_decryptor_key|.
+  // |keystore_decryptor_key| must be not null. Returns false if can't decrypt
+  // or crypto error occurs.
+  bool DecryptKeystoreDecryptorToken(
+      const sync_pb::EncryptedData& keystore_decryptor_token,
+      sync_pb::NigoriKey* keystore_decryptor_key) const;
+
+ private:
+  KeystoreKeysCryptographer(std::unique_ptr<CryptographerImpl> cryptographer,
+                            const std::vector<std::string>& keystore_keys);
+
+  std::unique_ptr<CryptographerImpl> cryptographer_;
+  std::vector<std::string> keystore_keys_;
+
+  DISALLOW_COPY_AND_ASSIGN(KeystoreKeysCryptographer);
+};
+
+}  // namespace syncer
+
+#endif  // COMPONENTS_SYNC_NIGORI_KEYSTORE_KEYS_CRYPTOGRAPHER_H_

components/sync/nigori/nigori_state.cc

@@ -9,6 +9,7 @@
 #include "components/sync/base/time.h"
 #include "components/sync/engine/sync_encryption_handler.h"
 #include "components/sync/nigori/cryptographer_impl.h"
+#include "components/sync/nigori/keystore_keys_cryptographer.h"
 #include "components/sync/protocol/nigori_local_data.pb.h"
 
 namespace syncer {
@@ -116,54 +117,8 @@ void UpdateSpecificsFromKeyDerivationParams(
   }
 }
 
-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::CreateFromLocalProto(
     const sync_pb::NigoriModel& proto) {
@@ -187,9 +142,20 @@ NigoriState NigoriState::CreateFromLocalProto(
             proto.custom_passphrase_key_derivation_params());
   }
   state.encrypt_everything = proto.encrypt_everything();
-  for (int i = 0; i < proto.keystore_key_size(); ++i) {
-    state.keystore_keys.push_back(proto.keystore_key(i));
+
+  std::vector<std::string> keystore_keys;
+  for (const std::string& keystore_key : proto.keystore_key()) {
+    keystore_keys.push_back(keystore_key);
   }
+  state.keystore_keys_cryptographer =
+      KeystoreKeysCryptographer::FromKeystoreKeys(keystore_keys);
+  if (!state.keystore_keys_cryptographer) {
+    // Crypto error occurs, create empty |keystore_keys_cryptographer|.
+    // Effectively it resets keystore keys.
+    state.keystore_keys_cryptographer =
+        KeystoreKeysCryptographer::CreateEmpty();
+  }
+
   if (proto.has_pending_keystore_decryptor_token()) {
     state.pending_keystore_decryptor_token =
         proto.pending_keystore_decryptor_token();
@@ -200,7 +166,8 @@ NigoriState NigoriState::CreateFromLocalProto(
 NigoriState::NigoriState()
     : cryptographer(CryptographerImpl::CreateEmpty()),
       passphrase_type(kInitialPassphraseType),
-      encrypt_everything(kInitialEncryptEverything) {}
+      encrypt_everything(kInitialEncryptEverything),
+      keystore_keys_cryptographer(KeystoreKeysCryptographer::CreateEmpty()) {}
 
 NigoriState::NigoriState(NigoriState&& other) = default;
 
@@ -214,6 +181,8 @@ sync_pb::NigoriModel NigoriState::ToLocalProto() const {
   if (pending_keys.has_value()) {
     *proto.mutable_pending_keys() = *pending_keys;
   }
+  const std::vector<std::string>& keystore_keys =
+      keystore_keys_cryptographer->keystore_keys();
   if (!keystore_keys.empty()) {
     proto.set_current_keystore_key_name(
         ComputePbkdf2KeyName(keystore_keys.back()));
@@ -284,10 +253,12 @@ sync_pb::NigoriSpecifics NigoriState::ToSpecificsProto() const {
       *specifics.mutable_keystore_decryptor_token() =
           *pending_keystore_decryptor_token;
     } else {
-      DCHECK(!keystore_keys.empty());
-      EncryptKeystoreDecryptorToken(
-          *cryptographer, specifics.mutable_keystore_decryptor_token(),
-          keystore_keys);
+      // TODO(crbug.com/922900): error handling (crypto errors, which could
+      // cause empty |keystore_keys_cryptographer| or can occur during
+      // encryption).
+      keystore_keys_cryptographer->EncryptKeystoreDecryptorToken(
+          cryptographer->ExportDefaultKey(),
+          specifics.mutable_keystore_decryptor_token());
     }
   }
   if (!keystore_migration_time.is_null()) {
@@ -313,7 +284,7 @@ NigoriState NigoriState::Clone() const {
   result.custom_passphrase_key_derivation_params =
       custom_passphrase_key_derivation_params;
   result.encrypt_everything = encrypt_everything;
-  result.keystore_keys = keystore_keys;
+  result.keystore_keys_cryptographer = keystore_keys_cryptographer->Clone();
   result.pending_keystore_decryptor_token = pending_keystore_decryptor_token;
   return result;
 }

components/sync/nigori/nigori_state.h

@@ -23,13 +23,7 @@ class NigoriModel;
 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);
+class KeystoreKeysCryptographer;
 
 struct NigoriState {
   static constexpr sync_pb::NigoriSpecifics::PassphraseType
@@ -77,10 +71,10 @@ struct NigoriState {
   base::Optional<KeyDerivationParams> custom_passphrase_key_derivation_params;
   bool encrypt_everything;
 
-  // 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. Must be encrypted with OSCrypt before persisting.
-  std::vector<std::string> keystore_keys;
+  // Contains keystore keys. Uses last keystore key as encryption key. Must be
+  // not null. Serialized as keystore keys, which must be encrypted with
+  // OSCrypt before persisting.
+  std::unique_ptr<KeystoreKeysCryptographer> keystore_keys_cryptographer;
 
   // Represents |keystore_decryptor_token| from NigoriSpecifics in case it
   // can't be decrypted right after remote update arrival due to lack of

components/sync/nigori/nigori_state_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "components/sync/base/time.h"
 #include "components/sync/nigori/cryptographer_impl.h"
+#include "components/sync/nigori/keystore_keys_cryptographer.h"
 #include "components/sync/nigori/nigori.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -58,7 +59,9 @@ TEST(NigoriStateTest, ShouldConvertKeystoreStateToSpecifics) {
   const std::string kDefaultEncryptionKey = "defaultkey";
 
   NigoriState state;
-  state.keystore_keys = {kKeystoreKey1, kKeystoreKey2};
+  state.keystore_keys_cryptographer =
+      KeystoreKeysCryptographer::FromKeystoreKeys(
+          {kKeystoreKey1, kKeystoreKey2});
   state.passphrase_type = NigoriSpecifics::KEYSTORE_PASSPHRASE;
   state.keystore_migration_time = now;
   state.cryptographer = CryptographerImpl::CreateEmpty();

components/sync/nigori/nigori_sync_bridge_impl.cc

@@ -17,6 +17,7 @@
 #include "components/sync/base/time.h"
 #include "components/sync/engine/sync_engine_switches.h"
 #include "components/sync/model/entity_data.h"
+#include "components/sync/nigori/keystore_keys_cryptographer.h"
 #include "components/sync/nigori/nigori.h"
 #include "components/sync/nigori/nigori_storage.h"
 #include "components/sync/nigori/pending_local_nigori_commit.h"
@@ -32,10 +33,10 @@ using sync_pb::NigoriSpecifics;
 
 const char kNigoriNonUniqueName[] = "Nigori";
 
-// Creates keystore Nigori specifics given |keystore_keys|.
+// Creates keystore Nigori specifics given |keystore_keys_cryptographer|.
 // Returns NigoriSpecifics that contain:
 // 1. passphrase_type = KEYSTORE_PASSPHRASE.
-// 2. encryption_keybag contains all |keystore_keys| and encrypted with the
+// 2. encryption_keybag contains all keystore keys and encrypted with the
 // latest keystore key.
 // 3. keystore_decryptor_token contains latest keystore key encrypted with
 // itself.
@@ -43,14 +44,14 @@ const char kNigoriNonUniqueName[] = "Nigori";
 // 5. keystore_migration_time is current time.
 // 6. Other fields are default.
 NigoriSpecifics MakeDefaultKeystoreNigori(
-    const std::vector<std::string>& keystore_keys) {
-  DCHECK(!keystore_keys.empty());
+    const KeystoreKeysCryptographer& keystore_keys_cryptographer) {
+  DCHECK(!keystore_keys_cryptographer.IsEmpty());
 
   NigoriState state;
-  state.keystore_keys = keystore_keys;
   state.passphrase_type = NigoriSpecifics::KEYSTORE_PASSPHRASE;
   state.keystore_migration_time = base::Time::Now();
-  state.cryptographer = CreateCryptographerFromKeystoreKeys(keystore_keys);
+  state.cryptographer = keystore_keys_cryptographer.ToCryptographerImpl();
+  state.keystore_keys_cryptographer = keystore_keys_cryptographer.Clone();
 
   return state.ToSpecificsProto();
 }
@@ -488,7 +489,7 @@ bool NigoriSyncBridgeImpl::Init() {
     // nigori keybag's encryption key. Otherwise we're simply missing the
     // keystore key.
     UMA_HISTOGRAM_BOOLEAN("Sync.KeystoreDecryptionFailed",
-                          !state_.keystore_keys.empty());
+                          !state_.keystore_keys_cryptographer->IsEmpty());
   }
   return true;
 }
@@ -600,10 +601,12 @@ KeystoreKeysHandler* NigoriSyncBridgeImpl::GetKeystoreKeysHandler() {
 
 std::string NigoriSyncBridgeImpl::GetLastKeystoreKey() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (state_.keystore_keys.empty()) {
+  const std::vector<std::string> keystore_keys =
+      state_.keystore_keys_cryptographer->keystore_keys();
+  if (keystore_keys.empty()) {
     return std::string();
   }
-  return state_.keystore_keys.back();
+  return keystore_keys.back();
 }
 
 bool NigoriSyncBridgeImpl::NeedKeystoreKey() const {
@@ -613,7 +616,7 @@ bool NigoriSyncBridgeImpl::NeedKeystoreKey() const {
   // server responsibility to send updated keystore keys. |keystore_keys_| is
   // expected to be non-empty before MergeSyncData() call, regardless of
   // passphrase type.
-  return state_.keystore_keys.empty() ||
+  return state_.keystore_keys_cryptographer->IsEmpty() ||
          state_.pending_keystore_decryptor_token.has_value();
 }
 
@@ -624,13 +627,21 @@ bool NigoriSyncBridgeImpl::SetKeystoreKeys(
     return false;
   }
 
-  state_.keystore_keys.resize(keys.size());
+  std::vector<std::string> encoded_keystore_keys(keys.size());
   for (size_t i = 0; i < keys.size(); ++i) {
     // We need to apply base64 encoding before using the keys to provide
     // backward compatibility with non-USS implementation. It's actually needed
     // only for the keys persisting, but was applied before passing keys to
     // cryptographer, so we have to do the same.
-    base::Base64Encode(keys[i], &state_.keystore_keys[i]);
+    base::Base64Encode(keys[i], &encoded_keystore_keys[i]);
+  }
+
+  state_.keystore_keys_cryptographer =
+      KeystoreKeysCryptographer::FromKeystoreKeys(encoded_keystore_keys);
+  if (!state_.keystore_keys_cryptographer) {
+    state_.keystore_keys_cryptographer =
+        KeystoreKeysCryptographer::CreateEmpty();
+    return false;
   }
 
   if (state_.pending_keystore_decryptor_token.has_value()) {
@@ -679,7 +690,8 @@ base::Optional<ModelError> NigoriSyncBridgeImpl::MergeSyncData(
   // Ensure we have |keystore_keys| during the initial download, requested to
   // the server as per NeedKeystoreKey(), and required for initializing the
   // default keystore Nigori.
-  if (state_.keystore_keys.empty()) {
+  DCHECK(state_.keystore_keys_cryptographer);
+  if (state_.keystore_keys_cryptographer->IsEmpty()) {
     // TODO(crbug.com/922900): try to relax this requirement for Nigori
     // initialization as well. Keystore keys might not arrive, for example, due
     // to throttling.
@@ -690,11 +702,9 @@ base::Optional<ModelError> NigoriSyncBridgeImpl::MergeSyncData(
   // keystore Nigori.
   // TODO(crbug.com/922900): Adopt QueuePendingLocalCommit().
   NigoriSpecifics initialized_specifics =
-      MakeDefaultKeystoreNigori(state_.keystore_keys);
-  // In rare cases the crypto operations may fail.
-  if (!IsValidNigoriSpecifics(initialized_specifics)) {
-    return ModelError(FROM_HERE, "Failed to initialize keystore Nigori.");
-  }
+      MakeDefaultKeystoreNigori(*state_.keystore_keys_cryptographer);
+  DCHECK(IsValidNigoriSpecifics(initialized_specifics));
+
   *data->specifics.mutable_nigori() = initialized_specifics;
   processor_->Put(std::make_unique<EntityData>(std::move(*data)));
   return UpdateLocalState(initialized_specifics);
@@ -834,17 +844,10 @@ NigoriSyncBridgeImpl::UpdateCryptographerFromKeystoreNigori(
   DCHECK(!keystore_decryptor_token.blob().empty());
 
   // Decryption of |keystore_decryptor_token|.
-  std::unique_ptr<Cryptographer> keystore_cryptographer =
-      CreateCryptographerFromKeystoreKeys(state_.keystore_keys);
-  if (!keystore_cryptographer) {
-    return ModelError(FROM_HERE,
-                      "Failed to create cryptographer from keystore keys.");
-  }
-
   NigoriKeyBag keystore_decryptor_key_bag = NigoriKeyBag::CreateEmpty();
   sync_pb::NigoriKey keystore_decryptor_key;
-  if (keystore_cryptographer->Decrypt(keystore_decryptor_token,
-                                      &keystore_decryptor_key)) {
+  if (state_.keystore_keys_cryptographer->DecryptKeystoreDecryptorToken(
+          keystore_decryptor_token, &keystore_decryptor_key)) {
     keystore_decryptor_key_bag.AddKeyFromProto(keystore_decryptor_key);
     state_.pending_keystore_decryptor_token.reset();
   } else {
@@ -982,7 +985,7 @@ void NigoriSyncBridgeImpl::ApplyDisableSyncChanges() {
   // |explicit_passphrase_key_| will become not working, once we clean up
   // storing explicit passphrase key in prefs, we need to find better solution.
   storage_->ClearData();
-  state_.keystore_keys.clear();
+  state_.keystore_keys_cryptographer = KeystoreKeysCryptographer::CreateEmpty();
   state_.cryptographer = CryptographerImpl::CreateEmpty();
   state_.pending_keys.reset();
   state_.pending_keystore_decryptor_token.reset();