diff options
26 files changed, 324 insertions, 162 deletions
diff --git a/chrome/browser/sync/engine/apply_updates_command_unittest.cc b/chrome/browser/sync/engine/apply_updates_command_unittest.cc index 4af85cc..e8d4604 100644 --- a/chrome/browser/sync/engine/apply_updates_command_unittest.cc +++ b/chrome/browser/sync/engine/apply_updates_command_unittest.cc @@ -19,6 +19,7 @@ #include "chrome/browser/sync/test/engine/fake_model_worker.h" #include "chrome/browser/sync/test/engine/syncer_command_test.h" #include "chrome/browser/sync/test/engine/test_id_factory.h" +#include "chrome/browser/sync/test/fake_encryptor.h" #include "chrome/browser/sync/util/cryptographer.h" #include "testing/gtest/include/gtest/gtest.h" @@ -208,6 +209,7 @@ class ApplyUpdatesCommandTest : public SyncerCommandTest { } ApplyUpdatesCommand apply_updates_command_; + FakeEncryptor encryptor_; TestIdFactory id_factory_; private: int64 next_revision_; @@ -627,7 +629,7 @@ TEST_F(ApplyUpdatesCommandTest, SomeUndecryptablePassword) { } { // Create a new cryptographer, independent of the one in the session. - Cryptographer cryptographer; + Cryptographer cryptographer(&encryptor_); KeyParams params = {"localhost", "dummy", "bazqux"}; cryptographer.AddKey(params); @@ -678,7 +680,7 @@ TEST_F(ApplyUpdatesCommandTest, NigoriUpdate) { } // Nigori node updates should update the Cryptographer. - Cryptographer other_cryptographer; + Cryptographer other_cryptographer(&encryptor_); KeyParams params = {"localhost", "dummy", "foobar"}; other_cryptographer.AddKey(params); @@ -727,7 +729,7 @@ TEST_F(ApplyUpdatesCommandTest, NigoriUpdateForDisabledTypes) { } // Nigori node updates should update the Cryptographer. - Cryptographer other_cryptographer; + Cryptographer other_cryptographer(&encryptor_); KeyParams params = {"localhost", "dummy", "foobar"}; other_cryptographer.AddKey(params); @@ -948,7 +950,7 @@ TEST_F(ApplyUpdatesCommandTest, CannotEncryptUnsyncedChanges) { // We encrypt with new keys, triggering the local cryptographer to be unready // and unable to decrypt data (once updated). - Cryptographer other_cryptographer; + Cryptographer other_cryptographer(&encryptor_); KeyParams params = {"localhost", "dummy", "foobar"}; other_cryptographer.AddKey(params); sync_pb::EntitySpecifics specifics; diff --git a/chrome/browser/sync/engine/syncer_unittest.cc b/chrome/browser/sync/engine/syncer_unittest.cc index d8c8220..374dec0 100644 --- a/chrome/browser/sync/engine/syncer_unittest.cc +++ b/chrome/browser/sync/engine/syncer_unittest.cc @@ -44,6 +44,7 @@ #include "chrome/browser/sync/test/engine/test_directory_setter_upper.h" #include "chrome/browser/sync/test/engine/test_id_factory.h" #include "chrome/browser/sync/test/engine/test_syncable_utils.h" +#include "chrome/browser/sync/test/fake_encryptor.h" #include "chrome/browser/sync/test/fake_extensions_activity_monitor.h" #include "chrome/browser/sync/util/cryptographer.h" #include "chrome/browser/sync/util/time.h" @@ -491,6 +492,7 @@ class SyncerTest : public testing::Test, TestIdFactory ids_; TestDirectorySetterUpper dir_maker_; + FakeEncryptor encryptor_; FakeExtensionsActivityMonitor extensions_activity_monitor_; scoped_ptr<MockConnectionManager> mock_server_; @@ -658,7 +660,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) { // Mark bookmarks as encrypted and set the cryptographer to have pending // keys. WriteTransaction wtrans(FROM_HERE, UNITTEST, directory()); - browser_sync::Cryptographer other_cryptographer; + browser_sync::Cryptographer other_cryptographer(&encryptor_); other_cryptographer.AddKey(other_params); sync_pb::EntitySpecifics specifics; sync_pb::NigoriSpecifics* nigori = @@ -757,7 +759,7 @@ TEST_F(SyncerTest, GetCommitIdsFiltersUnreadyEntries) { TEST_F(SyncerTest, EncryptionAwareConflicts) { KeyParams key_params = {"localhost", "dummy", "foobar"}; - browser_sync::Cryptographer other_cryptographer; + browser_sync::Cryptographer other_cryptographer(&encryptor_); other_cryptographer.AddKey(key_params); sync_pb::EntitySpecifics bookmark, encrypted_bookmark, modified_bookmark; bookmark.MutableExtension(sync_pb::bookmark)->set_title("title"); @@ -941,7 +943,7 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) { TEST_F(SyncerTest, NigoriConflicts) { KeyParams local_key_params = {"localhost", "dummy", "blargle"}; KeyParams other_key_params = {"localhost", "dummy", "foobar"}; - browser_sync::Cryptographer other_cryptographer; + browser_sync::Cryptographer other_cryptographer(&encryptor_); other_cryptographer.AddKey(other_key_params); syncable::ModelTypeSet encrypted_types(syncable::PASSWORDS, syncable::NIGORI); sync_pb::EntitySpecifics initial_nigori_specifics; diff --git a/chrome/browser/sync/glue/chrome_encryptor.cc b/chrome/browser/sync/glue/chrome_encryptor.cc new file mode 100644 index 0000000..4f08684 --- /dev/null +++ b/chrome/browser/sync/glue/chrome_encryptor.cc @@ -0,0 +1,23 @@ +// Copyright (c) 2012 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/glue/chrome_encryptor.h" + +#include "chrome/browser/password_manager/encryptor.h" + +namespace browser_sync { + +ChromeEncryptor::~ChromeEncryptor() {} + +bool ChromeEncryptor::EncryptString(const std::string& plaintext, + std::string* ciphertext) { + return ::Encryptor::EncryptString(plaintext, ciphertext); +} + +bool ChromeEncryptor::DecryptString(const std::string& ciphertext, + std::string* plaintext) { + return ::Encryptor::EncryptString(ciphertext, plaintext); +} + +} // namespace browser_sync diff --git a/chrome/browser/sync/glue/chrome_encryptor.h b/chrome/browser/sync/glue/chrome_encryptor.h new file mode 100644 index 0000000..a0f2a50 --- /dev/null +++ b/chrome/browser/sync/glue/chrome_encryptor.h @@ -0,0 +1,28 @@ +// Copyright (c) 2012 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. + +#ifndef CHROME_BROWSER_SYNC_GLUE_CHROME_ENCRYPTOR_H_ +#define CHROME_BROWSER_SYNC_GLUE_CHROME_ENCRYPTOR_H_ +#pragma once + +#include "base/compiler_specific.h" +#include "chrome/browser/sync/util/encryptor.h" + +namespace browser_sync { + +// Encryptor that uses the Chrome password manager's encryptor. +class ChromeEncryptor : public Encryptor { + public: + virtual ~ChromeEncryptor(); + + virtual bool EncryptString(const std::string& plaintext, + std::string* ciphertext) OVERRIDE; + + virtual bool DecryptString(const std::string& ciphertext, + std::string* plaintext) OVERRIDE; +}; + +} // namespace browser_sync + +#endif // CHROME_BROWSER_SYNC_GLUE_CHROME_ENCRYPTOR_H_ diff --git a/chrome/browser/sync/glue/sync_backend_host.cc b/chrome/browser/sync/glue/sync_backend_host.cc index 6bdc86b..abb0840 100644 --- a/chrome/browser/sync/glue/sync_backend_host.cc +++ b/chrome/browser/sync/glue/sync_backend_host.cc @@ -21,6 +21,7 @@ #include "chrome/browser/profiles/profile.h" #include "chrome/browser/signin/token_service.h" #include "chrome/browser/sync/glue/change_processor.h" +#include "chrome/browser/sync/glue/chrome_encryptor.h" #include "chrome/browser/sync/glue/http_bridge.h" #include "chrome/browser/sync/glue/sync_backend_registrar.h" #include "chrome/browser/sync/internal_api/base_transaction.h" @@ -212,6 +213,9 @@ class SyncBackendHost::Core // The timer used to periodically call SaveChanges. base::RepeatingTimer<Core> save_changes_timer_; + // Our encryptor, which uses Chrome's encryption functions. + ChromeEncryptor encryptor_; + // The top-level syncapi entry point. Lives on the sync thread. scoped_ptr<sync_api::SyncManager> sync_manager_; @@ -970,6 +974,7 @@ void SyncBackendHost::Core::DoInitialize(const DoInitializeOptions& options) { options.sync_notifier_factory->CreateSyncNotifier(), options.restored_key_for_bootstrapping, options.setup_for_test_mode, + &encryptor_, options.unrecoverable_error_handler, options.report_unrecoverable_error_function); LOG_IF(ERROR, !success) << "Syncapi initialization failed!"; diff --git a/chrome/browser/sync/internal_api/DEPS b/chrome/browser/sync/internal_api/DEPS index 656563b..486abe5 100644 --- a/chrome/browser/sync/internal_api/DEPS +++ b/chrome/browser/sync/internal_api/DEPS @@ -12,8 +12,5 @@ include_rules = [ # We store the Chrome version in the nigori node. "+chrome/common/chrome_version_info.h", - # unittests need this for mac osx keychain overriding - "+chrome/browser/password_manager/encryptor.h", - "+chrome/common/net/gaia/google_service_auth_error.h", ] diff --git a/chrome/browser/sync/internal_api/sync_manager.cc b/chrome/browser/sync/internal_api/sync_manager.cc index 3ea1667..01541fe 100644 --- a/chrome/browser/sync/internal_api/sync_manager.cc +++ b/chrome/browser/sync/internal_api/sync_manager.cc @@ -62,6 +62,7 @@ using std::string; using base::TimeDelta; using browser_sync::AllStatus; using browser_sync::Cryptographer; +using browser_sync::Encryptor; using browser_sync::JsArgList; using browser_sync::JsBackend; using browser_sync::JsEventDetails; @@ -142,6 +143,7 @@ class SyncManager::SyncInternal initialized_(false), setup_for_test_mode_(false), observing_ip_address_changes_(false), + encryptor_(NULL), unrecoverable_error_handler_(NULL), report_unrecoverable_error_function_(NULL), created_on_loop_(MessageLoop::current()) { @@ -195,6 +197,7 @@ class SyncManager::SyncInternal sync_notifier::SyncNotifier* sync_notifier, const std::string& restored_key_for_bootstrapping, bool setup_for_test_mode, + Encryptor* encryptor, UnrecoverableErrorHandler* unrecoverable_error_handler, ReportUnrecoverableErrorFunction report_unrecoverable_error_function); @@ -592,6 +595,7 @@ class SyncManager::SyncInternal // This is for keeping track of client events to send to the server. DebugInfoEventListener debug_info_event_listener_; + Encryptor* encryptor_; UnrecoverableErrorHandler* unrecoverable_error_handler_; ReportUnrecoverableErrorFunction report_unrecoverable_error_function_; @@ -726,6 +730,7 @@ bool SyncManager::Init( sync_notifier::SyncNotifier* sync_notifier, const std::string& restored_key_for_bootstrapping, bool setup_for_test_mode, + Encryptor* encryptor, UnrecoverableErrorHandler* unrecoverable_error_handler, ReportUnrecoverableErrorFunction report_unrecoverable_error_function) { DCHECK(thread_checker_.CalledOnValidThread()); @@ -746,6 +751,7 @@ bool SyncManager::Init( sync_notifier, restored_key_for_bootstrapping, setup_for_test_mode, + encryptor, unrecoverable_error_handler, report_unrecoverable_error_function); } @@ -861,6 +867,7 @@ bool SyncManager::SyncInternal::Init( sync_notifier::SyncNotifier* sync_notifier, const std::string& restored_key_for_bootstrapping, bool setup_for_test_mode, + Encryptor* encryptor, UnrecoverableErrorHandler* unrecoverable_error_handler, ReportUnrecoverableErrorFunction report_unrecoverable_error_function) { CHECK(!initialized_); @@ -884,10 +891,12 @@ bool SyncManager::SyncInternal::Init( database_path_ = database_location.Append( syncable::Directory::kSyncDatabaseFilename); + encryptor_ = encryptor; unrecoverable_error_handler_ = unrecoverable_error_handler; report_unrecoverable_error_function_ = report_unrecoverable_error_function; share_.directory.reset( - new syncable::Directory(unrecoverable_error_handler_, + new syncable::Directory(encryptor_, + unrecoverable_error_handler_, report_unrecoverable_error_function_)); connection_manager_.reset(new SyncAPIServerConnectionManager( @@ -1375,7 +1384,7 @@ bool SyncManager::SyncInternal::SetDecryptionPassphrase( // password, we need to generate a new bootstrap token to preserve it. // We build a temporary cryptographer to allow us to extract these // params without polluting our current cryptographer. - Cryptographer temp_cryptographer; + Cryptographer temp_cryptographer(encryptor_); temp_cryptographer.AddKey(key_params); temp_cryptographer.GetBootstrapToken(bootstrap_token); // We then set the new passphrase as the default passphrase of the @@ -1395,7 +1404,7 @@ bool SyncManager::SyncInternal::SetDecryptionPassphrase( // Otherwise, we're in a situation where the pending keys are // encrypted with an old gaia passphrase, while the default is the // current gaia passphrase. In that case, we preserve the default. - Cryptographer temp_cryptographer; + Cryptographer temp_cryptographer(encryptor_); temp_cryptographer.SetPendingKeys(cryptographer->GetPendingKeys()); if (temp_cryptographer.DecryptPendingKeys(key_params)) { // Check to see if the pending bag of keys contains the current diff --git a/chrome/browser/sync/internal_api/sync_manager.h b/chrome/browser/sync/internal_api/sync_manager.h index 013d7df..0948353 100644 --- a/chrome/browser/sync/internal_api/sync_manager.h +++ b/chrome/browser/sync/internal_api/sync_manager.h @@ -23,6 +23,7 @@ #include "chrome/common/net/gaia/google_service_auth_error.h" namespace browser_sync { +class Encryptor; class ExtensionsActivityMonitor; class JsBackend; class JsEventHandler; @@ -461,6 +462,7 @@ class SyncManager { sync_notifier::SyncNotifier* sync_notifier, const std::string& restored_key_for_bootstrapping, bool setup_for_test_mode, + browser_sync::Encryptor* encryptor, browser_sync::UnrecoverableErrorHandler* unrecoverable_error_handler, browser_sync::ReportUnrecoverableErrorFunction diff --git a/chrome/browser/sync/internal_api/syncapi_unittest.cc b/chrome/browser/sync/internal_api/syncapi_unittest.cc index f64e83fc..7b7c3e28 100644 --- a/chrome/browser/sync/internal_api/syncapi_unittest.cc +++ b/chrome/browser/sync/internal_api/syncapi_unittest.cc @@ -23,7 +23,6 @@ #include "base/test/values_test_util.h" #include "base/utf_string_conversions.h" #include "base/values.h" -#include "chrome/browser/password_manager/encryptor.h" #include "chrome/browser/sync/engine/model_safe_worker.h" #include "chrome/browser/sync/engine/nigori_util.h" #include "chrome/browser/sync/engine/polling_constants.h" @@ -56,6 +55,7 @@ #include "chrome/browser/sync/syncable/syncable.h" #include "chrome/browser/sync/syncable/syncable_id.h" #include "chrome/browser/sync/test/engine/test_user_share.h" +#include "chrome/browser/sync/test/fake_encryptor.h" #include "chrome/browser/sync/test/fake_extensions_activity_monitor.h" #include "chrome/browser/sync/util/cryptographer.h" #include "chrome/browser/sync/util/extensions_activity_monitor.h" @@ -66,6 +66,7 @@ using base::ExpectDictStringValue; using browser_sync::Cryptographer; +using browser_sync::FakeEncryptor; using browser_sync::FakeExtensionsActivityMonitor; using browser_sync::HasArgsAsList; using browser_sync::HasDetailsAsDictionary; @@ -81,6 +82,7 @@ using browser_sync::ModelSafeRoutingInfo; using browser_sync::ModelSafeWorker; using browser_sync::ModelSafeWorkerRegistrar; using browser_sync::sessions::SyncSessionSnapshot; +using browser_sync::TestUnrecoverableErrorHandler; using browser_sync::WeakHandle; using content::BrowserThread; using syncable::IS_DEL; @@ -758,7 +760,6 @@ class SyncManagerTest : public testing::Test, EXPECT_FALSE(sync_notifier_observer_); EXPECT_FALSE(js_backend_.IsInitialized()); - browser_sync::TestUnrecoverableErrorHandler handler; // Takes ownership of |sync_notifier_mock_|. sync_manager_.Init(temp_dir_.path(), WeakHandle<JsEventHandler>(), @@ -767,7 +768,8 @@ class SyncManagerTest : public testing::Test, &extensions_activity_monitor_, this, "bogus", credentials, sync_notifier_mock_, "", true /* setup_for_test_mode */, - &handler, + &encryptor_, + &handler_, NULL); EXPECT_TRUE(sync_notifier_observer_); @@ -817,11 +819,6 @@ class SyncManagerTest : public testing::Test, // Helper methods. bool SetUpEncryption(NigoriStatus nigori_status, EncryptionStatus encryption_status) { - // Mock the Mac Keychain service. The real Keychain can block on user input. - #if defined(OS_MACOSX) - Encryptor::UseMockKeychain(true); - #endif - UserShare* share = sync_manager_.GetUserShare(); share->directory->set_initial_sync_ended_for_type(syncable::NIGORI, true); @@ -931,6 +928,8 @@ class SyncManagerTest : public testing::Test, StrictMock<SyncNotifierMock>* sync_notifier_mock_; protected: + FakeEncryptor encryptor_; + TestUnrecoverableErrorHandler handler_; SyncManager sync_manager_; WeakHandle<JsBackend> js_backend_; StrictMock<SyncManagerObserverMock> observer_; @@ -1515,7 +1514,7 @@ TEST_F(SyncManagerTest, SetInitialGaiaPass) { // (case 1 in SyncManager::SyncInternalSetPassphrase) TEST_F(SyncManagerTest, UpdateGaiaPass) { EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION)); - Cryptographer verifier; + Cryptographer verifier(&encryptor_); { ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); Cryptographer* cryptographer = trans.GetCryptographer(); @@ -1543,7 +1542,7 @@ TEST_F(SyncManagerTest, UpdateGaiaPass) { // and re-encrypt everything. // (case 2 in SyncManager::SyncInternalSetPassphrase) TEST_F(SyncManagerTest, SetPassphraseWithPassword) { - Cryptographer verifier; + Cryptographer verifier(&encryptor_); EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION)); { WriteTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); @@ -1592,7 +1591,7 @@ TEST_F(SyncManagerTest, SetPassphraseWithPassword) { // (case 3 in SyncManager::SyncInternalSetPassphrase) TEST_F(SyncManagerTest, SupplyPendingGAIAPass) { EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION)); - Cryptographer other_cryptographer; + Cryptographer other_cryptographer(&encryptor_); { WriteTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); Cryptographer* cryptographer = trans.GetCryptographer(); @@ -1636,7 +1635,7 @@ TEST_F(SyncManagerTest, SupplyPendingGAIAPass) { // (case 4 in SyncManager::SyncInternalSetPassphrase) TEST_F(SyncManagerTest, SupplyPendingOldGAIAPass) { EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION)); - Cryptographer other_cryptographer; + Cryptographer other_cryptographer(&encryptor_); { WriteTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); Cryptographer* cryptographer = trans.GetCryptographer(); @@ -1694,7 +1693,7 @@ TEST_F(SyncManagerTest, SupplyPendingOldGAIAPass) { EXPECT_TRUE(cryptographer->CanDecrypt(encrypted)); // Verify the saved bootstrap token is based on the new gaia password. - Cryptographer temp_cryptographer; + Cryptographer temp_cryptographer(&encryptor_); temp_cryptographer.Bootstrap(bootstrap_token); EXPECT_TRUE(temp_cryptographer.CanDecrypt(encrypted)); } @@ -1706,7 +1705,7 @@ TEST_F(SyncManagerTest, SupplyPendingOldGAIAPass) { // (case 5 in SyncManager::SyncInternalSetPassphrase) TEST_F(SyncManagerTest, SupplyPendingExplicitPass) { EXPECT_TRUE(SetUpEncryption(WRITE_TO_NIGORI, DEFAULT_ENCRYPTION)); - Cryptographer other_cryptographer; + Cryptographer other_cryptographer(&encryptor_); { WriteTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); Cryptographer* cryptographer = trans.GetCryptographer(); diff --git a/chrome/browser/sync/syncable/syncable.cc b/chrome/browser/sync/syncable/syncable.cc index bc12516..ce2d8bf 100644 --- a/chrome/browser/sync/syncable/syncable.cc +++ b/chrome/browser/sync/syncable/syncable.cc @@ -94,6 +94,7 @@ bool VerifyReferenceIntegrityUnsafe(const syncable::MetahandlesIndex &index) { } // namespace using std::string; +using browser_sync::Encryptor; using browser_sync::ReportUnrecoverableErrorFunction; using browser_sync::UnrecoverableErrorHandler; @@ -514,9 +515,11 @@ Directory::Kernel::~Kernel() { } Directory::Directory( + Encryptor* encryptor, UnrecoverableErrorHandler* unrecoverable_error_handler, ReportUnrecoverableErrorFunction report_unrecoverable_error_function) - : kernel_(NULL), + : cryptographer_(encryptor), + kernel_(NULL), store_(NULL), unrecoverable_error_handler_(unrecoverable_error_handler), report_unrecoverable_error_function_( diff --git a/chrome/browser/sync/syncable/syncable.h b/chrome/browser/sync/syncable/syncable.h index d28e4c0..e2922f6 100644 --- a/chrome/browser/sync/syncable/syncable.h +++ b/chrome/browser/sync/syncable/syncable.h @@ -43,6 +43,10 @@ class DictionaryValue; class ListValue; } +namespace browser_sync { +class Encryptor; +} // namespace browser_sync + namespace sync_api { class ReadTransaction; class WriteNode; @@ -811,8 +815,10 @@ class Directory { MetahandleSet metahandles_to_purge; }; + // Does not take ownership of |encryptor|. // |report_unrecoverable_error_function| may be NULL. Directory( + browser_sync::Encryptor* encryptor, browser_sync::UnrecoverableErrorHandler* unrecoverable_error_handler, browser_sync::ReportUnrecoverableErrorFunction report_unrecoverable_error_function); diff --git a/chrome/browser/sync/syncable/syncable_mock.cc b/chrome/browser/sync/syncable/syncable_mock.cc index 0ef8542..1fb55fa 100644 --- a/chrome/browser/sync/syncable/syncable_mock.cc +++ b/chrome/browser/sync/syncable/syncable_mock.cc @@ -8,7 +8,7 @@ #include "chrome/browser/sync/test/null_transaction_observer.h" MockDirectory::MockDirectory(browser_sync::UnrecoverableErrorHandler* handler) - : Directory(handler, NULL) { + : Directory(&encryptor_, handler, NULL) { InitKernelForTest("myk", &delegate_, syncable::NullTransactionObserver()); } diff --git a/chrome/browser/sync/syncable/syncable_mock.h b/chrome/browser/sync/syncable/syncable_mock.h index d3e136b..1f8ed48 100644 --- a/chrome/browser/sync/syncable/syncable_mock.h +++ b/chrome/browser/sync/syncable/syncable_mock.h @@ -9,6 +9,7 @@ #include <string> #include "chrome/browser/sync/syncable/syncable.h" +#include "chrome/browser/sync/test/fake_encryptor.h" #include "chrome/browser/sync/test/null_directory_change_delegate.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -31,6 +32,7 @@ class MockDirectory : public Directory { MOCK_METHOD1(PurgeEntriesWithTypeIn, void(syncable::ModelTypeSet)); private: + browser_sync::FakeEncryptor encryptor_; syncable::NullDirectoryChangeDelegate delegate_; }; diff --git a/chrome/browser/sync/syncable/syncable_unittest.cc b/chrome/browser/sync/syncable/syncable_unittest.cc index 4d1ffe6..a18930a 100644 --- a/chrome/browser/sync/syncable/syncable_unittest.cc +++ b/chrome/browser/sync/syncable/syncable_unittest.cc @@ -26,14 +26,17 @@ #include "chrome/browser/sync/syncable/directory_change_delegate.h" #include "chrome/browser/sync/test/engine/test_id_factory.h" #include "chrome/browser/sync/test/engine/test_syncable_utils.h" +#include "chrome/browser/sync/test/fake_encryptor.h" #include "chrome/browser/sync/test/null_directory_change_delegate.h" #include "chrome/browser/sync/test/null_transaction_observer.h" #include "testing/gtest/include/gtest/gtest.h" #include "third_party/sqlite/sqlite3.h" -using browser_sync::TestIdFactory; using base::ExpectDictBooleanValue; using base::ExpectDictStringValue; +using browser_sync::FakeEncryptor; +using browser_sync::TestIdFactory; +using browser_sync::TestUnrecoverableErrorHandler; namespace syncable { @@ -94,12 +97,13 @@ class SyncableGeneralTest : public testing::Test { MessageLoop message_loop_; ScopedTempDir temp_dir_; NullDirectoryChangeDelegate delegate_; + FakeEncryptor encryptor_; + TestUnrecoverableErrorHandler handler_; FilePath db_path_; }; TEST_F(SyncableGeneralTest, General) { - browser_sync::TestUnrecoverableErrorHandler handler; - Directory dir(&handler, NULL); + Directory dir(&encryptor_, &handler_, NULL); ASSERT_EQ(OPENED, dir.Open(db_path_, "SimpleTest", &delegate_, NullTransactionObserver())); @@ -199,8 +203,7 @@ TEST_F(SyncableGeneralTest, General) { } TEST_F(SyncableGeneralTest, ChildrenOps) { - browser_sync::TestUnrecoverableErrorHandler handler; - Directory dir(&handler, NULL); + Directory dir(&encryptor_, &handler_, NULL); ASSERT_EQ(OPENED, dir.Open(db_path_, "SimpleTest", &delegate_, NullTransactionObserver())); @@ -273,8 +276,7 @@ TEST_F(SyncableGeneralTest, ClientIndexRebuildsProperly) { // Test creating a new meta entry. { - browser_sync::TestUnrecoverableErrorHandler handler; - Directory dir(&handler, NULL); + Directory dir(&encryptor_, &handler_, NULL); ASSERT_EQ(OPENED, dir.Open(db_path_, "IndexTest", &delegate_, NullTransactionObserver())); { @@ -291,8 +293,7 @@ TEST_F(SyncableGeneralTest, ClientIndexRebuildsProperly) { // The DB was closed. Now reopen it. This will cause index regeneration. { - browser_sync::TestUnrecoverableErrorHandler handler; - Directory dir(&handler, NULL); + Directory dir(&encryptor_, &handler_, NULL); ASSERT_EQ(OPENED, dir.Open(db_path_, "IndexTest", &delegate_, NullTransactionObserver())); @@ -313,8 +314,7 @@ TEST_F(SyncableGeneralTest, ClientIndexRebuildsDeletedProperly) { // Test creating a deleted, unsynced, server meta entry. { - browser_sync::TestUnrecoverableErrorHandler handler; - Directory dir(&handler, NULL); + Directory dir(&encryptor_, &handler_, NULL); ASSERT_EQ(OPENED, dir.Open(db_path_, "IndexTest", &delegate_, NullTransactionObserver())); { @@ -333,8 +333,7 @@ TEST_F(SyncableGeneralTest, ClientIndexRebuildsDeletedProperly) { // The DB was closed. Now reopen it. This will cause index regeneration. // Should still be present and valid in the client tag index. { - browser_sync::TestUnrecoverableErrorHandler handler; - Directory dir(&handler, NULL); + Directory dir(&encryptor_, &handler_, NULL); ASSERT_EQ(OPENED, dir.Open(db_path_, "IndexTest", &delegate_, NullTransactionObserver())); @@ -349,8 +348,7 @@ TEST_F(SyncableGeneralTest, ClientIndexRebuildsDeletedProperly) { } TEST_F(SyncableGeneralTest, ToValue) { - browser_sync::TestUnrecoverableErrorHandler handler; - Directory dir(&handler, NULL); + Directory dir(&encryptor_, &handler_, NULL); ASSERT_EQ(OPENED, dir.Open(db_path_, "SimpleTest", &delegate_, NullTransactionObserver())); @@ -387,7 +385,7 @@ TEST_F(SyncableGeneralTest, ToValue) { // A Directory whose backing store always fails SaveChanges by returning false. class TestUnsaveableDirectory : public Directory { public: - TestUnsaveableDirectory() : Directory(&handler_, NULL) {} + TestUnsaveableDirectory() : Directory(&encryptor_, &handler_, NULL) {} class UnsaveableBackingStore : public DirectoryBackingStore { public: UnsaveableBackingStore(const std::string& dir_name, @@ -403,7 +401,8 @@ class TestUnsaveableDirectory : public Directory { return new UnsaveableBackingStore(dir_name, backing_filepath); } private: - browser_sync::TestUnrecoverableErrorHandler handler_; + FakeEncryptor encryptor_; + TestUnrecoverableErrorHandler handler_; }; // Test suite for syncable::Directory. @@ -421,7 +420,7 @@ class SyncableDirectoryTest : public testing::Test { file_path_ = temp_dir_.path().Append( FILE_PATH_LITERAL("Test.sqlite3")); file_util::Delete(file_path_, true); - dir_.reset(new Directory(&handler_, NULL)); + dir_.reset(new Directory(&encryptor_, &handler_, NULL)); ASSERT_TRUE(dir_.get()); ASSERT_EQ(OPENED, dir_->Open(file_path_, kName, &delegate_, NullTransactionObserver())); @@ -436,7 +435,7 @@ class SyncableDirectoryTest : public testing::Test { } void ReloadDir() { - dir_.reset(new Directory(&handler_, NULL)); + dir_.reset(new Directory(&encryptor_, &handler_, NULL)); ASSERT_TRUE(dir_.get()); ASSERT_EQ(OPENED, dir_->Open(file_path_, kName, &delegate_, NullTransactionObserver())); @@ -489,7 +488,8 @@ class SyncableDirectoryTest : public testing::Test { EXPECT_TRUE(dir_->initial_sync_ended_for_type(BOOKMARKS)); } - browser_sync::TestUnrecoverableErrorHandler handler_; + FakeEncryptor encryptor_; + TestUnrecoverableErrorHandler handler_; scoped_ptr<Directory> dir_; FilePath file_path_; NullDirectoryChangeDelegate delegate_; @@ -1209,8 +1209,7 @@ TEST_F(SyncableDirectoryTest, TestSimpleFieldsPreservedDuringSaveChanges) { } dir_->SaveChanges(); - browser_sync::TestUnrecoverableErrorHandler handler; - dir_.reset(new Directory(&handler, NULL)); + dir_.reset(new Directory(&encryptor_, &handler_, NULL)); ASSERT_TRUE(dir_.get()); ASSERT_EQ(OPENED, dir_->Open(file_path_, kName, &delegate_, NullTransactionObserver())); @@ -1510,15 +1509,16 @@ class SyncableDirectoryManagement : public testing::Test { protected: MessageLoop message_loop_; ScopedTempDir temp_dir_; + FakeEncryptor encryptor_; + TestUnrecoverableErrorHandler handler_; NullDirectoryChangeDelegate delegate_; }; TEST_F(SyncableDirectoryManagement, TestFileRelease) { - browser_sync::TestUnrecoverableErrorHandler handler; FilePath path = temp_dir_.path().Append( Directory::kSyncDatabaseFilename); - syncable::Directory dir(&handler, NULL); + syncable::Directory dir(&encryptor_, &handler_, NULL); DirOpenResult result = dir.Open(path, "ScopeTest", &delegate_, NullTransactionObserver()); ASSERT_EQ(result, OPENED); @@ -1573,9 +1573,10 @@ TEST(SyncableDirectory, StressTransactions) { MessageLoop message_loop; ScopedTempDir temp_dir; ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + FakeEncryptor encryptor; + TestUnrecoverableErrorHandler handler; NullDirectoryChangeDelegate delegate; - browser_sync::TestUnrecoverableErrorHandler handler; - Directory dir(&handler, NULL); + Directory dir(&encryptor, &handler, NULL); FilePath path = temp_dir.path().Append(Directory::kSyncDatabaseFilename); file_util::Delete(path, true); std::string dirname = "stress"; diff --git a/chrome/browser/sync/test/engine/test_directory_setter_upper.cc b/chrome/browser/sync/test/engine/test_directory_setter_upper.cc index 35fb116..97acd33 100644 --- a/chrome/browser/sync/test/engine/test_directory_setter_upper.cc +++ b/chrome/browser/sync/test/engine/test_directory_setter_upper.cc @@ -22,7 +22,7 @@ TestDirectorySetterUpper::TestDirectorySetterUpper() : name_("Test") {} TestDirectorySetterUpper::~TestDirectorySetterUpper() {} void TestDirectorySetterUpper::SetUp() { - directory_.reset(new syncable::Directory(&handler_, NULL)); + directory_.reset(new syncable::Directory(&encryptor_, &handler_, NULL)); ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); ASSERT_EQ(syncable::OPENED, directory_->Open( temp_dir_.path().Append(syncable::Directory::kSyncDatabaseFilename), diff --git a/chrome/browser/sync/test/engine/test_directory_setter_upper.h b/chrome/browser/sync/test/engine/test_directory_setter_upper.h index a9365a8..b47fe89 100644 --- a/chrome/browser/sync/test/engine/test_directory_setter_upper.h +++ b/chrome/browser/sync/test/engine/test_directory_setter_upper.h @@ -38,6 +38,7 @@ #include "base/scoped_temp_dir.h" #include "chrome/browser/sync/internal_api/includes/test_unrecoverable_error_handler.h" #include "chrome/browser/sync/syncable/syncable.h" +#include "chrome/browser/sync/test/fake_encryptor.h" #include "chrome/browser/sync/test/null_directory_change_delegate.h" #include "testing/gmock/include/gmock/gmock.h" @@ -66,6 +67,7 @@ class TestDirectorySetterUpper { void RunInvariantCheck(); ScopedTempDir temp_dir_; + FakeEncryptor encryptor_; scoped_ptr<syncable::Directory> directory_; std::string name_; diff --git a/chrome/browser/sync/test/fake_encryptor.cc b/chrome/browser/sync/test/fake_encryptor.cc new file mode 100644 index 0000000..c109ad7 --- /dev/null +++ b/chrome/browser/sync/test/fake_encryptor.cc @@ -0,0 +1,23 @@ +// Copyright (c) 2012 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/test/fake_encryptor.h" + +#include "base/base64.h" + +namespace browser_sync { + +FakeEncryptor::~FakeEncryptor() {} + +bool FakeEncryptor::EncryptString(const std::string& plaintext, + std::string* ciphertext) { + return base::Base64Encode(plaintext, ciphertext); +} + +bool FakeEncryptor::DecryptString(const std::string& ciphertext, + std::string* plaintext) { + return base::Base64Decode(ciphertext, plaintext); +} + +} // namespace browser_sync diff --git a/chrome/browser/sync/test/fake_encryptor.h b/chrome/browser/sync/test/fake_encryptor.h new file mode 100644 index 0000000..d08d7d8 --- /dev/null +++ b/chrome/browser/sync/test/fake_encryptor.h @@ -0,0 +1,29 @@ +// Copyright (c) 2012 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. + +#ifndef CHROME_BROWSER_SYNC_TEST_FAKE_ENCRYPTOR_H_ +#define CHROME_BROWSER_SYNC_TEST_FAKE_ENCRYPTOR_H_ +#pragma once + +#include "base/compiler_specific.h" +#include "chrome/browser/sync/util/encryptor.h" + +namespace browser_sync { + +// Encryptor which simply base64-encodes the plaintext to get the +// ciphertext. Obviously, this should be used only for testing. +class FakeEncryptor : public Encryptor { + public: + virtual ~FakeEncryptor(); + + virtual bool EncryptString(const std::string& plaintext, + std::string* ciphertext) OVERRIDE; + + virtual bool DecryptString(const std::string& ciphertext, + std::string* plaintext) OVERRIDE; +}; + +} // namespace browser_sync + +#endif // CHROME_BROWSER_SYNC_TEST_FAKE_ENCRYPTOR_H_ diff --git a/chrome/browser/sync/util/DEPS b/chrome/browser/sync/util/DEPS index cb1b125..c45c30e 100644 --- a/chrome/browser/sync/util/DEPS +++ b/chrome/browser/sync/util/DEPS @@ -4,11 +4,9 @@ include_rules = [ "+chrome/browser/sync/protocol", "+chrome/browser/sync/sessions", "+chrome/browser/sync/syncable", + "+chrome/browser/sync/test", "+chrome/browser/sync/util", # this file is weird. "+chrome/browser/sync/engine/syncproto.h", - - # unittests need this for mac osx keychain overriding - "+chrome/browser/password_manager/encryptor.h", ] diff --git a/chrome/browser/sync/util/cryptographer.cc b/chrome/browser/sync/util/cryptographer.cc index 26fbc55..30d0e91 100644 --- a/chrome/browser/sync/util/cryptographer.cc +++ b/chrome/browser/sync/util/cryptographer.cc @@ -5,8 +5,9 @@ #include <algorithm> #include "base/base64.h" +#include "base/logging.h" #include "chrome/browser/sync/util/cryptographer.h" -#include "chrome/browser/password_manager/encryptor.h" +#include "chrome/browser/sync/util/encryptor.h" namespace browser_sync { @@ -20,10 +21,13 @@ const char kNigoriKeyName[] = "nigori-key"; Cryptographer::Observer::~Observer() {} -Cryptographer::Cryptographer() - : default_nigori_(NULL), +Cryptographer::Cryptographer(Encryptor* encryptor) + : encryptor_(encryptor), + default_nigori_(NULL), encrypted_types_(SensitiveTypes()), - encrypt_everything_(false) {} + encrypt_everything_(false) { + DCHECK(encryptor); +} Cryptographer::~Cryptographer() {} @@ -235,7 +239,7 @@ bool Cryptographer::PackBootstrapToken(const Nigori* nigori, } std::string encrypted_token; - if (!Encryptor::EncryptString(unencrypted_token, &encrypted_token)) { + if (!encryptor_->EncryptString(unencrypted_token, &encrypted_token)) { NOTREACHED(); return false; } @@ -258,7 +262,7 @@ Nigori* Cryptographer::UnpackBootstrapToken(const std::string& token) const { } std::string unencrypted_token; - if (!Encryptor::DecryptString(encrypted_data, &unencrypted_token)) { + if (!encryptor_->DecryptString(encrypted_data, &unencrypted_token)) { DLOG(WARNING) << "Decryption of bootstrap token failed."; return NULL; } diff --git a/chrome/browser/sync/util/cryptographer.h b/chrome/browser/sync/util/cryptographer.h index 6426a3d..e4a5d1e 100644 --- a/chrome/browser/sync/util/cryptographer.h +++ b/chrome/browser/sync/util/cryptographer.h @@ -20,6 +20,8 @@ namespace browser_sync { +class Encryptor; + extern const char kNigoriTag[]; // The parameters used to initialize a Nigori instance. @@ -70,7 +72,8 @@ class Cryptographer { virtual ~Observer(); }; - Cryptographer(); + // Does not take ownership of |encryptor|. + explicit Cryptographer(Encryptor* encryptor); ~Cryptographer(); // When update on cryptographer is called this enum tells if the @@ -224,6 +227,8 @@ class Cryptographer { bool PackBootstrapToken(const Nigori* nigori, std::string* pack_into) const; Nigori* UnpackBootstrapToken(const std::string& token) const; + Encryptor* const encryptor_; + ObserverList<Observer> observers_; NigoriMap nigoris_; // The Nigoris we know about, mapped by key name. diff --git a/chrome/browser/sync/util/cryptographer_unittest.cc b/chrome/browser/sync/util/cryptographer_unittest.cc index 5d3361c..5d6c006 100644 --- a/chrome/browser/sync/util/cryptographer_unittest.cc +++ b/chrome/browser/sync/util/cryptographer_unittest.cc @@ -8,22 +8,22 @@ #include "base/memory/scoped_ptr.h" #include "base/string_util.h" -#include "chrome/browser/password_manager/encryptor.h" #include "chrome/browser/sync/protocol/nigori_specifics.pb.h" #include "chrome/browser/sync/protocol/password_specifics.pb.h" #include "chrome/browser/sync/syncable/model_type_test_util.h" +#include "chrome/browser/sync/test/fake_encryptor.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" namespace browser_sync { +namespace { + using ::testing::_; using ::testing::Mock; using ::testing::StrictMock; using syncable::ModelTypeSet; -namespace { - class MockObserver : public Cryptographer::Observer { public: MOCK_METHOD2(OnEncryptedTypesChanged, @@ -32,46 +32,48 @@ class MockObserver : public Cryptographer::Observer { } // namespace -TEST(SyncCryptographerTest, EmptyCantDecrypt) { - Cryptographer cryptographer; - EXPECT_FALSE(cryptographer.is_ready()); +class SyncCryptographerTest : public ::testing::Test { + protected: + SyncCryptographerTest() : cryptographer_(&encryptor_) {} + + FakeEncryptor encryptor_; + Cryptographer cryptographer_; +}; + +TEST_F(SyncCryptographerTest, EmptyCantDecrypt) { + EXPECT_FALSE(cryptographer_.is_ready()); sync_pb::EncryptedData encrypted; encrypted.set_key_name("foo"); encrypted.set_blob("bar"); - EXPECT_FALSE(cryptographer.CanDecrypt(encrypted)); + EXPECT_FALSE(cryptographer_.CanDecrypt(encrypted)); } -TEST(SyncCryptographerTest, EmptyCantEncrypt) { - Cryptographer cryptographer; - EXPECT_FALSE(cryptographer.is_ready()); +TEST_F(SyncCryptographerTest, EmptyCantEncrypt) { + EXPECT_FALSE(cryptographer_.is_ready()); sync_pb::EncryptedData encrypted; sync_pb::PasswordSpecificsData original; - EXPECT_FALSE(cryptographer.Encrypt(original, &encrypted)); + EXPECT_FALSE(cryptographer_.Encrypt(original, &encrypted)); } -TEST(SyncCryptographerTest, MissingCantDecrypt) { - Cryptographer cryptographer; - +TEST_F(SyncCryptographerTest, MissingCantDecrypt) { KeyParams params = {"localhost", "dummy", "dummy"}; - cryptographer.AddKey(params); - EXPECT_TRUE(cryptographer.is_ready()); + cryptographer_.AddKey(params); + EXPECT_TRUE(cryptographer_.is_ready()); sync_pb::EncryptedData encrypted; encrypted.set_key_name("foo"); encrypted.set_blob("bar"); - EXPECT_FALSE(cryptographer.CanDecrypt(encrypted)); + EXPECT_FALSE(cryptographer_.CanDecrypt(encrypted)); } -TEST(SyncCryptographerTest, CanEncryptAndDecrypt) { - Cryptographer cryptographer; - +TEST_F(SyncCryptographerTest, CanEncryptAndDecrypt) { KeyParams params = {"localhost", "dummy", "dummy"}; - EXPECT_TRUE(cryptographer.AddKey(params)); - EXPECT_TRUE(cryptographer.is_ready()); + EXPECT_TRUE(cryptographer_.AddKey(params)); + EXPECT_TRUE(cryptographer_.is_ready()); sync_pb::PasswordSpecificsData original; original.set_origin("http://example.com"); @@ -79,20 +81,18 @@ TEST(SyncCryptographerTest, CanEncryptAndDecrypt) { original.set_password_value("hunter2"); sync_pb::EncryptedData encrypted; - EXPECT_TRUE(cryptographer.Encrypt(original, &encrypted)); + EXPECT_TRUE(cryptographer_.Encrypt(original, &encrypted)); sync_pb::PasswordSpecificsData decrypted; - EXPECT_TRUE(cryptographer.Decrypt(encrypted, &decrypted)); + EXPECT_TRUE(cryptographer_.Decrypt(encrypted, &decrypted)); EXPECT_EQ(original.SerializeAsString(), decrypted.SerializeAsString()); } -TEST(SyncCryptographerTest, EncryptOnlyIfDifferent) { - Cryptographer cryptographer; - +TEST_F(SyncCryptographerTest, EncryptOnlyIfDifferent) { KeyParams params = {"localhost", "dummy", "dummy"}; - EXPECT_TRUE(cryptographer.AddKey(params)); - EXPECT_TRUE(cryptographer.is_ready()); + EXPECT_TRUE(cryptographer_.AddKey(params)); + EXPECT_TRUE(cryptographer_.is_ready()); sync_pb::PasswordSpecificsData original; original.set_origin("http://example.com"); @@ -100,32 +100,30 @@ TEST(SyncCryptographerTest, EncryptOnlyIfDifferent) { original.set_password_value("hunter2"); sync_pb::EncryptedData encrypted; - EXPECT_TRUE(cryptographer.Encrypt(original, &encrypted)); + EXPECT_TRUE(cryptographer_.Encrypt(original, &encrypted)); sync_pb::EncryptedData encrypted2, encrypted3; encrypted2.CopyFrom(encrypted); encrypted3.CopyFrom(encrypted); - EXPECT_TRUE(cryptographer.Encrypt(original, &encrypted2)); + EXPECT_TRUE(cryptographer_.Encrypt(original, &encrypted2)); // Now encrypt with a new default key. Should overwrite the old data. KeyParams params_new = {"localhost", "dummy", "dummy2"}; - cryptographer.AddKey(params_new); - EXPECT_TRUE(cryptographer.Encrypt(original, &encrypted3)); + cryptographer_.AddKey(params_new); + EXPECT_TRUE(cryptographer_.Encrypt(original, &encrypted3)); sync_pb::PasswordSpecificsData decrypted; - EXPECT_TRUE(cryptographer.Decrypt(encrypted2, &decrypted)); + EXPECT_TRUE(cryptographer_.Decrypt(encrypted2, &decrypted)); // encrypted2 should match encrypted, encrypted3 should not (due to salting). EXPECT_EQ(encrypted.SerializeAsString(), encrypted2.SerializeAsString()); EXPECT_NE(encrypted.SerializeAsString(), encrypted3.SerializeAsString()); EXPECT_EQ(original.SerializeAsString(), decrypted.SerializeAsString()); } -TEST(SyncCryptographerTest, AddKeySetsDefault) { - Cryptographer cryptographer; - +TEST_F(SyncCryptographerTest, AddKeySetsDefault) { KeyParams params1 = {"localhost", "dummy", "dummy1"}; - EXPECT_TRUE(cryptographer.AddKey(params1)); - EXPECT_TRUE(cryptographer.is_ready()); + EXPECT_TRUE(cryptographer_.AddKey(params1)); + EXPECT_TRUE(cryptographer_.is_ready()); sync_pb::PasswordSpecificsData original; original.set_origin("http://example.com"); @@ -133,18 +131,18 @@ TEST(SyncCryptographerTest, AddKeySetsDefault) { original.set_password_value("hunter2"); sync_pb::EncryptedData encrypted1; - EXPECT_TRUE(cryptographer.Encrypt(original, &encrypted1)); + EXPECT_TRUE(cryptographer_.Encrypt(original, &encrypted1)); sync_pb::EncryptedData encrypted2; - EXPECT_TRUE(cryptographer.Encrypt(original, &encrypted2)); + EXPECT_TRUE(cryptographer_.Encrypt(original, &encrypted2)); KeyParams params2 = {"localhost", "dummy", "dummy2"}; - EXPECT_TRUE(cryptographer.AddKey(params2)); - EXPECT_TRUE(cryptographer.is_ready()); + EXPECT_TRUE(cryptographer_.AddKey(params2)); + EXPECT_TRUE(cryptographer_.is_ready()); sync_pb::EncryptedData encrypted3; - EXPECT_TRUE(cryptographer.Encrypt(original, &encrypted3)); + EXPECT_TRUE(cryptographer_.Encrypt(original, &encrypted3)); sync_pb::EncryptedData encrypted4; - EXPECT_TRUE(cryptographer.Encrypt(original, &encrypted4)); + EXPECT_TRUE(cryptographer_.Encrypt(original, &encrypted4)); EXPECT_EQ(encrypted1.key_name(), encrypted2.key_name()); EXPECT_NE(encrypted1.key_name(), encrypted3.key_name()); @@ -157,7 +155,7 @@ TEST(SyncCryptographerTest, AddKeySetsDefault) { #else #define MAYBE_EncryptExportDecrypt EncryptExportDecrypt #endif -TEST(SyncCryptographerTest, MAYBE_EncryptExportDecrypt) { +TEST_F(SyncCryptographerTest, MAYBE_EncryptExportDecrypt) { sync_pb::EncryptedData nigori; sync_pb::EncryptedData encrypted; @@ -167,7 +165,7 @@ TEST(SyncCryptographerTest, MAYBE_EncryptExportDecrypt) { original.set_password_value("hunter2"); { - Cryptographer cryptographer; + Cryptographer cryptographer(&encryptor_); KeyParams params = {"localhost", "dummy", "dummy"}; cryptographer.AddKey(params); @@ -178,7 +176,7 @@ TEST(SyncCryptographerTest, MAYBE_EncryptExportDecrypt) { } { - Cryptographer cryptographer; + Cryptographer cryptographer(&encryptor_); EXPECT_FALSE(cryptographer.CanDecrypt(nigori)); cryptographer.SetPendingKeys(nigori); @@ -202,23 +200,18 @@ TEST(SyncCryptographerTest, MAYBE_EncryptExportDecrypt) { #else #define MAYBE_PackUnpack PackUnpack #endif -TEST(SyncCryptographerTest, MAYBE_PackUnpack) { -#if defined(OS_MACOSX) - Encryptor::UseMockKeychain(true); -#endif - +TEST_F(SyncCryptographerTest, MAYBE_PackUnpack) { Nigori nigori; ASSERT_TRUE(nigori.InitByDerivation("example.com", "username", "password")); std::string expected_user, expected_encryption, expected_mac; ASSERT_TRUE(nigori.ExportKeys(&expected_user, &expected_encryption, &expected_mac)); - Cryptographer cryptographer; std::string token; - EXPECT_TRUE(cryptographer.PackBootstrapToken(&nigori, &token)); + EXPECT_TRUE(cryptographer_.PackBootstrapToken(&nigori, &token)); EXPECT_TRUE(IsStringUTF8(token)); - scoped_ptr<Nigori> unpacked(cryptographer.UnpackBootstrapToken(token)); + scoped_ptr<Nigori> unpacked(cryptographer_.UnpackBootstrapToken(token)); EXPECT_NE(static_cast<Nigori*>(NULL), unpacked.get()); std::string user_key, encryption_key, mac_key; @@ -229,23 +222,22 @@ TEST(SyncCryptographerTest, MAYBE_PackUnpack) { EXPECT_EQ(expected_mac, mac_key); } -TEST(SyncCryptographerTest, NigoriEncryptionTypes) { - Cryptographer cryptographer; - Cryptographer cryptographer2; +TEST_F(SyncCryptographerTest, NigoriEncryptionTypes) { + Cryptographer cryptographer2(&encryptor_); sync_pb::NigoriSpecifics nigori; StrictMock<MockObserver> observer; - cryptographer.AddObserver(&observer); + cryptographer_.AddObserver(&observer); StrictMock<MockObserver> observer2; cryptographer2.AddObserver(&observer2); // Just set the sensitive types (shouldn't trigger any // notifications). ModelTypeSet encrypted_types(Cryptographer::SensitiveTypes()); - cryptographer.MergeEncryptedTypesForTest(encrypted_types); - cryptographer.UpdateNigoriFromEncryptedTypes(&nigori); + cryptographer_.MergeEncryptedTypesForTest(encrypted_types); + cryptographer_.UpdateNigoriFromEncryptedTypes(&nigori); cryptographer2.UpdateEncryptedTypesFromNigori(nigori); - EXPECT_TRUE(encrypted_types.Equals(cryptographer.GetEncryptedTypes())); + EXPECT_TRUE(encrypted_types.Equals(cryptographer_.GetEncryptedTypes())); EXPECT_TRUE(encrypted_types.Equals(cryptographer2.GetEncryptedTypes())); Mock::VerifyAndClearExpectations(&observer); @@ -262,35 +254,34 @@ TEST(SyncCryptographerTest, NigoriEncryptionTypes) { // Set all encrypted types encrypted_types = syncable::ModelTypeSet::All(); - cryptographer.MergeEncryptedTypesForTest(encrypted_types); - cryptographer.UpdateNigoriFromEncryptedTypes(&nigori); + cryptographer_.MergeEncryptedTypesForTest(encrypted_types); + cryptographer_.UpdateNigoriFromEncryptedTypes(&nigori); cryptographer2.UpdateEncryptedTypesFromNigori(nigori); - EXPECT_TRUE(encrypted_types.Equals(cryptographer.GetEncryptedTypes())); + EXPECT_TRUE(encrypted_types.Equals(cryptographer_.GetEncryptedTypes())); EXPECT_TRUE(encrypted_types.Equals(cryptographer2.GetEncryptedTypes())); // Receiving an empty nigori should not reset any encrypted types or trigger // an observer notification. Mock::VerifyAndClearExpectations(&observer); nigori = sync_pb::NigoriSpecifics(); - cryptographer.UpdateEncryptedTypesFromNigori(nigori); - EXPECT_TRUE(encrypted_types.Equals(cryptographer.GetEncryptedTypes())); + cryptographer_.UpdateEncryptedTypesFromNigori(nigori); + EXPECT_TRUE(encrypted_types.Equals(cryptographer_.GetEncryptedTypes())); } -TEST(SyncCryptographerTest, EncryptEverythingExplicit) { +TEST_F(SyncCryptographerTest, EncryptEverythingExplicit) { ModelTypeSet real_types = syncable::ModelTypeSet::All(); sync_pb::NigoriSpecifics specifics; specifics.set_encrypt_everything(true); - Cryptographer cryptographer; StrictMock<MockObserver> observer; - cryptographer.AddObserver(&observer); + cryptographer_.AddObserver(&observer); EXPECT_CALL(observer, OnEncryptedTypesChanged( HasModelTypes(syncable::ModelTypeSet::All()), true)); - EXPECT_FALSE(cryptographer.encrypt_everything()); - ModelTypeSet encrypted_types = cryptographer.GetEncryptedTypes(); + EXPECT_FALSE(cryptographer_.encrypt_everything()); + ModelTypeSet encrypted_types = cryptographer_.GetEncryptedTypes(); for (ModelTypeSet::Iterator iter = real_types.First(); iter.Good(); iter.Inc()) { if (iter.Get() == syncable::PASSWORDS || iter.Get() == syncable::NIGORI) @@ -299,10 +290,10 @@ TEST(SyncCryptographerTest, EncryptEverythingExplicit) { EXPECT_FALSE(encrypted_types.Has(iter.Get())); } - cryptographer.UpdateEncryptedTypesFromNigori(specifics); + cryptographer_.UpdateEncryptedTypesFromNigori(specifics); - EXPECT_TRUE(cryptographer.encrypt_everything()); - encrypted_types = cryptographer.GetEncryptedTypes(); + EXPECT_TRUE(cryptographer_.encrypt_everything()); + encrypted_types = cryptographer_.GetEncryptedTypes(); for (ModelTypeSet::Iterator iter = real_types.First(); iter.Good(); iter.Inc()) { EXPECT_TRUE(encrypted_types.Has(iter.Get())); @@ -311,24 +302,23 @@ TEST(SyncCryptographerTest, EncryptEverythingExplicit) { // Shouldn't trigger another notification. specifics.set_encrypt_everything(true); - cryptographer.RemoveObserver(&observer); + cryptographer_.RemoveObserver(&observer); } -TEST(SyncCryptographerTest, EncryptEverythingImplicit) { +TEST_F(SyncCryptographerTest, EncryptEverythingImplicit) { ModelTypeSet real_types = syncable::ModelTypeSet::All(); sync_pb::NigoriSpecifics specifics; specifics.set_encrypt_bookmarks(true); // Non-passwords = encrypt everything - Cryptographer cryptographer; StrictMock<MockObserver> observer; - cryptographer.AddObserver(&observer); + cryptographer_.AddObserver(&observer); EXPECT_CALL(observer, OnEncryptedTypesChanged( HasModelTypes(syncable::ModelTypeSet::All()), true)); - EXPECT_FALSE(cryptographer.encrypt_everything()); - ModelTypeSet encrypted_types = cryptographer.GetEncryptedTypes(); + EXPECT_FALSE(cryptographer_.encrypt_everything()); + ModelTypeSet encrypted_types = cryptographer_.GetEncryptedTypes(); for (ModelTypeSet::Iterator iter = real_types.First(); iter.Good(); iter.Inc()) { if (iter.Get() == syncable::PASSWORDS || iter.Get() == syncable::NIGORI) @@ -337,10 +327,10 @@ TEST(SyncCryptographerTest, EncryptEverythingImplicit) { EXPECT_FALSE(encrypted_types.Has(iter.Get())); } - cryptographer.UpdateEncryptedTypesFromNigori(specifics); + cryptographer_.UpdateEncryptedTypesFromNigori(specifics); - EXPECT_TRUE(cryptographer.encrypt_everything()); - encrypted_types = cryptographer.GetEncryptedTypes(); + EXPECT_TRUE(cryptographer_.encrypt_everything()); + encrypted_types = cryptographer_.GetEncryptedTypes(); for (ModelTypeSet::Iterator iter = real_types.First(); iter.Good(); iter.Inc()) { EXPECT_TRUE(encrypted_types.Has(iter.Get())); @@ -349,10 +339,10 @@ TEST(SyncCryptographerTest, EncryptEverythingImplicit) { // Shouldn't trigger another notification. specifics.set_encrypt_everything(true); - cryptographer.RemoveObserver(&observer); + cryptographer_.RemoveObserver(&observer); } -TEST(SyncCryptographerTest, UnknownSensitiveTypes) { +TEST_F(SyncCryptographerTest, UnknownSensitiveTypes) { ModelTypeSet real_types = syncable::ModelTypeSet::All(); sync_pb::NigoriSpecifics specifics; // Explicitly setting encrypt everything should override logic for implicit @@ -360,9 +350,8 @@ TEST(SyncCryptographerTest, UnknownSensitiveTypes) { specifics.set_encrypt_everything(false); specifics.set_encrypt_bookmarks(true); - Cryptographer cryptographer; StrictMock<MockObserver> observer; - cryptographer.AddObserver(&observer); + cryptographer_.AddObserver(&observer); syncable::ModelTypeSet expected_encrypted_types = Cryptographer::SensitiveTypes(); @@ -372,8 +361,8 @@ TEST(SyncCryptographerTest, UnknownSensitiveTypes) { OnEncryptedTypesChanged( HasModelTypes(expected_encrypted_types), false)); - EXPECT_FALSE(cryptographer.encrypt_everything()); - ModelTypeSet encrypted_types = cryptographer.GetEncryptedTypes(); + EXPECT_FALSE(cryptographer_.encrypt_everything()); + ModelTypeSet encrypted_types = cryptographer_.GetEncryptedTypes(); for (ModelTypeSet::Iterator iter = real_types.First(); iter.Good(); iter.Inc()) { if (iter.Get() == syncable::PASSWORDS || iter.Get() == syncable::NIGORI) @@ -382,10 +371,10 @@ TEST(SyncCryptographerTest, UnknownSensitiveTypes) { EXPECT_FALSE(encrypted_types.Has(iter.Get())); } - cryptographer.UpdateEncryptedTypesFromNigori(specifics); + cryptographer_.UpdateEncryptedTypesFromNigori(specifics); - EXPECT_FALSE(cryptographer.encrypt_everything()); - encrypted_types = cryptographer.GetEncryptedTypes(); + EXPECT_FALSE(cryptographer_.encrypt_everything()); + encrypted_types = cryptographer_.GetEncryptedTypes(); for (ModelTypeSet::Iterator iter = real_types.First(); iter.Good(); iter.Inc()) { if (iter.Get() == syncable::PASSWORDS || @@ -396,7 +385,7 @@ TEST(SyncCryptographerTest, UnknownSensitiveTypes) { EXPECT_FALSE(encrypted_types.Has(iter.Get())); } - cryptographer.RemoveObserver(&observer); + cryptographer_.RemoveObserver(&observer); } } // namespace browser_sync diff --git a/chrome/browser/sync/util/encryptor.h b/chrome/browser/sync/util/encryptor.h new file mode 100644 index 0000000..1c9eab9 --- /dev/null +++ b/chrome/browser/sync/util/encryptor.h @@ -0,0 +1,28 @@ +// Copyright (c) 2012 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. + +#ifndef CHROME_BROWSER_SYNC_UTIL_ENCRYPTOR_H_ +#define CHROME_BROWSER_SYNC_UTIL_ENCRYPTOR_H_ +#pragma once + +#include <string> + +namespace browser_sync { + +class Encryptor { + public: + // All methods below should be thread-safe. + virtual bool EncryptString(const std::string& plaintext, + std::string* ciphertext) = 0; + + virtual bool DecryptString(const std::string& ciphertext, + std::string* plaintext) = 0; + + protected: + virtual ~Encryptor() {} +}; + +} // namespace browser_sync + +#endif // CHROME_BROWSER_SYNC_UTIL_ENCRYPTOR_H_ diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp index f07d554..08a07a1 100644 --- a/chrome/chrome.gyp +++ b/chrome/chrome.gyp @@ -476,6 +476,7 @@ 'browser/sync/util/cryptographer.cc', 'browser/sync/util/cryptographer.h', 'browser/sync/util/data_type_histogram.h', + 'browser/sync/util/encryptor.h', 'browser/sync/util/enum_set.h', 'browser/sync/util/extensions_activity_monitor.cc', 'browser/sync/util/extensions_activity_monitor.h', diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 4b0995d..d801023 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -2385,6 +2385,8 @@ 'browser/sync/glue/browser_thread_model_worker.h', 'browser/sync/glue/change_processor.cc', 'browser/sync/glue/change_processor.h', + 'browser/sync/glue/chrome_encryptor.cc', + 'browser/sync/glue/chrome_encryptor.h', 'browser/sync/glue/chrome_extensions_activity_monitor.cc', 'browser/sync/glue/chrome_extensions_activity_monitor.h', 'browser/sync/glue/chrome_report_unrecoverable_error.cc', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index a379efe..de7d276 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -377,6 +377,8 @@ 'browser/sync/js/js_test_util.h', 'browser/sync/syncable/model_type_test_util.cc', 'browser/sync/syncable/model_type_test_util.h', + 'browser/sync/test/fake_encryptor.cc', + 'browser/sync/test/fake_encryptor.h', 'browser/sync/test/fake_extensions_activity_monitor.cc', 'browser/sync/test/fake_extensions_activity_monitor.h', 'browser/sync/test/null_directory_change_delegate.cc', @@ -1781,7 +1783,6 @@ 'browser/sync/test_profile_sync_service.h', 'browser/sync/test/test_http_bridge_factory.cc', 'browser/sync/test/test_http_bridge_factory.h', - 'browser/sync/util/cryptographer_unittest.cc', 'browser/sync/util/get_session_name_task_unittest.cc', 'browser/sync/util/nigori_unittest.cc', 'browser/tab_contents/render_view_context_menu_unittest.cc', @@ -3653,6 +3654,7 @@ 'browser/sync/test/engine/test_syncable_utils.cc', 'browser/sync/test/engine/test_syncable_utils.h', 'browser/sync/test/sessions/test_scoped_session_event_listener.h', + 'browser/sync/util/cryptographer_unittest.cc', 'browser/sync/util/data_encryption_unittest.cc', 'browser/sync/util/data_type_histogram_unittest.cc', 'browser/sync/util/enum_set_unittest.cc', |