[Sync] Ensure cryptographer ready before encrypting.

chrome/browser/sync/engine/syncapi.cc

 // Copyright (c) 2011 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 "chrome/browser/sync/engine/syncapi.h"
 
 #include <algorithm>
 #include <bitset>
 #include <iomanip>
 #include <list>
@@ skipping 184 matching lines @@
   }
 }
 
 UserShare::UserShare() {}
 
 UserShare::~UserShare() {}
 
 ////////////////////////////////////
 // BaseNode member definitions.
 
-BaseNode::BaseNode() {}
+BaseNode::BaseNode() : password_data_(new sync_pb::PasswordSpecificsData) {}
 
 BaseNode::~BaseNode() {}
 
 std::string BaseNode::GenerateSyncableHash(
     syncable::ModelType model_type, const std::string& client_tag) {
   // blank PB with just the extension in it has termination symbol,
   // handy for delimiter
   sync_pb::EntitySpecifics serialized_type;
   syncable::AddDefaultExtensionValue(model_type, &serialized_type);
   std::string hash_input;
@@ skipping 22 matching lines @@
 }
 
 bool BaseNode::DecryptIfNecessary() {
   if (GetIsFolder()) return true;  // Ignore the top-level datatype folder.
   const sync_pb::EntitySpecifics& specifics =
       GetEntry()->Get(syncable::SPECIFICS);
   if (specifics.HasExtension(sync_pb::password)) {
     // Passwords have their own legacy encryption structure.
     scoped_ptr<sync_pb::PasswordSpecificsData> data(DecryptPasswordSpecifics(
         specifics, GetTransaction()->GetCryptographer()));
-    if (!data.get())
+    if (!data.get()) {
+      LOG(ERROR) << "Failed to decrypt password specifics.";
       return false;
+    }
     password_data_.swap(data);
     return true;
   }
 
   // We assume any node with the encrypted field set has encrypted data.
   if (!specifics.has_encrypted())
     return true;
 
   const sync_pb::EncryptedData& encrypted =
       specifics.encrypted();
   std::string plaintext_data = GetTransaction()->GetCryptographer()->
       DecryptToString(encrypted);
-  if (plaintext_data.length() == 0)
-    return false;
-  if (!unencrypted_data_.ParseFromString(plaintext_data)) {
+  if (plaintext_data.length() == 0 ||
+      !unencrypted_data_.ParseFromString(plaintext_data)) {
     LOG(ERROR) << "Failed to decrypt encrypted node of type " <<
       syncable::ModelTypeToString(GetModelType()) << ".";
     return false;
   }
   return true;
 }
 
 const sync_pb::EntitySpecifics& BaseNode::GetUnencryptedSpecifics(
     const syncable::Entry* entry) const {
   const sync_pb::EntitySpecifics& specifics = entry->Get(SPECIFICS);
@@ skipping 122 matching lines @@
   return GetEntitySpecifics().GetExtension(sync_pb::bookmark);
 }
 
 const sync_pb::NigoriSpecifics& BaseNode::GetNigoriSpecifics() const {
   DCHECK_EQ(syncable::NIGORI, GetModelType());
   return GetEntitySpecifics().GetExtension(sync_pb::nigori);
 }
 
 const sync_pb::PasswordSpecificsData& BaseNode::GetPasswordSpecifics() const {
   DCHECK_EQ(syncable::PASSWORDS, GetModelType());
-  DCHECK(password_data_.get());
   return *password_data_;
 }
 
 const sync_pb::ThemeSpecifics& BaseNode::GetThemeSpecifics() const {
   DCHECK_EQ(syncable::THEMES, GetModelType());
   return GetEntitySpecifics().GetExtension(sync_pb::theme);
 }
 
 const sync_pb::TypedUrlSpecifics& BaseNode::GetTypedUrlSpecifics() const {
   DCHECK_EQ(syncable::TYPED_URLS, GetModelType());
@@ skipping 146 matching lines @@
   scoped_ptr<sync_pb::PasswordSpecificsData> old_plaintext(
       DecryptPasswordSpecifics(GetEntry()->Get(SPECIFICS), cryptographer));
   if (old_plaintext.get() &&
       old_plaintext->SerializeAsString() == data.SerializeAsString() &&
       cryptographer->CanDecryptUsingDefaultKey(old_ciphertext)) {
     return;
   }
 
   sync_pb::PasswordSpecifics new_value;
   if (!cryptographer->Encrypt(data, new_value.mutable_encrypted())) {
-    NOTREACHED();
+    NOTREACHED() << "Failed to encrypt password, possibly due to sync node "
+                 << "corruption";
+    return;
   }
 
   sync_pb::EntitySpecifics entity_specifics;
   entity_specifics.MutableExtension(sync_pb::password)->CopyFrom(new_value);
   SetEntitySpecifics(entity_specifics);
 }
 
 void WriteNode::SetThemeSpecifics(
     const sync_pb::ThemeSpecifics& new_value) {
   sync_pb::EntitySpecifics entity_specifics;
@@ skipping 611 matching lines @@
 
   // Called when the user disables or enables a sync type.
   void UpdateEnabledTypes();
 
   // Tell the sync engine to start the syncing process.
   void StartSyncingNormally();
 
   // Whether or not the Nigori node is encrypted using an explicit passphrase.
   bool IsUsingExplicitPassphrase();
 
+  // Update the Cryptographer from the current nigori node.
+  // Note: opens a transaction and can trigger an ON_PASSPHRASE_REQUIRED, so
+  // should only be called after syncapi is fully initialized.
+  // Returns true if cryptographer is ready, false otherwise.
+  bool UpdateCryptographerFromNigori();
+
   // Set the datatypes we want to encrypt and encrypt any nodes as necessary.
   void EncryptDataTypes(const syncable::ModelTypeSet& encrypted_types);
 
   // Try to set the current passphrase to |passphrase|, and record whether
   // it is an explicit passphrase or implicitly using gaia in the Nigori
   // node.
   void SetPassphrase(const std::string& passphrase, bool is_explicit);
 
   // Call periodically from a database-safe thread to persist recent changes
   // to the syncapi model.
@@ skipping 572 matching lines @@
   }
 
   // Do this once the directory is opened.
   BootstrapEncryption(restored_key_for_bootstrapping);
   MarkAndNotifyInitializationComplete();
   return signed_in;
 }
 
 void SyncManager::SyncInternal::BootstrapEncryption(
     const std::string& restored_key_for_bootstrapping) {
+  // Cryptographer should only be accessed while holding a transaction.
+  ReadTransaction trans(GetUserShare());
+  Cryptographer* cryptographer = trans.GetCryptographer();
+
+  // Set the bootstrap token before bailing out if nigori node is not there.
+  // This could happen if server asked us to migrate nigri.
+  cryptographer->Bootstrap(restored_key_for_bootstrapping);
+}
+
+bool SyncManager::SyncInternal::UpdateCryptographerFromNigori() {
   syncable::ScopedDirLookup lookup(dir_manager(), username_for_share());
   if (!lookup.good()) {
-    VLOG(0) << "BootstrapEncryption: lookup not good so bailing out";
+    NOTREACHED() << "BootstrapEncryption: lookup not good so bailing out";
+    return false;
+  }
+  if (!lookup->initial_sync_ended_for_type(syncable::NIGORI))
+    return false;  // Should only happen during first time sync.
+
+  ReadTransaction trans(GetUserShare());
+  Cryptographer* cryptographer = trans.GetCryptographer();
+
+  ReadNode node(&trans);
+  if (!node.InitByTagLookup(kNigoriTag)) {
     NOTREACHED();
-    return;
+    return false;
+  }
+  Cryptographer::UpdateResult result =
+      cryptographer->Update(node.GetNigoriSpecifics());
+  if (result == Cryptographer::NEEDS_PASSPHRASE) {
+    ObserverList<SyncManager::Observer> temp_obs_list;
+    CopyObservers(&temp_obs_list);
+    FOR_EACH_OBSERVER(SyncManager::Observer, temp_obs_list,
+                      OnPassphraseRequired(sync_api::REASON_DECRYPTION));
   }
 
-  sync_pb::NigoriSpecifics nigori;
-  syncable::ModelTypeSet encrypted_types;
-  {
-    // Cryptographer should only be accessed while holding a transaction.
-    ReadTransaction trans(GetUserShare());
-    Cryptographer* cryptographer = trans.GetCryptographer();
-
-    // Set the bootstrap token before bailing out if nigori node is not there.
-    // This could happen if server asked us to migrate nigri.
-    cryptographer->Bootstrap(restored_key_for_bootstrapping);
-
-    if (!lookup->initial_sync_ended_for_type(syncable::NIGORI))
-      return;
-
-    ReadNode node(&trans);
-    if (!node.InitByTagLookup(kNigoriTag)) {
-      NOTREACHED();
-      return;
-    }
-
-    nigori.CopyFrom(node.GetNigoriSpecifics());
-    Cryptographer::UpdateResult result = cryptographer->Update(nigori);
-    if (result == Cryptographer::NEEDS_PASSPHRASE) {
-      ObserverList<SyncManager::Observer> temp_obs_list;
-      CopyObservers(&temp_obs_list);
-      FOR_EACH_OBSERVER(SyncManager::Observer, temp_obs_list,
-                        OnPassphraseRequired(sync_api::REASON_DECRYPTION));
-    }
-
-    // Refresh list of encrypted datatypes.
-    encrypted_types = GetEncryptedTypes(&trans);
-  }
-
-
-
-  // Ensure any datatypes that need encryption are encrypted.
-  EncryptDataTypes(encrypted_types);
+  return cryptographer->is_ready();
 }
 
 void SyncManager::SyncInternal::StartSyncingNormally() {
   // Start the syncer thread. This won't actually
   // result in any syncing until at least the
   // DirectoryManager broadcasts the OPENED event,
   // and a valid server connection is detected.
   if (syncer_thread())  // NULL during certain unittests.
     syncer_thread()->Start(SyncerThread::NORMAL_MODE, NULL);
 }
