[Sync] Add password support to sync api

This change modifies the password protobuf definition and the sync generic
change processor in order to support passwords custom encryption scheme.

BUG=117445

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

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

7 files changed, 196 insertions, 15 deletions

diff --git a/chrome/browser/sync/glue/generic_change_processor.cc b/chrome/browser/sync/glue/generic_change_processor.cc
index 591e717..fe11df0 100644
--- a/chrome/browser/sync/glue/generic_change_processor.cc
+++ b/chrome/browser/sync/glue/generic_change_processor.cc
@@ -24,6 +24,39 @@ using content::BrowserThread;
 
 namespace browser_sync {
 
+namespace {
+
+void SetNodeSpecifics(const sync_pb::EntitySpecifics& entity_specifics,
+                      syncer::WriteNode* write_node) {
+  if (syncer::GetModelTypeFromSpecifics(entity_specifics) ==
+          syncer::PASSWORDS) {
+    write_node->SetPasswordSpecifics(
+        entity_specifics.password().client_only_encrypted_data());
+  } else {
+    write_node->SetEntitySpecifics(entity_specifics);
+  }
+}
+
+syncer::SyncData BuildRemoteSyncData(
+    int64 sync_id,
+    const syncer::BaseNode& read_node) {
+  // Use the specifics of non-password datatypes directly (encryption has
+  // already been handled).
+  if (read_node.GetModelType() != syncer::PASSWORDS) {
+    return syncer::SyncData::CreateRemoteData(sync_id,
+                                              read_node.GetEntitySpecifics());
+  }
+
+  // Passwords must be accessed differently, to account for their encryption,
+  // and stored into a temporary EntitySpecifics.
+  sync_pb::EntitySpecifics password_holder;
+  password_holder.mutable_password()->mutable_client_only_encrypted_data()->
+      CopyFrom(read_node.GetPasswordSpecifics());
+  return syncer::SyncData::CreateRemoteData(sync_id, password_holder);
+}
+
+}  // namespace
+
 GenericChangeProcessor::GenericChangeProcessor(
     DataTypeErrorHandler* error_handler,
     const base::WeakPtr<syncer::SyncableService>& local_service,
@@ -71,8 +104,7 @@ void GenericChangeProcessor::ApplyChangesFromSyncModel(
           syncer::SyncChange(
               FROM_HERE,
               action,
-              syncer::SyncData::CreateRemoteData(
-                  it->id, read_node.GetEntitySpecifics())));
+              BuildRemoteSyncData(it->id, read_node)));
     }
   }
 }
@@ -136,8 +168,8 @@ syncer::SyncError GenericChangeProcessor::GetSyncDataForType(
                               type);
       return error;
     }
-    current_sync_data->push_back(syncer::SyncData::CreateRemoteData(
-        sync_child_node.GetId(), sync_child_node.GetEntitySpecifics()));
+    current_sync_data->push_back(BuildRemoteSyncData(sync_child_node.GetId(),
+                                                     sync_child_node));
   }
   return syncer::SyncError();
 }
@@ -317,8 +349,8 @@ syncer::SyncError GenericChangeProcessor::ProcessSyncChanges(
       }
       syncer::WriteNode::InitUniqueByCreationResult result =
           sync_node.InitUniqueByCreation(change.sync_data().GetDataType(),
-                                          root_node,
-                                          change.sync_data().GetTag());
+                                         root_node,
+                                         change.sync_data().GetTag());
       if (result != syncer::WriteNode::INIT_SUCCESS) {
         std::string error_prefix = "Failed to create " + type_str + " node: " +
             change.location().ToString() + ", ";
@@ -368,7 +400,7 @@ syncer::SyncError GenericChangeProcessor::ProcessSyncChanges(
         }
       }
       sync_node.SetTitle(UTF8ToWide(change.sync_data().GetTitle()));
