diff options
Diffstat (limited to 'chrome/browser/sync')
33 files changed, 210 insertions, 1600 deletions
diff --git a/chrome/browser/sync/engine/apply_updates_command_unittest.cc b/chrome/browser/sync/engine/apply_updates_command_unittest.cc index df1a1ae..b3ca18c 100644 --- a/chrome/browser/sync/engine/apply_updates_command_unittest.cc +++ b/chrome/browser/sync/engine/apply_updates_command_unittest.cc @@ -2,21 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include <string> - -#include "base/format_macros.h" -#include "base/string_util.h" #include "chrome/browser/sync/engine/apply_updates_command.h" -#include "chrome/browser/sync/engine/syncer.h" -#include "chrome/browser/sync/engine/syncer_util.h" #include "chrome/browser/sync/protocol/bookmark_specifics.pb.h" #include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/syncable/directory_manager.h" -#include "chrome/browser/sync/syncable/nigori_util.h" #include "chrome/browser/sync/syncable/syncable.h" #include "chrome/browser/sync/syncable/syncable_id.h" #include "chrome/test/sync/engine/syncer_command_test.h" -#include "chrome/test/sync/engine/test_id_factory.h" #include "testing/gtest/include/gtest/gtest.h" namespace browser_sync { @@ -24,7 +16,6 @@ namespace browser_sync { using sessions::SyncSession; using std::string; using syncable::Entry; -using syncable::GetEncryptedDataTypes; using syncable::Id; using syncable::MutableEntry; using syncable::ReadTransaction; @@ -50,7 +41,7 @@ class ApplyUpdatesCommandTest : public SyncerCommandTest { SyncerCommandTest::SetUp(); } - // Create a new unapplied bookmark node with a parent. + // Create a new unapplied update. void CreateUnappliedNewItemWithParent(const string& item_id, const string& parent_id) { ScopedDirLookup dir(syncdb()->manager(), syncdb()->name()); @@ -70,10 +61,8 @@ class ApplyUpdatesCommandTest : public SyncerCommandTest { entry.Put(syncable::SERVER_SPECIFICS, default_bookmark_specifics); } - // Create a new unapplied update without a parent. void CreateUnappliedNewItem(const string& item_id, - const sync_pb::EntitySpecifics& specifics, - bool is_unique) { + const sync_pb::EntitySpecifics& specifics) { ScopedDirLookup dir(syncdb()->manager(), syncdb()->name()); ASSERT_TRUE(dir.good()); WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); @@ -82,53 +71,15 @@ class ApplyUpdatesCommandTest : public SyncerCommandTest { ASSERT_TRUE(entry.good()); entry.Put(syncable::SERVER_VERSION, next_revision_++); entry.Put(syncable::IS_UNAPPLIED_UPDATE, true); + entry.Put(syncable::SERVER_NON_UNIQUE_NAME, item_id); entry.Put(syncable::SERVER_PARENT_ID, syncable::kNullId); entry.Put(syncable::SERVER_IS_DIR, false); entry.Put(syncable::SERVER_SPECIFICS, specifics); - if (is_unique) // For top-level nodes. - entry.Put(syncable::UNIQUE_SERVER_TAG, item_id); - } - - // Create an unsynced item in the database. If item_id is a local ID, it - // will be treated as a create-new. Otherwise, if it's a server ID, we'll - // fake the server data so that it looks like it exists on the server. - // Returns the methandle of the created item in |metahandle_out| if not NULL. - void CreateUnsyncedItem(const Id& item_id, - const Id& parent_id, - const string& name, - bool is_folder, - syncable::ModelType model_type, - int64* metahandle_out) { - ScopedDirLookup dir(syncdb()->manager(), syncdb()->name()); - ASSERT_TRUE(dir.good()); - WriteTransaction trans(dir, UNITTEST, __FILE__, __LINE__); - Id predecessor_id = dir->GetLastChildId(&trans, parent_id); - MutableEntry entry(&trans, syncable::CREATE, parent_id, name); - ASSERT_TRUE(entry.good()); - entry.Put(syncable::ID, item_id); - entry.Put(syncable::BASE_VERSION, - item_id.ServerKnows() ? next_revision_++ : 0); - entry.Put(syncable::IS_UNSYNCED, true); - entry.Put(syncable::IS_DIR, is_folder); - entry.Put(syncable::IS_DEL, false); - entry.Put(syncable::PARENT_ID, parent_id); - entry.PutPredecessor(predecessor_id); - sync_pb::EntitySpecifics default_specifics; - syncable::AddDefaultExtensionValue(model_type, &default_specifics); - entry.Put(syncable::SPECIFICS, default_specifics); - if (item_id.ServerKnows()) { - entry.Put(syncable::SERVER_SPECIFICS, default_specifics); - entry.Put(syncable::SERVER_IS_DIR, is_folder); - entry.Put(syncable::SERVER_PARENT_ID, parent_id); - entry.Put(syncable::SERVER_IS_DEL, false); - } - if (metahandle_out) - *metahandle_out = entry.Get(syncable::META_HANDLE); } ApplyUpdatesCommand apply_updates_command_; - TestIdFactory id_factory_; + private: int64 next_revision_; DISALLOW_COPY_AND_ASSIGN(ApplyUpdatesCommandTest); @@ -227,7 +178,7 @@ TEST_F(ApplyUpdatesCommandTest, DecryptablePassword) { cryptographer->Encrypt(data, specifics.MutableExtension(sync_pb::password)->mutable_encrypted()); - CreateUnappliedNewItem("item", specifics, false); + CreateUnappliedNewItem("item", specifics); apply_updates_command_.ExecuteImpl(session()); @@ -245,7 +196,7 @@ TEST_F(ApplyUpdatesCommandTest, UndecryptablePassword) { // Undecryptable password updates should not be applied. sync_pb::EntitySpecifics specifics; specifics.MutableExtension(sync_pb::password); - CreateUnappliedNewItem("item", specifics, false); + CreateUnappliedNewItem("item", specifics); apply_updates_command_.ExecuteImpl(session()); @@ -274,7 +225,7 @@ TEST_F(ApplyUpdatesCommandTest, SomeUndecryptablePassword) { cryptographer->Encrypt(data, specifics.MutableExtension(sync_pb::password)->mutable_encrypted()); - CreateUnappliedNewItem("item1", specifics, false); + CreateUnappliedNewItem("item1", specifics); } { // Create a new cryptographer, independent of the one in the session. @@ -288,7 +239,7 @@ TEST_F(ApplyUpdatesCommandTest, SomeUndecryptablePassword) { cryptographer.Encrypt(data, specifics.MutableExtension(sync_pb::password)->mutable_encrypted()); - CreateUnappliedNewItem("item2", specifics, false); + CreateUnappliedNewItem("item2", specifics); } apply_updates_command_.ExecuteImpl(session()); @@ -304,109 +255,20 @@ TEST_F(ApplyUpdatesCommandTest, SomeUndecryptablePassword) { } TEST_F(ApplyUpdatesCommandTest, NigoriUpdate) { - syncable::ModelTypeSet encrypted_types; - { - ScopedDirLookup dir(syncdb()->manager(), syncdb()->name()); - ASSERT_TRUE(dir.good()); - ReadTransaction trans(dir, __FILE__, __LINE__); - EXPECT_EQ(encrypted_types, GetEncryptedDataTypes(&trans)); - } - // Nigori node updates should update the Cryptographer. Cryptographer other_cryptographer; KeyParams params = {"localhost", "dummy", "foobar"}; other_cryptographer.AddKey(params); sync_pb::EntitySpecifics specifics; - sync_pb::NigoriSpecifics* nigori = - specifics.MutableExtension(sync_pb::nigori); - other_cryptographer.GetKeys(nigori->mutable_encrypted()); - nigori->set_encrypt_bookmarks(true); - encrypted_types.insert(syncable::BOOKMARKS); - CreateUnappliedNewItem(syncable::ModelTypeToRootTag(syncable::NIGORI), - specifics, true); - - Cryptographer* cryptographer = - session()->context()->directory_manager()->cryptographer(); - EXPECT_FALSE(cryptographer->has_pending_keys()); - - apply_updates_command_.ExecuteImpl(session()); - - sessions::StatusController* status = session()->status_controller(); - sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE); - EXPECT_EQ(1, status->update_progress().AppliedUpdatesSize()) - << "All updates should have been attempted"; - EXPECT_EQ(0, status->conflict_progress().ConflictingItemsSize()) - << "The nigori update shouldn't be in conflict"; - EXPECT_EQ(1, status->update_progress().SuccessfullyAppliedUpdateCount()) - << "The nigori update should be applied"; - - EXPECT_FALSE(cryptographer->is_ready()); - EXPECT_TRUE(cryptographer->has_pending_keys()); -} - -TEST_F(ApplyUpdatesCommandTest, EncryptUnsyncedChanges) { - syncable::ModelTypeSet encrypted_types; - { - ScopedDirLookup dir(syncdb()->manager(), syncdb()->name()); - ASSERT_TRUE(dir.good()); - ReadTransaction trans(dir, __FILE__, __LINE__); - EXPECT_EQ(encrypted_types, GetEncryptedDataTypes(&trans)); - - // With empty encrypted_types, this should be true. - EXPECT_TRUE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types)); - - Syncer::UnsyncedMetaHandles handles; - SyncerUtil::GetUnsyncedEntries(&trans, &handles); - EXPECT_TRUE(handles.empty()); - } + other_cryptographer.GetKeys( + specifics.MutableExtension(sync_pb::nigori)->mutable_encrypted()); - // Create unsynced bookmarks without encryption. - // First item is a folder - Id folder_id = id_factory_.NewLocalId(); - CreateUnsyncedItem(folder_id, id_factory_.root(), "folder", - true, syncable::BOOKMARKS, NULL); - // Next five items are children of the folder - size_t i; - size_t batch_s = 5; - for (i = 0; i < batch_s; ++i) { - CreateUnsyncedItem(id_factory_.NewLocalId(), folder_id, - StringPrintf("Item %"PRIuS"", i), false, - syncable::BOOKMARKS, NULL); - } - // Next five items are children of the root. - for (; i < 2*batch_s; ++i) { - CreateUnsyncedItem(id_factory_.NewLocalId(), id_factory_.root(), - StringPrintf("Item %"PRIuS"", i), false, - syncable::BOOKMARKS, NULL); - } + CreateUnappliedNewItem("item", specifics); Cryptographer* cryptographer = session()->context()->directory_manager()->cryptographer(); - KeyParams params = {"localhost", "dummy", "foobar"}; - cryptographer->AddKey(params); - sync_pb::EntitySpecifics specifics; - sync_pb::NigoriSpecifics* nigori = - specifics.MutableExtension(sync_pb::nigori); - cryptographer->GetKeys(nigori->mutable_encrypted()); - nigori->set_encrypt_bookmarks(true); - encrypted_types.insert(syncable::BOOKMARKS); - CreateUnappliedNewItem(syncable::ModelTypeToRootTag(syncable::NIGORI), - specifics, true); EXPECT_FALSE(cryptographer->has_pending_keys()); - EXPECT_TRUE(cryptographer->is_ready()); - - { - // Ensure we have unsynced nodes that aren't properly encrypted. - ScopedDirLookup dir(syncdb()->manager(), syncdb()->name()); - ASSERT_TRUE(dir.good()); - ReadTransaction trans(dir, __FILE__, __LINE__); - EXPECT_FALSE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types)); - - Syncer::UnsyncedMetaHandles handles; - SyncerUtil::GetUnsyncedEntries(&trans, &handles); - EXPECT_EQ(2*batch_s+1, handles.size()); - } apply_updates_command_.ExecuteImpl(session()); @@ -418,116 +280,9 @@ TEST_F(ApplyUpdatesCommandTest, EncryptUnsyncedChanges) { << "The nigori update shouldn't be in conflict"; EXPECT_EQ(1, status->update_progress().SuccessfullyAppliedUpdateCount()) << "The nigori update should be applied"; - EXPECT_FALSE(cryptographer->has_pending_keys()); - EXPECT_TRUE(cryptographer->is_ready()); - { - ScopedDirLookup dir(syncdb()->manager(), syncdb()->name()); - ASSERT_TRUE(dir.good()); - ReadTransaction trans(dir, __FILE__, __LINE__); - - // If ProcessUnsyncedChangesForEncryption worked, all our unsynced changes - // should be encrypted now. - EXPECT_EQ(encrypted_types, GetEncryptedDataTypes(&trans)); - EXPECT_TRUE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types)); - - Syncer::UnsyncedMetaHandles handles; - SyncerUtil::GetUnsyncedEntries(&trans, &handles); - EXPECT_EQ(2*batch_s+1, handles.size()); - } -} - -TEST_F(ApplyUpdatesCommandTest, CannotEncryptUnsyncedChanges) { - syncable::ModelTypeSet encrypted_types; - { - ScopedDirLookup dir(syncdb()->manager(), syncdb()->name()); - ASSERT_TRUE(dir.good()); - ReadTransaction trans(dir, __FILE__, __LINE__); - EXPECT_EQ(encrypted_types, GetEncryptedDataTypes(&trans)); - - // With empty encrypted_types, this should be true. - EXPECT_TRUE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types)); - - Syncer::UnsyncedMetaHandles handles; - SyncerUtil::GetUnsyncedEntries(&trans, &handles); - EXPECT_TRUE(handles.empty()); - } - - // Create unsynced bookmarks without encryption. - // First item is a folder - Id folder_id = id_factory_.NewLocalId(); - CreateUnsyncedItem(folder_id, id_factory_.root(), "folder", true, - syncable::BOOKMARKS, NULL); - // Next five items are children of the folder - size_t i; - size_t batch_s = 5; - for (i = 0; i < batch_s; ++i) { - CreateUnsyncedItem(id_factory_.NewLocalId(), folder_id, - StringPrintf("Item %"PRIuS"", i), false, - syncable::BOOKMARKS, NULL); - } - // Next five items are children of the root. - for (; i < 2*batch_s; ++i) { - CreateUnsyncedItem(id_factory_.NewLocalId(), id_factory_.root(), - StringPrintf("Item %"PRIuS"", i), false, - syncable::BOOKMARKS, NULL); - } - - // We encrypt with new keys, triggering the local cryptographer to be unready - // and unable to decrypt data (once updated). - Cryptographer other_cryptographer; - KeyParams params = {"localhost", "dummy", "foobar"}; - other_cryptographer.AddKey(params); - sync_pb::EntitySpecifics specifics; - sync_pb::NigoriSpecifics* nigori = - specifics.MutableExtension(sync_pb::nigori); - other_cryptographer.GetKeys(nigori->mutable_encrypted()); - nigori->set_encrypt_bookmarks(true); - encrypted_types.insert(syncable::BOOKMARKS); - CreateUnappliedNewItem(syncable::ModelTypeToRootTag(syncable::NIGORI), - specifics, true); - Cryptographer* cryptographer = - session()->context()->directory_manager()->cryptographer(); - EXPECT_FALSE(cryptographer->has_pending_keys()); - - { - // Ensure we have unsynced nodes that aren't properly encrypted. - ScopedDirLookup dir(syncdb()->manager(), syncdb()->name()); - ASSERT_TRUE(dir.good()); - ReadTransaction trans(dir, __FILE__, __LINE__); - EXPECT_FALSE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types)); - Syncer::UnsyncedMetaHandles handles; - SyncerUtil::GetUnsyncedEntries(&trans, &handles); - EXPECT_EQ(2*batch_s+1, handles.size()); - } - - apply_updates_command_.ExecuteImpl(session()); - sessions::StatusController* status = session()->status_controller(); - sessions::ScopedModelSafeGroupRestriction r(status, GROUP_PASSIVE); - EXPECT_EQ(1, status->update_progress().AppliedUpdatesSize()) - << "All updates should have been attempted"; - EXPECT_EQ(1, status->conflict_progress().ConflictingItemsSize()) - << "The unsynced chnages trigger a conflict with the nigori update."; - EXPECT_EQ(0, status->update_progress().SuccessfullyAppliedUpdateCount()) - << "The nigori update should not be applied"; EXPECT_FALSE(cryptographer->is_ready()); EXPECT_TRUE(cryptographer->has_pending_keys()); - { - // Ensure the unsynced nodes are still not encrypted. - ScopedDirLookup dir(syncdb()->manager(), syncdb()->name()); - ASSERT_TRUE(dir.good()); - ReadTransaction trans(dir, __FILE__, __LINE__); - - // Since we're in conflict, the specifics don't reflect the unapplied - // changes. - EXPECT_FALSE(VerifyUnsyncedChangesAreEncrypted(&trans, encrypted_types)); - encrypted_types.clear(); - EXPECT_EQ(encrypted_types, GetEncryptedDataTypes(&trans)); - - Syncer::UnsyncedMetaHandles handles; - SyncerUtil::GetUnsyncedEntries(&trans, &handles); - EXPECT_EQ(2*batch_s+1, handles.size()); - } } } // namespace browser_sync diff --git a/chrome/browser/sync/engine/change_reorder_buffer.h b/chrome/browser/sync/engine/change_reorder_buffer.h index 4e0fd82..e2e091a 100644 --- a/chrome/browser/sync/engine/change_reorder_buffer.h +++ b/chrome/browser/sync/engine/change_reorder_buffer.h @@ -39,7 +39,7 @@ namespace sync_api { class ChangeReorderBuffer { public: typedef SyncManager::ChangeRecord ChangeRecord; - typedef SyncManager::ExtraPasswordChangeRecordData ExtraChangeRecordData; + typedef SyncManager::ExtraChangeRecordData ExtraChangeRecordData; ChangeReorderBuffer(); ~ChangeReorderBuffer(); diff --git a/chrome/browser/sync/engine/syncapi.cc b/chrome/browser/sync/engine/syncapi.cc index a90b658..018c5cf 100644 --- a/chrome/browser/sync/engine/syncapi.cc +++ b/chrome/browser/sync/engine/syncapi.cc @@ -4,11 +4,9 @@ #include "chrome/browser/sync/engine/syncapi.h" -#include <algorithm> #include <bitset> #include <iomanip> #include <list> -#include <queue> #include <string> #include <vector> @@ -54,7 +52,6 @@ #include "chrome/browser/sync/sessions/sync_session_context.h" #include "chrome/browser/sync/syncable/autofill_migration.h" #include "chrome/browser/sync/syncable/directory_manager.h" -#include "chrome/browser/sync/syncable/nigori_util.h" #include "chrome/browser/sync/syncable/syncable.h" #include "chrome/browser/sync/util/crypto_helpers.h" #include "chrome/common/deprecated/event_sys.h" @@ -195,11 +192,8 @@ sync_pb::PasswordSpecificsData* DecryptPasswordSpecifics( const sync_pb::EntitySpecifics& specifics, Cryptographer* crypto) { if (!specifics.HasExtension(sync_pb::password)) return NULL; - const sync_pb::PasswordSpecifics& password_specifics = - specifics.GetExtension(sync_pb::password); - if (!password_specifics.has_encrypted()) - return NULL; - const sync_pb::EncryptedData& encrypted = password_specifics.encrypted(); + const sync_pb::EncryptedData& encrypted = + specifics.GetExtension(sync_pb::password).encrypted(); scoped_ptr<sync_pb::PasswordSpecificsData> data( new sync_pb::PasswordSpecificsData); if (!crypto->Decrypt(encrypted, data.get())) @@ -208,51 +202,19 @@ sync_pb::PasswordSpecificsData* DecryptPasswordSpecifics( } bool BaseNode::DecryptIfNecessary(Entry* entry) { - if (GetIsFolder()) return true; // Ignore the top-level datatype folder. + if (GetIsFolder()) return true; // Ignore the top-level password folder. const sync_pb::EntitySpecifics& specifics = entry->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()) 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)) { - LOG(ERROR) << "Failed to decrypt encrypted node of type " << - syncable::ModelTypeToString(entry->GetModelType()) << "."; - return false; } return true; } -const sync_pb::EntitySpecifics& BaseNode::GetUnencryptedSpecifics( - const syncable::Entry* entry) const { - const sync_pb::EntitySpecifics& specifics = entry->Get(SPECIFICS); - if (specifics.has_encrypted()) { - DCHECK(syncable::GetModelTypeFromSpecifics(unencrypted_data_) != - syncable::UNSPECIFIED); - return unencrypted_data_; - } else { - DCHECK(syncable::GetModelTypeFromSpecifics(unencrypted_data_) == - syncable::UNSPECIFIED); - return specifics; - } -} - int64 BaseNode::GetParentId() const { return IdToMetahandle(GetTransaction()->GetWrappedTrans(), GetEntry()->Get(syncable::PARENT_ID)); @@ -354,79 +316,59 @@ int64 BaseNode::GetExternalId() const { } const sync_pb::AppSpecifics& BaseNode::GetAppSpecifics() const { - DCHECK_EQ(syncable::APPS, GetModelType()); - const sync_pb::EntitySpecifics& unencrypted = - GetUnencryptedSpecifics(GetEntry()); - return unencrypted.GetExtension(sync_pb::app); + DCHECK(GetModelType() == syncable::APPS); + return GetEntry()->Get(SPECIFICS).GetExtension(sync_pb::app); } const sync_pb::AutofillSpecifics& BaseNode::GetAutofillSpecifics() const { - DCHECK_EQ(syncable::AUTOFILL, GetModelType()); - const sync_pb::EntitySpecifics& unencrypted = - GetUnencryptedSpecifics(GetEntry()); - return unencrypted.GetExtension(sync_pb::autofill); + DCHECK(GetModelType() == syncable::AUTOFILL); + return GetEntry()->Get(SPECIFICS).GetExtension(sync_pb::autofill); } const AutofillProfileSpecifics& BaseNode::GetAutofillProfileSpecifics() const { DCHECK_EQ(GetModelType(), syncable::AUTOFILL_PROFILE); - const sync_pb::EntitySpecifics& unencrypted = - GetUnencryptedSpecifics(GetEntry()); - return unencrypted.GetExtension(sync_pb::autofill_profile); + return GetEntry()->Get(SPECIFICS).GetExtension(sync_pb::autofill_profile); } const sync_pb::BookmarkSpecifics& BaseNode::GetBookmarkSpecifics() const { - DCHECK_EQ(syncable::BOOKMARKS, GetModelType()); - const sync_pb::EntitySpecifics& unencrypted = - GetUnencryptedSpecifics(GetEntry()); - return unencrypted.GetExtension(sync_pb::bookmark); + DCHECK(GetModelType() == syncable::BOOKMARKS); + return GetEntry()->Get(SPECIFICS).GetExtension(sync_pb::bookmark); } const sync_pb::NigoriSpecifics& BaseNode::GetNigoriSpecifics() const { - DCHECK_EQ(syncable::NIGORI, GetModelType()); - const sync_pb::EntitySpecifics& unencrypted = - GetUnencryptedSpecifics(GetEntry()); - return unencrypted.GetExtension(sync_pb::nigori); + DCHECK(GetModelType() == syncable::NIGORI); + return GetEntry()->Get(SPECIFICS).GetExtension(sync_pb::nigori); } const sync_pb::PasswordSpecificsData& BaseNode::GetPasswordSpecifics() const { - DCHECK_EQ(syncable::PASSWORDS, GetModelType()); + DCHECK(GetModelType() == syncable::PASSWORDS); DCHECK(password_data_.get()); return *password_data_; } const sync_pb::PreferenceSpecifics& BaseNode::GetPreferenceSpecifics() const { - DCHECK_EQ(syncable::PREFERENCES, GetModelType()); - const sync_pb::EntitySpecifics& unencrypted = - GetUnencryptedSpecifics(GetEntry()); - return unencrypted.GetExtension(sync_pb::preference); + DCHECK(GetModelType() == syncable::PREFERENCES); + return GetEntry()->Get(SPECIFICS).GetExtension(sync_pb::preference); } const sync_pb::ThemeSpecifics& BaseNode::GetThemeSpecifics() const { - DCHECK_EQ(syncable::THEMES, GetModelType()); - const sync_pb::EntitySpecifics& unencrypted = - GetUnencryptedSpecifics(GetEntry()); - return unencrypted.GetExtension(sync_pb::theme); + DCHECK(GetModelType() == syncable::THEMES); + return GetEntry()->Get(SPECIFICS).GetExtension(sync_pb::theme); } const sync_pb::TypedUrlSpecifics& BaseNode::GetTypedUrlSpecifics() const { - DCHECK_EQ(syncable::TYPED_URLS, GetModelType()); - const sync_pb::EntitySpecifics& unencrypted = - GetUnencryptedSpecifics(GetEntry()); - return unencrypted.GetExtension(sync_pb::typed_url); + DCHECK(GetModelType() == syncable::TYPED_URLS); + return GetEntry()->Get(SPECIFICS).GetExtension(sync_pb::typed_url); } const sync_pb::ExtensionSpecifics& BaseNode::GetExtensionSpecifics() const { - DCHECK_EQ(syncable::EXTENSIONS, GetModelType()); - const sync_pb::EntitySpecifics& unencrypted = - GetUnencryptedSpecifics(GetEntry()); - return unencrypted.GetExtension(sync_pb::extension); + DCHECK(GetModelType() == syncable::EXTENSIONS); + return GetEntry()->Get(SPECIFICS).GetExtension(sync_pb::extension); } const sync_pb::SessionSpecifics& BaseNode::GetSessionSpecifics() const { - DCHECK_EQ(syncable::SESSIONS, GetModelType()); - const sync_pb::EntitySpecifics& unencrypted = - GetUnencryptedSpecifics(GetEntry()); - return unencrypted.GetExtension(sync_pb::session); + DCHECK(GetModelType() == syncable::SESSIONS); + return GetEntry()->Get(SPECIFICS).GetExtension(sync_pb::session); } syncable::ModelType BaseNode::GetModelType() const { @@ -435,40 +377,6 @@ syncable::ModelType BaseNode::GetModelType() const { //////////////////////////////////// // WriteNode member definitions -void WriteNode::EncryptIfNecessary(sync_pb::EntitySpecifics* unencrypted) { - syncable::ModelType type = syncable::GetModelTypeFromSpecifics(*unencrypted); - DCHECK_NE(type, syncable::UNSPECIFIED); - DCHECK_NE(type, syncable::PASSWORDS); // Passwords use their own encryption. - DCHECK_NE(type, syncable::NIGORI); // Nigori is encrypted separately. - - syncable::ModelTypeSet encrypted_types = - GetEncryptedDataTypes(GetTransaction()->GetWrappedTrans()); - if (encrypted_types.count(type) == 0) { - // This datatype does not require encryption. - return; - } - - if (unencrypted->has_encrypted()) { - // This specifics is already encrypted, our work is done. - LOG(WARNING) << "Attempted to encrypt an already encrypted entity" - << " specifics of type " << syncable::ModelTypeToString(type) - << ". Dropping."; - return; - } - sync_pb::EntitySpecifics encrypted; - syncable::AddDefaultExtensionValue(type, &encrypted); - VLOG(2) << "Encrypted specifics of type " << syncable::ModelTypeToString(type) - << " with content: " << unencrypted->SerializeAsString() << "\n"; - if (!GetTransaction()->GetCryptographer()->Encrypt( - *unencrypted, - encrypted.mutable_encrypted())) { - LOG(ERROR) << "Could not encrypt data for node of type " << - syncable::ModelTypeToString(type); - NOTREACHED(); - } - unencrypted->CopyFrom(encrypted); -} - void WriteNode::SetIsFolder(bool folder) { if (entry_->Get(syncable::IS_DIR) == folder) return; // Skip redundant changes. @@ -498,13 +406,13 @@ void WriteNode::SetURL(const GURL& url) { void WriteNode::SetAppSpecifics( const sync_pb::AppSpecifics& new_value) { - DCHECK_EQ(syncable::APPS, GetModelType()); + DCHECK(GetModelType() == syncable::APPS); PutAppSpecificsAndMarkForSyncing(new_value); } void WriteNode::SetAutofillSpecifics( const sync_pb::AutofillSpecifics& new_value) { - DCHECK_EQ(syncable::AUTOFILL, GetModelType()); + DCHECK(GetModelType() == syncable::AUTOFILL); PutAutofillSpecificsAndMarkForSyncing(new_value); } @@ -512,7 +420,6 @@ void WriteNode::PutAutofillSpecificsAndMarkForSyncing( const sync_pb::AutofillSpecifics& new_value) { sync_pb::EntitySpecifics entity_specifics; entity_specifics.MutableExtension(sync_pb::autofill)->CopyFrom(new_value); - EncryptIfNecessary(&entity_specifics); PutSpecificsAndMarkForSyncing(entity_specifics); } @@ -527,13 +434,12 @@ void WriteNode::PutAutofillProfileSpecificsAndMarkForSyncing( sync_pb::EntitySpecifics entity_specifics; entity_specifics.MutableExtension(sync_pb::autofill_profile)->CopyFrom( new_value); - EncryptIfNecessary(&entity_specifics); PutSpecificsAndMarkForSyncing(entity_specifics); } void WriteNode::SetBookmarkSpecifics( const sync_pb::BookmarkSpecifics& new_value) { - DCHECK_EQ(syncable::BOOKMARKS, GetModelType()); + DCHECK(GetModelType() == syncable::BOOKMARKS); PutBookmarkSpecificsAndMarkForSyncing(new_value); } @@ -541,13 +447,12 @@ void WriteNode::PutBookmarkSpecificsAndMarkForSyncing( const sync_pb::BookmarkSpecifics& new_value) { sync_pb::EntitySpecifics entity_specifics; entity_specifics.MutableExtension(sync_pb::bookmark)->CopyFrom(new_value); - EncryptIfNecessary(&entity_specifics); PutSpecificsAndMarkForSyncing(entity_specifics); } void WriteNode::SetNigoriSpecifics( const sync_pb::NigoriSpecifics& new_value) { - DCHECK_EQ(syncable::NIGORI, GetModelType()); + DCHECK(GetModelType() == syncable::NIGORI); PutNigoriSpecificsAndMarkForSyncing(new_value); } @@ -560,40 +465,36 @@ void WriteNode::PutNigoriSpecificsAndMarkForSyncing( void WriteNode::SetPasswordSpecifics( const sync_pb::PasswordSpecificsData& data) { - DCHECK_EQ(syncable::PASSWORDS, GetModelType()); + DCHECK(GetModelType() == syncable::PASSWORDS); + sync_pb::PasswordSpecifics new_value; if (!GetTransaction()->GetCryptographer()->Encrypt( data, new_value.mutable_encrypted())) { NOTREACHED(); } + PutPasswordSpecificsAndMarkForSyncing(new_value); } void WriteNode::SetPreferenceSpecifics( const sync_pb::PreferenceSpecifics& new_value) { - DCHECK_EQ(syncable::PREFERENCES, GetModelType()); + DCHECK(GetModelType() == syncable::PREFERENCES); PutPreferenceSpecificsAndMarkForSyncing(new_value); } void WriteNode::SetThemeSpecifics( const sync_pb::ThemeSpecifics& new_value) { - DCHECK_EQ(syncable::THEMES, GetModelType()); + DCHECK(GetModelType() == syncable::THEMES); PutThemeSpecificsAndMarkForSyncing(new_value); } void WriteNode::SetSessionSpecifics( const sync_pb::SessionSpecifics& new_value) { - DCHECK_EQ(syncable::SESSIONS, GetModelType()); + DCHECK(GetModelType() == syncable::SESSIONS); PutSessionSpecificsAndMarkForSyncing(new_value); } -void WriteNode::ResetFromSpecifics() { - sync_pb::EntitySpecifics new_data; - new_data.CopyFrom(GetUnencryptedSpecifics(GetEntry())); - EncryptIfNecessary(&new_data); - PutSpecificsAndMarkForSyncing(new_data); -} void WriteNode::PutPasswordSpecificsAndMarkForSyncing( const sync_pb::PasswordSpecifics& new_value) { @@ -606,19 +507,18 @@ void WriteNode::PutPreferenceSpecificsAndMarkForSyncing( const sync_pb::PreferenceSpecifics& new_value) { sync_pb::EntitySpecifics entity_specifics; entity_specifics.MutableExtension(sync_pb::preference)->CopyFrom(new_value); - EncryptIfNecessary(&entity_specifics); PutSpecificsAndMarkForSyncing(entity_specifics); } void WriteNode::SetTypedUrlSpecifics( const sync_pb::TypedUrlSpecifics& new_value) { - DCHECK_EQ(syncable::TYPED_URLS, GetModelType()); + DCHECK(GetModelType() == syncable::TYPED_URLS); PutTypedUrlSpecificsAndMarkForSyncing(new_value); } void WriteNode::SetExtensionSpecifics( const sync_pb::ExtensionSpecifics& new_value) { - DCHECK_EQ(syncable::EXTENSIONS, GetModelType()); + DCHECK(GetModelType() == syncable::EXTENSIONS); PutExtensionSpecificsAndMarkForSyncing(new_value); } @@ -626,7 +526,6 @@ void WriteNode::PutAppSpecificsAndMarkForSyncing( const sync_pb::AppSpecifics& new_value) { sync_pb::EntitySpecifics entity_specifics; entity_specifics.MutableExtension(sync_pb::app)->CopyFrom(new_value); - EncryptIfNecessary(&entity_specifics); PutSpecificsAndMarkForSyncing(entity_specifics); } @@ -634,7 +533,6 @@ void WriteNode::PutThemeSpecificsAndMarkForSyncing( const sync_pb::ThemeSpecifics& new_value) { sync_pb::EntitySpecifics entity_specifics; entity_specifics.MutableExtension(sync_pb::theme)->CopyFrom(new_value); - EncryptIfNecessary(&entity_specifics); PutSpecificsAndMarkForSyncing(entity_specifics); } @@ -642,7 +540,6 @@ void WriteNode::PutTypedUrlSpecificsAndMarkForSyncing( const sync_pb::TypedUrlSpecifics& new_value) { sync_pb::EntitySpecifics entity_specifics; entity_specifics.MutableExtension(sync_pb::typed_url)->CopyFrom(new_value); - EncryptIfNecessary(&entity_specifics); PutSpecificsAndMarkForSyncing(entity_specifics); } @@ -650,18 +547,18 @@ void WriteNode::PutExtensionSpecificsAndMarkForSyncing( const sync_pb::ExtensionSpecifics& new_value) { sync_pb::EntitySpecifics entity_specifics; entity_specifics.MutableExtension(sync_pb::extension)->CopyFrom(new_value); - EncryptIfNecessary(&entity_specifics); PutSpecificsAndMarkForSyncing(entity_specifics); } + void WriteNode::PutSessionSpecificsAndMarkForSyncing( const sync_pb::SessionSpecifics& new_value) { sync_pb::EntitySpecifics entity_specifics; entity_specifics.MutableExtension(sync_pb::session)->CopyFrom(new_value); - EncryptIfNecessary(&entity_specifics); PutSpecificsAndMarkForSyncing(entity_specifics); } + void WriteNode::PutSpecificsAndMarkForSyncing( const sync_pb::EntitySpecifics& specifics) { // Skip redundant changes. @@ -726,7 +623,7 @@ bool WriteNode::InitByTagLookup(const std::string& tag) { if (entry_->Get(syncable::IS_DEL)) return false; syncable::ModelType model_type = GetModelType(); - DCHECK_EQ(syncable::NIGORI, model_type); + DCHECK(model_type == syncable::NIGORI); return true; } @@ -739,7 +636,7 @@ void WriteNode::PutModelType(syncable::ModelType model_type) { sync_pb::EntitySpecifics specifics; syncable::AddDefaultExtensionValue(model_type, &specifics); PutSpecificsAndMarkForSyncing(specifics); - DCHECK_EQ(model_type, GetModelType()); + DCHECK(GetModelType() == model_type); } // Create a new node with default properties, and bind this WriteNode to it. @@ -1037,6 +934,8 @@ syncable::BaseTransaction* WriteTransaction::GetWrappedTrans() const { return transaction_; } +SyncManager::ExtraChangeRecordData::~ExtraChangeRecordData() {} + SyncManager::ChangeRecord::ChangeRecord() : id(kInvalidId), action(ACTION_ADD) {} @@ -1086,12 +985,9 @@ DictionaryValue* SyncManager::ChangeRecord::ToValue( return value; } -SyncManager::ExtraPasswordChangeRecordData::ExtraPasswordChangeRecordData() {} - SyncManager::ExtraPasswordChangeRecordData::ExtraPasswordChangeRecordData( const sync_pb::PasswordSpecificsData& data) - : unencrypted_(data) { -} + : unencrypted_(data) {} SyncManager::ExtraPasswordChangeRecordData::~ExtraPasswordChangeRecordData() {} @@ -1164,9 +1060,6 @@ class SyncManager::SyncInternal // Whether or not the Nigori node is encrypted using an explicit passphrase. bool IsUsingExplicitPassphrase(); - // 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. @@ -1242,9 +1135,6 @@ class SyncManager::SyncInternal return initialized_; } - // If this is a deletion for a password, sets the legacy - // ExtraPasswordChangeRecordData field of |buffer|. Otherwise sets - // |buffer|'s specifics field to contain the unencrypted data. void SetExtraChangeRecordData(int64 id, syncable::ModelType type, ChangeReorderBuffer* buffer, @@ -1370,8 +1260,7 @@ class SyncManager::SyncInternal // differ between the versions of an entry stored in |a| and |b|. A return // value of false means that it should be OK to ignore this change. static bool VisiblePropertiesDiffer(const syncable::EntryKernel& a, - const syncable::Entry& b, - Cryptographer* cryptographer) { + const syncable::Entry& b) { syncable::ModelType model_type = b.GetModelType(); // Suppress updates to items that aren't tracked by any browser model. if (model_type == syncable::UNSPECIFIED || @@ -1382,21 +1271,8 @@ class SyncManager::SyncInternal return true; if (a.ref(syncable::IS_DIR) != b.Get(syncable::IS_DIR)) return true; - // Check if data has changed (account for encryption). - std::string a_str, b_str; - if (a.ref(SPECIFICS).has_encrypted()) { - const sync_pb::EncryptedData& encrypted = a.ref(SPECIFICS).encrypted(); - a_str = cryptographer->DecryptToString(encrypted); - } else { - a_str = a.ref(SPECIFICS).SerializeAsString(); - } - if (b.Get(SPECIFICS).has_encrypted()) { - const sync_pb::EncryptedData& encrypted = b.Get(SPECIFICS).encrypted(); - b_str = cryptographer->DecryptToString(encrypted); - } else { - b_str = b.Get(SPECIFICS).SerializeAsString(); - } - if (a_str != b_str) { + if (a.ref(SPECIFICS).SerializeAsString() != + b.Get(SPECIFICS).SerializeAsString()) { return true; } if (VisiblePositionsDiffer(a, b)) @@ -1601,11 +1477,6 @@ void SyncManager::SetPassphrase(const std::string& passphrase, data_->SetPassphrase(passphrase, is_explicit); } -void SyncManager::EncryptDataTypes( - const syncable::ModelTypeSet& encrypted_types) { - data_->EncryptDataTypes(encrypted_types); -} - bool SyncManager::IsUsingExplicitPassphrase() { return data_ && data_->IsUsingExplicitPassphrase(); } @@ -1711,33 +1582,23 @@ void SyncManager::SyncInternal::BootstrapEncryption( Cryptographer* cryptographer = share_.dir_manager->cryptographer(); cryptographer->Bootstrap(restored_key_for_bootstrapping); - sync_pb::NigoriSpecifics nigori; - { - ReadTransaction trans(GetUserShare()); - ReadNode node(&trans); - if (!node.InitByTagLookup(kNigoriTag)) { - NOTREACHED(); - return; - } + ReadTransaction trans(GetUserShare()); + ReadNode node(&trans); + if (!node.InitByTagLookup(kNigoriTag)) { + NOTREACHED(); + return; + } - nigori.CopyFrom(node.GetNigoriSpecifics()); - if (!nigori.encrypted().blob().empty()) { - if (cryptographer->CanDecrypt(nigori.encrypted())) { - cryptographer->SetKeys(nigori.encrypted()); - } else { - cryptographer->SetPendingKeys(nigori.encrypted()); - FOR_EACH_OBSERVER(SyncManager::Observer, observers_, - OnPassphraseRequired(true)); - } + const sync_pb::NigoriSpecifics& nigori = node.GetNigoriSpecifics(); + if (!nigori.encrypted().blob().empty()) { + if (cryptographer->CanDecrypt(nigori.encrypted())) { + cryptographer->SetKeys(nigori.encrypted()); + } else { + cryptographer->SetPendingKeys(nigori.encrypted()); + FOR_EACH_OBSERVER(SyncManager::Observer, observers_, + OnPassphraseRequired(true)); } } - - // Refresh list of encrypted datatypes. - syncable::ModelTypeSet encrypted_types = - syncable::GetEncryptedDataTypesFromNigori(nigori); - - // Ensure any datatypes that need encryption are encrypted. - EncryptDataTypes(encrypted_types); } void SyncManager::SyncInternal::StartSyncing() { @@ -1848,7 +1709,7 @@ bool SyncManager::SyncInternal::SignIn(const SyncCredentials& credentials) { void SyncManager::SyncInternal::UpdateCredentials( const SyncCredentials& credentials) { DCHECK_EQ(MessageLoop::current(), core_message_loop_); - DCHECK_EQ(credentials.email, share_.name); + DCHECK(share_.name == credentials.email); connection_manager()->set_auth_token(credentials.sync_token); TalkMediatorLogin(credentials.email, credentials.sync_token); CheckServerReachable(); @@ -1942,8 +1803,8 @@ void SyncManager::SyncInternal::SetPassphrase( if (is_explicit) SetUsingExplicitPassphrasePrefForMigration(); - // Nudge the syncer so that encrypted datatype updates that were waiting for - // this passphrase get applied as soon as possible. + // Nudge the syncer so that passwords updates that were waiting for this + // passphrase get applied as soon as possible. sync_manager_->RequestNudge(); } else { WriteTransaction trans(GetUserShare()); @@ -1965,8 +1826,7 @@ void SyncManager::SyncInternal::SetPassphrase( // messing with the Nigori node, because we can't call SetPassphrase until // download conditions are met vs Cryptographer init. It seems like it's // safe to defer this work. - sync_pb::NigoriSpecifics specifics(node.GetNigoriSpecifics()); - specifics.clear_encrypted(); + sync_pb::NigoriSpecifics specifics; cryptographer->GetKeys(specifics.mutable_encrypted()); specifics.set_using_explicit_passphrase(is_explicit); node.SetNigoriSpecifics(specifics); @@ -1991,109 +1851,28 @@ bool SyncManager::SyncInternal::IsUsingExplicitPassphrase() { return node.GetNigoriSpecifics().using_explicit_passphrase(); } -void SyncManager::SyncInternal::EncryptDataTypes( - const syncable::ModelTypeSet& encrypted_types) { - // Verify the encrypted types are all enabled. - ModelSafeRoutingInfo routes; - registrar_->GetModelSafeRoutingInfo(&routes); - for (syncable::ModelTypeSet::const_iterator iter = encrypted_types.begin(); - iter != encrypted_types.end(); ++iter) { - if (routes.count(*iter) == 0) { - LOG(WARNING) << "Attempted to encrypt non-enabled datatype " - << syncable::ModelTypeToString(*iter) << ", dropping type."; - routes.erase(*iter); - } - } - - WriteTransaction trans(GetUserShare()); - WriteNode node(&trans); - if (!node.InitByTagLookup(kNigoriTag)) { - LOG(ERROR) << "Unable to set encrypted datatypes because Nigori node not " - << "found."; - NOTREACHED(); +void SyncManager::SyncInternal::ReEncryptEverything(WriteTransaction* trans) { + // TODO(tim): bug 59242. We shouldn't lookup by data type and instead use + // a protocol flag or existence of an EncryptedData message, but for now, + // encryption is on if-and-only-if the type is passwords, and we haven't + // ironed out the protocol for generic encryption. + static const char* passwords_tag = "google_chrome_passwords"; + ReadNode passwords_root(trans); + if (!passwords_root.InitByTagLookup(passwords_tag)) { + LOG(WARNING) << "No passwords to reencrypt."; return; } - // Update the Nigori node set of encrypted datatypes so other machines notice. - sync_pb::NigoriSpecifics nigori; - nigori.CopyFrom(node.GetNigoriSpecifics()); - syncable::FillNigoriEncryptedTypes(encrypted_types, &nigori); - node.SetNigoriSpecifics(nigori); - - // TODO(zea): only reencrypt this datatype? ReEncrypting everything is a - // safer approach, and should not impact anything that is already encrypted - // (redundant changes are ignored). - ReEncryptEverything(&trans); - return; -} - -void SyncManager::SyncInternal::ReEncryptEverything(WriteTransaction* trans) { - syncable::ModelTypeSet encrypted_types = - GetEncryptedDataTypes(trans->GetWrappedTrans()); - ModelSafeRoutingInfo routes; - registrar_->GetModelSafeRoutingInfo(&routes); - std::string tag; - for (syncable::ModelTypeSet::iterator iter = encrypted_types.begin(); - iter != encrypted_types.end(); ++iter) { - if (*iter == syncable::PASSWORDS || routes.count(*iter) == 0) - continue; - ReadNode type_root(trans); - tag = syncable::ModelTypeToRootTag(*iter); - if (!type_root.InitByTagLookup(tag)) { + int64 child_id = passwords_root.GetFirstChildId(); + while (child_id != kInvalidId) { + WriteNode child(trans); + if (!child.InitByIdLookup(child_id)) { NOTREACHED(); return; } - - // Iterate through all children of this datatype. - std::queue<int64> to_visit; - int64 child_id = type_root.GetFirstChildId(); - to_visit.push(child_id); - while (!to_visit.empty()) { - child_id = to_visit.front(); - to_visit.pop(); - if (child_id == kInvalidId) - continue; - - WriteNode child(trans); - if (!child.InitByIdLookup(child_id)) { - NOTREACHED(); - return; - } - if (child.GetIsFolder()) { - to_visit.push(child.GetFirstChildId()); - } else { - // Rewrite the specifics of the node with encrypted data if necessary. - child.ResetFromSpecifics(); - } - to_visit.push(child.GetSuccessorId()); - } + child.SetPasswordSpecifics(child.GetPasswordSpecifics()); + child_id = child.GetSuccessorId(); } - - if (routes.count(syncable::PASSWORDS) > 0) { - // Passwords are encrypted with their own legacy scheme. - encrypted_types.insert(syncable::PASSWORDS); - ReadNode passwords_root(trans); - std::string passwords_tag = - syncable::ModelTypeToRootTag(syncable::PASSWORDS); - if (!passwords_root.InitByTagLookup(passwords_tag)) { - LOG(WARNING) << "No passwords to reencrypt."; - return; - } - - int64 child_id = passwords_root.GetFirstChildId(); - while (child_id != kInvalidId) { - WriteNode child(trans); - if (!child.InitByIdLookup(child_id)) { - NOTREACHED(); - return; - } - child.SetPasswordSpecifics(child.GetPasswordSpecifics()); - child_id = child.GetSuccessorId(); - } - } - - FOR_EACH_OBSERVER(SyncManager::Observer, observers_, - OnEncryptionComplete(encrypted_types)); } SyncManager::~SyncManager() { @@ -2341,29 +2120,20 @@ void SyncManager::SyncInternal::SetExtraChangeRecordData(int64 id, syncable::ModelType type, ChangeReorderBuffer* buffer, Cryptographer* cryptographer, const syncable::EntryKernel& original, bool existed_before, bool exists_now) { - // If this is a deletion and the datatype was encrypted, we need to decrypt it - // and attach it to the buffer. + // If this is a deletion, attach the entity specifics as extra data + // so that the delete can be processed. if (!exists_now && existed_before) { - sync_pb::EntitySpecifics original_specifics(original.ref(SPECIFICS)); + buffer->SetSpecificsForId(id, original.ref(SPECIFICS)); if (type == syncable::PASSWORDS) { - // Passwords must use their own legacy ExtraPasswordChangeRecordData. + // Need to dig a bit deeper as passwords are encrypted. scoped_ptr<sync_pb::PasswordSpecificsData> data( - DecryptPasswordSpecifics(original_specifics, cryptographer)); + DecryptPasswordSpecifics(original.ref(SPECIFICS), cryptographer)); if (!data.get()) { NOTREACHED(); return; } buffer->SetExtraDataForId(id, new ExtraPasswordChangeRecordData(*data)); - } else if (original_specifics.has_encrypted()) { - // All other datatypes can just create a new unencrypted specifics and - // attach it. - const sync_pb::EncryptedData& encrypted = original_specifics.encrypted(); - if (!cryptographer->Decrypt(encrypted, &original_specifics)) { - NOTREACHED(); - return; - } } - buffer->SetSpecificsForId(id, original_specifics); } } @@ -2394,10 +2164,8 @@ void SyncManager::SyncInternal::HandleCalculateChangesChangeEventFromSyncer( change_buffers_[type].PushAddedItem(id); else if (!exists_now && existed_before) change_buffers_[type].PushDeletedItem(id); - else if (exists_now && existed_before && - VisiblePropertiesDiffer(*i, e, dir_manager()->cryptographer())) { + else if (exists_now && existed_before && VisiblePropertiesDiffer(*i, e)) change_buffers_[type].PushUpdatedItem(id, VisiblePositionsDiffer(*i, e)); - } SetExtraChangeRecordData(id, type, &change_buffers_[type], dir_manager()->cryptographer(), *i, @@ -2424,44 +2192,32 @@ void SyncManager::SyncInternal::OnSyncEngineEvent( if (event.what_happened == SyncEngineEvent::SYNC_CYCLE_ENDED) { ModelSafeRoutingInfo enabled_types; registrar_->GetModelSafeRoutingInfo(&enabled_types); - { - // Check to see if we need to notify the frontend that we have newly - // encrypted types or that we require a passphrase. - sync_api::ReadTransaction trans(GetUserShare()); - sync_api::ReadNode node(&trans); - if (!node.InitByTagLookup(kNigoriTag)) { - DCHECK(!event.snapshot->is_share_usable); - return; - } - const sync_pb::NigoriSpecifics& nigori = node.GetNigoriSpecifics(); - syncable::ModelTypeSet encrypted_types = - syncable::GetEncryptedDataTypesFromNigori(nigori); - // If passwords are enabled, they're automatically considered encrypted. - if (enabled_types.count(syncable::PASSWORDS) > 0) - encrypted_types.insert(syncable::PASSWORDS); - if (encrypted_types.size() > 0) { - Cryptographer* cryptographer = - GetUserShare()->dir_manager->cryptographer(); - if (!cryptographer->is_ready() && !cryptographer->has_pending_keys()) { - if (!nigori.encrypted().blob().empty()) { - DCHECK(!cryptographer->CanDecrypt(nigori.encrypted())); - cryptographer->SetPendingKeys(nigori.encrypted()); - } + if (enabled_types.count(syncable::PASSWORDS) > 0) { + Cryptographer* cryptographer = + GetUserShare()->dir_manager->cryptographer(); + if (!cryptographer->is_ready() && !cryptographer->has_pending_keys()) { + sync_api::ReadTransaction trans(GetUserShare()); + sync_api::ReadNode node(&trans); + if (!node.InitByTagLookup(kNigoriTag)) { + DCHECK(!event.snapshot->is_share_usable); + return; } - - // If we've completed a sync cycle and the cryptographer isn't ready - // yet, prompt the user for a passphrase. - if (cryptographer->has_pending_keys()) { - FOR_EACH_OBSERVER(SyncManager::Observer, observers_, - OnPassphraseRequired(true)); - } else if (!cryptographer->is_ready()) { - FOR_EACH_OBSERVER(SyncManager::Observer, observers_, - OnPassphraseRequired(false)); - } else { - FOR_EACH_OBSERVER(SyncManager::Observer, observers_, - OnEncryptionComplete(encrypted_types)); + const sync_pb::NigoriSpecifics& nigori = node.GetNigoriSpecifics(); + if (!nigori.encrypted().blob().empty()) { + DCHECK(!cryptographer->CanDecrypt(nigori.encrypted())); + cryptographer->SetPendingKeys(nigori.encrypted()); } } + + // If we've completed a sync cycle and the cryptographer isn't ready yet, + // prompt the user for a passphrase. + if (cryptographer->has_pending_keys()) { + FOR_EACH_OBSERVER(SyncManager::Observer, observers_, + OnPassphraseRequired(true)); + } else if (!cryptographer->is_ready()) { + FOR_EACH_OBSERVER(SyncManager::Observer, observers_, + OnPassphraseRequired(false)); + } } if (!initialized()) diff --git a/chrome/browser/sync/engine/syncapi.h b/chrome/browser/sync/engine/syncapi.h index abc91d8..35207b6 100644 --- a/chrome/browser/sync/engine/syncapi.h +++ b/chrome/browser/sync/engine/syncapi.h @@ -266,26 +266,14 @@ class BaseNode { // Determines whether part of the entry is encrypted, and if so attempts to // decrypt it. Unless decryption is necessary and fails, this will always - // return |true|. If the contents are encrypted, the decrypted data will be - // stored in |unencrypted_data_|. - // This method is invoked once when the BaseNode is initialized. + // return |true|. bool DecryptIfNecessary(syncable::Entry* entry); - // Returns the unencrypted specifics associated with |entry|. If |entry| was - // not encrypted, it directly returns |entry|'s EntitySpecifics. Otherwise, - // returns |unencrypted_data_|. - // This method is invoked by the datatype specific Get<datatype>Specifics - // methods. - const sync_pb::EntitySpecifics& GetUnencryptedSpecifics( - const syncable::Entry* entry) const; - private: void* operator new(size_t size); // Node is meant for stack use only. - // A holder for the unencrypted data stored in an encrypted node. - sync_pb::EntitySpecifics unencrypted_data_; - - // Same as |unencrypted_data_|, but for legacy password encryption. + // If this node represents a password, this field will hold the actual + // decrypted password data. scoped_ptr<sync_pb::PasswordSpecificsData> password_data_; friend class SyncApiTest; @@ -400,10 +388,6 @@ class WriteNode : public BaseNode { // Should only be called if GetModelType() == SESSIONS. void SetSessionSpecifics(const sync_pb::SessionSpecifics& specifics); - // Resets the EntitySpecifics for this node based on the unencrypted data. - // Will encrypt if necessary. - void ResetFromSpecifics(); - // Implementation of BaseNode's abstract virtual accessors. virtual const syncable::Entry* GetEntry() const; @@ -452,9 +436,6 @@ class WriteNode : public BaseNode { // upcoming commit pass. void MarkForSyncing(); - // Encrypt the specifics if the datatype requries it. - void EncryptIfNecessary(sync_pb::EntitySpecifics* new_value); - // The underlying syncable object which this class wraps. syncable::MutableEntry* entry_; @@ -606,21 +587,15 @@ class SyncManager { // internal types from clients of the interface. class SyncInternal; - // TODO(zea): One day get passwords playing nicely with the rest of encryption - // and get rid of this. - class ExtraPasswordChangeRecordData { + // TODO(tim): Depending on how multi-type encryption pans out, maybe we + // should turn ChangeRecord itself into a class. Or we could template this + // wrapper / add a templated method to return unencrypted protobufs. + class ExtraChangeRecordData { public: - ExtraPasswordChangeRecordData(); - explicit ExtraPasswordChangeRecordData( - const sync_pb::PasswordSpecificsData& data); - virtual ~ExtraPasswordChangeRecordData(); + virtual ~ExtraChangeRecordData(); // Transfers ownership of the DictionaryValue to the caller. - virtual DictionaryValue* ToValue() const; - - const sync_pb::PasswordSpecificsData& unencrypted() const; - private: - sync_pb::PasswordSpecificsData unencrypted_; + virtual DictionaryValue* ToValue() const = 0; }; // ChangeRecord indicates a single item that changed as a result of a sync @@ -642,7 +617,24 @@ class SyncManager { int64 id; Action action; sync_pb::EntitySpecifics specifics; - linked_ptr<ExtraPasswordChangeRecordData> extra; + linked_ptr<ExtraChangeRecordData> extra; + }; + + // Since PasswordSpecifics is just an encrypted blob, we extend to provide + // access to unencrypted bits. + class ExtraPasswordChangeRecordData : public ExtraChangeRecordData { + public: + explicit ExtraPasswordChangeRecordData( + const sync_pb::PasswordSpecificsData& data); + virtual ~ExtraPasswordChangeRecordData(); + + // Transfers ownership of the DictionaryValue to the caller. + virtual DictionaryValue* ToValue() const; + + const sync_pb::PasswordSpecificsData& unencrypted() const; + + private: + sync_pb::PasswordSpecificsData unencrypted_; }; // Status encapsulates detailed state about the internals of the SyncManager. @@ -809,14 +801,10 @@ class SyncManager { virtual void OnStopSyncingPermanently() = 0; // After a request to clear server data, these callbacks are invoked to - // indicate success or failure. + // indicate success or failure virtual void OnClearServerDataSucceeded() = 0; virtual void OnClearServerDataFailed() = 0; - // Called after we finish encrypting all appropriate datatypes. - virtual void OnEncryptionComplete( - const syncable::ModelTypeSet& encrypted_types) = 0; - protected: virtual ~Observer(); }; @@ -897,9 +885,6 @@ class SyncManager { // *not* override an explicit passphrase set previously. void SetPassphrase(const std::string& passphrase, bool is_explicit); - // Set the datatypes we want to encrypt and encrypt any nodes as necessary. - void EncryptDataTypes(const syncable::ModelTypeSet& encrypted_types); - // Requests the syncer thread to pause. The observer's OnPause // method will be called when the syncer thread is paused. Returns // false if the syncer thread can not be paused (e.g. if it is not diff --git a/chrome/browser/sync/engine/syncapi_unittest.cc b/chrome/browser/sync/engine/syncapi_unittest.cc index ff6d022..c2e3584 100644 --- a/chrome/browser/sync/engine/syncapi_unittest.cc +++ b/chrome/browser/sync/engine/syncapi_unittest.cc @@ -6,19 +6,14 @@ // functionality is provided by the Syncable layer, which has its own // unit tests. We'll test SyncApi specific things in this harness. -#include <map> - #include "base/basictypes.h" -#include "base/format_macros.h" #include "base/message_loop.h" #include "base/scoped_ptr.h" #include "base/scoped_temp_dir.h" #include "base/string_number_conversions.h" -#include "base/string_util.h" #include "base/utf_string_conversions.h" #include "base/values.h" #include "chrome/browser/browser_thread.h" -#include "chrome/browser/sync/engine/model_safe_worker.h" #include "chrome/browser/sync/engine/syncapi.h" #include "chrome/browser/sync/js_arg_list.h" #include "chrome/browser/sync/js_backend.h" @@ -27,30 +22,19 @@ #include "chrome/browser/sync/js_test_util.h" #include "chrome/browser/sync/protocol/password_specifics.pb.h" #include "chrome/browser/sync/protocol/proto_value_conversions.h" -#include "chrome/browser/sync/sessions/sync_session.h" #include "chrome/browser/sync/syncable/directory_manager.h" -#include "chrome/browser/sync/syncable/nigori_util.h" #include "chrome/browser/sync/syncable/syncable.h" -#include "chrome/browser/sync/syncable/syncable_id.h" -#include "chrome/browser/sync/util/cryptographer.h" #include "chrome/test/sync/engine/test_directory_setter_upper.h" #include "chrome/test/values_test_util.h" #include "jingle/notifier/base/notifier_options.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" -using browser_sync::Cryptographer; using browser_sync::HasArgsAsList; using browser_sync::KeyParams; using browser_sync::JsArgList; using browser_sync::MockJsEventHandler; using browser_sync::MockJsEventRouter; -using browser_sync::ModelSafeRoutingInfo; -using browser_sync::ModelSafeWorker; -using browser_sync::ModelSafeWorkerRegistrar; -using browser_sync::sessions::SyncSessionSnapshot; -using syncable::ModelType; -using syncable::ModelTypeSet; using test::ExpectDictionaryValue; using test::ExpectStringValue; using testing::_; @@ -74,7 +58,7 @@ void ExpectInt64Value(int64 expected_value, // Makes a non-folder child of the root node. Returns the id of the // newly-created node. int64 MakeNode(UserShare* share, - ModelType model_type, + syncable::ModelType model_type, const std::string& client_tag) { WriteTransaction trans(share); ReadNode root_node(&trans); @@ -85,80 +69,6 @@ int64 MakeNode(UserShare* share, return node.GetId(); } -// Make a folder as a child of the root node. Returns the id of the -// newly-created node. -int64 MakeFolder(UserShare* share, - syncable::ModelType model_type, - const std::string& client_tag) { - WriteTransaction trans(share); - ReadNode root_node(&trans); - root_node.InitByRootLookup(); - WriteNode node(&trans); - EXPECT_TRUE(node.InitUniqueByCreation(model_type, root_node, client_tag)); - node.SetIsFolder(true); - return node.GetId(); -} - -// Makes a non-folder child of a non-root node. Returns the id of the -// newly-created node. -int64 MakeNodeWithParent(UserShare* share, - ModelType model_type, - const std::string& client_tag, - int64 parent_id) { - WriteTransaction trans(share); - ReadNode parent_node(&trans); - parent_node.InitByIdLookup(parent_id); - WriteNode node(&trans); - EXPECT_TRUE(node.InitUniqueByCreation(model_type, parent_node, client_tag)); - node.SetIsFolder(false); - return node.GetId(); -} - -// Makes a folder child of a non-root node. Returns the id of the -// newly-created node. -int64 MakeFolderWithParent(UserShare* share, - ModelType model_type, - int64 parent_id, - BaseNode* predecessor) { - WriteTransaction trans(share); - ReadNode parent_node(&trans); - parent_node.InitByIdLookup(parent_id); - WriteNode node(&trans); - EXPECT_TRUE(node.InitByCreation(model_type, parent_node, predecessor)); - node.SetIsFolder(true); - return node.GetId(); -} - -// Creates the "synced" root node for a particular datatype. We use the syncable -// methods here so that the syncer treats these nodes as if they were already -// received from the server. -int64 MakeServerNodeForType(UserShare* share, - ModelType model_type) { - sync_pb::EntitySpecifics specifics; - syncable::AddDefaultExtensionValue(model_type, &specifics); - syncable::ScopedDirLookup dir(share->dir_manager.get(), share->name); - EXPECT_TRUE(dir.good()); - syncable::WriteTransaction trans(dir, syncable::UNITTEST, __FILE__, __LINE__); - // Attempt to lookup by nigori tag. - std::string type_tag = syncable::ModelTypeToRootTag(model_type); - syncable::Id node_id = syncable::Id::CreateFromServerId(type_tag); - syncable::MutableEntry entry(&trans, syncable::CREATE_NEW_UPDATE_ITEM, - node_id); - EXPECT_TRUE(entry.good()); - entry.Put(syncable::BASE_VERSION, 1); - entry.Put(syncable::SERVER_VERSION, 1); - entry.Put(syncable::IS_UNAPPLIED_UPDATE, false); - entry.Put(syncable::SERVER_PARENT_ID, syncable::kNullId); - entry.Put(syncable::SERVER_IS_DIR, true); - entry.Put(syncable::IS_DIR, true); - entry.Put(syncable::SERVER_SPECIFICS, specifics); - entry.Put(syncable::UNIQUE_SERVER_TAG, type_tag); - entry.Put(syncable::NON_UNIQUE_NAME, type_tag); - entry.Put(syncable::IS_DEL, false); - entry.Put(syncable::SPECIFICS, specifics); - return entry.Get(syncable::META_HANDLE); -} - } // namespace class SyncApiTest : public testing::Test { @@ -288,6 +198,7 @@ TEST_F(SyncApiTest, ReadMissingTagsFails) { // TODO(chron): Hook this all up to the server and write full integration tests // for update->undelete behavior. TEST_F(SyncApiTest, TestDeleteBehavior) { + int64 node_id; int64 folder_id; std::wstring test_title(L"test1"); @@ -405,11 +316,11 @@ void CheckNodeValue(const BaseNode& node, const DictionaryValue& value) { } ExpectStringValue(WideToUTF8(node.GetTitle()), value, "title"); { - ModelType expected_model_type = node.GetModelType(); + syncable::ModelType expected_model_type = node.GetModelType(); std::string type_str; EXPECT_TRUE(value.GetString("type", &type_str)); if (expected_model_type >= syncable::FIRST_REAL_MODEL_TYPE) { - ModelType model_type = + syncable::ModelType model_type = syncable::ModelTypeFromString(type_str); EXPECT_EQ(expected_model_type, model_type); } else if (expected_model_type == syncable::TOP_LEVEL_FOLDER) { @@ -509,8 +420,7 @@ void CheckDeleteChangeRecordValue(const SyncManager::ChangeRecord& record, } } -class MockExtraChangeRecordData - : public SyncManager::ExtraPasswordChangeRecordData { +class MockExtraChangeRecordData : public SyncManager::ExtraChangeRecordData { public: MOCK_CONST_METHOD0(ToValue, DictionaryValue*()); }; @@ -597,102 +507,22 @@ class TestHttpPostProviderFactory : public HttpPostProviderFactory { } }; -class SyncManagerObserverMock : public SyncManager::Observer { - public: - MOCK_METHOD4(OnChangesApplied, - void(ModelType, - const BaseTransaction*, - const SyncManager::ChangeRecord*, - int)); // NOLINT - MOCK_METHOD1(OnChangesComplete, void(ModelType)); // NOLINT - MOCK_METHOD1(OnSyncCycleCompleted, - void(const SyncSessionSnapshot*)); // NOLINT - MOCK_METHOD0(OnInitializationComplete, void()); // NOLINT - MOCK_METHOD1(OnAuthError, void(const GoogleServiceAuthError&)); // NOLINT - MOCK_METHOD1(OnPassphraseRequired, void(bool)); // NOLINT - MOCK_METHOD1(OnPassphraseAccepted, void(const std::string&)); // NOLINT - MOCK_METHOD0(OnPaused, void()); // NOLINT - MOCK_METHOD0(OnResumed, void()); // NOLINT - MOCK_METHOD0(OnStopSyncingPermanently, void()); // NOLINT - MOCK_METHOD1(OnUpdatedToken, void(const std::string&)); // NOLINT - MOCK_METHOD0(OnClearServerDataFailed, void()); // NOLINT - MOCK_METHOD0(OnClearServerDataSucceeded, void()); // NOLINT - MOCK_METHOD1(OnEncryptionComplete, void(const ModelTypeSet&)); // NOLINT -}; - -class SyncManagerTest : public testing::Test, - public ModelSafeWorkerRegistrar { +class SyncManagerTest : public testing::Test { protected: SyncManagerTest() : ui_thread_(BrowserThread::UI, &ui_loop_) {} - // Test implementation. void SetUp() { ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); sync_manager_.Init(temp_dir_.path(), "bogus", 0, false, - new TestHttpPostProviderFactory(), this, "bogus", + new TestHttpPostProviderFactory(), NULL, "bogus", SyncCredentials(), notifier::NotifierOptions(), "", true /* setup_for_test_mode */); - sync_manager_.AddObserver(&observer_); - ModelSafeRoutingInfo routes; - GetModelSafeRoutingInfo(&routes); - for (ModelSafeRoutingInfo::iterator i = routes.begin(); i != routes.end(); - ++i) { - EXPECT_CALL(observer_, OnChangesApplied(i->first, _, _, 1)) - .RetiresOnSaturation(); - EXPECT_CALL(observer_, OnChangesComplete(i->first)) - .RetiresOnSaturation(); - type_roots_[i->first] = MakeServerNodeForType( - sync_manager_.GetUserShare(), i->first); - } } void TearDown() { - sync_manager_.RemoveObserver(&observer_); sync_manager_.Shutdown(); } - // ModelSafeWorkerRegistrar implementation. - virtual void GetWorkers(std::vector<ModelSafeWorker*>* out) { - NOTIMPLEMENTED(); - out->clear(); - } - virtual void GetModelSafeRoutingInfo(ModelSafeRoutingInfo* out) { - (*out)[syncable::NIGORI] = browser_sync::GROUP_PASSIVE; - (*out)[syncable::BOOKMARKS] = browser_sync::GROUP_PASSIVE; - (*out)[syncable::THEMES] = browser_sync::GROUP_PASSIVE; - (*out)[syncable::SESSIONS] = browser_sync::GROUP_PASSIVE; - (*out)[syncable::PASSWORDS] = browser_sync::GROUP_PASSIVE; - } - - // Helper methods. - bool SetUpEncryption() { - // We need to create the nigori node as if it were an applied server update. - UserShare* share = sync_manager_.GetUserShare(); - int64 nigori_id = GetIdForDataType(syncable::NIGORI); - if (nigori_id == kInvalidId) - return false; - - // Set the nigori cryptographer information. - Cryptographer* cryptographer = share->dir_manager->cryptographer(); - if (!cryptographer) - return false; - KeyParams params = {"localhost", "dummy", "foobar"}; - cryptographer->AddKey(params); - sync_pb::NigoriSpecifics nigori; - cryptographer->GetKeys(nigori.mutable_encrypted()); - WriteTransaction trans(share); - WriteNode node(&trans); - node.InitByIdLookup(nigori_id); - node.SetNigoriSpecifics(nigori); - return cryptographer->is_ready(); - } - - int64 GetIdForDataType(ModelType type) { - if (type_roots_.count(type) == 0) - return 0; - return type_roots_[type]; - } - private: // Needed by |ui_thread_|. MessageLoopForUI ui_loop_; @@ -700,12 +530,9 @@ class SyncManagerTest : public testing::Test, BrowserThread ui_thread_; // Needed by |sync_manager_|. ScopedTempDir temp_dir_; - // Sync Id's for the roots of the enabled datatypes. - std::map<ModelType, int64> type_roots_; protected: SyncManager sync_manager_; - StrictMock<SyncManagerObserverMock> observer_; }; TEST_F(SyncManagerTest, ParentJsEventRouter) { @@ -976,88 +803,6 @@ TEST_F(SyncManagerTest, OnIncomingNotification) { sync_manager_.TriggerOnIncomingNotificationForTest(model_types); } -TEST_F(SyncManagerTest, EncryptDataTypesWithNoData) { - EXPECT_TRUE(SetUpEncryption()); - ModelTypeSet encrypted_types; - encrypted_types.insert(syncable::BOOKMARKS); - // Even though Passwords isn't marked for encryption, it's enabled, so it - // should automatically be added to the response of OnEncryptionComplete. - ModelTypeSet expected_types = encrypted_types; - expected_types.insert(syncable::PASSWORDS); - EXPECT_CALL(observer_, OnEncryptionComplete(expected_types)); - sync_manager_.EncryptDataTypes(encrypted_types); - { - ReadTransaction trans(sync_manager_.GetUserShare()); - EXPECT_EQ(encrypted_types, - GetEncryptedDataTypes(trans.GetWrappedTrans())); - } -} - -TEST_F(SyncManagerTest, EncryptDataTypesWithData) { - size_t batch_size = 5; - EXPECT_TRUE(SetUpEncryption()); - - // Create some unencrypted unsynced data. - int64 folder = MakeFolderWithParent(sync_manager_.GetUserShare(), - syncable::BOOKMARKS, - GetIdForDataType(syncable::BOOKMARKS), - NULL); - // First batch_size nodes are children of folder. - size_t i; - for (i = 0; i < batch_size; ++i) { - MakeNodeWithParent(sync_manager_.GetUserShare(), syncable::BOOKMARKS, - StringPrintf("%"PRIuS"", i), folder); - } - // Next batch_size nodes are a different type and on their own. - for (; i < 2*batch_size; ++i) { - MakeNodeWithParent(sync_manager_.GetUserShare(), syncable::SESSIONS, - StringPrintf("%"PRIuS"", i), - GetIdForDataType(syncable::SESSIONS)); - } - // Last batch_size nodes are a third type that will not need encryption. - for (; i < 3*batch_size; ++i) { - MakeNodeWithParent(sync_manager_.GetUserShare(), syncable::THEMES, - StringPrintf("%"PRIuS"", i), - GetIdForDataType(syncable::THEMES)); - } - - { - ReadTransaction trans(sync_manager_.GetUserShare()); - EXPECT_TRUE(syncable::VerifyDataTypeEncryption(trans.GetWrappedTrans(), - syncable::BOOKMARKS, - false /* not encrypted */)); - EXPECT_TRUE(syncable::VerifyDataTypeEncryption(trans.GetWrappedTrans(), - syncable::SESSIONS, - false /* not encrypted */)); - EXPECT_TRUE(syncable::VerifyDataTypeEncryption(trans.GetWrappedTrans(), - syncable::THEMES, - false /* not encrypted */)); - } - - ModelTypeSet encrypted_types; - encrypted_types.insert(syncable::BOOKMARKS); - encrypted_types.insert(syncable::SESSIONS); - encrypted_types.insert(syncable::PASSWORDS); - EXPECT_CALL(observer_, OnEncryptionComplete(encrypted_types)); - sync_manager_.EncryptDataTypes(encrypted_types); - - { - ReadTransaction trans(sync_manager_.GetUserShare()); - encrypted_types.erase(syncable::PASSWORDS); // Not stored in nigori node. - EXPECT_EQ(encrypted_types, - GetEncryptedDataTypes(trans.GetWrappedTrans())); - EXPECT_TRUE(syncable::VerifyDataTypeEncryption(trans.GetWrappedTrans(), - syncable::BOOKMARKS, - true /* is encrypted */)); - EXPECT_TRUE(syncable::VerifyDataTypeEncryption(trans.GetWrappedTrans(), - syncable::SESSIONS, - true /* is encrypted */)); - EXPECT_TRUE(syncable::VerifyDataTypeEncryption(trans.GetWrappedTrans(), - syncable::THEMES, - false /* not encrypted */)); - } -} - } // namespace } // namespace browser_sync diff --git a/chrome/browser/sync/engine/syncer_util.cc b/chrome/browser/sync/engine/syncer_util.cc index b17f852..ba9d3a2 100644 --- a/chrome/browser/sync/engine/syncer_util.cc +++ b/chrome/browser/sync/engine/syncer_util.cc @@ -4,7 +4,6 @@ #include "chrome/browser/sync/engine/syncer_util.h" -#include <algorithm> #include <set> #include <string> #include <vector> @@ -18,7 +17,6 @@ #include "chrome/browser/sync/protocol/sync.pb.h" #include "chrome/browser/sync/syncable/directory_manager.h" #include "chrome/browser/sync/syncable/model_type.h" -#include "chrome/browser/sync/syncable/nigori_util.h" #include "chrome/browser/sync/syncable/syncable.h" #include "chrome/browser/sync/syncable/syncable_changes_version.h" @@ -221,8 +219,8 @@ syncable::Id SyncerUtil::FindLocalIdToUpdate( if (local_entry.good() && !local_entry.Get(IS_DEL)) { int64 old_version = local_entry.Get(BASE_VERSION); int64 new_version = update.version(); - DCHECK_LE(old_version, 0); - DCHECK_GT(new_version, 0); + DCHECK(old_version <= 0); + DCHECK(new_version > 0); // Otherwise setting the base version could cause a consistency failure. // An entry should never be version 0 and SYNCED. DCHECK(local_entry.Get(IS_UNSYNCED)); @@ -288,9 +286,8 @@ UpdateAttemptResponse SyncerUtil::AttemptToUpdateEntry( } } - // We intercept updates to the Nigori node, update the Cryptographer and - // encrypt any unsynced changes here because there is no Nigori - // ChangeProcessor. + // We intercept updates to the Nigori node and update the Cryptographer here + // because there is no Nigori ChangeProcessor. const sync_pb::EntitySpecifics& specifics = entry->Get(SERVER_SPECIFICS); if (specifics.HasExtension(sync_pb::nigori)) { const sync_pb::NigoriSpecifics& nigori = @@ -302,49 +299,17 @@ UpdateAttemptResponse SyncerUtil::AttemptToUpdateEntry( cryptographer->SetPendingKeys(nigori.encrypted()); } } - - // Make sure any unsynced changes are properly encrypted as necessary. - syncable::ModelTypeSet encrypted_types = - syncable::GetEncryptedDataTypesFromNigori(nigori); - if (!VerifyUnsyncedChangesAreEncrypted(trans, encrypted_types) && - (!cryptographer->is_ready() || - !syncable::ProcessUnsyncedChangesForEncryption(trans, encrypted_types, - cryptographer))) { - // We were unable to encrypt the changes, possibly due to a missing - // passphrase. We return conflict, even though the conflict is with the - // unsynced change and not the nigori node. We ensure foward progress - // because the cryptographer already has the pending keys set, so once - // the new passphrase is entered we should be able to encrypt properly. - // And, because this update will not be applied yet, next time around - // we will properly encrypt all appropriate unsynced data. - VLOG(1) << "Marking nigori node update as conflicting due to being unable" - << " to encrypt all necessary unsynced changes."; - return CONFLICT; - } - - // Note that we don't bother to encrypt any synced data that now requires - // encryption. The machine that turned on encryption should encrypt - // everything itself. It's possible it could get interrupted during this - // process, but we currently reencrypt everything at startup as well, - // so as soon as a client is restarted with this datatype encrypted, all the - // data should be updated as necessary. } // Only apply updates that we can decrypt. Updates that can't be decrypted yet // will stay in conflict until the user provides a passphrase that lets the // Cryptographer decrypt them. - if (!entry->Get(SERVER_IS_DIR)) { - if (specifics.has_encrypted() && - !cryptographer->CanDecrypt(specifics.encrypted())) { + if (!entry->Get(SERVER_IS_DIR) && specifics.HasExtension(sync_pb::password)) { + const sync_pb::PasswordSpecifics& password = + specifics.GetExtension(sync_pb::password); + if (!cryptographer->CanDecrypt(password.encrypted())) { // We can't decrypt this node yet. return CONFLICT; - } else if (specifics.HasExtension(sync_pb::password)) { - // Passwords use their own legacy encryption scheme. - const sync_pb::PasswordSpecifics& password = - specifics.GetExtension(sync_pb::password); - if (!cryptographer->CanDecrypt(password.encrypted())) { - return CONFLICT; - } } } diff --git a/chrome/browser/sync/glue/password_change_processor.cc b/chrome/browser/sync/glue/password_change_processor.cc index 30d77fa..adb7d35 100644 --- a/chrome/browser/sync/glue/password_change_processor.cc +++ b/chrome/browser/sync/glue/password_change_processor.cc @@ -150,7 +150,8 @@ void PasswordChangeProcessor::ApplyChangesFromSyncModel( << "Password specifics data not present on delete!"; DCHECK(changes[i].extra.get()); sync_api::SyncManager::ExtraPasswordChangeRecordData* extra = - changes[i].extra.get(); + static_cast<sync_api::SyncManager::ExtraPasswordChangeRecordData*>( + changes[i].extra.get()); const sync_pb::PasswordSpecificsData& password = extra->unencrypted(); webkit_glue::PasswordForm form; PasswordModelAssociator::CopyPassword(password, &form); diff --git a/chrome/browser/sync/glue/session_change_processor.cc b/chrome/browser/sync/glue/session_change_processor.cc index 56a99fb..8823af0 100644 --- a/chrome/browser/sync/glue/session_change_processor.cc +++ b/chrome/browser/sync/glue/session_change_processor.cc @@ -10,10 +10,8 @@ #include "base/logging.h" #include "base/scoped_vector.h" -#include "chrome/browser/profiles/profile.h" #include "chrome/browser/sync/engine/syncapi.h" #include "chrome/browser/sync/glue/session_model_associator.h" -#include "chrome/browser/sync/profile_sync_service.h" #include "chrome/browser/tab_contents/navigation_controller.h" #include "chrome/browser/tab_contents/tab_contents.h" #include "chrome/common/notification_details.h" @@ -33,19 +31,6 @@ SessionChangeProcessor::SessionChangeProcessor( DCHECK(session_model_associator_); } -SessionChangeProcessor::SessionChangeProcessor( - UnrecoverableErrorHandler* error_handler, - SessionModelAssociator* session_model_associator, - bool setup_for_test) - : ChangeProcessor(error_handler), - session_model_associator_(session_model_associator), - profile_(NULL), - setup_for_test_(setup_for_test) { - DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - DCHECK(error_handler); - DCHECK(session_model_associator_); -} - SessionChangeProcessor::~SessionChangeProcessor() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); } @@ -198,15 +183,6 @@ void SessionChangeProcessor::ApplyChangesFromSyncModel( const sync_pb::SessionSpecifics& specifics( sync_node.GetSessionSpecifics()); - if (specifics.session_tag() == - session_model_associator_->GetCurrentMachineTag() && - !setup_for_test_) { - // We should only ever receive a change to our own machine's session info - // if encryption was turned on. In that case, the data is still the same, - // so we can ignore. - LOG(WARNING) << "Dropping modification to local session."; - return; - } const int64 mtime = sync_node.GetModificationTime(); // Model associator handles foreign session update and add the same. session_model_associator_->AssociateForeignSpecifics(specifics, mtime); diff --git a/chrome/browser/sync/glue/session_change_processor.h b/chrome/browser/sync/glue/session_change_processor.h index b8bcc8f..9d772da 100644 --- a/chrome/browser/sync/glue/session_change_processor.h +++ b/chrome/browser/sync/glue/session_change_processor.h @@ -35,10 +35,6 @@ class SessionChangeProcessor : public ChangeProcessor, SessionChangeProcessor( UnrecoverableErrorHandler* error_handler, SessionModelAssociator* session_model_associator); - SessionChangeProcessor( - UnrecoverableErrorHandler* error_handler, - SessionModelAssociator* session_model_associator, - bool setup_for_test); virtual ~SessionChangeProcessor(); // NotificationObserver implementation. @@ -67,9 +63,6 @@ class SessionChangeProcessor : public ChangeProcessor, // Owner of the SessionService. Non-NULL iff |running()| is true. Profile* profile_; - // To bypass some checks/codepaths not applicable in tests. - bool setup_for_test_; - DISALLOW_COPY_AND_ASSIGN(SessionChangeProcessor); }; diff --git a/chrome/browser/sync/glue/session_model_associator.cc b/chrome/browser/sync/glue/session_model_associator.cc index 75c7a0d..cad5b98 100644 --- a/chrome/browser/sync/glue/session_model_associator.cc +++ b/chrome/browser/sync/glue/session_model_associator.cc @@ -40,16 +40,6 @@ SessionModelAssociator::SessionModelAssociator(ProfileSyncService* sync_service) DCHECK(sync_service_); } -SessionModelAssociator::SessionModelAssociator(ProfileSyncService* sync_service, - bool setup_for_test) - : tab_pool_(sync_service), - local_session_syncid_(sync_api::kInvalidId), - sync_service_(sync_service), - setup_for_test_(setup_for_test) { - DCHECK(CalledOnValidThread()); - DCHECK(sync_service_); -} - SessionModelAssociator::~SessionModelAssociator() { DCHECK(CalledOnValidThread()); } @@ -503,7 +493,8 @@ bool SessionModelAssociator::AssociateForeignSpecifics( const int64 modification_time) { DCHECK(CalledOnValidThread()); std::string foreign_session_tag = specifics.session_tag(); - DCHECK(foreign_session_tag != GetCurrentMachineTag() || setup_for_test_); + DCHECK(foreign_session_tag != GetCurrentMachineTag() || + sync_service_->cros_user() == "test user"); // For tests. if (specifics.has_header()) { // Read in the header data for this foreign session. diff --git a/chrome/browser/sync/glue/session_model_associator.h b/chrome/browser/sync/glue/session_model_associator.h index 7fe19c3..7864c123 100644 --- a/chrome/browser/sync/glue/session_model_associator.h +++ b/chrome/browser/sync/glue/session_model_associator.h @@ -11,7 +11,6 @@ #include <vector> #include "base/basictypes.h" -#include "base/format_macros.h" #include "base/gtest_prod_util.h" #include "base/observer_list.h" #include "base/scoped_vector.h" @@ -54,8 +53,6 @@ class SessionModelAssociator public: // Does not take ownership of sync_service. explicit SessionModelAssociator(ProfileSyncService* sync_service); - SessionModelAssociator(ProfileSyncService* sync_service, - bool setup_for_test); virtual ~SessionModelAssociator(); // The has_nodes out parameter is set to true if the sync model has @@ -304,8 +301,8 @@ class SessionModelAssociator static inline std::string TabIdToTag( const std::string machine_tag, size_t tab_node_id) { - return StringPrintf("%s %"PRIuS"", - machine_tag.c_str(), tab_node_id); + return StringPrintf("%s %lu", + machine_tag.c_str(), static_cast<unsigned long>(tab_node_id)); } // Initializes the tag corresponding to this machine. @@ -402,9 +399,6 @@ class SessionModelAssociator // Consumer used to obtain the current session. CancelableRequestConsumer consumer_; - // To avoid certain checks not applicable to tests. - bool setup_for_test_; - DISALLOW_COPY_AND_ASSIGN(SessionModelAssociator); }; diff --git a/chrome/browser/sync/glue/session_model_associator_unittest.cc b/chrome/browser/sync/glue/session_model_associator_unittest.cc index 9f0ce3c..77f3866 100644 --- a/chrome/browser/sync/glue/session_model_associator_unittest.cc +++ b/chrome/browser/sync/glue/session_model_associator_unittest.cc @@ -11,6 +11,7 @@ #include "chrome/browser/sync/glue/session_model_associator.h" #include "chrome/common/page_transition_types.h" #include "chrome/common/url_constants.h" +#include "chrome/test/testing_profile.h" #include "testing/gtest/include/gtest/gtest.h" using browser_sync::SessionModelAssociator; diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc index 6e7cf4b..493593c 100644 --- a/chrome/browser/sync/glue/sync_backend_host.cc +++ b/chrome/browser/sync/glue/sync_backend_host.cc @@ -30,7 +30,6 @@ // TODO(tim): Remove this! We should have a syncapi pass-thru instead. #include "chrome/browser/sync/syncable/directory_manager.h" // Cryptographer. #include "chrome/browser/sync/syncable/model_type.h" -#include "chrome/browser/sync/syncable/nigori_util.h" #include "chrome/common/chrome_switches.h" #include "chrome/common/chrome_version_info.h" #include "chrome/common/net/gaia/gaia_constants.h" @@ -126,13 +125,17 @@ void SyncBackendHost::Initialize( registrar_.workers[GROUP_PASSWORD] = new PasswordModelWorker(password_store); } else { - LOG_IF(WARNING, types.count(syncable::PASSWORDS) > 0) << "Password store " - << "not initialized, cannot sync passwords"; + LOG(WARNING) << "Password store not initialized, cannot sync passwords"; registrar_.routing_info.erase(syncable::PASSWORDS); } - // Nigori is populated by default now. - registrar_.routing_info[syncable::NIGORI] = GROUP_PASSIVE; + // TODO(tim): Remove this special case once NIGORI is populated by + // default. We piggy back off of the passwords flag for now to not + // require both encryption and passwords flags. + bool enable_encryption = !CommandLine::ForCurrentProcess()->HasSwitch( + switches::kDisableSyncPasswords) || types.count(syncable::PASSWORDS); + if (enable_encryption) + registrar_.routing_info[syncable::NIGORI] = GROUP_PASSIVE; InitCore(Core::DoInitializeOptions( sync_service_url, @@ -404,14 +407,6 @@ void SyncBackendHost::ConfigureDataTypes( } } -void SyncBackendHost::EncryptDataTypes( - const syncable::ModelTypeSet& encrypted_types) { - core_thread_.message_loop()->PostTask(FROM_HERE, - NewRunnableMethod(core_.get(), - &SyncBackendHost::Core::DoEncryptDataTypes, - encrypted_types)); -} - void SyncBackendHost::RequestNudge() { core_thread_.message_loop()->PostTask(FROM_HERE, NewRunnableMethod(core_.get(), &SyncBackendHost::Core::DoRequestNudge)); @@ -516,14 +511,6 @@ void SyncBackendHost::Core::NotifyUpdatedToken(const std::string& token) { Details<const TokenAvailableDetails>(&details)); } -void SyncBackendHost::Core::NotifyEncryptionComplete( - const syncable::ModelTypeSet& encrypted_types) { - if (!host_) - return; - DCHECK_EQ(MessageLoop::current(), host_->frontend_loop_); - host_->frontend_->OnEncryptionComplete(encrypted_types); -} - SyncBackendHost::Core::DoInitializeOptions::DoInitializeOptions( const GURL& service_url, sync_api::HttpPostProviderFactory* http_bridge_factory, @@ -682,12 +669,6 @@ void SyncBackendHost::Core::DoSetPassphrase(const std::string& passphrase, syncapi_->SetPassphrase(passphrase, is_explicit); } -void SyncBackendHost::Core::DoEncryptDataTypes( - const syncable::ModelTypeSet& encrypted_types) { - DCHECK(MessageLoop::current() == host_->core_thread_.message_loop()); - syncapi_->EncryptDataTypes(encrypted_types); -} - UIModelWorker* SyncBackendHost::ui_worker() { ModelSafeWorker* w = registrar_.workers[GROUP_UI]; if (w == NULL) @@ -907,14 +888,6 @@ void SyncBackendHost::Core::OnClearServerDataFailed() { &Core::HandleClearServerDataFailedOnFrontendLoop)); } -void SyncBackendHost::Core::OnEncryptionComplete( - const syncable::ModelTypeSet& encrypted_types) { - host_->frontend_loop_->PostTask( - FROM_HERE, - NewRunnableMethod(this, &Core::NotifyEncryptionComplete, - encrypted_types)); -} - void SyncBackendHost::Core::RouteJsEvent( const std::string& name, const JsArgList& args, const JsEventHandler* target) { diff --git a/chrome/browser/sync/glue/sync_backend_host.h b/chrome/browser/sync/glue/sync_backend_host.h index 317cbcc..4b3d164 100644 --- a/chrome/browser/sync/glue/sync_backend_host.h +++ b/chrome/browser/sync/glue/sync_backend_host.h @@ -88,9 +88,6 @@ class SyncFrontend { // encrypted using the accepted passphrase. virtual void OnPassphraseAccepted() = 0; - virtual void OnEncryptionComplete( - const syncable::ModelTypeSet& encrypted_types) = 0; - protected: // Don't delete through SyncFrontend interface. virtual ~SyncFrontend() { @@ -163,13 +160,6 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar { const syncable::ModelTypeSet& types, CancelableTask* ready_task); - // Encrypts the specified datatypes and marks them as needing encryption on - // other machines. This affects all machines synced to this account and all - // data belonging to the specified types. - // Note: actual work is done on core_thread_'s message loop. - virtual void EncryptDataTypes( - const syncable::ModelTypeSet& encrypted_types); - syncable::AutofillMigrationState GetAutofillMigrationState(); @@ -286,8 +276,6 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar { virtual void OnUpdatedToken(const std::string& token); virtual void OnClearServerDataFailed(); virtual void OnClearServerDataSucceeded(); - virtual void OnEncryptionComplete( - const syncable::ModelTypeSet& encrypted_types); // JsBackend implementation. virtual void SetParentJsEventRouter(JsEventRouter* router); @@ -355,10 +343,6 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar { // on behalf of SyncBackendHost::SupplyPassphrase. void DoSetPassphrase(const std::string& passphrase, bool is_explicit); - // Called on SyncBackendHost's |core_thread_| to set the datatypes we need - // to encrypt as well as encrypt all local data of that type. - void DoEncryptDataTypes(const syncable::ModelTypeSet& encrypted_types); - // The shutdown order is a bit complicated: // 1) From |core_thread_|, invoke the syncapi Shutdown call to do a final // SaveChanges, close sqlite handles, and halt the syncer thread (which @@ -396,6 +380,7 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar { void DoInitializeForTest(const std::wstring& test_user, sync_api::HttpPostProviderFactory* factory, bool delete_sync_data_folder) { + // Construct dummy credentials for test. sync_api::SyncCredentials credentials; credentials.email = WideToUTF8(test_user); @@ -454,11 +439,6 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar { // Invoked when an updated token is available from the sync server. void NotifyUpdatedToken(const std::string& token); - // Invoked when sync finishes encrypting new datatypes or has become aware - // of new datatypes requiring encryption. - void NotifyEncryptionComplete(const syncable::ModelTypeSet& - encrypted_types); - // Called from Core::OnSyncCycleCompleted to handle updating frontend // thread components. void HandleSyncCycleCompletedOnFrontendLoop( @@ -527,6 +507,7 @@ class SyncBackendHost : public browser_sync::ModelSafeWorkerRegistrar { scoped_refptr<Core> core_; private: + UIModelWorker* ui_worker(); void ConfigureAutofillMigration(); diff --git a/chrome/browser/sync/js_sync_manager_observer.cc b/chrome/browser/sync/js_sync_manager_observer.cc index 85fc5c4..407ce98 100644 --- a/chrome/browser/sync/js_sync_manager_observer.cc +++ b/chrome/browser/sync/js_sync_manager_observer.cc @@ -12,6 +12,7 @@ #include "chrome/browser/sync/js_event_router.h" #include "chrome/browser/sync/sessions/session_state.h" #include "chrome/browser/sync/syncable/model_type.h" +#include "chrome/browser/sync/sessions/session_state.h" namespace browser_sync { @@ -86,14 +87,6 @@ void JsSyncManagerObserver::OnPassphraseAccepted( JsArgList(return_args), NULL); } -void JsSyncManagerObserver::OnEncryptionComplete( - const syncable::ModelTypeSet& encrypted_types) { - ListValue return_args; - return_args.Append(syncable::ModelTypeSetToValue(encrypted_types)); - parent_router_->RouteJsEvent("onEncryptionComplete", - JsArgList(return_args), NULL); -} - void JsSyncManagerObserver::OnInitializationComplete() { parent_router_->RouteJsEvent("onInitializationComplete", JsArgList(), NULL); diff --git a/chrome/browser/sync/js_sync_manager_observer.h b/chrome/browser/sync/js_sync_manager_observer.h index a912d7f..aa710ef 100644 --- a/chrome/browser/sync/js_sync_manager_observer.h +++ b/chrome/browser/sync/js_sync_manager_observer.h @@ -36,8 +36,6 @@ class JsSyncManagerObserver : public sync_api::SyncManager::Observer { virtual void OnUpdatedToken(const std::string& token); virtual void OnPassphraseRequired(bool for_decryption); virtual void OnPassphraseAccepted(const std::string& bootstrap_token); - virtual void OnEncryptionComplete( - const syncable::ModelTypeSet& encrypted_types); virtual void OnInitializationComplete(); virtual void OnPaused(); virtual void OnResumed(); diff --git a/chrome/browser/sync/js_sync_manager_observer_unittest.cc b/chrome/browser/sync/js_sync_manager_observer_unittest.cc index 573ba8e..11df4d5 100644 --- a/chrome/browser/sync/js_sync_manager_observer_unittest.cc +++ b/chrome/browser/sync/js_sync_manager_observer_unittest.cc @@ -149,26 +149,6 @@ TEST_F(JsSyncManagerObserverTest, SensitiveNotifiations) { sync_manager_observer_.OnPassphraseAccepted("sensitive_token"); } -TEST_F(JsSyncManagerObserverTest, OnEncryptionComplete) { - ListValue expected_args; - ListValue* encrypted_type_values = new ListValue(); - syncable::ModelTypeSet encrypted_types; - - expected_args.Append(encrypted_type_values); - for (int i = syncable::FIRST_REAL_MODEL_TYPE; - i < syncable::MODEL_TYPE_COUNT; ++i) { - syncable::ModelType type = syncable::ModelTypeFromInt(i); - encrypted_types.insert(type); - encrypted_type_values->Append(Value::CreateStringValue( - syncable::ModelTypeToString(type))); - } - - EXPECT_CALL(mock_router_, - RouteJsEvent("onEncryptionComplete", - HasArgsAsList(expected_args), NULL)); - - sync_manager_observer_.OnEncryptionComplete(encrypted_types); -} namespace { // Makes a node of the given model type. Returns the id of the diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc index b04142d..2e44ff2 100644 --- a/chrome/browser/sync/profile_sync_service.cc +++ b/chrome/browser/sync/profile_sync_service.cc @@ -433,7 +433,11 @@ void ProfileSyncService::CreateBackend() { } bool ProfileSyncService::IsEncryptedDatatypeEnabled() const { - return encrypted_types_.size() > 0; + // Currently on passwords are an encrypted datatype, so + // we check to see if it is enabled. + syncable::ModelTypeSet types; + GetPreferredDataTypes(&types); + return types.count(syncable::PASSWORDS) != 0; } void ProfileSyncService::StartUp() { @@ -747,14 +751,6 @@ void ProfileSyncService::OnPassphraseAccepted() { wizard_.Step(SyncSetupWizard::DONE); } -void ProfileSyncService::OnEncryptionComplete( - const syncable::ModelTypeSet& encrypted_types) { - if (encrypted_types_ != encrypted_types) { - encrypted_types_ = encrypted_types; - NotifyObservers(); - } -} - void ProfileSyncService::ShowLoginDialog(gfx::NativeWindow parent_window) { if (!cros_user_.empty()) { // For ChromeOS, any login UI needs to be handled by the settings page. @@ -1174,16 +1170,6 @@ void ProfileSyncService::SetPassphrase(const std::string& passphrase, tried_setting_explicit_passphrase_ = true; } -void ProfileSyncService::EncryptDataTypes( - const syncable::ModelTypeSet& encrypted_types) { - backend_->EncryptDataTypes(encrypted_types); -} - -void ProfileSyncService::GetEncryptedDataTypes( - syncable::ModelTypeSet* encrypted_types) const { - *encrypted_types = encrypted_types_; -} - void ProfileSyncService::Observe(NotificationType type, const NotificationSource& source, const NotificationDetails& details) { diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h index f0f057e..ec9ce6b 100644 --- a/chrome/browser/sync/profile_sync_service.h +++ b/chrome/browser/sync/profile_sync_service.h @@ -190,8 +190,6 @@ class ProfileSyncService : public browser_sync::SyncFrontend, virtual void OnClearServerDataSucceeded(); virtual void OnPassphraseRequired(bool for_decryption); virtual void OnPassphraseAccepted(); - virtual void OnEncryptionComplete( - const syncable::ModelTypeSet& encrypted_types); // Called when a user enters credentials through UI. virtual void OnUserSubmittedAuth(const std::string& username, @@ -436,19 +434,6 @@ class ProfileSyncService : public browser_sync::SyncFrontend, bool is_explicit, bool is_creation); - // Changes the set of datatypes that require encryption. This affects all - // machines synced to this account and all data belonging to the specified - // types. - // Note that this is an asynchronous operation (the encryption of data is - // performed on SyncBackendHost's core thread) and may not have an immediate - // effect. - virtual void EncryptDataTypes( - const syncable::ModelTypeSet& encrypted_types); - - // Get the currently encrypted data types. - virtual void GetEncryptedDataTypes( - syncable::ModelTypeSet* encrypted_types) const; - // Returns whether processing changes is allowed. Check this before doing // any model-modifying operations. bool ShouldPushChanges(); @@ -655,10 +640,6 @@ class ProfileSyncService : public browser_sync::SyncFrontend, // is reworked to allow one-shot commands like clearing server data. base::OneShotTimer<ProfileSyncService> clear_server_data_timer_; - // The set of encrypted types. This is updated whenever datatypes are - // encrypted through the OnEncryptionComplete callback of SyncFrontend. - syncable::ModelTypeSet encrypted_types_; - DISALLOW_COPY_AND_ASSIGN(ProfileSyncService); }; diff --git a/chrome/browser/sync/profile_sync_service_harness.cc b/chrome/browser/sync/profile_sync_service_harness.cc index 72bb584d..95eaff1 100644 --- a/chrome/browser/sync/profile_sync_service_harness.cc +++ b/chrome/browser/sync/profile_sync_service_harness.cc @@ -84,8 +84,7 @@ ProfileSyncServiceHarness::ProfileSyncServiceHarness( const std::string& username, const std::string& password, int id) - : waiting_for_encryption_type_(syncable::UNSPECIFIED), - wait_state_(INITIAL_WAIT_STATE), + : wait_state_(INITIAL_WAIT_STATE), profile_(profile), service_(NULL), timestamp_match_partner_(NULL), @@ -289,14 +288,6 @@ bool ProfileSyncServiceHarness::RunStateChangeMachine() { LogClientInfo("SYNC_DISABLED"); break; } - case WAITING_FOR_ENCRYPTION: { - // If the type whose encryption we are waiting for is now complete, there - // is nothing to do. - LogClientInfo("WAITING_FOR_ENCRYPTION"); - if (IsTypeEncrypted(waiting_for_encryption_type_)) - SignalStateCompleteWithNextState(FULLY_SYNCED); - break; - } default: // Invalid state during observer callback which may be triggered by other // classes using the the UI message loop. Defer to their handling. @@ -606,36 +597,3 @@ void ProfileSyncServiceHarness::LogClientInfo(std::string message) { << ": Sync service not available."; } } - -bool ProfileSyncServiceHarness::EnableEncryptionForType( - syncable::ModelType type) { - syncable::ModelTypeSet encrypted_types; - service_->GetEncryptedDataTypes(&encrypted_types); - if (encrypted_types.count(type) > 0) - return true; - encrypted_types.insert(type); - service_->EncryptDataTypes(encrypted_types); - - // Wait some time to let the enryption finish. - std::string reason = "Waiting for encryption."; - DCHECK_EQ(FULLY_SYNCED, wait_state_); - wait_state_ = WAITING_FOR_ENCRYPTION; - waiting_for_encryption_type_ = type; - if (!AwaitStatusChangeWithTimeout(kLiveSyncOperationTimeoutMs, reason)) { - LOG(ERROR) << "Did not receive EncryptionComplete notification after" - << kLiveSyncOperationTimeoutMs / 1000 - << " seconds."; - return false; - } - - return IsTypeEncrypted(type); -} - -bool ProfileSyncServiceHarness::IsTypeEncrypted(syncable::ModelType type) { - syncable::ModelTypeSet encrypted_types; - service_->GetEncryptedDataTypes(&encrypted_types); - if (encrypted_types.count(type) == 0) { - return false; - } - return true; -} diff --git a/chrome/browser/sync/profile_sync_service_harness.h b/chrome/browser/sync/profile_sync_service_harness.h index d895e6a..ddbfbe2 100644 --- a/chrome/browser/sync/profile_sync_service_harness.h +++ b/chrome/browser/sync/profile_sync_service_harness.h @@ -114,17 +114,6 @@ class ProfileSyncServiceHarness : public ProfileSyncServiceObserver { // Returns a snapshot of the current sync session. const SyncSessionSnapshot* GetLastSessionSnapshot() const; - // Encrypt the datatype |type|. This method will block while the sync backend - // host performs the encryption or a timeout is reached. Returns false if - // encryption failed, else true. - // Note: this method does not currently support tracking encryption status - // while other sync activities are being performed. Sync should be fully - // synced when this is called. - bool EnableEncryptionForType(syncable::ModelType type); - - // Check if |type| is encrypted. - bool IsTypeEncrypted(syncable::ModelType type); - private: friend class StateChangeTimeoutEvent; @@ -150,9 +139,6 @@ class ProfileSyncServiceHarness : public ProfileSyncServiceObserver { // The sync client anticipates incoming updates leading to a new sync cycle. WAITING_FOR_UPDATES, - // The sync client anticipates encryption of new datatypes. - WAITING_FOR_ENCRYPTION, - // The sync client cannot reach the server. SERVER_UNREACHABLE, @@ -193,10 +179,6 @@ class ProfileSyncServiceHarness : public ProfileSyncServiceObserver { // for a particular datatype. std::string GetUpdatedTimestamp(syncable::ModelType model_type); - // When in WAITING_FOR_ENCRYPTION state, we check to see if this type is now - // encrypted to determine if we're done. - syncable::ModelType waiting_for_encryption_type_; - WaitState wait_state_; Profile* profile_; diff --git a/chrome/browser/sync/profile_sync_service_session_unittest.cc b/chrome/browser/sync/profile_sync_service_session_unittest.cc index 1570ff3..fef7e14 100644 --- a/chrome/browser/sync/profile_sync_service_session_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_session_unittest.cc @@ -56,6 +56,7 @@ class ProfileSyncServiceSessionTest ProfileSyncServiceSessionTest() : window_bounds_(0, 1, 2, 3), notified_of_update_(false) {} + ProfileSyncService* sync_service() { return sync_service_.get(); } TestIdFactory* ids() { return sync_service_->id_factory(); } @@ -64,9 +65,7 @@ class ProfileSyncServiceSessionTest SessionService* service() { return helper_.service(); } virtual void SetUp() { - // BrowserWithTestWindowTest implementation. BrowserWithTestWindowTest::SetUp(); - profile()->set_has_history_service(true); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); SessionService* session_service = new SessionService(temp_dir_.path()); @@ -99,17 +98,16 @@ class ProfileSyncServiceSessionTest bool StartSyncService(Task* task, bool will_fail_association) { if (sync_service_.get()) return false; + sync_service_.reset(new TestProfileSyncService( &factory_, profile(), "test user", false, task)); profile()->set_session_service(helper_.service()); // Register the session data type. model_associator_ = - new SessionModelAssociator(sync_service_.get(), - true /* setup_for_test */); + new SessionModelAssociator(sync_service_.get()); change_processor_ = new SessionChangeProcessor( - sync_service_.get(), model_associator_, - true /* setup_for_test */); + sync_service_.get(), model_associator_); EXPECT_CALL(factory_, CreateSessionSyncComponents(_, _)). WillOnce(Return(ProfileSyncFactory::SyncComponents( model_associator_, change_processor_))); diff --git a/chrome/browser/sync/protocol/nigori_specifics.proto b/chrome/browser/sync/protocol/nigori_specifics.proto index 4e8b875..98ee1b7 100644 --- a/chrome/browser/sync/protocol/nigori_specifics.proto +++ b/chrome/browser/sync/protocol/nigori_specifics.proto @@ -34,18 +34,6 @@ message NigoriSpecifics { // True if |encrypted| is encrypted using a passphrase // explicitly set by the user. optional bool using_explicit_passphrase = 2; - - // Booleans corresponding to whether a datatype should be encrypted. - // Passwords are always encrypted, so we don't need a field here. - optional bool encrypt_bookmarks = 3; - optional bool encrypt_preferences = 4; - optional bool encrypt_autofill_profile = 5; - optional bool encrypt_autofill = 6; - optional bool encrypt_themes = 7; - optional bool encrypt_typed_urls = 8; - optional bool encrypt_extensions = 9; - optional bool encrypt_sessions = 10; - optional bool encrypt_apps = 11; } extend EntitySpecifics { diff --git a/chrome/browser/sync/protocol/sync.proto b/chrome/browser/sync/protocol/sync.proto index a87b0b3..6af021b 100644 --- a/chrome/browser/sync/protocol/sync.proto +++ b/chrome/browser/sync/protocol/sync.proto @@ -14,8 +14,6 @@ option retain_unknown_fields = true; package sync_pb; -import "encryption.proto"; - // Used for inspecting how long we spent performing operations in different // backends. All times must be in millis. message ProfilingData { @@ -28,13 +26,6 @@ message ProfilingData { } message EntitySpecifics { - // If a datatype is encrypted, this field will contain the encrypted - // original EntitySpecifics. The extension for the datatype will continue - // to exist, but contain only the default values. - // Note that currently passwords employ their own legacy encryption scheme and - // do not use this field. - optional EncryptedData encrypted = 1; - // To add new datatype-specific fields to the protocol, extend // EntitySpecifics. First, pick a non-colliding tag number by // picking a revision number of one of your past commits diff --git a/chrome/browser/sync/syncable/model_type.cc b/chrome/browser/sync/syncable/model_type.cc index b9e42e2..0ff67bd 100644 --- a/chrome/browser/sync/syncable/model_type.cc +++ b/chrome/browser/sync/syncable/model_type.cc @@ -261,54 +261,13 @@ ListValue* ModelTypeBitSetToValue(const ModelTypeBitSet& model_types) { for (int i = FIRST_REAL_MODEL_TYPE; i < MODEL_TYPE_COUNT; ++i) { if (model_types[i]) { value->Append( - Value::CreateStringValue(ModelTypeToString(ModelTypeFromInt(i)))); + Value::CreateStringValue( + ModelTypeToString(ModelTypeFromInt(i)))); } } return value; } -ListValue* ModelTypeSetToValue(const ModelTypeSet& model_types) { - ListValue* value = new ListValue(); - for (ModelTypeSet::const_iterator i = model_types.begin(); - i != model_types.end(); ++i) { - value->Append(Value::CreateStringValue(ModelTypeToString(*i))); - } - return value; -} - -// TODO(zea): remove all hardcoded tags in model associators and have them use -// this instead. -std::string ModelTypeToRootTag(ModelType type) { - switch (type) { - case BOOKMARKS: - return "google_chrome_bookmarks"; - case PREFERENCES: - return "google_chrome_preferences"; - case PASSWORDS: - return "google_chrome_passwords"; - case AUTOFILL: - return "google_chrome_autofill"; - case THEMES: - return "google_chrome_themes"; - case TYPED_URLS: - return "google_chrome_typed_urls"; - case EXTENSIONS: - return "google_chrome_extensions"; - case NIGORI: - return "google_chrome_nigori"; - case SESSIONS: - return "google_chrome_sessions"; - case APPS: - return "google_chrome_apps"; - case AUTOFILL_PROFILE: - return "google_chrome_autofill_profiles"; - default: - break; - } - NOTREACHED() << "No known extension for model type."; - return "INVALID"; -} - // For now, this just implements UMA_HISTOGRAM_LONG_TIMES. This can be adjusted // if we feel the min, max, or bucket count amount are not appropriate. #define SYNC_FREQ_HISTOGRAM(name, time) UMA_HISTOGRAM_CUSTOM_TIMES( \ diff --git a/chrome/browser/sync/syncable/model_type.h b/chrome/browser/sync/syncable/model_type.h index 3c4254c..62f1787 100644 --- a/chrome/browser/sync/syncable/model_type.h +++ b/chrome/browser/sync/syncable/model_type.h @@ -118,12 +118,6 @@ bool ModelTypeBitSetFromString( // Caller takes ownership of returned list. ListValue* ModelTypeBitSetToValue(const ModelTypeBitSet& model_types); -// Caller takes ownership of returned list. -ListValue* ModelTypeSetToValue(const ModelTypeSet& model_types); - -// Returns a string corresponding to the syncable tag for this datatype. -std::string ModelTypeToRootTag(ModelType type); - // Posts timedeltas to histogram of datatypes. Allows tracking of the frequency // at which datatypes cause syncs. void PostTimeToTypeHistogram(ModelType model_type, base::TimeDelta time); diff --git a/chrome/browser/sync/syncable/model_type_unittest.cc b/chrome/browser/sync/syncable/model_type_unittest.cc index 59ed1f9..b15f0a0 100644 --- a/chrome/browser/sync/syncable/model_type_unittest.cc +++ b/chrome/browser/sync/syncable/model_type_unittest.cc @@ -29,19 +29,5 @@ TEST_F(ModelTypeTest, ModelTypeBitSetToValue) { EXPECT_EQ("Apps", types[1]); } -TEST_F(ModelTypeTest, ModelTypeSetToValue) { - ModelTypeSet model_types; - model_types.insert(syncable::BOOKMARKS); - model_types.insert(syncable::APPS); - - scoped_ptr<ListValue> value(ModelTypeSetToValue(model_types)); - EXPECT_EQ(2u, value->GetSize()); - std::string types[2]; - EXPECT_TRUE(value->GetString(0, &types[0])); - EXPECT_TRUE(value->GetString(1, &types[1])); - EXPECT_EQ("Bookmarks", types[0]); - EXPECT_EQ("Apps", types[1]); -} - } // namespace } // namespace syncable diff --git a/chrome/browser/sync/syncable/nigori_util.cc b/chrome/browser/sync/syncable/nigori_util.cc deleted file mode 100644 index 71af205..0000000 --- a/chrome/browser/sync/syncable/nigori_util.cc +++ /dev/null @@ -1,196 +0,0 @@ -// 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/syncable/nigori_util.h" - -#include <queue> -#include <string> -#include <vector> - -#include "chrome/browser/sync/engine/syncer_util.h" -#include "chrome/browser/sync/syncable/syncable.h" -#include "chrome/browser/sync/util/cryptographer.h" - -namespace syncable { - -ModelTypeSet GetEncryptedDataTypes(BaseTransaction* const trans) { - std::string nigori_tag = ModelTypeToRootTag(syncable::NIGORI); - Entry entry(trans, GET_BY_SERVER_TAG, nigori_tag); - if (!entry.good()) { - VLOG(1) << "Nigori node not found, assuming no encrypted datatypes."; - return ModelTypeSet(); - } - if (NIGORI != entry.GetModelType()) { - // Can happen if we fail to apply the nigori node due to a conflict. - VLOG(1) << "Nigori node does not have nigori extension. Assuming no" - << " encrypted datatypes."; - return ModelTypeSet(); - } - const sync_pb::EntitySpecifics& specifics = entry.Get(SPECIFICS); - return GetEncryptedDataTypesFromNigori( - specifics.GetExtension(sync_pb::nigori)); -} - -ModelTypeSet GetEncryptedDataTypesFromNigori( - const sync_pb::NigoriSpecifics& nigori) { - // We don't check NIGORI datatype, it uses its own encryption scheme. - ModelTypeSet encrypted_types; - if (nigori.encrypt_bookmarks()) - encrypted_types.insert(BOOKMARKS); - if (nigori.encrypt_preferences()) - encrypted_types.insert(PREFERENCES); - if (nigori.encrypt_autofill_profile()) - encrypted_types.insert(AUTOFILL_PROFILE); - if (nigori.encrypt_autofill()) - encrypted_types.insert(AUTOFILL); - if (nigori.encrypt_themes()) - encrypted_types.insert(THEMES); - if (nigori.encrypt_typed_urls()) - encrypted_types.insert(TYPED_URLS); - if (nigori.encrypt_extensions()) - encrypted_types.insert(EXTENSIONS); - if (nigori.encrypt_sessions()) - encrypted_types.insert(SESSIONS); - if (nigori.encrypt_apps()) - encrypted_types.insert(APPS); - return encrypted_types; -} - -void FillNigoriEncryptedTypes(const ModelTypeSet& types, - sync_pb::NigoriSpecifics* nigori) { - DCHECK(nigori); - nigori->set_encrypt_bookmarks(types.count(BOOKMARKS) > 0); - nigori->set_encrypt_preferences(types.count(PREFERENCES) > 0); - nigori->set_encrypt_autofill_profile(types.count(AUTOFILL_PROFILE) > 0); - nigori->set_encrypt_autofill(types.count(AUTOFILL) > 0); - nigori->set_encrypt_themes(types.count(THEMES) > 0); - nigori->set_encrypt_typed_urls(types.count(TYPED_URLS) > 0); - nigori->set_encrypt_extensions(types.count(EXTENSIONS) > 0); - nigori->set_encrypt_sessions(types.count(SESSIONS) > 0); - nigori->set_encrypt_apps(types.count(APPS) > 0); -} - -bool ProcessUnsyncedChangesForEncryption( - WriteTransaction* const trans, - const ModelTypeSet& encrypted_types, - browser_sync::Cryptographer* cryptographer) { - // Get list of all datatypes with unsynced changes. It's possible that our - // local changes need to be encrypted if encryption for that datatype was - // just turned on (and vice versa). This should never affect passwords. - std::vector<int64> handles; - browser_sync::SyncerUtil::GetUnsyncedEntries(trans, &handles); - for (size_t i = 0; i < handles.size(); ++i) { - MutableEntry entry(trans, GET_BY_HANDLE, handles[i]); - sync_pb::EntitySpecifics new_specifics; - const sync_pb::EntitySpecifics& entry_specifics = entry.Get(SPECIFICS); - ModelType type = entry.GetModelType(); - if (type == PASSWORDS) - continue; - if (encrypted_types.count(type) > 0 && - !entry_specifics.has_encrypted()) { - // This entry now requires encryption. - AddDefaultExtensionValue(type, &new_specifics); - if (!cryptographer->Encrypt( - entry_specifics, - new_specifics.mutable_encrypted())) { - LOG(ERROR) << "Could not encrypt data for newly encrypted type " << - ModelTypeToString(type); - NOTREACHED(); - return false; - } else { - VLOG(1) << "Encrypted change for newly encrypted type " << - ModelTypeToString(type); - entry.Put(SPECIFICS, new_specifics); - } - } else if (encrypted_types.count(type) == 0 && - entry_specifics.has_encrypted()) { - // This entry no longer requires encryption. - if (!cryptographer->Decrypt(entry_specifics.encrypted(), - &new_specifics)) { - LOG(ERROR) << "Could not decrypt data for newly unencrypted type " << - ModelTypeToString(type); - NOTREACHED(); - return false; - } else { - VLOG(1) << "Decrypted change for newly unencrypted type " << - ModelTypeToString(type); - entry.Put(SPECIFICS, new_specifics); - } - } - } - return true; -} - -bool VerifyUnsyncedChangesAreEncrypted( - BaseTransaction* const trans, - const ModelTypeSet& encrypted_types) { - std::vector<int64> handles; - browser_sync::SyncerUtil::GetUnsyncedEntries(trans, &handles); - for (size_t i = 0; i < handles.size(); ++i) { - Entry entry(trans, GET_BY_HANDLE, handles[i]); - if (!entry.good()) { - NOTREACHED(); - return false; - } - const sync_pb::EntitySpecifics& entry_specifics = entry.Get(SPECIFICS); - ModelType type = entry.GetModelType(); - if (type == PASSWORDS) - continue; - if (encrypted_types.count(type) > 0 && - !entry_specifics.has_encrypted()) { - // This datatype requires encryption but this data is not encrypted. - return false; - } - } - return true; -} - -// Mainly for testing. -bool VerifyDataTypeEncryption(BaseTransaction* const trans, - ModelType type, - bool is_encrypted) { - if (type == PASSWORDS || type == NIGORI) { - NOTREACHED(); - return true; - } - std::string type_tag = ModelTypeToRootTag(type); - Entry type_root(trans, GET_BY_SERVER_TAG, type_tag); - if (!type_root.good()) { - NOTREACHED(); - return false; - } - - std::queue<Id> to_visit; - Id id_string = - trans->directory()->GetFirstChildId(trans, type_root.Get(ID)); - to_visit.push(id_string); - while (!to_visit.empty()) { - id_string = to_visit.front(); - to_visit.pop(); - if (id_string.IsRoot()) - continue; - - Entry child(trans, GET_BY_ID, id_string); - if (!child.good()) { - NOTREACHED(); - return false; - } - if (child.Get(IS_DIR)) { - // Traverse the children. - to_visit.push( - trans->directory()->GetFirstChildId(trans, child.Get(ID))); - } else { - const sync_pb::EntitySpecifics& specifics = child.Get(SPECIFICS); - DCHECK_EQ(type, child.GetModelType()); - DCHECK_EQ(type, GetModelTypeFromSpecifics(specifics)); - if (specifics.has_encrypted() != is_encrypted) - return false; - } - // Push the successor. - to_visit.push(child.Get(NEXT_ID)); - } - return true; -} - -} // namespace syncable diff --git a/chrome/browser/sync/syncable/nigori_util.h b/chrome/browser/sync/syncable/nigori_util.h deleted file mode 100644 index 3ad8fcd..0000000 --- a/chrome/browser/sync/syncable/nigori_util.h +++ /dev/null @@ -1,62 +0,0 @@ -// 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. - -// Various utility methods for nigory-based multi-type encryption. - -#ifndef CHROME_BROWSER_SYNC_SYNCABLE_NIGORI_UTIL_H_ -#define CHROME_BROWSER_SYNC_SYNCABLE_NIGORI_UTIL_H_ -#pragma once - -#include "chrome/browser/sync/protocol/nigori_specifics.pb.h" -#include "chrome/browser/sync/syncable/model_type.h" - -namespace browser_sync { -class Cryptographer; -} - -namespace syncable { - -class BaseTransaction; -class ReadTransaction; -class WriteTransaction; - -// Returns the set of datatypes that require encryption as specified by the -// Sync DB's nigori node. This will never include passwords, as the encryption -// status of that is always on if passwords are enabled.. -ModelTypeSet GetEncryptedDataTypes(BaseTransaction* const trans); - -// Extract the set of encrypted datatypes from a nigori node. -ModelTypeSet GetEncryptedDataTypesFromNigori( - const sync_pb::NigoriSpecifics& nigori); - -// Set the encrypted datatypes on the nigori node. -void FillNigoriEncryptedTypes(const ModelTypeSet& types, - sync_pb::NigoriSpecifics* nigori); - -// Check if our unsyced changes are encrypted if they need to be based on -// |encrypted_types|. -// Returns: true if all unsynced data that should be encrypted is. -// false if some unsynced changes need to be encrypted. -// This method is similar to ProcessUnsyncedChangesForEncryption but does not -// modify the data and does not care if data is unnecessarily encrypted. -bool VerifyUnsyncedChangesAreEncrypted( - BaseTransaction* const trans, - const ModelTypeSet& encrypted_types); - -// Processes all unsynced changes and ensures they are appropriately encrypted -// or unencrypted, based on |encrypted_types|. -bool ProcessUnsyncedChangesForEncryption( - WriteTransaction* const trans, - const syncable::ModelTypeSet& encrypted_types, - browser_sync::Cryptographer* cryptographer); - -// Verifies all data of type |type| is encrypted if |is_encrypted| is true or is -// unencrypted otherwise. -bool VerifyDataTypeEncryption(BaseTransaction* const trans, - ModelType type, - bool is_encrypted); - -} // namespace syncable - -#endif // CHROME_BROWSER_SYNC_SYNCABLE_NIGORI_UTIL_H_ diff --git a/chrome/browser/sync/syncable/nigori_util_unittest.cc b/chrome/browser/sync/syncable/nigori_util_unittest.cc deleted file mode 100644 index 14f147d..0000000 --- a/chrome/browser/sync/syncable/nigori_util_unittest.cc +++ /dev/null @@ -1,38 +0,0 @@ -// 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/syncable/nigori_util.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace syncable { - -typedef testing::Test NigoriUtilTest; - -TEST_F(NigoriUtilTest, NigoriEncryptionTypes) { - sync_pb::NigoriSpecifics nigori; - ModelTypeSet encrypted_types; - FillNigoriEncryptedTypes(encrypted_types, &nigori); - ModelTypeSet test_types = GetEncryptedDataTypesFromNigori(nigori); - EXPECT_EQ(encrypted_types, test_types); - - for (int i = FIRST_REAL_MODEL_TYPE; i < MODEL_TYPE_COUNT; ++i) { - encrypted_types.insert(ModelTypeFromInt(i)); - } - FillNigoriEncryptedTypes(encrypted_types, &nigori); - test_types = GetEncryptedDataTypesFromNigori(nigori); - encrypted_types.erase(syncable::NIGORI); // Should not get set. - encrypted_types.erase(syncable::PASSWORDS); // Should not get set. - EXPECT_EQ(encrypted_types, test_types); - - encrypted_types.erase(syncable::BOOKMARKS); - encrypted_types.erase(syncable::SESSIONS); - FillNigoriEncryptedTypes(encrypted_types, &nigori); - test_types = GetEncryptedDataTypesFromNigori(nigori); - EXPECT_EQ(encrypted_types, test_types); -} - -// ProcessUnsyncedChangesForEncryption and other methods that rely on the syncer -// are tested in apply_updates_command_unittest.cc - -} // namespace syncable diff --git a/chrome/browser/sync/syncable/syncable.cc b/chrome/browser/sync/syncable/syncable.cc index 7c0dcb4..ac66083 100644 --- a/chrome/browser/sync/syncable/syncable.cc +++ b/chrome/browser/sync/syncable/syncable.cc @@ -38,6 +38,10 @@ #include "base/time.h" #include "chrome/browser/sync/engine/syncer.h" #include "chrome/browser/sync/engine/syncer_util.h" +#include "chrome/browser/sync/protocol/autofill_specifics.pb.h" +#include "chrome/browser/sync/protocol/bookmark_specifics.pb.h" +#include "chrome/browser/sync/protocol/password_specifics.pb.h" +#include "chrome/browser/sync/protocol/preference_specifics.pb.h" #include "chrome/browser/sync/protocol/service_constants.h" #include "chrome/browser/sync/protocol/theme_specifics.pb.h" #include "chrome/browser/sync/protocol/typed_url_specifics.pb.h" @@ -223,7 +227,7 @@ void Directory::Kernel::Release() { } Directory::Kernel::~Kernel() { - CHECK_EQ(0, refcount); + CHECK(0 == refcount); delete channel; changes_channel.Notify(kShutdownChangesEvent); delete unsynced_metahandles; @@ -486,14 +490,14 @@ void Directory::Delete(EntryKernel* const entry) { entry->put(IS_DEL, true); entry->mark_dirty(kernel_->dirty_metahandles); ScopedKernelLock lock(this); - CHECK_EQ(1U, kernel_->parent_id_child_index->erase(entry)); + CHECK(1 == kernel_->parent_id_child_index->erase(entry)); } bool Directory::ReindexId(EntryKernel* const entry, const Id& new_id) { ScopedKernelLock lock(this); if (NULL != GetEntryById(new_id, &lock)) return false; - CHECK_EQ(1U, kernel_->ids_index->erase(entry)); + CHECK(1 == kernel_->ids_index->erase(entry)); entry->put(ID, new_id); CHECK(kernel_->ids_index->insert(entry).second); return true; @@ -501,6 +505,7 @@ bool Directory::ReindexId(EntryKernel* const entry, const Id& new_id) { void Directory::ReindexParentId(EntryKernel* const entry, const Id& new_parent_id) { + ScopedKernelLock lock(this); if (entry->ref(IS_DEL)) { entry->put(PARENT_ID, new_parent_id); @@ -511,7 +516,7 @@ void Directory::ReindexParentId(EntryKernel* const entry, return; } - CHECK_EQ(1U, kernel_->parent_id_child_index->erase(entry)); + CHECK(1 == kernel_->parent_id_child_index->erase(entry)); entry->put(PARENT_ID, new_parent_id); CHECK(kernel_->parent_id_child_index->insert(entry).second); } @@ -528,7 +533,7 @@ bool Directory::SafeToPurgeFromMemory(const EntryKernel* const entry) const { if (safe) { int64 handle = entry->ref(META_HANDLE); - CHECK_EQ(kernel_->dirty_metahandles->count(handle), 0U); + CHECK(kernel_->dirty_metahandles->count(handle) == 0); // TODO(tim): Bug 49278. CHECK(!kernel_->unsynced_metahandles->count(handle)); CHECK(!kernel_->unapplied_update_metahandles->count(handle)); @@ -996,7 +1001,7 @@ void Directory::CheckTreeInvariants(syncable::BaseTransaction* trans, CHECK(handles.end() != handles.find(parent.Get(META_HANDLE))) << e << parent; parentid = parent.Get(PARENT_ID); - CHECK_GE(--safety_count, 0) << e << parent; + CHECK(--safety_count >= 0) << e << parent; } } int64 base_version = e.Get(BASE_VERSION); @@ -1025,7 +1030,7 @@ void Directory::CheckTreeInvariants(syncable::BaseTransaction* trans, // on the server, isn't waiting for application locally, but either // is an unsynced create or a sucessful delete in the local copy. // Either way, that's a mismatch. - CHECK_EQ(0, server_version) << e; + CHECK(0 == server_version) << e; // Items that aren't using the unique client tag should have a zero // base version only if they have a local ID. Items with unique client // tags are allowed to use the zero base version for undeletion and @@ -1265,7 +1270,7 @@ syncable::ModelType Entry::GetServerModelType() const { // It's possible we'll need to relax these checks in the future; they're // just here for now as a safety measure. DCHECK(Get(IS_UNSYNCED)); - DCHECK_EQ(Get(SERVER_VERSION), 0); + DCHECK(Get(SERVER_VERSION) == 0); DCHECK(Get(SERVER_IS_DEL)); // Note: can't enforce !Get(ID).ServerKnows() here because that could // actually happen if we hit AttemptReuniteLostCommitResponses. @@ -1496,7 +1501,7 @@ bool MutableEntry::Put(IndexedBitField field, bool value) { if (value) CHECK(index->insert(kernel_->ref(META_HANDLE)).second); else - CHECK_EQ(1U, index->erase(kernel_->ref(META_HANDLE))); + CHECK(1 == index->erase(kernel_->ref(META_HANDLE))); kernel_->put(field, value); kernel_->mark_dirty(dir()->kernel_->dirty_metahandles); } diff --git a/chrome/browser/sync/util/cryptographer.cc b/chrome/browser/sync/util/cryptographer.cc index da94681..747b094 100644 --- a/chrome/browser/sync/util/cryptographer.cc +++ b/chrome/browser/sync/util/cryptographer.cc @@ -59,24 +59,19 @@ bool Cryptographer::Encrypt(const ::google::protobuf::MessageLite& message, bool Cryptographer::Decrypt(const sync_pb::EncryptedData& encrypted, ::google::protobuf::MessageLite* message) const { DCHECK(message); - std::string plaintext = DecryptToString(encrypted); - return message->ParseFromString(plaintext); -} -std::string Cryptographer::DecryptToString( - const sync_pb::EncryptedData& encrypted) const { NigoriMap::const_iterator it = nigoris_.find(encrypted.key_name()); if (nigoris_.end() == it) { NOTREACHED() << "Cannot decrypt message"; - return std::string(""); // Caller should have called CanDecrypt(encrypt). + return false; // Caller should have called CanDecrypt(encrypt). } std::string plaintext; if (!it->second->Decrypt(encrypted.blob(), &plaintext)) { - return std::string(""); + return false; } - return plaintext; + return message->ParseFromString(plaintext); } bool Cryptographer::GetKeys(sync_pb::EncryptedData* encrypted) const { @@ -209,7 +204,7 @@ Nigori* Cryptographer::UnpackBootstrapToken(const std::string& token) const { return NULL; std::string encrypted_data; - if (!base::Base64Decode(token, &encrypted_data)) { + if (!base::Base64Decode(token, &encrypted_data)){ DLOG(WARNING) << "Could not decode token."; return NULL; } diff --git a/chrome/browser/sync/util/cryptographer.h b/chrome/browser/sync/util/cryptographer.h index adb809b..ada084cc 100644 --- a/chrome/browser/sync/util/cryptographer.h +++ b/chrome/browser/sync/util/cryptographer.h @@ -71,10 +71,6 @@ class Cryptographer { bool Decrypt(const sync_pb::EncryptedData& encrypted, ::google::protobuf::MessageLite* message) const; - // Decrypts |encrypted| and returns plaintext decrypted data. If decryption - // fails, returns empty string. - std::string DecryptToString(const sync_pb::EncryptedData& encrypted) const; - // Encrypts the set of currently known keys into |encrypted|. Returns true if // successful. bool GetKeys(sync_pb::EncryptedData* encrypted) const; |