@@ skipping 226 matching lines @@
 }
 
 void SyncManager::SyncInternal::EncryptDataTypes(
     const syncable::ModelTypeSet& encrypted_types) {
   VLOG(1) << "Attempting to encrypt datatypes "
           << syncable::ModelTypeSetToString(encrypted_types);
 
   WriteTransaction trans(GetUserShare());
   WriteNode node(&trans);
   if (!node.InitByTagLookup(kNigoriTag)) {
-    LOG(ERROR) << "Unable to set encrypted datatypes because Nigori node not "
-               << "found.";
-    NOTREACHED();
+    NOTREACHED() << "Unable to set encrypted datatypes because Nigori node not "
+                 << "found.";
     return;
   }
 
   Cryptographer* cryptographer = trans.GetCryptographer();
 
+  if (!cryptographer->is_ready()) {
+    NOTREACHED() << "Attempting to encrypt datatypes when cryptographer not "
+                 << "ready.";
+    return;
+  }
+
   // Update the Nigori node set of encrypted datatypes so other machines notice.
   // Note, we merge the current encrypted types with those requested. Once a
   // datatypes is marked as needing encryption, it is never unmarked.
   sync_pb::NigoriSpecifics nigori;
   nigori.CopyFrom(node.GetNigoriSpecifics());
   syncable::ModelTypeSet current_encrypted_types = GetEncryptedTypes(&trans);
   syncable::ModelTypeSet newly_encrypted_types;
   std::set_union(current_encrypted_types.begin(), current_encrypted_types.end(),
                  encrypted_types.begin(), encrypted_types.end(),
                  std::inserter(newly_encrypted_types,
@@ skipping 877 matching lines @@
 }
 BaseTransaction::~BaseTransaction() {
   delete lookup_;
 }
 
 UserShare* SyncManager::GetUserShare() const {
   DCHECK(data_->initialized()) << "GetUserShare requires initialization!";
   return data_->GetUserShare();
 }
 
+void SyncManager::ReloadNigori() {
+  DCHECK(data_->initialized());
+  if (data_->UpdateCryptographerFromNigori())

[tim (not reviewing), 2011/06/14 01:32:58]

is it possible for is_ready() to change from true to false after this call?

[Nicolas Zea, 2011/06/14 16:52:38]

You're right, once we release the transaction it's possible a new passphrase
will arrive. But, in EncryptDataTypes, as long as the cryptographer has been
initialized, we can still encrypt with the old passphrase. I've changed
EncryptDataTypes to just ensure the cryptographer is initialized.

+    data_->EncryptDataTypes(syncable::ModelTypeSet());
+}
+
 syncable::ModelTypeSet SyncManager::GetEncryptedDataTypes() const {
   sync_api::ReadTransaction trans(GetUserShare());
   return GetEncryptedTypes(&trans);
 }
 
-
 bool SyncManager::HasUnsyncedItems() const {
   sync_api::ReadTransaction trans(GetUserShare());
   return (trans.GetWrappedTrans()->directory()->unsynced_entity_count() != 0);
 }
 
 void SyncManager::LogUnsyncedItems(int level) const {
   std::vector<int64> unsynced_handles;
   sync_api::ReadTransaction trans(GetUserShare());
   trans.GetWrappedTrans()->directory()->GetUnsyncedMetaHandles(
       trans.GetWrappedTrans(), &unsynced_handles);
@@ skipping 18 matching lines @@
 void SyncManager::TriggerOnIncomingNotificationForTest(
     const syncable::ModelTypeBitSet& model_types) {
   syncable::ModelTypePayloadMap model_types_with_payloads =
       syncable::ModelTypePayloadMapFromBitSet(model_types,
           std::string());
 
   data_->OnIncomingNotification(model_types_with_payloads);
 }
 
 }  // namespace sync_api