[Sync] Stop requiring backwards compatbility for sync encryption

A GAIA password is no longer required to set up a keystore encrypted nigori
node. This has the effect of not enabling backwards compatibility for new
nigori nodes.

At this point we also enable keystore encryption by default.

BUG=222304

Review URL: https://codereview.chromium.org/12440044

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

2 files changed, 69 insertions, 29 deletions

diff --git a/sync/internal_api/sync_encryption_handler_impl.cc b/sync/internal_api/sync_encryption_handler_impl.cc
index 3d0264c..1a645bf 100644
--- a/sync/internal_api/sync_encryption_handler_impl.cc
+++ b/sync/internal_api/sync_encryption_handler_impl.cc
@@ -1297,11 +1297,9 @@ bool SyncEncryptionHandlerImpl::ShouldTriggerMigration(
     const sync_pb::NigoriSpecifics& nigori,
     const Cryptographer& cryptographer) const {
   DCHECK(thread_checker_.CalledOnValidThread());
-  // TODO(zea): once we're willing to have the keystore key be the only
-  // encryption key, change this to !has_pending_keys(). For now, we need the
-  // cryptographer to be initialized with the current GAIA pass so that older
-  // clients (that don't have keystore support) can decrypt the keybag.
-  if (!cryptographer.is_ready())
+  // Don't migrate if there are pending encryption keys (because data
+  // encrypted with the pending keys will not be decryptable).
+  if (cryptographer.has_pending_keys())
     return false;
   if (IsNigoriMigratedToKeystore(nigori)) {
     // If the nigori is already migrated but does not reflect the explicit
@@ -1390,12 +1388,16 @@ bool SyncEncryptionHandlerImpl::AttemptToMigrateNigoriToKeystore(
 
   if (!keystore_key_.empty()) {
     KeyParams key_params = {"localhost", "dummy", keystore_key_};
-    if (old_keystore_keys_.size() > 0 &&
-        new_passphrase_type == KEYSTORE_PASSPHRASE) {
-      // At least one key rotation has been performed, so we no longer care
-      // about backwards compatibility. Ensure the keystore key is the default
-      // key.
+    if ((old_keystore_keys_.size() > 0 &&
+         new_passphrase_type == KEYSTORE_PASSPHRASE) ||
+        !cryptographer->is_initialized()) {
+      // Either at least one key rotation has been performed, so we no longer
+      // care about backwards compatibility, or we're generating keystore-based
+      // encryption keys without knowing the GAIA password (and therefore the
+      // cryptographer is not initialized), so we can't support backwards
+      // compatibility. Ensure the keystore key is the default key.
       DVLOG(1) << "Migrating keybag to keystore key.";
+      bool cryptographer_was_ready = cryptographer->is_ready();
       if (!cryptographer->AddKey(key_params)) {
         LOG(ERROR) << "Failed to add keystore key as default key";
         UMA_HISTOGRAM_ENUMERATION("Sync.AttemptNigoriMigration",
@@ -1403,6 +1405,12 @@ bool SyncEncryptionHandlerImpl::AttemptToMigrateNigoriToKeystore(
                                   MIGRATION_RESULT_SIZE);
         return false;
       }
+      if (!cryptographer_was_ready && cryptographer->is_ready()) {
+        FOR_EACH_OBSERVER(
+            SyncEncryptionHandler::Observer,
+            observers_,
+            OnPassphraseAccepted());
+      }
     } else {
       // We're in backwards compatible mode -- either the account has an
       // explicit passphrase, or we want to preserve the current GAIA-based key
diff --git a/sync/internal_api/sync_encryption_handler_impl_unittest.cc b/sync/internal_api/sync_encryption_handler_impl_unittest.cc
index a08de63..8abbd7e 100644
--- a/sync/internal_api/sync_encryption_handler_impl_unittest.cc
+++ b/sync/internal_api/sync_encryption_handler_impl_unittest.cc
@@ -601,19 +601,21 @@ TEST_F(SyncEncryptionHandlerImplTest, ReceiveOldNigori) {
 }
 
 // Ensure setting the keystore key works, updates the bootstrap token, and
-// doesn't modify the cryptographer. Then verify that the bootstrap token
-// can be correctly parsed by the encryption handler at startup time.
-TEST_F(SyncEncryptionHandlerImplTest, SetKeystoreUpdatesBootstrap) {
-  WriteTransaction trans(FROM_HERE, user_share());
-
+// triggers a non-backwards compatible migration. Then verify that the
+// bootstrap token can be correctly parsed by the encryption handler at startup
+// time.
+TEST_F(SyncEncryptionHandlerImplTest, SetKeystoreMigratesAndUpdatesBootstrap) {
   // Passing no keys should do nothing.
   EXPECT_CALL(*observer(), OnBootstrapTokenUpdated(_, _)).Times(0);
-  EXPECT_FALSE(GetCryptographer()->is_initialized());
-  EXPECT_TRUE(encryption_handler()->NeedKeystoreKey(trans.GetWrappedTrans()));
-  EXPECT_FALSE(
-      encryption_handler()->SetKeystoreKeys(BuildEncryptionKeyProto(""),
-                                            trans.GetWrappedTrans()));
-  EXPECT_TRUE(encryption_handler()->NeedKeystoreKey(trans.GetWrappedTrans()));
+  {
+    WriteTransaction trans(FROM_HERE, user_share());
+    EXPECT_FALSE(GetCryptographer()->is_initialized());
+    EXPECT_TRUE(encryption_handler()->NeedKeystoreKey(trans.GetWrappedTrans()));
+    EXPECT_FALSE(
+        encryption_handler()->SetKeystoreKeys(BuildEncryptionKeyProto(""),
+                                              trans.GetWrappedTrans()));
+    EXPECT_TRUE(encryption_handler()->NeedKeystoreKey(trans.GetWrappedTrans()));
+  }
   Mock::VerifyAndClearExpectations(observer());
 
   // Build a set of keystore keys.
@@ -624,19 +626,31 @@ TEST_F(SyncEncryptionHandlerImplTest, SetKeystoreUpdatesBootstrap) {
   keys.Add()->assign(kRawOldKeystoreKey);
   keys.Add()->assign(kRawKeystoreKey);
 
-  // Pass them to the encryption handler, triggering a bootstrap token update.
+  // Pass them to the encryption handler, triggering a migration and bootstrap
+  // token update.
   std::string encoded_key;
   std::string keystore_bootstrap;
+  EXPECT_CALL(*observer(), OnEncryptionComplete());
+  EXPECT_CALL(*observer(), OnCryptographerStateChanged(_));
+  EXPECT_CALL(*observer(), OnPassphraseAccepted());
+  EXPECT_CALL(*observer(), OnPassphraseTypeChanged(KEYSTORE_PASSPHRASE, _));
   EXPECT_CALL(*observer(),
               OnBootstrapTokenUpdated(_,
                                       KEYSTORE_BOOTSTRAP_TOKEN)).
       WillOnce(SaveArg<0>(&keystore_bootstrap));
-  EXPECT_TRUE(
-      encryption_handler()->SetKeystoreKeys(
-          keys,
-          trans.GetWrappedTrans()));
-  EXPECT_FALSE(encryption_handler()->NeedKeystoreKey(trans.GetWrappedTrans()));
-  EXPECT_FALSE(GetCryptographer()->is_initialized());
+  {
+    WriteTransaction trans(FROM_HERE, user_share());
+    EXPECT_TRUE(
+        encryption_handler()->SetKeystoreKeys(
+            keys,
+            trans.GetWrappedTrans()));
+    EXPECT_FALSE(
+        encryption_handler()->NeedKeystoreKey(trans.GetWrappedTrans()));
+    EXPECT_FALSE(GetCryptographer()->is_initialized());
+  }
+  PumpLoop();
+  EXPECT_TRUE(GetCryptographer()->is_initialized());
+  VerifyMigratedNigori(KEYSTORE_PASSPHRASE, kKeystoreKey);
 
   // Ensure the bootstrap is encoded properly (a base64 encoded encrypted blob
   // of list values containing the keystore keys).
@@ -667,7 +681,11 @@ TEST_F(SyncEncryptionHandlerImplTest, SetKeystoreUpdatesBootstrap) {
                                      &encryptor_,
                                      "",  // Cryptographer bootstrap.
                                      keystore_bootstrap);
-  EXPECT_FALSE(handler2.NeedKeystoreKey(trans.GetWrappedTrans()));
+
+  {
+    WriteTransaction trans(FROM_HERE, user_share());
+    EXPECT_FALSE(handler2.NeedKeystoreKey(trans.GetWrappedTrans()));
+  }
 }
 
 // Ensure GetKeystoreDecryptor only updates the keystore decryptor token if it
@@ -1942,6 +1960,11 @@ TEST_F(SyncEncryptionHandlerImplTest,
 // migrated nigori with the gaia key as the default (still in backwards
 // compatible mode).
 TEST_F(SyncEncryptionHandlerImplTest, RotateKeysGaiaDefault) {
+  // Destroy the existing nigori node so we init without a nigori node.
+  TearDown();
+  test_user_share_.SetUp();
+  SetUpEncryption();
+
   const char kOldGaiaKey[] = "old_gaia_key";
   const char kRawOldKeystoreKey[] = "old_keystore_key";
   std::string old_keystore_key;
@@ -1957,6 +1980,8 @@ TEST_F(SyncEncryptionHandlerImplTest, RotateKeysGaiaDefault) {
   PumpLoop();
   Mock::VerifyAndClearExpectations(observer());
 
+  // Then init the nigori node with a backwards compatible set of keys.
+  CreateRootForType(NIGORI);
   EXPECT_CALL(*observer(), OnPassphraseAccepted());
   InitKeystoreMigratedNigori(1, kOldGaiaKey, old_keystore_key);
 
@@ -1990,6 +2015,11 @@ TEST_F(SyncEncryptionHandlerImplTest, RotateKeysGaiaDefault) {
 // Trigger a key rotation upon receiving new keys if we already had a keystore
 // migrated nigori with the keystore key as the default.
 TEST_F(SyncEncryptionHandlerImplTest, RotateKeysKeystoreDefault) {
+  // Destroy the existing nigori node so we init without a nigori node.
+  TearDown();
+  test_user_share_.SetUp();
+  SetUpEncryption();
+
   const char kRawOldKeystoreKey[] = "old_keystore_key";
   std::string old_keystore_key;
   base::Base64Encode(kRawOldKeystoreKey, &old_keystore_key);
@@ -2004,6 +2034,8 @@ TEST_F(SyncEncryptionHandlerImplTest, RotateKeysKeystoreDefault) {
   PumpLoop();
   Mock::VerifyAndClearExpectations(observer());
 
+  // Then init the nigori node with a non-backwards compatible set of keys.
+  CreateRootForType(NIGORI);
   EXPECT_CALL(*observer(), OnPassphraseAccepted());
   InitKeystoreMigratedNigori(1, old_keystore_key, old_keystore_key);