[Sync] Persist keystore key across restarts

Adds the preference for the bootstrap token and the bootstrapping functionality
in the cryptographer so that we persist the keystore key across restarts. With
this, we should only ever do one GetKey per client, and only on the first time
that client signs in/restarts on a version with GetKey support (although it's 
not persisted across signouts/profile nukes)

BUG=129665
TEST=unit_tests


Review URL: https://chromiumcodereview.appspot.com/10540149

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149344 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/sync/glue/sync_backend_host.cc

@@ -411,6 +411,7 @@ void SyncBackendHost::Initialize(
       sync_manager_factory,
       delete_sync_data_folder,
       sync_prefs_->GetEncryptionBootstrapToken(),
+      sync_prefs_->GetKeystoreEncryptionBootstrapToken(),
       new InternalComponentsFactoryImpl(),
       unrecoverable_error_handler,
       report_unrecoverable_error_function));
@@ -786,6 +787,7 @@ SyncBackendHost::DoInitializeOptions::DoInitializeOptions(
     syncer::SyncManagerFactory* sync_manager_factory,
     bool delete_sync_data_folder,
     const std::string& restored_key_for_bootstrapping,
+    const std::string& restored_keystore_key_for_bootstrapping,
     InternalComponentsFactory* internal_components_factory,
     syncer::UnrecoverableErrorHandler* unrecoverable_error_handler,
     syncer::ReportUnrecoverableErrorFunction
@@ -804,6 +806,8 @@ SyncBackendHost::DoInitializeOptions::DoInitializeOptions(
       sync_manager_factory(sync_manager_factory),
       delete_sync_data_folder(delete_sync_data_folder),
       restored_key_for_bootstrapping(restored_key_for_bootstrapping),
+      restored_keystore_key_for_bootstrapping(
+          restored_keystore_key_for_bootstrapping),
       internal_components_factory(internal_components_factory),
       unrecoverable_error_handler(unrecoverable_error_handler),
       report_unrecoverable_error_function(
@@ -834,6 +838,24 @@ void SyncBackendHost::Core::OnSyncCycleCompleted(
   if (!sync_loop_)
     return;
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
+
+  if (snapshot.model_neutral_state().last_get_key_result ==
+          syncer::SYNCER_OK) {
+    // If we just received a new keystore key, get it and make sure we update
+    // the bootstrap token with it.
+    std::string keystore_token;
+    sync_manager_->GetKeystoreKeyBootstrapToken(&keystore_token);
+    if (!keystore_token.empty()) {
+      DVLOG(1) << "Persisting keystore encryption bootstrap token.";
+      host_.Call(FROM_HERE,
+                 &SyncBackendHost::PersistEncryptionBootstrapToken,
+                 keystore_token,
+                 KEYSTORE_BOOTSTRAP_TOKEN);
+    } else {
+      NOTREACHED();
+    }
+  }
+
   host_.Call(
       FROM_HERE,
       &SyncBackendHost::HandleSyncCycleCompletedOnFrontendLoop,
@@ -900,9 +922,10 @@ void SyncBackendHost::Core::OnBootstrapTokenUpdated(
   if (!sync_loop_)
     return;
   DCHECK_EQ(MessageLoop::current(), sync_loop_);
-  host_.Call(
-      FROM_HERE,
-      &SyncBackendHost::PersistEncryptionBootstrapToken, bootstrap_token);
+  host_.Call(FROM_HERE,
+             &SyncBackendHost::PersistEncryptionBootstrapToken,
+             bootstrap_token,
+             PASSPHRASE_BOOTSTRAP_TOKEN);
 }
 
 void SyncBackendHost::Core::OnStopSyncingPermanently() {
@@ -999,6 +1022,7 @@ void SyncBackendHost::Core::DoInitialize(const DoInitializeOptions& options) {
           options.chrome_sync_notification_bridge,
           options.sync_notifier_factory->CreateSyncNotifier())),
       options.restored_key_for_bootstrapping,
+      options.restored_keystore_key_for_bootstrapping,
       CommandLine::ForCurrentProcess()->HasSwitch(
               switches::kSyncKeystoreEncryption),
       scoped_ptr<InternalComponentsFactory>(
@@ -1271,9 +1295,14 @@ void SyncBackendHost::RetryConfigurationOnFrontendLoop(
 }
 
 void SyncBackendHost::PersistEncryptionBootstrapToken(
-    const std::string& token) {
+    const std::string& token,
+    BootstrapTokenType token_type) {
   CHECK(sync_prefs_.get());
-  sync_prefs_->SetEncryptionBootstrapToken(token);
+  DCHECK(!token.empty());
+  if (token_type == PASSPHRASE_BOOTSTRAP_TOKEN)
+    sync_prefs_->SetEncryptionBootstrapToken(token);
+  else
+    sync_prefs_->SetKeystoreEncryptionBootstrapToken(token);
 }
 
 void SyncBackendHost::HandleActionableErrorEventOnFrontendLoop(

chrome/browser/sync/glue/sync_backend_host.h

@@ -297,6 +297,7 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
         syncer::SyncManagerFactory* sync_manager_factory,
         bool delete_sync_data_folder,
         const std::string& restored_key_for_bootstrapping,
+        const std::string& restored_keystore_key_for_bootstrapping,
         syncer::InternalComponentsFactory* internal_components_factory,
         syncer::UnrecoverableErrorHandler* unrecoverable_error_handler,
         syncer::ReportUnrecoverableErrorFunction
@@ -319,6 +320,7 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
     std::string lsid;
     bool delete_sync_data_folder;
     std::string restored_key_for_bootstrapping;
+    std::string restored_keystore_key_for_bootstrapping;
     syncer::InternalComponentsFactory* internal_components_factory;
     syncer::UnrecoverableErrorHandler* unrecoverable_error_handler;
     syncer::ReportUnrecoverableErrorFunction
@@ -368,6 +370,12 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
     INITIALIZED,            // Initialization is complete.
   };
 
+  // Enum used to distinguish which bootstrap encryption token is being updated.
+  enum BootstrapTokenType {
+    PASSPHRASE_BOOTSTRAP_TOKEN,
+    KEYSTORE_BOOTSTRAP_TOKEN
+  };
+
   // Checks if we have received a notice to turn on experimental datatypes
   // (via the nigori node) and informs the frontend if that is the case.
   // Note: it is illegal to call this before the backend is initialized.
@@ -397,7 +405,9 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
   // across browser restart to avoid requiring the user to re-enter their
   // passphrase.  |token| must be valid UTF-8 as we use the PrefService for
   // storage.
-  void PersistEncryptionBootstrapToken(const std::string& token);
+  void PersistEncryptionBootstrapToken(
+      const std::string& token,
+      BootstrapTokenType token_type);
 
   // For convenience, checks if initialization state is INITIALIZED.
   bool initialized() const { return initialization_state_ == INITIALIZED; }

chrome/browser/sync/sync_prefs.cc

@@ -52,6 +52,7 @@ void SyncPrefs::ClearPreferences() {
   pref_service_->ClearPref(prefs::kSyncLastSyncedTime);
   pref_service_->ClearPref(prefs::kSyncHasSetupCompleted);
   pref_service_->ClearPref(prefs::kSyncEncryptionBootstrapToken);
+  pref_service_->ClearPref(prefs::kSyncKeystoreEncryptionBootstrapToken);
 
   // TODO(nick): The current behavior does not clear
   // e.g. prefs::kSyncBookmarks.  Is that really what we want?
@@ -177,6 +178,19 @@ void SyncPrefs::SetEncryptionBootstrapToken(const std::string& token) {
   pref_service_->SetString(prefs::kSyncEncryptionBootstrapToken, token);
 }
 
+std::string SyncPrefs::GetKeystoreEncryptionBootstrapToken() const {
+  DCHECK(CalledOnValidThread());
+  return
+      pref_service_ ?
+      pref_service_->GetString(prefs::kSyncKeystoreEncryptionBootstrapToken) :
+      "";
+}
+
+void SyncPrefs::SetKeystoreEncryptionBootstrapToken(const std::string& token) {
+  DCHECK(CalledOnValidThread());
+  pref_service_->SetString(prefs::kSyncKeystoreEncryptionBootstrapToken, token);
+}
+
 // static
 const char* SyncPrefs::GetPrefNameForDataType(syncer::ModelType data_type) {
   switch (data_type) {
@@ -339,6 +353,10 @@ void SyncPrefs::RegisterPreferences() {
   pref_service_->RegisterStringPref(prefs::kSyncEncryptionBootstrapToken,
                                     "",
                                     PrefService::UNSYNCABLE_PREF);
+  pref_service_->RegisterStringPref(
+      prefs::kSyncKeystoreEncryptionBootstrapToken,
+      "",
+      PrefService::UNSYNCABLE_PREF);
 #if defined(OS_CHROMEOS)
   pref_service_->RegisterStringPref(prefs::kSyncSpareBootstrapToken,
                                     "",

chrome/browser/sync/sync_prefs.h

@@ -94,10 +94,15 @@ class SyncPrefs : NON_EXPORTED_BASE(public base::NonThreadSafe),
   // This pref is set outside of sync.
   bool IsManaged() const;
 
-  // Use this encryption bootstrap token once already syncing.
+  // Use this encryption bootstrap token if we're using an explicit passphrase.
   std::string GetEncryptionBootstrapToken() const;
   void SetEncryptionBootstrapToken(const std::string& token);
 
+  // Use this keystore bootstrap token if we're not using an explicit
+  // passphrase.
+  std::string GetKeystoreEncryptionBootstrapToken() const;
+  void SetKeystoreEncryptionBootstrapToken(const std::string& token);
+
   // Maps |data_type| to its corresponding preference name.
   static const char* GetPrefNameForDataType(syncer::ModelType data_type);
 

chrome/common/pref_names.cc

@@ -1646,6 +1646,11 @@ const char kInvalidatorMaxInvalidationVersions[] =
 const char kSyncEncryptionBootstrapToken[] =
     "sync.encryption_bootstrap_token";
 
+// Same as kSyncEncryptionBootstrapToken, but derived from the keystore key,
+// so we don't have to do a GetKey command at restart.
+const char kSyncKeystoreEncryptionBootstrapToken[] =
+    "sync.keystore_encryption_bootstrap_token";
+
 // Boolean tracking whether the user chose to specify a secondary encryption
 // passphrase.
 const char kSyncUsingSecondaryPassphrase[] = "sync.using_secondary_passphrase";

chrome/common/pref_names.h

@@ -588,6 +588,7 @@ extern const char kGoogleServicesUsername[];
 extern const char kGoogleServicesUsernamePattern[];
 extern const char kSyncUsingSecondaryPassphrase[];
 extern const char kSyncEncryptionBootstrapToken[];
+extern const char kSyncKeystoreEncryptionBootstrapToken[];
 extern const char kSyncAcknowledgedSyncTypes[];
 // Deprecated in favor of kInvalidatorMaxInvalidationVersions.
 extern const char kSyncMaxInvalidationVersions[];

sync/internal_api/public/sync_manager.h

@@ -381,6 +381,7 @@ class SyncManager {
       const SyncCredentials& credentials,
       scoped_ptr<SyncNotifier> sync_notifier,
       const std::string& restored_key_for_bootstrapping,
+      const std::string& restored_keystore_key_for_bootstrapping,
       bool keystore_encryption_enabled,
       scoped_ptr<InternalComponentsFactory> internal_components_factory,
       Encryptor* encryptor,
@@ -464,6 +465,10 @@ class SyncManager {
   // May be called on any thread.
   virtual bool IsUsingExplicitPassphrase() = 0;
 
+  // Extracts the keystore encryption bootstrap token if a keystore key existed.
+  // Returns true if bootstrap token successfully extracted, false otherwise.
+  virtual bool GetKeystoreKeyBootstrapToken(std::string* token) = 0;
+
   // Call periodically from a database-safe thread to persist recent changes
   // to the syncapi model.
   virtual void SaveChanges() = 0;

sync/internal_api/public/test/fake_sync_manager.h

@@ -67,6 +67,7 @@ class FakeSyncManager : public SyncManager {
       const SyncCredentials& credentials,
       scoped_ptr<SyncNotifier> sync_notifier,
       const std::string& restored_key_for_bootstrapping,
+      const std::string& restored_keystore_key_for_bootstrapping,
       bool keystore_encryption_enabled,
       scoped_ptr<InternalComponentsFactory> internal_components_factory,
       Encryptor* encryptor,
@@ -95,6 +96,7 @@ class FakeSyncManager : public SyncManager {
   virtual void RemoveObserver(Observer* observer) OVERRIDE;
   virtual SyncStatus GetDetailedStatus() const OVERRIDE;
   virtual bool IsUsingExplicitPassphrase() OVERRIDE;
+  virtual bool GetKeystoreKeyBootstrapToken(std::string* token) OVERRIDE;
   virtual void SaveChanges() OVERRIDE;
   virtual void StopSyncingForShutdown(const base::Closure& callback) OVERRIDE;
   virtual void ShutdownOnSyncThread() OVERRIDE;

sync/internal_api/sync_manager_impl.cc

@@ -376,6 +376,7 @@ bool SyncManagerImpl::Init(
     const SyncCredentials& credentials,
     scoped_ptr<SyncNotifier> sync_notifier,
     const std::string& restored_key_for_bootstrapping,
+    const std::string& restored_keystore_key_for_bootstrapping,
     bool keystore_encryption_enabled,
     scoped_ptr<InternalComponentsFactory> internal_components_factory,
     Encryptor* encryptor,
@@ -486,6 +487,8 @@ bool SyncManagerImpl::Init(
   // |initialized_| is set to true.
   ReadTransaction trans(FROM_HERE, GetUserShare());
   trans.GetCryptographer()->Bootstrap(restored_key_for_bootstrapping);
+  trans.GetCryptographer()->BootstrapKeystoreKey(
+      restored_keystore_key_for_bootstrapping);
   trans.GetCryptographer()->AddObserver(this);
 
   FOR_EACH_OBSERVER(SyncManager::Observer, observers_,
@@ -1062,6 +1065,11 @@ bool SyncManagerImpl::IsUsingExplicitPassphrase() {
   return node.GetNigoriSpecifics().using_explicit_passphrase();
 }
 
+bool SyncManagerImpl::GetKeystoreKeyBootstrapToken(std::string* token) {
+  ReadTransaction trans(FROM_HERE, GetUserShare());
+  return trans.GetCryptographer()->GetKeystoreKeyBootstrapToken(token);
+}
+
 void SyncManagerImpl::RefreshEncryption() {
   DCHECK(initialized_);
 
@@ -1505,13 +1513,15 @@ void SyncManagerImpl::OnSyncEngineEvent(const SyncEngineEvent& event) {
     }
 
     if (!event.snapshot.has_more_to_sync()) {
-      // To account for a nigori node arriving with stale/bad data, we ensure
-      // that the nigori node is up to date at the end of each cycle.
-      WriteTransaction trans(FROM_HERE, GetUserShare());
-      WriteNode nigori_node(&trans);
-      if (nigori_node.InitByTagLookup(kNigoriTag) == BaseNode::INIT_OK) {
-        Cryptographer* cryptographer = trans.GetCryptographer();
-        UpdateNigoriEncryptionState(cryptographer, &nigori_node);
+      {
+        // To account for a nigori node arriving with stale/bad data, we ensure
+        // that the nigori node is up to date at the end of each cycle.
+        WriteTransaction trans(FROM_HERE, GetUserShare());
+        WriteNode nigori_node(&trans);
+        if (nigori_node.InitByTagLookup(kNigoriTag) == BaseNode::INIT_OK) {
+          Cryptographer* cryptographer = trans.GetCryptographer();
+          UpdateNigoriEncryptionState(cryptographer, &nigori_node);
+        }
       }
 
       DVLOG(1) << "Sending OnSyncCycleCompleted";

sync/internal_api/sync_manager_impl.h

@@ -71,6 +71,7 @@ class SyncManagerImpl : public SyncManager,
       const SyncCredentials& credentials,
       scoped_ptr<SyncNotifier> sync_notifier,
       const std::string& restored_key_for_bootstrapping,
+      const std::string& restored_keystore_key_for_bootstrapping,
       bool keystore_encryption_enabled,
       scoped_ptr<InternalComponentsFactory> internal_components_factory,
       Encryptor* encryptor,
@@ -100,6 +101,7 @@ class SyncManagerImpl : public SyncManager,
   virtual void RemoveObserver(SyncManager::Observer* observer) OVERRIDE;
   virtual SyncStatus GetDetailedStatus() const OVERRIDE;
   virtual bool IsUsingExplicitPassphrase() OVERRIDE;
+  virtual bool GetKeystoreKeyBootstrapToken(std::string* token) OVERRIDE;
   virtual void SaveChanges() OVERRIDE;
   virtual void StopSyncingForShutdown(const base::Closure& callback) OVERRIDE;
   virtual void ShutdownOnSyncThread() OVERRIDE;

sync/internal_api/syncapi_unittest.cc

@@ -763,7 +763,7 @@ class SyncManagerTest : public testing::Test,
                        workers, &extensions_activity_monitor_, this,
                        credentials,
                        scoped_ptr<SyncNotifier>(sync_notifier_mock_),
-                       "",
+                       "", "",  // bootstrap tokens
                        true,  // enable keystore encryption
                        scoped_ptr<InternalComponentsFactory>(GetFactory()),
                        &encryptor_,

sync/internal_api/test/fake_sync_manager.cc

@@ -62,6 +62,7 @@ bool FakeSyncManager::Init(
     const SyncCredentials& credentials,
     scoped_ptr<SyncNotifier> sync_notifier,
     const std::string& restored_key_for_bootstrapping,
+    const std::string& restored_keystore_key_for_bootstrapping,
     bool keystore_encryption_enabled,
     scoped_ptr<InternalComponentsFactory> internal_components_factory,
     Encryptor* encryptor,
@@ -177,6 +178,10 @@ bool FakeSyncManager::IsUsingExplicitPassphrase() {
   return false;
 }
 
+bool FakeSyncManager::GetKeystoreKeyBootstrapToken(std::string* token) {
+  return false;
+}
+
 void FakeSyncManager::SaveChanges() {
   // Do nothing.
 }

sync/tools/sync_client.cc

@@ -345,6 +345,7 @@ int SyncClientMain(int argc, char* argv[]) {
   ExtensionsActivityMonitor* extensions_activity_monitor = NULL;
   LoggingChangeDelegate change_delegate;
   const char kRestoredKeyForBootstrapping[] = "";
+  const char kRestoredKeystoreKeyForBootstrapping[] = "";
   NullEncryptor null_encryptor;
   LoggingUnrecoverableErrorHandler unrecoverable_error_handler;
   sync_manager->Init(database_dir.path(),
@@ -362,6 +363,7 @@ int SyncClientMain(int argc, char* argv[]) {
                     scoped_ptr<SyncNotifier>(
                         sync_notifier_factory.CreateSyncNotifier()),
                     kRestoredKeyForBootstrapping,
+                    kRestoredKeystoreKeyForBootstrapping,
                     true,  // enable keystore encryption
                     scoped_ptr<InternalComponentsFactory>(
                         new InternalComponentsFactoryImpl()),