-      sync_node.SetEntitySpecifics(change.sync_data().GetSpecifics());
+      SetNodeSpecifics(change.sync_data().GetSpecifics(), &sync_node);
       if (merge_result_.get()) {
         merge_result_->set_num_items_added(merge_result_->num_items_added() +
                                            1);
@@ -461,7 +493,7 @@ syncer::SyncError GenericChangeProcessor::ProcessSyncChanges(
       }
 
       sync_node.SetTitle(UTF8ToWide(change.sync_data().GetTitle()));
-      sync_node.SetEntitySpecifics(change.sync_data().GetSpecifics());
+      SetNodeSpecifics(change.sync_data().GetSpecifics(), &sync_node);
       if (merge_result_.get()) {
         merge_result_->set_num_items_modified(
             merge_result_->num_items_modified() + 1);
diff --git a/chrome/browser/sync/glue/generic_change_processor_unittest.cc b/chrome/browser/sync/glue/generic_change_processor_unittest.cc
index bb90a50..25b562c 100644
--- a/chrome/browser/sync/glue/generic_change_processor_unittest.cc
+++ b/chrome/browser/sync/glue/generic_change_processor_unittest.cc
@@ -10,9 +10,11 @@
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/sync/glue/data_type_error_handler_mock.h"
 #include "sync/api/fake_syncable_service.h"
+#include "sync/api/sync_change.h"
 #include "sync/api/sync_merge_result.h"
 #include "sync/internal_api/public/base/model_type.h"
 #include "sync/internal_api/public/read_node.h"
+#include "sync/internal_api/public/read_transaction.h"
 #include "sync/internal_api/public/test/test_user_share.h"
 #include "sync/internal_api/public/user_share.h"
 #include "sync/internal_api/public/write_node.h"
@@ -23,12 +25,12 @@ namespace browser_sync {
 
 namespace {
 
-class GenericChangeProcessorTest : public testing::Test {
+class SyncGenericChangeProcessorTest : public testing::Test {
  public:
   // It doesn't matter which type we use.  Just pick one.
   static const syncer::ModelType kType = syncer::PREFERENCES;
 
-  GenericChangeProcessorTest() :
+  SyncGenericChangeProcessorTest() :
       loop_(base::MessageLoop::TYPE_UI),
       sync_merge_result_(kType),
       merge_result_ptr_factory_(&sync_merge_result_),
@@ -37,7 +39,13 @@ class GenericChangeProcessorTest : public testing::Test {
 
   virtual void SetUp() OVERRIDE {
     test_user_share_.SetUp();
-    syncer::TestUserShare::CreateRoot(kType, test_user_share_.user_share());
+    syncer::ModelTypeSet types = syncer::ProtocolTypes();
+    for (syncer::ModelTypeSet::Iterator iter = types.First(); iter.Good();
+         iter.Inc()) {
+      syncer::TestUserShare::CreateRoot(iter.Get(),
+                                        test_user_share_.user_share());
+    }
+    test_user_share_.encryption_handler()->Init();
     change_processor_.reset(
         new GenericChangeProcessor(
             &data_type_error_handler_,
@@ -88,7 +96,7 @@ class GenericChangeProcessorTest : public testing::Test {
 // This test exercises GenericChangeProcessor's GetSyncDataForType function.
 // It's not a great test, but, by modifying some of the parameters, you could
 // turn it into a micro-benchmark for model association.
-TEST_F(GenericChangeProcessorTest, StressGetSyncDataForType) {
+TEST_F(SyncGenericChangeProcessorTest, StressGetSyncDataForType) {
   const int kNumChildNodes = 1000;
   const int kRepeatCount = 1;
 
@@ -103,6 +111,138 @@ TEST_F(GenericChangeProcessorTest, StressGetSyncDataForType) {
   }
 }
 
+TEST_F(SyncGenericChangeProcessorTest, SetGetPasswords) {
+  const int kNumPasswords = 10;
+  sync_pb::PasswordSpecificsData password_data;
+  password_data.set_username_value("user");
+
+  sync_pb::EntitySpecifics password_holder;
+
+  syncer::SyncChangeList change_list;
+  for (int i = 0; i < kNumPasswords; ++i) {
+    password_data.set_password_value(
+        base::StringPrintf("password%i", i));
+    password_holder.mutable_password()->mutable_client_only_encrypted_data()->
+        CopyFrom(password_data);
+    change_list.push_back(
+        syncer::SyncChange(FROM_HERE,
+                           syncer::SyncChange::ACTION_ADD,
+                           syncer::SyncData::CreateLocalData(
+                               base::StringPrintf("tag%i", i),
+                               base::StringPrintf("title%i", i),
+                               password_holder)));
+  }
+
+  ASSERT_FALSE(
+      change_processor()->ProcessSyncChanges(FROM_HERE, change_list).IsSet());
+
+  syncer::SyncDataList password_list;
+  ASSERT_FALSE(
+      change_processor()->GetSyncDataForType(syncer::PASSWORDS, &password_list).
+          IsSet());
+
+  ASSERT_EQ(password_list.size(), change_list.size());
+  for (int i = 0; i < kNumPasswords; ++i) {
+    // Verify the password is returned properly.
+    ASSERT_TRUE(password_list[i].GetSpecifics().has_password());
+    ASSERT_TRUE(password_list[i].GetSpecifics().password().
+                    has_client_only_encrypted_data());
+    ASSERT_FALSE(password_list[i].GetSpecifics().password().has_encrypted());
+    const sync_pb::PasswordSpecificsData& sync_password =
+        password_list[i].GetSpecifics().password().client_only_encrypted_data();
+    const sync_pb::PasswordSpecificsData& change_password =
+        change_list[i].sync_data().GetSpecifics().password().
+            client_only_encrypted_data();
+    ASSERT_EQ(sync_password.password_value(), change_password.password_value());
+    ASSERT_EQ(sync_password.username_value(), change_password.username_value());
+
+    // Verify the raw sync data was stored securely.
+    syncer::ReadTransaction read_transaction(FROM_HERE, user_share());
+    syncer::ReadNode node(&read_transaction);
+    ASSERT_EQ(node.InitByClientTagLookup(syncer::PASSWORDS,
+                                         base::StringPrintf("tag%i", i)),
+              syncer::BaseNode::INIT_OK);
+    ASSERT_EQ(node.GetTitle(), "encrypted");
+    const sync_pb::EntitySpecifics& raw_specifics = node.GetEntitySpecifics();
+    ASSERT_TRUE(raw_specifics.has_password());
+    ASSERT_TRUE(raw_specifics.password().has_encrypted());
+    ASSERT_FALSE(raw_specifics.password().has_client_only_encrypted_data());
+  }
+}
+
+TEST_F(SyncGenericChangeProcessorTest, UpdatePasswords) {
+  const int kNumPasswords = 10;
+  sync_pb::PasswordSpecificsData password_data;
+  password_data.set_username_value("user");
+
+  sync_pb::EntitySpecifics password_holder;
+
+  syncer::SyncChangeList change_list;
+  syncer::SyncChangeList change_list2;
+  for (int i = 0; i < kNumPasswords; ++i) {
+    password_data.set_password_value(
+        base::StringPrintf("password%i", i));
+    password_holder.mutable_password()->mutable_client_only_encrypted_data()->
+        CopyFrom(password_data);
+    change_list.push_back(
+        syncer::SyncChange(FROM_HERE,
+                           syncer::SyncChange::ACTION_ADD,
+                           syncer::SyncData::CreateLocalData(
+                               base::StringPrintf("tag%i", i),
+                               base::StringPrintf("title%i", i),
+                               password_holder)));
+    password_data.set_password_value(
+        base::StringPrintf("password_m%i", i));
+    password_holder.mutable_password()->mutable_client_only_encrypted_data()->
+        CopyFrom(password_data);
+    change_list2.push_back(
+        syncer::SyncChange(FROM_HERE,
+                           syncer::SyncChange::ACTION_UPDATE,
+                           syncer::SyncData::CreateLocalData(
+                               base::StringPrintf("tag%i", i),
+                               base::StringPrintf("title_m%i", i),
+                               password_holder)));
+  }
+
+  ASSERT_FALSE(
+      change_processor()->ProcessSyncChanges(FROM_HERE, change_list).IsSet());
+  ASSERT_FALSE(
+      change_processor()->ProcessSyncChanges(FROM_HERE, change_list2).IsSet());
+
+  syncer::SyncDataList password_list;
+  ASSERT_FALSE(
+      change_processor()->GetSyncDataForType(syncer::PASSWORDS, &password_list).
+          IsSet());
+
+  ASSERT_EQ(password_list.size(), change_list2.size());
+  for (int i = 0; i < kNumPasswords; ++i) {
+    // Verify the password is returned properly.
+    ASSERT_TRUE(password_list[i].GetSpecifics().has_password());
+    ASSERT_TRUE(password_list[i].GetSpecifics().password().
+                    has_client_only_encrypted_data());
+    ASSERT_FALSE(password_list[i].GetSpecifics().password().has_encrypted());
+    const sync_pb::PasswordSpecificsData& sync_password =
+        password_list[i].GetSpecifics().password().client_only_encrypted_data();
+    const sync_pb::PasswordSpecificsData& change_password =
+        change_list2[i].sync_data().GetSpecifics().password().
+            client_only_encrypted_data();
+    ASSERT_EQ(sync_password.password_value(), change_password.password_value());
+    ASSERT_EQ(sync_password.username_value(), change_password.username_value());
+
+    // Verify the raw sync data was stored securely.
+    syncer::ReadTransaction read_transaction(FROM_HERE, user_share());
+    syncer::ReadNode node(&read_transaction);
+    ASSERT_EQ(node.InitByClientTagLookup(syncer::PASSWORDS,
+                                         base::StringPrintf("tag%i", i)),
+              syncer::BaseNode::INIT_OK);
+    ASSERT_EQ(node.GetTitle(), "encrypted");
+    const sync_pb::EntitySpecifics& raw_specifics = node.GetEntitySpecifics();
+    ASSERT_TRUE(raw_specifics.has_password());
+    ASSERT_TRUE(raw_specifics.password().has_encrypted());
+    ASSERT_FALSE(raw_specifics.password().has_client_only_encrypted_data());
+  }
+}
+
 }  // namespace
 
 }  // namespace browser_sync
diff --git a/sync/engine/build_commit_command.cc b/sync/engine/build_commit_command.cc
index 0f16b80..a846cfd 100644
--- a/sync/engine/build_commit_command.cc
+++ b/sync/engine/build_commit_command.cc
@@ -104,6 +104,7 @@ void SetEntrySpecifics(Entry* meta_entry,
   sync_entry->mutable_specifics()->CopyFrom(meta_entry->Get(SPECIFICS));
   sync_entry->set_folder(meta_entry->Get(syncable::IS_DIR));
 
+  CHECK(!sync_entry->specifics().password().has_client_only_encrypted_data());
   DCHECK_EQ(meta_entry->GetModelType(), GetModelType(*sync_entry));
 }
 }  // namespace
diff --git a/sync/internal_api/write_node.cc b/sync/internal_api/write_node.cc
index 44a2a50..17a7fdb 100644
--- a/sync/internal_api/write_node.cc
+++ b/sync/internal_api/write_node.cc
@@ -225,6 +225,7 @@ void WriteNode::SetEntitySpecifics(
     const sync_pb::EntitySpecifics& new_value) {
   ModelType new_specifics_type =
       GetModelTypeFromSpecifics(new_value);
+  CHECK(!new_value.password().has_client_only_encrypted_data());
   DCHECK_NE(new_specifics_type, UNSPECIFIED);
   DVLOG(1) << "Writing entity specifics of type "
            << ModelTypeToString(new_specifics_type);
diff --git a/sync/protocol/password_specifics.proto b/sync/protocol/password_specifics.proto
index 769914433..ad5a512 100644
--- a/sync/protocol/password_specifics.proto
+++ b/sync/protocol/password_specifics.proto
@@ -33,9 +33,13 @@ message PasswordSpecificsData {
   optional bool blacklisted = 12;
 }
 
-// Properties of password sync objects. The actual password data is held in a
-// PasswordSpecificsData that is encrypted into |encrypted|.
+// Properties of password sync objects.
 message PasswordSpecifics {
+  // The actual password data. Contains an encrypted PasswordSpecificsData
+  // message.
   optional EncryptedData encrypted = 1;
+  // An unsynced field for use internally on the client. This field should
+  // never be set in any network-based communications.
+  optional PasswordSpecificsData client_only_encrypted_data = 2;
 }
 
diff --git a/sync/syncable/mutable_entry.cc b/sync/syncable/mutable_entry.cc
index 1d5524d..9562f31 100644
--- a/sync/syncable/mutable_entry.cc
+++ b/sync/syncable/mutable_entry.cc
@@ -261,6 +261,7 @@ bool MutableEntry::Put(StringField field, const string& value) {
 bool MutableEntry::Put(ProtoField field,
                        const sync_pb::EntitySpecifics& value) {
   DCHECK(kernel_);
+  CHECK(!value.password().has_client_only_encrypted_data());
   write_transaction_->SaveOriginal(kernel_);
   // TODO(ncarter): This is unfortunately heavyweight.  Can we do
   // better?
diff --git a/sync/test/fake_sync_encryption_handler.cc b/sync/test/fake_sync_encryption_handler.cc
index 1a83e55..3a60b07 100644
--- a/sync/test/fake_sync_encryption_handler.cc
+++ b/sync/test/fake_sync_encryption_handler.cc
@@ -18,7 +18,9 @@ FakeSyncEncryptionHandler::FakeSyncEncryptionHandler()
 FakeSyncEncryptionHandler::~FakeSyncEncryptionHandler() {}
 
 void FakeSyncEncryptionHandler::Init() {
-  // Do nothing.
+  // Set up a basic cryptographer.
+  KeyParams keystore_params = {"localhost", "dummy", "keystore_key"};
+  cryptographer_.AddKey(keystore_params);
 }
 
 void FakeSyncEncryptionHandler::ApplyNigoriUpdate(