[Sync] Split encryption state and logic out of PSS and SBHI.

components/sync/driver/sync_service_crypto.h

Index: components/sync/driver/sync_service_crypto.h
diff --git a/components/sync/driver/sync_service_crypto.h b/components/sync/driver/sync_service_crypto.h
new file mode 100644
index 0000000000000000000000000000000000000000..1d73b07463d3273ffe101806d7ca2da9cb2987b3
--- /dev/null
+++ b/components/sync/driver/sync_service_crypto.h
@@ -0,0 +1,180 @@
+// Copyright 2017 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_DRIVER_SYNC_SERVICE_CRYPTO_H_
+#define COMPONENTS_SYNC_DRIVER_SYNC_SERVICE_CRYPTO_H_
+
+#include <memory>
+#include <string>
+
+#include "base/callback.h"
+#include "base/memory/weak_ptr.h"
+#include "components/sync/base/model_type.h"
+#include "components/sync/engine/sync_encryption_handler.h"
+#include "components/sync/engine/sync_engine.h"
+
+namespace syncer {
+
+class DataTypeManager;
+class SyncPrefs;
+
+// This class functions as an extension of SyncServiceBase that handles things

[skym, 2017/01/31 01:58:54]

Extension is an odd word that I think you're trying to convey certain things,
but they are not conveyed, the reader doesn't know what you mean. It seems
you're explicitly trying to avoid having a dependency on the PSS/SSB, would it
be worth noting that?

[maxbogue, 2017/01/31 07:51:10]

Good point. Changed the wording to be "a mostly independent component of
SyncServiceBase" (since it gets some of the methods injected). Hopefully that's
better...

+// related to encryption, including holding lots of state and handling
+// communications with the sync thread.
+class SyncServiceCrypto : public SyncEncryptionHandler::Observer {
+ public:
+  SyncServiceCrypto(const base::Closure& notify_observers,
+                    const base::Callback<ModelTypeSet()>& get_preferred_types,
+                    SyncPrefs* sync_prefs);
+  ~SyncServiceCrypto() override;
+
+  // See the SyncService header.

[skym, 2017/01/31 01:58:54]

Usually we use interfaces when multiple classes implement the same exact
methods. Though I don't know if it'd simplify things here. Exposing
SyncServiceCrypto through SyncService would definitely solve this. It's kind of
nice that external to sync can always interact with us through SS, but at the
same time, does that really make sense if it grows to be monolithic?

[maxbogue, 2017/01/31 07:51:10]

I think it would be good to do, but quite frankly I don't have time.
SyncServiceCrypto could just be an interface masking everything related to
crypto on the UI thread and SyncServiceCryptoImpl could go in driver_impl/ and
do the stuff. The only challenging part really would be fixing the tests that
rely on calling the SyncEncryptionHandler::Observer methods directly.

Sadly the SyncEncryptionHandler interface itself doesn't completely align, and
doesn't look like it would really make sense to.

+  base::Time GetExplicitPassphraseTime() const;
+  bool IsUsingSecondaryPassphrase() const;
+  void EnableEncryptEverything();
+  bool IsEncryptEverythingEnabled() const;
+  void SetEncryptionPassphrase(const std::string& passphrase, bool is_explicit);
+  bool SetDecryptionPassphrase(const std::string& passphrase);
+
+  // Returns the actual passphrase type being used for encryption.
+  PassphraseType GetPassphraseType() const;
+
+  // Returns true if encrypting all the sync data is allowed. If this method
+  // returns false, EnableEncryptEverything() should not be called.
+  bool IsEncryptEverythingAllowed() const;
+
+  // Sets whether encrypting all the sync data is allowed or not.
+  void SetEncryptEverythingAllowed(bool allowed);
+
+  // Returns the current set of encrypted data types.
+  ModelTypeSet GetEncryptedDataTypes() const;
+
+  // SyncEncryptionHandler::Observer implementation.
+  void OnPassphraseRequired(
+      PassphraseRequiredReason reason,
+      const sync_pb::EncryptedData& pending_keys) override;
+  void OnPassphraseAccepted() override;
+  void OnBootstrapTokenUpdated(const std::string& bootstrap_token,
+                               BootstrapTokenType type) override;
+  void OnEncryptedTypesChanged(ModelTypeSet encrypted_types,
+                               bool encrypt_everything) override;
+  void OnEncryptionComplete() override;
+  void OnCryptographerStateChanged(Cryptographer* cryptographer) override;
+  void OnPassphraseTypeChanged(PassphraseType type,
+                               base::Time passphrase_time) override;
+  void OnLocalSetPassphraseEncryption(
+      const SyncEncryptionHandler::NigoriState& nigori_state) override;
+
+  // Calls data type manager to start catch up configure.
+  void BeginConfigureCatchUpBeforeClear();
+
+  // Used to provide the engine and DTM when the engine is initialized.
+  void SetSyncEngine(SyncEngine* engine) { engine_ = engine; }
+  void SetDataTypeManager(DataTypeManager* dtm) { data_type_manager_ = dtm; }
+
+  // Creates a proxy observer object that will post calls to this thread.
+  std::unique_ptr<SyncEncryptionHandler::Observer> GetEncryptionObserverProxy();
+
+  // Takes the previously saved nigori state; null if there isn't any.
+  std::unique_ptr<SyncEncryptionHandler::NigoriState> TakeSavedNigoriState();
+
+  // Sets the cached passphrase.
+  void CachePassphrase(const std::string& passphrase) {
+    cached_passphrase_ = passphrase;
+  }
+
+  // During initial signin, ProfileSyncService caches the user's signin
+  // passphrase so it can be used to encrypt/decrypt data after sync starts up.
+  // This routine is invoked once the engine has started up to use the
+  // cached passphrase and clear it out when it is done.
+  void ConsumeCachedPassphraseIfPossible();
+
+  PassphraseRequiredReason passphrase_required_reason() const {
+    return passphrase_required_reason_;
+  }
+  const std::string& cached_passphrase() { return cached_passphrase_; }
+  bool encryption_pending() { return encryption_pending_; }
+
+ private:
+  // Checks if |passphrase| can be used to decrypt the cryptographer's pending
+  // keys that were cached during NotifyPassphraseRequired. Returns true if
+  // decryption was successful. Returns false otherwise. Must be called with a
+  // non-empty pending keys cache.
+  bool CheckPassphraseAgainstCachedPendingKeys(
+      const std::string& passphrase) const;
+
+  // Calls SyncServiceBase::NotifyObservers(). Never null.
+  const base::Closure notify_observers_;
+
+  // Calls SyncService::GetPreferredDataTypes(). Never null.
+  const base::Callback<ModelTypeSet()> get_preferred_types_;
+
+  // A pointer to the sync prefs. Never null and guaranteed to outlive us.
+  SyncPrefs* const sync_prefs_;
+
+  // These are only not-null when the engine is initialized.
+  SyncEngine* engine_ = nullptr;
+  DataTypeManager* data_type_manager_ = nullptr;
+
+  // Was the last SYNC_PASSPHRASE_REQUIRED notification sent because it
+  // was required for encryption, decryption with a cached passphrase, or
+  // because a new passphrase is required?
+  PassphraseRequiredReason passphrase_required_reason_ =
+      REASON_PASSPHRASE_NOT_REQUIRED;
+
+  // Sometimes we need to temporarily hold on to a passphrase because we don't
+  // yet have a engine to send it to.  This happens during initialization as
+  // we don't StartUp until we have a valid token, which happens after valid
+  // credentials were provided.
+  std::string cached_passphrase_;
+
+  // The current set of encrypted types.  Always a superset of
+  // Cryptographer::SensitiveTypes().
+  ModelTypeSet encrypted_types_ = SyncEncryptionHandler::SensitiveTypes();
+
+  // Whether encrypting everything is allowed.
+  bool encrypt_everything_allowed_ = true;
+
+  // Whether we want to encrypt everything.
+  bool encrypt_everything_ = false;
+
+  // Whether we're waiting for an attempt to encryption all sync data to
+  // complete. We track this at this layer in order to allow the user to cancel
+  // if they e.g. don't remember their explicit passphrase.
+  bool encryption_pending_ = false;
+
+  // Nigori state after user switching to custom passphrase, saved until
+  // transition steps complete. It will be injected into new engine after sync
+  // restart.
+  std::unique_ptr<SyncEncryptionHandler::NigoriState> saved_nigori_state_;
+
+  // We cache the cryptographer's pending keys whenever NotifyPassphraseRequired
+  // is called. This way, before the UI calls SetDecryptionPassphrase on the
+  // syncer, it can avoid the overhead of an asynchronous decryption call and
+  // give the user immediate feedback about the passphrase entered by first
+  // trying to decrypt the cached pending keys on the UI thread. Note that
+  // SetDecryptionPassphrase can still fail after the cached pending keys are
+  // successfully decrypted if the pending keys have changed since the time they
+  // were cached.
+  sync_pb::EncryptedData cached_pending_keys_;
+
+  // The state of the passphrase required to decrypt the bag of encryption keys
+  // in the nigori node. Updated whenever a new nigori node arrives or the user
+  // manually changes their passphrase state. Cached so we can synchronously
+  // check it from the UI thread.
+  PassphraseType cached_passphrase_type_ = PassphraseType::IMPLICIT_PASSPHRASE;
+
+  // If an explicit passphrase is in use, the time at which the passphrase was
+  // first set (if available).
+  base::Time cached_explicit_passphrase_time_;
+
+  base::ThreadChecker thread_checker_;
+  base::WeakPtrFactory<SyncServiceCrypto> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(SyncServiceCrypto);
+};
+
+}  // namespace syncer
+
+#endif  // COMPONENTS_SYNC_DRIVER_SYNC_SERVICE_CRYPTO_H_