[Sync] Add intelligent re-encryption support.

chrome/browser/sync/internal_api/write_node.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/internal_api/write_node.h"
 
 #include "base/json/json_writer.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/sync/engine/nigori_util.h"
@@ skipping 20 matching lines @@
 namespace sync_api {
 
 static const char kDefaultNameForNewNodes[] = " ";
 
 bool WriteNode::UpdateEntryWithEncryption(
     browser_sync::Cryptographer* cryptographer,
     const sync_pb::EntitySpecifics& new_specifics,
     syncable::MutableEntry* entry) {
   syncable::ModelType type = syncable::GetModelTypeFromSpecifics(new_specifics);
   DCHECK_GE(type, syncable::FIRST_REAL_MODEL_TYPE);
+  const sync_pb::EntitySpecifics& old_specifics = entry->Get(SPECIFICS);
   syncable::ModelTypeSet encrypted_types = cryptographer->GetEncryptedTypes();
   sync_pb::EntitySpecifics generated_specifics;
   if (!SpecificsNeedsEncryption(encrypted_types, new_specifics) ||
       !cryptographer->is_initialized()) {
     // No encryption required or we are unable to encrypt.
     generated_specifics.CopyFrom(new_specifics);
   } else {
     // Encrypt new_specifics into generated_specifics.
     if (VLOG_IS_ON(2)) {
       scoped_ptr<DictionaryValue> value(entry->ToValue());
       std::string info;
       base::JSONWriter::Write(value.get(), true, &info);
       DVLOG(2) << "Encrypting specifics of type "
                << syncable::ModelTypeToString(type)
                << " with content: "
                << info;
     }
-    syncable::AddDefaultExtensionValue(type, &generated_specifics);
+    // Only copy over the old specifics if it is of the right type and already
+    // encrypted. The first time we encrypt a node we start from scratch, hence
+    // removing all the unencrypted data, but from then on we only want to
+    // update the node if the data changes or the encryption key changes.
+    if (syncable::GetModelTypeFromSpecifics(old_specifics) == type &&
+        old_specifics.has_encrypted()) {
+      generated_specifics.CopyFrom(old_specifics);
+    } else {
+      syncable::AddDefaultExtensionValue(type, &generated_specifics);
+    }
+    // Does not change anything if underlying encrypted blob was already up
+    // to date and encrypted with the default key.
     if (!cryptographer->Encrypt(new_specifics,
                                 generated_specifics.mutable_encrypted())) {
       NOTREACHED() << "Could not encrypt data for node of type "
                    << syncable::ModelTypeToString(type);
       return false;
     }
   }
 
-  const sync_pb::EntitySpecifics& old_specifics = entry->Get(SPECIFICS);
-  if (AreSpecificsEqual(cryptographer, old_specifics, generated_specifics) &&
-      (entry->Get(syncable::NON_UNIQUE_NAME) == kEncryptedString ||
-       !generated_specifics.has_encrypted())) {
-    // Even if the data is the same but the old specifics are encrypted with an
-    // old key, we should go ahead and re-encrypt with the new key.
-    if ((!old_specifics.has_encrypted() &&
-         !generated_specifics.has_encrypted()) ||
-         cryptographer->CanDecryptUsingDefaultKey(old_specifics.encrypted())) {
-      DVLOG(2) << "Specifics of type " << syncable::ModelTypeToString(type)
-               << " already match, dropping change.";
-      return true;
-    }
-    // TODO(zea): Add some way to keep track of how often we're reencrypting
-    // because of a passphrase change.
+  // It's possible this entry was encrypted but didn't properly overwrite the
+  // non_unique_name (see crbug.com/96314).
+  bool encrypted_without_overwriting_name = (old_specifics.has_encrypted() &&
+      entry->Get(syncable::NON_UNIQUE_NAME) != kEncryptedString);
+
+  // If we're encrypted but the name wasn't overwritten properly we still want
+  // to rewrite the entry, irrespective of whether the specifics match.
+  if (!encrypted_without_overwriting_name &&
+      old_specifics.SerializeAsString() ==
+          generated_specifics.SerializeAsString()) {
+    DVLOG(2) << "Specifics of type " << syncable::ModelTypeToString(type)
+             << " already match, dropping change.";
+    return true;
   }
 
   if (generated_specifics.has_encrypted()) {
     // Overwrite the possibly sensitive non-specifics data.
     entry->Put(syncable::NON_UNIQUE_NAME, kEncryptedString);
     // For bookmarks we actually put bogus data into the unencrypted specifics,
     // else the server will try to do it for us.
     if (type == syncable::BOOKMARKS) {
       sync_pb::BookmarkSpecifics* bookmark_specifics =
           generated_specifics.MutableExtension(sync_pb::bookmark);
@@ skipping 85 matching lines @@
   entity_specifics.MutableExtension(sync_pb::nigori)->CopyFrom(new_value);
   SetEntitySpecifics(entity_specifics);
 }
 
 void WriteNode::SetPasswordSpecifics(
     const sync_pb::PasswordSpecificsData& data) {
   DCHECK_EQ(syncable::PASSWORDS, GetModelType());
 
   Cryptographer* cryptographer = GetTransaction()->GetCryptographer();
 
-  // Idempotency check to prevent unnecessary syncing: if the plaintexts match
-  // and the old ciphertext is encrypted with the most current key, there's
-  // nothing to do here.  Because each encryption is seeded with a different
-  // random value, checking for equivalence post-encryption doesn't suffice.
-  const sync_pb::EncryptedData& old_ciphertext =
-      GetEntry()->Get(SPECIFICS).GetExtension(sync_pb::password).encrypted();
-  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() << "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);
@@ skipping 307 matching lines @@
   sync_pb::BookmarkSpecifics new_value = GetBookmarkSpecifics();
   new_value.set_favicon(bytes.empty() ? NULL : &bytes[0], bytes.size());
   SetBookmarkSpecifics(new_value);
 }
 
 void WriteNode::MarkForSyncing() {
   syncable::MarkForSyncing(entry_);
 }
 
 } // namespace sync_api