diff options
22 files changed, 323 insertions, 395 deletions
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc index 862d014..0944f03 100644 --- a/chrome/browser/sync/profile_sync_service.cc +++ b/chrome/browser/sync/profile_sync_service.cc @@ -373,8 +373,6 @@ void ProfileSyncService::TrySyncDatatypePrefRecovery() { UMA_HISTOGRAM_COUNTS("Sync.DatatypePrefRecovery", 1); sync_prefs_.SetKeepEverythingSynced(true); syncer::ModelTypeSet registered_types = GetRegisteredDataTypes(); - sync_prefs_.SetPreferredDataTypes(registered_types, - registered_types); } void ProfileSyncService::StartSyncingWithServer() { @@ -1140,6 +1138,9 @@ void ProfileSyncService::OnBackendInitialized( // Initialize local device info. local_device_->Initialize(cache_guid, signin_scoped_device_id); + DVLOG(1) << "Setting preferred types for non-blocking DTM"; + non_blocking_data_type_manager_.SetPreferredTypes(GetPreferredDataTypes()); + // Give the DataTypeControllers a handle to the now initialized backend // as a UserShare. for (DataTypeController::TypeMap::iterator it = diff --git a/sync/BUILD.gn b/sync/BUILD.gn index daa4d5d..e3e8914 100644 --- a/sync/BUILD.gn +++ b/sync/BUILD.gn @@ -60,14 +60,10 @@ source_set("sync_core") { "engine/conflict_resolver.h", "engine/conflict_util.cc", "engine/conflict_util.h", - "engine/cryptographer_provider.cc", - "engine/cryptographer_provider.h", "engine/directory_commit_contribution.cc", "engine/directory_commit_contribution.h", "engine/directory_commit_contributor.cc", "engine/directory_commit_contributor.h", - "engine/directory_cryptographer_provider.cc", - "engine/directory_cryptographer_provider.h", "engine/directory_update_handler.cc", "engine/directory_update_handler.h", "engine/entity_tracker.cc", @@ -445,8 +441,6 @@ static_library("test_support_sync_core") { "test/engine/mock_nudge_handler.h", "test/engine/mock_update_handler.cc", "test/engine/mock_update_handler.h", - "test/engine/simple_cryptographer_provider.cc", - "test/engine/simple_cryptographer_provider.h", "test/engine/single_type_mock_server.cc", "test/engine/single_type_mock_server.h", "test/engine/test_directory_setter_upper.cc", diff --git a/sync/engine/cryptographer_provider.cc b/sync/engine/cryptographer_provider.cc deleted file mode 100644 index 2c16bc4..0000000 --- a/sync/engine/cryptographer_provider.cc +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 2014 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 "sync/engine/cryptographer_provider.h" - -namespace syncer { - -CryptographerProvider::CryptographerProvider() { -} - -CryptographerProvider::~CryptographerProvider() { -} - -ScopedCryptographerRef::ScopedCryptographerRef() { -} - -ScopedCryptographerRef::~ScopedCryptographerRef() { -} - -bool ScopedCryptographerRef::Initialize(ScopedCryptographerInternal* impl) { - scoped_cryptographer_internal_.reset(impl); - return IsValid(); -} - -bool ScopedCryptographerRef::IsValid() const { - return !!Get(); -} - -Cryptographer* ScopedCryptographerRef::Get() const { - if (!scoped_cryptographer_internal_) - return NULL; - return scoped_cryptographer_internal_->Get(); -} - -ScopedCryptographerInternal::ScopedCryptographerInternal() { -} - -ScopedCryptographerInternal::~ScopedCryptographerInternal() { -} - -} // namespace syncer diff --git a/sync/engine/cryptographer_provider.h b/sync/engine/cryptographer_provider.h deleted file mode 100644 index f38bd185..0000000 --- a/sync/engine/cryptographer_provider.h +++ /dev/null @@ -1,64 +0,0 @@ -// Copyright 2014 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 SYNC_ENGINE_CRYPTOGRAPHER_PROVIDER_H_ -#define SYNC_ENGINE_CRYPTOGRAPHER_PROVIDER_H_ - -#include "base/memory/scoped_ptr.h" -#include "sync/base/sync_export.h" - -namespace syncer { - -class Cryptographer; -class ScopedCryptographerRef; -class ScopedCryptographerInternal; - -// An interface for providing clients with a ScopedCryptographerRef. -// -// Used to expose the syncable::Directory's cryptographer to clients that -// would otherwise not have access to the Directory. -class SYNC_EXPORT_PRIVATE CryptographerProvider { - public: - CryptographerProvider(); - virtual ~CryptographerProvider(); - - virtual bool InitScopedCryptographerRef(ScopedCryptographerRef* scoped) = 0; -}; - -// A concrete class representing a reference to a cryptographer. -// -// Access to the cryptographer is lost when this class goes out of scope. -class SYNC_EXPORT_PRIVATE ScopedCryptographerRef { - public: - ScopedCryptographerRef(); - ~ScopedCryptographerRef(); - - bool Initialize(ScopedCryptographerInternal* impl); - bool IsValid() const; - Cryptographer* Get() const; - - private: - scoped_ptr<ScopedCryptographerInternal> scoped_cryptographer_internal_; - - DISALLOW_COPY_AND_ASSIGN(ScopedCryptographerRef); -}; - -// An interface class used in the implementation of the ScopedCryptographerRef. -// -// We use this class to allow different implementations of -// CryptographerProvider to implement InitScopedCryptographer in different -// ways. The ScopedCryptographerRef itself must be stack-allocated, so it -// can't vary depending on which kind of CryptographerProvider is used to -// intialize it. -class SYNC_EXPORT_PRIVATE ScopedCryptographerInternal { - public: - ScopedCryptographerInternal(); - virtual ~ScopedCryptographerInternal(); - - virtual Cryptographer* Get() const = 0; -}; - -} // namespace syncer - -#endif // SYNC_ENGINE_CRYPTOGRAPHER_PROVIDER_H_ diff --git a/sync/engine/directory_cryptographer_provider.cc b/sync/engine/directory_cryptographer_provider.cc deleted file mode 100644 index a9cd2a2..0000000 --- a/sync/engine/directory_cryptographer_provider.cc +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2014 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 "sync/engine/directory_cryptographer_provider.h" - -#include "sync/syncable/directory.h" -#include "sync/syncable/syncable_read_transaction.h" - -namespace syncer { - -DirectoryCryptographerProvider::DirectoryCryptographerProvider( - syncable::Directory* dir) - : dir_(dir) { -} - -DirectoryCryptographerProvider::~DirectoryCryptographerProvider() { -} - -bool DirectoryCryptographerProvider::InitScopedCryptographerRef( - ScopedCryptographerRef* scoped) { - scoped->Initialize(new ScopedDirectoryCryptographerInternal(dir_)); - return scoped->IsValid(); -} - -ScopedDirectoryCryptographerInternal::ScopedDirectoryCryptographerInternal( - syncable::Directory* dir) - : dir_(dir), trans_(FROM_HERE, dir) { -} - -ScopedDirectoryCryptographerInternal::~ScopedDirectoryCryptographerInternal() { -} - -Cryptographer* ScopedDirectoryCryptographerInternal::Get() const { - return dir_->GetCryptographer(&trans_); -} - -} // namespace syncer diff --git a/sync/engine/directory_cryptographer_provider.h b/sync/engine/directory_cryptographer_provider.h deleted file mode 100644 index 36b1c0f..0000000 --- a/sync/engine/directory_cryptographer_provider.h +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2014 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 SYNC_ENGINE_DIRECTORY_CRYPTOGRAPHER_PROVIDER_H_ -#define SYNC_ENGINE_DIRECTORY_CRYPTOGRAPHER_PROVIDER_H_ - -#include "sync/engine/cryptographer_provider.h" - -#include "sync/syncable/syncable_read_transaction.h" - -namespace syncer { - -namespace syncable { -class Directory; -} - -// Provides access to the Directory's cryptographer through the -// CryptographerProvider interface. -class DirectoryCryptographerProvider : public CryptographerProvider { - public: - DirectoryCryptographerProvider(syncable::Directory* dir); - virtual ~DirectoryCryptographerProvider(); - - virtual bool InitScopedCryptographerRef( - ScopedCryptographerRef* scoped) OVERRIDE; - - private: - syncable::Directory* dir_; -}; - -// An implementation detail of the DirectoryCryptographerProvider. Contains -// the ReadTransaction used to safely access the Cryptographer. -class ScopedDirectoryCryptographerInternal - : public ScopedCryptographerInternal { - public: - ScopedDirectoryCryptographerInternal(syncable::Directory* dir); - virtual ~ScopedDirectoryCryptographerInternal(); - - virtual Cryptographer* Get() const OVERRIDE; - - private: - syncable::Directory* dir_; - syncable::ReadTransaction trans_; - - DISALLOW_COPY_AND_ASSIGN(ScopedDirectoryCryptographerInternal); -}; - -} // namespace syncer - -#endif // SYNC_ENGINE_DIRECTORY_CRYPTOGRAPHER_PROVIDER_H_ diff --git a/sync/engine/model_type_entity.cc b/sync/engine/model_type_entity.cc index 2f758a8..3249a16 100644 --- a/sync/engine/model_type_entity.cc +++ b/sync/engine/model_type_entity.cc @@ -131,6 +131,9 @@ void ModelTypeEntity::UpdateDesiredEncryptionKey(const std::string& name) { if (encryption_key_name_ == name) return; + DVLOG(2) << id_ << ": Encryption triggered commit: " << encryption_key_name_ + << " -> " << name; + // Schedule commit with the expectation that the worker will re-encrypt with // the latest encryption key as it does. sequence_number_++; diff --git a/sync/engine/model_type_sync_proxy_impl.cc b/sync/engine/model_type_sync_proxy_impl.cc index 7ea5d52..d3e1d77 100644 --- a/sync/engine/model_type_sync_proxy_impl.cc +++ b/sync/engine/model_type_sync_proxy_impl.cc @@ -240,6 +240,9 @@ void ModelTypeSyncProxyImpl::OnUpdateReceived( // commit to fix it. if (data_type_state_.encryption_key_name != response_data.encryption_key_name) { + DVLOG(2) << ModelTypeToString(type_) << ": Requesting re-encrypt commit " + << response_data.encryption_key_name << " -> " + << data_type_state_.encryption_key_name; EntityMap::iterator it2 = entities_.find(client_tag_hash); it2->second->UpdateDesiredEncryptionKey( data_type_state_.encryption_key_name); diff --git a/sync/engine/model_type_sync_worker_impl.cc b/sync/engine/model_type_sync_worker_impl.cc index a58400f4..5a47f5e 100644 --- a/sync/engine/model_type_sync_worker_impl.cc +++ b/sync/engine/model_type_sync_worker_impl.cc @@ -22,13 +22,13 @@ ModelTypeSyncWorkerImpl::ModelTypeSyncWorkerImpl( ModelType type, const DataTypeState& initial_state, const UpdateResponseDataList& saved_pending_updates, - CryptographerProvider* cryptographer_provider, + scoped_ptr<Cryptographer> cryptographer, NudgeHandler* nudge_handler, scoped_ptr<ModelTypeSyncProxy> type_sync_proxy) : type_(type), data_type_state_(initial_state), type_sync_proxy_(type_sync_proxy.Pass()), - cryptographer_provider_(cryptographer_provider), + cryptographer_(cryptographer.Pass()), nudge_handler_(nudge_handler), entities_deleter_(&entities_), weak_ptr_factory_(this) { @@ -47,7 +47,11 @@ ModelTypeSyncWorkerImpl::ModelTypeSyncWorkerImpl( entities_.insert(std::make_pair(it->client_tag_hash, entity_tracker)); } - TryDecryptPendingUpdates(); + if (cryptographer_) { + DVLOG(1) << ModelTypeToString(type_) << ": Starting with encryption key " + << cryptographer_->GetDefaultNigoriKeyName(); + OnCryptographerUpdated(); + } } ModelTypeSyncWorkerImpl::~ModelTypeSyncWorkerImpl() { @@ -59,29 +63,19 @@ ModelType ModelTypeSyncWorkerImpl::GetModelType() const { } bool ModelTypeSyncWorkerImpl::IsEncryptionRequired() const { - return !data_type_state_.encryption_key_name.empty(); + return !!cryptographer_; } -void ModelTypeSyncWorkerImpl::SetEncryptionKeyName(const std::string& name) { - if (data_type_state_.encryption_key_name == name) - return; - - data_type_state_.encryption_key_name = name; - - // Pretend to send an update. This will cause the TypeSyncProxy to notice - // the new encryption key and take appropriate action. - type_sync_proxy_->OnUpdateReceived( - data_type_state_, UpdateResponseDataList(), UpdateResponseDataList()); -} +void ModelTypeSyncWorkerImpl::UpdateCryptographer( + scoped_ptr<Cryptographer> cryptographer) { + DCHECK(cryptographer); + cryptographer_ = cryptographer.Pass(); -void ModelTypeSyncWorkerImpl::OnCryptographerStateChanged() { - TryDecryptPendingUpdates(); + // Update our state and that of the proxy. + OnCryptographerUpdated(); - ScopedCryptographerRef scoped_cryptographer_ref; - cryptographer_provider_->InitScopedCryptographerRef( - &scoped_cryptographer_ref); - Cryptographer* cryptographer = scoped_cryptographer_ref.Get(); - if (CanCommitItems(cryptographer)) + // Nudge the scheduler if we're now allowed to commit. + if (CanCommitItems()) nudge_handler_->NudgeForCommit(type_); } @@ -109,12 +103,6 @@ SyncerError ModelTypeSyncWorkerImpl::ProcessGetUpdatesResponse( data_type_state_.type_context = mutated_context; data_type_state_.progress_marker = progress_marker; - ScopedCryptographerRef scoped_cryptographer_ref; - cryptographer_provider_->InitScopedCryptographerRef( - &scoped_cryptographer_ref); - Cryptographer* cryptographer = scoped_cryptographer_ref.Get(); - DCHECK(cryptographer); - UpdateResponseDataList response_datas; UpdateResponseDataList pending_updates; @@ -166,17 +154,18 @@ SyncerError ModelTypeSyncWorkerImpl::ProcessGetUpdatesResponse( entity_tracker->ReceiveUpdate(update_entity->version()); response_data.specifics = specifics; response_datas.push_back(response_data); - } else if (specifics.has_encrypted() && - cryptographer->CanDecrypt(specifics.encrypted())) { + } else if (specifics.has_encrypted() && cryptographer_ && + cryptographer_->CanDecrypt(specifics.encrypted())) { // Encrypted, but we know the key. if (DecryptSpecifics( - cryptographer, specifics, &response_data.specifics)) { + cryptographer_.get(), specifics, &response_data.specifics)) { entity_tracker->ReceiveUpdate(update_entity->version()); response_data.encryption_key_name = specifics.encrypted().key_name(); response_datas.push_back(response_data); } } else if (specifics.has_encrypted() && - !cryptographer->CanDecrypt(specifics.encrypted())) { + (!cryptographer_ || + !cryptographer_->CanDecrypt(specifics.encrypted()))) { // Can't decrypt right now. Ask the entity tracker to handle it. response_data.specifics = specifics; if (entity_tracker->ReceivePendingUpdate(response_data)) { @@ -188,6 +177,12 @@ SyncerError ModelTypeSyncWorkerImpl::ProcessGetUpdatesResponse( } } + DVLOG(1) << ModelTypeToString(type_) << ": " + << base::StringPrintf( + "Delivering %zd applicable and %zd pending updates.", + response_datas.size(), + pending_updates.size()); + // Forward these updates to the model thread so it can do the rest. type_sync_proxy_->OnUpdateReceived( data_type_state_, response_datas, pending_updates); @@ -202,6 +197,8 @@ void ModelTypeSyncWorkerImpl::ApplyUpdates(sessions::StatusController* status) { // cycle, we should update our state so the ModelTypeSyncProxy knows that // it's safe to commit items now. if (!data_type_state_.initial_sync_done) { + DVLOG(1) << "Delivering 'initial sync done' ping."; + data_type_state_.initial_sync_done = true; type_sync_proxy_->OnUpdateReceived( @@ -230,11 +227,7 @@ void ModelTypeSyncWorkerImpl::EnqueueForCommit( StorePendingCommit(*it); } - ScopedCryptographerRef scoped_cryptographer_ref; - cryptographer_provider_->InitScopedCryptographerRef( - &scoped_cryptographer_ref); - Cryptographer* cryptographer = scoped_cryptographer_ref.Get(); - if (CanCommitItems(cryptographer)) + if (CanCommitItems()) nudge_handler_->NudgeForCommit(type_); } @@ -247,12 +240,7 @@ scoped_ptr<CommitContribution> ModelTypeSyncWorkerImpl::GetContribution( std::vector<int64> sequence_numbers; google::protobuf::RepeatedPtrField<sync_pb::SyncEntity> commit_entities; - ScopedCryptographerRef scoped_cryptographer_ref; - cryptographer_provider_->InitScopedCryptographerRef( - &scoped_cryptographer_ref); - Cryptographer* cryptographer = scoped_cryptographer_ref.Get(); - - if (!CanCommitItems(cryptographer)) + if (!CanCommitItems()) return scoped_ptr<CommitContribution>(); // TODO(rlarocque): Avoid iterating here. @@ -265,7 +253,7 @@ scoped_ptr<CommitContribution> ModelTypeSyncWorkerImpl::GetContribution( int64 sequence_number = -1; entity->PrepareCommitProto(commit_entity, &sequence_number); - HelpInitializeCommitEntity(cryptographer, commit_entity); + HelpInitializeCommitEntity(commit_entity); sequence_numbers.push_back(sequence_number); space_remaining--; @@ -350,17 +338,15 @@ bool ModelTypeSyncWorkerImpl::IsTypeInitialized() const { data_type_state_.initial_sync_done; } -bool ModelTypeSyncWorkerImpl::CanCommitItems( - Cryptographer* cryptographer) const { +bool ModelTypeSyncWorkerImpl::CanCommitItems() const { // We can't commit anything until we know the type's parent node. // We'll get it in the first update response. if (!IsTypeInitialized()) return false; // Don't commit if we should be encrypting but don't have the required keys. - if (IsEncryptionRequired() && (!cryptographer || !cryptographer->is_ready() || - cryptographer->GetDefaultNigoriKeyName() != - data_type_state_.encryption_key_name)) { + if (IsEncryptionRequired() && + (!cryptographer_ || !cryptographer_->is_ready())) { return false; } @@ -368,8 +354,9 @@ bool ModelTypeSyncWorkerImpl::CanCommitItems( } void ModelTypeSyncWorkerImpl::HelpInitializeCommitEntity( - Cryptographer* cryptographer, sync_pb::SyncEntity* sync_entity) { + DCHECK(CanCommitItems()); + // Initial commits need our help to generate a client ID. if (!sync_entity->has_id_string()) { DCHECK_EQ(kUncommittedVersion, sync_entity->version()); @@ -380,9 +367,12 @@ void ModelTypeSyncWorkerImpl::HelpInitializeCommitEntity( // Encrypt the specifics and hide the title if necessary. if (IsEncryptionRequired()) { + // IsEncryptionRequired() && CanCommitItems() implies + // that the cryptographer is valid and ready to encrypt. sync_pb::EntitySpecifics encrypted_specifics; - cryptographer->Encrypt(sync_entity->specifics(), - encrypted_specifics.mutable_encrypted()); + bool result = cryptographer_->Encrypt( + sync_entity->specifics(), encrypted_specifics.mutable_encrypted()); + DCHECK(result); sync_entity->mutable_specifics()->CopyFrom(encrypted_specifics); sync_entity->set_name("encrypted"); } @@ -395,14 +385,21 @@ void ModelTypeSyncWorkerImpl::HelpInitializeCommitEntity( sync_entity->set_parent_id_string(data_type_state_.type_root_id); } -void ModelTypeSyncWorkerImpl::TryDecryptPendingUpdates() { +void ModelTypeSyncWorkerImpl::OnCryptographerUpdated() { + DCHECK(cryptographer_); + + bool new_encryption_key = false; UpdateResponseDataList response_datas; - ScopedCryptographerRef scoped_cryptographer_ref; - cryptographer_provider_->InitScopedCryptographerRef( - &scoped_cryptographer_ref); - Cryptographer* cryptographer = scoped_cryptographer_ref.Get(); - DCHECK(cryptographer); + const std::string& new_key_name = cryptographer_->GetDefaultNigoriKeyName(); + + // Handle a change in encryption key. + if (data_type_state_.encryption_key_name != new_key_name) { + DVLOG(1) << ModelTypeToString(type_) << ": Updating encryption key " + << data_type_state_.encryption_key_name << " -> " << new_key_name; + data_type_state_.encryption_key_name = new_key_name; + new_encryption_key = true; + } for (EntityMap::const_iterator it = entities_.begin(); it != entities_.end(); ++it) { @@ -413,9 +410,9 @@ void ModelTypeSyncWorkerImpl::TryDecryptPendingUpdates() { // don't have the key. DCHECK(saved_pending.specifics.has_encrypted()); - if (cryptographer->CanDecrypt(saved_pending.specifics.encrypted())) { + if (cryptographer_->CanDecrypt(saved_pending.specifics.encrypted())) { UpdateResponseData decrypted_response = saved_pending; - if (DecryptSpecifics(cryptographer, + if (DecryptSpecifics(cryptographer_.get(), saved_pending.specifics, &decrypted_response.specifics)) { decrypted_response.encryption_key_name = @@ -428,7 +425,11 @@ void ModelTypeSyncWorkerImpl::TryDecryptPendingUpdates() { } } - if (!response_datas.empty()) { + if (new_encryption_key || response_datas.size() > 0) { + DVLOG(1) << ModelTypeToString(type_) << ": " + << base::StringPrintf( + "Delivering encryption key and %zd decrypted updates.", + response_datas.size()); type_sync_proxy_->OnUpdateReceived( data_type_state_, response_datas, UpdateResponseDataList()); } diff --git a/sync/engine/model_type_sync_worker_impl.h b/sync/engine/model_type_sync_worker_impl.h index f2df976..10aa080 100644 --- a/sync/engine/model_type_sync_worker_impl.h +++ b/sync/engine/model_type_sync_worker_impl.h @@ -10,7 +10,6 @@ #include "base/threading/non_thread_safe.h" #include "sync/base/sync_export.h" #include "sync/engine/commit_contributor.h" -#include "sync/engine/cryptographer_provider.h" #include "sync/engine/model_type_sync_worker.h" #include "sync/engine/nudge_handler.h" #include "sync/engine/update_handler.h" @@ -18,6 +17,7 @@ #include "sync/internal_api/public/non_blocking_sync_common.h" #include "sync/internal_api/public/sync_encryption_handler.h" #include "sync/protocol/sync.pb.h" +#include "sync/util/cryptographer.h" namespace base { class SingleThreadTaskRunner; @@ -56,7 +56,7 @@ class SYNC_EXPORT ModelTypeSyncWorkerImpl : public UpdateHandler, ModelTypeSyncWorkerImpl(ModelType type, const DataTypeState& initial_state, const UpdateResponseDataList& saved_pending_updates, - CryptographerProvider* cryptographer_provider, + scoped_ptr<Cryptographer> cryptographer, NudgeHandler* nudge_handler, scoped_ptr<ModelTypeSyncProxy> type_sync_proxy); virtual ~ModelTypeSyncWorkerImpl(); @@ -64,9 +64,7 @@ class SYNC_EXPORT ModelTypeSyncWorkerImpl : public UpdateHandler, ModelType GetModelType() const; bool IsEncryptionRequired() const; - void SetEncryptionKeyName(const std::string& name); - - void OnCryptographerStateChanged(); + void UpdateCryptographer(scoped_ptr<Cryptographer> cryptographer); // UpdateHandler implementation. virtual void GetDownloadProgress( @@ -109,19 +107,19 @@ class SYNC_EXPORT ModelTypeSyncWorkerImpl : public UpdateHandler, // Returns true if this type is prepared to commit items. Currently, this // depends on having downloaded the initial data and having the encryption // settings in a good state. - bool CanCommitItems(Cryptographer* cryptographer) const; + bool CanCommitItems() const; // Initializes the parts of a commit entity that are the responsibility of // this class, and not the EntityTracker. Some fields, like the // client-assigned ID, can only be set by an entity with knowledge of the // entire data type's state. - void HelpInitializeCommitEntity(Cryptographer* cryptographer, - sync_pb::SyncEntity* commit_entity); + void HelpInitializeCommitEntity(sync_pb::SyncEntity* commit_entity); // Attempts to decrypt pending updates stored in the EntityMap. If // successful, will remove the update from the its EntityTracker and forward - // it to the proxy thread for application. - void TryDecryptPendingUpdates(); + // it to the proxy thread for application. Will forward any new encryption + // keys to the proxy to trigger re-encryption if necessary. + void OnCryptographerUpdated(); // Attempts to decrypt the given specifics and return them in the |out| // parameter. Assumes cryptographer->CanDecrypt(specifics) returned true. @@ -145,9 +143,10 @@ class SYNC_EXPORT ModelTypeSyncWorkerImpl : public UpdateHandler, // This is NULL when no proxy is connected.. scoped_ptr<ModelTypeSyncProxy> type_sync_proxy_; - // A helper to provide access to the syncable::Directory's cryptographer. - // Not owned. - CryptographerProvider* cryptographer_provider_; + // A private copy of the most recent cryptographer known to sync. + // Initialized at construction time and updated with UpdateCryptographer(). + // NULL if encryption is not enabled for this type. + scoped_ptr<Cryptographer> cryptographer_; // Interface used to access and send nudges to the sync scheduler. Not owned. NudgeHandler* nudge_handler_; diff --git a/sync/engine/model_type_sync_worker_impl_unittest.cc b/sync/engine/model_type_sync_worker_impl_unittest.cc index df2ed66..5c62c9a 100644 --- a/sync/engine/model_type_sync_worker_impl_unittest.cc +++ b/sync/engine/model_type_sync_worker_impl_unittest.cc @@ -14,7 +14,6 @@ #include "sync/syncable/syncable_util.h" #include "sync/test/engine/mock_model_type_sync_proxy.h" #include "sync/test/engine/mock_nudge_handler.h" -#include "sync/test/engine/simple_cryptographer_provider.h" #include "sync/test/engine/single_type_mock_server.h" #include "sync/test/fake_encryptor.h" @@ -40,6 +39,7 @@ namespace syncer { // - Commit requests from the model thread. // - Update responses from the server. // - Commit responses from the server. +// - The cryptographer, if encryption is enabled. // // Outputs: // - Commit requests to the server. @@ -168,6 +168,11 @@ class ModelTypeSyncWorkerImplTest : public ::testing::Test { // Returns the number of initial sync nudges sent to the mock nudge handler. int GetNumInitialDownloadNudges() const; + // Returns the name of the encryption key in the cryptographer last passed to + // the ModelTypeSyncWorker. Returns an empty string if no crypgorapher is + // in use. See also: UpdateLocalCryptographer(). + std::string GetLocalCryptographerKeyName() const; + // Helpers for building various messages and structures. static std::string GenerateTagHash(const std::string& tag); static sync_pb::EntitySpecifics GenerateSpecifics(const std::string& tag, @@ -192,11 +197,8 @@ class ModelTypeSyncWorkerImplTest : public ::testing::Test { // An encryptor for our cryptographer. FakeEncryptor fake_encryptor_; - // The cryptographer itself. - Cryptographer cryptographer_; - - // A CryptographerProvider for the ModelTypeSyncWorkerImpl. - SimpleCryptographerProvider cryptographer_provider_; + // The cryptographer itself. NULL if we're not encrypting the type. + scoped_ptr<Cryptographer> cryptographer_; // The number of the most recent foreign encryption key known to our // cryptographer. Note that not all of these will be decryptable. @@ -224,9 +226,7 @@ class ModelTypeSyncWorkerImplTest : public ::testing::Test { }; ModelTypeSyncWorkerImplTest::ModelTypeSyncWorkerImplTest() - : cryptographer_(&fake_encryptor_), - cryptographer_provider_(&cryptographer_), - foreign_encryption_key_index_(0), + : foreign_encryption_key_index_(0), update_encryption_filter_index_(0), mock_type_sync_proxy_(NULL), mock_server_(kModelType) { @@ -273,15 +273,24 @@ void ModelTypeSyncWorkerImplTest::InitializeWithState( mock_type_sync_proxy_ = new MockModelTypeSyncProxy(); scoped_ptr<ModelTypeSyncProxy> proxy(mock_type_sync_proxy_); + scoped_ptr<Cryptographer> cryptographer_copy; + if (cryptographer_) { + cryptographer_copy.reset(new Cryptographer(*cryptographer_)); + } + worker_.reset(new ModelTypeSyncWorkerImpl(kModelType, state, initial_pending_updates, - &cryptographer_provider_, + cryptographer_copy.Pass(), &mock_nudge_handler_, proxy.Pass())); } void ModelTypeSyncWorkerImplTest::NewForeignEncryptionKey() { + if (!cryptographer_) { + cryptographer_.reset(new Cryptographer(&fake_encryptor_)); + } + foreign_encryption_key_index_++; sync_pb::NigoriKeyBag bag; @@ -314,20 +323,29 @@ void ModelTypeSyncWorkerImplTest::NewForeignEncryptionKey() { last_nigori.Encrypt(serialized_bag, encrypted.mutable_blob()); // Update the cryptographer with new pending keys. - cryptographer_.SetPendingKeys(encrypted); + cryptographer_->SetPendingKeys(encrypted); - // Update the worker with the latest encryption key name. - if (worker_) - worker_->SetEncryptionKeyName(encrypted.key_name()); + // Update the worker with the latest cryptographer. + if (worker_) { + worker_->UpdateCryptographer( + make_scoped_ptr<Cryptographer>(new Cryptographer(*cryptographer_))); + } } void ModelTypeSyncWorkerImplTest::UpdateLocalCryptographer() { + if (!cryptographer_) { + cryptographer_.reset(new Cryptographer(&fake_encryptor_)); + } + KeyParams params = GetNthKeyParams(foreign_encryption_key_index_); - bool success = cryptographer_.DecryptPendingKeys(params); + bool success = cryptographer_->DecryptPendingKeys(params); DCHECK(success); - if (worker_) - worker_->OnCryptographerStateChanged(); + // Update the worker with the latest cryptographer. + if (worker_) { + worker_->UpdateCryptographer( + make_scoped_ptr<Cryptographer>(new Cryptographer(*cryptographer_))); + } } void ModelTypeSyncWorkerImplTest::SetUpdateEncryptionFilter(int n) { @@ -558,6 +576,14 @@ int ModelTypeSyncWorkerImplTest::GetNumInitialDownloadNudges() const { return mock_nudge_handler_.GetNumInitialDownloadNudges(); } +std::string ModelTypeSyncWorkerImplTest::GetLocalCryptographerKeyName() const { + if (!cryptographer_) { + return std::string(); + } + + return cryptographer_->GetDefaultNigoriKeyName(); +} + // static. std::string ModelTypeSyncWorkerImplTest::GenerateTagHash( const std::string& tag) { @@ -829,9 +855,15 @@ TEST_F(ModelTypeSyncWorkerImplTest, ReceiveUpdates) { TEST_F(ModelTypeSyncWorkerImplTest, EncryptedCommit) { NormalInitialize(); + ASSERT_EQ(0U, GetNumModelThreadUpdateResponses()); + NewForeignEncryptionKey(); UpdateLocalCryptographer(); + ASSERT_EQ(1U, GetNumModelThreadUpdateResponses()); + EXPECT_EQ(GetLocalCryptographerKeyName(), + GetNthModelThreadUpdateState(0).encryption_key_name); + // Normal commit request stuff. CommitRequest("tag1", "value1"); DoSuccessfulCommit(); @@ -917,27 +949,42 @@ TEST_F(ModelTypeSyncWorkerImplTest, ReceiveDecryptableEntities) { EXPECT_FALSE(update2.encryption_key_name.empty()); } +// Test initializing a ModelTypeSyncWorker with a cryptographer at startup. +TEST_F(ModelTypeSyncWorkerImplTest, InitializeWithCryptographer) { + // Set up some encryption state. + NewForeignEncryptionKey(); + UpdateLocalCryptographer(); + + // Then initialize. + NormalInitialize(); + + // The worker should tell the model thread about encryption as soon as + // possible, so that it will have the chance to re-encrypt local data if + // necessary. + ASSERT_EQ(1U, GetNumModelThreadUpdateResponses()); + EXPECT_EQ(GetLocalCryptographerKeyName(), + GetNthModelThreadUpdateState(0).encryption_key_name); +} + // Receive updates that are initially undecryptable, then ensure they get // delivered to the model thread when decryption becomes possible. TEST_F(ModelTypeSyncWorkerImplTest, ReceiveUndecryptableEntries) { NormalInitialize(); - // Set a new encryption key. The model thread will be notified of the new - // encryption key through a faked update response. + // Receive a new foreign encryption key that we can't decrypt. NewForeignEncryptionKey(); - EXPECT_EQ(1U, GetNumModelThreadUpdateResponses()); - // Send an update using that new key. + // Receive an encrypted with that new key, which we can't access. SetUpdateEncryptionFilter(1); TriggerUpdateFromServer(10, "tag1", "value1"); // At this point, the cryptographer does not have access to the key, so the // updates will be undecryptable. They'll be transfered to the model thread // for safe-keeping as pending updates. - ASSERT_EQ(2U, GetNumModelThreadUpdateResponses()); - UpdateResponseDataList updates_list = GetNthModelThreadUpdateResponse(1); + ASSERT_EQ(1U, GetNumModelThreadUpdateResponses()); + UpdateResponseDataList updates_list = GetNthModelThreadUpdateResponse(0); EXPECT_EQ(0U, updates_list.size()); - UpdateResponseDataList pending_updates = GetNthModelThreadPendingUpdates(1); + UpdateResponseDataList pending_updates = GetNthModelThreadPendingUpdates(0); EXPECT_EQ(1U, pending_updates.size()); // The update will be delivered as soon as decryption becomes possible. diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc index e12ec8f..5f2c3de 100644 --- a/sync/internal_api/sync_manager_impl.cc +++ b/sync/internal_api/sync_manager_impl.cc @@ -388,6 +388,7 @@ void SyncManagerImpl::Init(InitArgs* args) { model_type_registry_.reset( new ModelTypeRegistry(args->workers, directory(), this)); + sync_encryption_handler_->AddObserver(model_type_registry_.get()); // Bind the SyncContext WeakPtr to this thread. This helps us crash earlier // if the pointer is misused in debug mode. @@ -608,6 +609,10 @@ void SyncManagerImpl::ShutdownOnSyncThread(ShutdownReason reason) { scheduler_.reset(); session_context_.reset(); + + if (model_type_registry_) + sync_encryption_handler_->RemoveObserver(model_type_registry_.get()); + model_type_registry_.reset(); if (sync_encryption_handler_) { diff --git a/sync/protocol/proto_value_conversions.cc b/sync/protocol/proto_value_conversions.cc index ba7fa53..62b611f 100644 --- a/sync/protocol/proto_value_conversions.cc +++ b/sync/protocol/proto_value_conversions.cc @@ -897,7 +897,6 @@ base::DictionaryValue* GetUpdateTriggersToValue( SET_BOOL(invalidations_out_of_sync); SET_INT64(local_modification_nudges); SET_INT64(datatype_refresh_nudges); - SET_BOOL(initial_sync_in_progress); return value; } diff --git a/sync/sessions/model_type_registry.cc b/sync/sessions/model_type_registry.cc index c2ba5f4..a3448464f 100644 --- a/sync/sessions/model_type_registry.cc +++ b/sync/sessions/model_type_registry.cc @@ -111,7 +111,6 @@ ModelTypeRegistry::ModelTypeRegistry( syncable::Directory* directory, NudgeHandler* nudge_handler) : directory_(directory), - cryptographer_provider_(directory_), nudge_handler_(nudge_handler), weak_ptr_factory_(this) { for (size_t i = 0u; i < workers.size(); ++i) { @@ -120,7 +119,8 @@ ModelTypeRegistry::ModelTypeRegistry( } } -ModelTypeRegistry::~ModelTypeRegistry() {} +ModelTypeRegistry::~ModelTypeRegistry() { +} void ModelTypeRegistry::SetEnabledDirectoryTypes( const ModelSafeRoutingInfo& routing_info) { @@ -198,11 +198,15 @@ void ModelTypeRegistry::ConnectSyncTypeToWorker( // Initialize Worker -> Proxy communication channel. scoped_ptr<ModelTypeSyncProxy> proxy( new ModelTypeSyncProxyWrapper(proxy_impl, type_task_runner)); + scoped_ptr<Cryptographer> cryptographer_copy; + if (encrypted_types_.Has(type)) + cryptographer_copy.reset(new Cryptographer(*cryptographer_)); + scoped_ptr<ModelTypeSyncWorkerImpl> worker( new ModelTypeSyncWorkerImpl(type, data_type_state, saved_pending_updates, - &cryptographer_provider_, + cryptographer_copy.Pass(), nudge_handler_, proxy.Pass())); @@ -299,10 +303,54 @@ base::WeakPtr<SyncContext> ModelTypeRegistry::AsWeakPtr() { return weak_ptr_factory_.GetWeakPtr(); } +void ModelTypeRegistry::OnPassphraseRequired( + PassphraseRequiredReason reason, + const sync_pb::EncryptedData& pending_keys) { +} + +void ModelTypeRegistry::OnPassphraseAccepted() { +} + +void ModelTypeRegistry::OnBootstrapTokenUpdated( + const std::string& bootstrap_token, + BootstrapTokenType type) { +} + +void ModelTypeRegistry::OnEncryptedTypesChanged(ModelTypeSet encrypted_types, + bool encrypt_everything) { + encrypted_types_ = encrypted_types; + OnEncryptionStateChanged(); +} + +void ModelTypeRegistry::OnEncryptionComplete() { +} + +void ModelTypeRegistry::OnCryptographerStateChanged( + Cryptographer* cryptographer) { + cryptographer_.reset(new Cryptographer(*cryptographer)); + OnEncryptionStateChanged(); +} + +void ModelTypeRegistry::OnPassphraseTypeChanged(PassphraseType type, + base::Time passphrase_time) { +} + ModelTypeSet ModelTypeRegistry::GetEnabledDirectoryTypes() const { return enabled_directory_types_; } +void ModelTypeRegistry::OnEncryptionStateChanged() { + for (ScopedVector<ModelTypeSyncWorkerImpl>::iterator it = + model_type_sync_workers_.begin(); + it != model_type_sync_workers_.end(); + ++it) { + if (encrypted_types_.Has((*it)->GetModelType())) { + (*it)->UpdateCryptographer( + make_scoped_ptr(new Cryptographer(*cryptographer_))); + } + } +} + ModelTypeSet ModelTypeRegistry::GetEnabledNonBlockingTypes() const { ModelTypeSet enabled_off_thread_types; for (ScopedVector<ModelTypeSyncWorkerImpl>::const_iterator it = diff --git a/sync/sessions/model_type_registry.h b/sync/sessions/model_type_registry.h index 2399f22..0b4a5b4 100644 --- a/sync/sessions/model_type_registry.h +++ b/sync/sessions/model_type_registry.h @@ -12,13 +12,13 @@ #include "base/memory/scoped_vector.h" #include "base/memory/weak_ptr.h" #include "sync/base/sync_export.h" -#include "sync/engine/directory_cryptographer_provider.h" #include "sync/engine/nudge_handler.h" #include "sync/internal_api/public/base/model_type.h" #include "sync/internal_api/public/engine/model_safe_worker.h" #include "sync/internal_api/public/non_blocking_sync_common.h" #include "sync/internal_api/public/sessions/type_debug_info_observer.h" #include "sync/internal_api/public/sync_context.h" +#include "sync/internal_api/public/sync_encryption_handler.h" namespace syncer { @@ -40,7 +40,9 @@ typedef std::map<ModelType, DirectoryTypeDebugInfoEmitter*> DirectoryTypeDebugInfoEmitterMap; // Keeps track of the sets of active update handlers and commit contributors. -class SYNC_EXPORT_PRIVATE ModelTypeRegistry : public SyncContext { +class SYNC_EXPORT_PRIVATE ModelTypeRegistry + : public SyncContext, + public SyncEncryptionHandler::Observer { public: // Constructs a ModelTypeRegistry that supports directory types. ModelTypeRegistry(const std::vector<scoped_refptr<ModelSafeWorker> >& workers, @@ -68,6 +70,21 @@ class SYNC_EXPORT_PRIVATE ModelTypeRegistry : public SyncContext { // Deletes the worker associated with the type. virtual void DisconnectSyncWorker(syncer::ModelType type) OVERRIDE; + // Implementation of SyncEncryptionHandler::Observer. + virtual void OnPassphraseRequired( + PassphraseRequiredReason reason, + const sync_pb::EncryptedData& pending_keys) OVERRIDE; + virtual void OnPassphraseAccepted() OVERRIDE; + virtual void OnBootstrapTokenUpdated(const std::string& bootstrap_token, + BootstrapTokenType type) OVERRIDE; + virtual void OnEncryptedTypesChanged(ModelTypeSet encrypted_types, + bool encrypt_everything) OVERRIDE; + virtual void OnEncryptionComplete() OVERRIDE; + virtual void OnCryptographerStateChanged( + Cryptographer* cryptographer) OVERRIDE; + virtual void OnPassphraseTypeChanged(PassphraseType type, + base::Time passphrase_time) OVERRIDE; + // Gets the set of enabled types. ModelTypeSet GetEnabledTypes() const; @@ -87,6 +104,8 @@ class SYNC_EXPORT_PRIVATE ModelTypeRegistry : public SyncContext { base::WeakPtr<SyncContext> AsWeakPtr(); private: + void OnEncryptionStateChanged(); + ModelTypeSet GetEnabledNonBlockingTypes() const; ModelTypeSet GetEnabledDirectoryTypes() const; @@ -114,8 +133,14 @@ class SYNC_EXPORT_PRIVATE ModelTypeRegistry : public SyncContext { // The directory. Not owned. syncable::Directory* directory_; - // Provides access to the Directory's cryptographer. - DirectoryCryptographerProvider cryptographer_provider_; + // A copy of the directory's most recent cryptographer. + scoped_ptr<Cryptographer> cryptographer_; + + // The set of encrypted types. + ModelTypeSet encrypted_types_; + + // A helper that manages cryptography state and preferences. + SyncEncryptionHandler* encryption_handler_; // The NudgeHandler. Not owned. NudgeHandler* nudge_handler_; diff --git a/sync/sync.gyp b/sync/sync.gyp index 9ced955..8646013 100644 --- a/sync/sync.gyp +++ b/sync/sync.gyp @@ -102,14 +102,10 @@ 'engine/conflict_resolver.h', 'engine/conflict_util.cc', 'engine/conflict_util.h', - 'engine/cryptographer_provider.cc', - 'engine/cryptographer_provider.h', 'engine/directory_commit_contribution.cc', 'engine/directory_commit_contribution.h', 'engine/directory_commit_contributor.cc', 'engine/directory_commit_contributor.h', - 'engine/directory_cryptographer_provider.cc', - 'engine/directory_cryptographer_provider.h', 'engine/directory_update_handler.cc', 'engine/directory_update_handler.h', 'engine/entity_tracker.cc', diff --git a/sync/sync_tests.gypi b/sync/sync_tests.gypi index ddcdf25..dbd1057 100644 --- a/sync/sync_tests.gypi +++ b/sync/sync_tests.gypi @@ -51,8 +51,6 @@ 'test/engine/mock_nudge_handler.h', 'test/engine/mock_update_handler.cc', 'test/engine/mock_update_handler.h', - 'test/engine/simple_cryptographer_provider.cc', - 'test/engine/simple_cryptographer_provider.h', 'test/engine/single_type_mock_server.cc', 'test/engine/single_type_mock_server.h', 'test/engine/test_directory_setter_upper.cc', diff --git a/sync/test/engine/simple_cryptographer_provider.cc b/sync/test/engine/simple_cryptographer_provider.cc deleted file mode 100644 index 0e2fa99..0000000 --- a/sync/test/engine/simple_cryptographer_provider.cc +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright 2014 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 "sync/test/engine/simple_cryptographer_provider.h" - -namespace syncer { - -SimpleCryptographerProvider::SimpleCryptographerProvider( - Cryptographer* cryptographer) - : cryptographer_(cryptographer) { -} - -SimpleCryptographerProvider::~SimpleCryptographerProvider() { -} - -bool SimpleCryptographerProvider::InitScopedCryptographerRef( - ScopedCryptographerRef* scoped) { - scoped->Initialize(new SimpleScopedCryptographerInternal(cryptographer_)); - return scoped->IsValid(); -} - -SimpleScopedCryptographerInternal::SimpleScopedCryptographerInternal( - Cryptographer* cryptographer) - : cryptographer_(cryptographer) { -} - -SimpleScopedCryptographerInternal::~SimpleScopedCryptographerInternal() { -} - -Cryptographer* SimpleScopedCryptographerInternal::Get() const { - return cryptographer_; -} - -} // namespace syncer diff --git a/sync/test/engine/simple_cryptographer_provider.h b/sync/test/engine/simple_cryptographer_provider.h deleted file mode 100644 index b23c5c4..0000000 --- a/sync/test/engine/simple_cryptographer_provider.h +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright 2014 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 SYNC_TEST_ENGINE_SIMPLE_CRYPTOGRAPHER_PROVIDER_H_ -#define SYNC_TEST_ENGINE_SIMPLE_CRYPTOGRAPHER_PROVIDER_H_ - -#include "sync/engine/cryptographer_provider.h" - -#include "sync/util/cryptographer.h" - -namespace syncer { - -// A trivial cryptographer provider. Exposes the given Cryptographer through -// the CryptographerProvider interface. Does not take ownership of the -// |cryptographer|. -class SimpleCryptographerProvider : public CryptographerProvider { - public: - explicit SimpleCryptographerProvider(Cryptographer* cryptographer); - virtual ~SimpleCryptographerProvider(); - - // Implementation of CryptographerProvider. - virtual bool InitScopedCryptographerRef( - ScopedCryptographerRef* scoped) OVERRIDE; - - private: - Cryptographer* cryptographer_; -}; - -class SimpleScopedCryptographerInternal : public ScopedCryptographerInternal { - public: - explicit SimpleScopedCryptographerInternal(Cryptographer* cryptographer); - virtual ~SimpleScopedCryptographerInternal(); - - // Implementation of ScopedCryptographerInternal. - virtual Cryptographer* Get() const OVERRIDE; - - private: - Cryptographer* cryptographer_; -}; - -} // namespace syncer - -#endif // SYNC_TEST_ENGINE_SIMPLE_CRYPTOGRAPHER_PROVIDER_H_ diff --git a/sync/util/cryptographer.cc b/sync/util/cryptographer.cc index cb155b5..c22f0a3 100644 --- a/sync/util/cryptographer.cc +++ b/sync/util/cryptographer.cc @@ -27,6 +27,24 @@ Cryptographer::Cryptographer(Encryptor* encryptor) DCHECK(encryptor); } +Cryptographer::Cryptographer(const Cryptographer& other) + : encryptor_(other.encryptor_), + default_nigori_name_(other.default_nigori_name_) { + for (NigoriMap::const_iterator it = other.nigoris_.begin(); + it != other.nigoris_.end(); + ++it) { + std::string user_key, encryption_key, mac_key; + it->second->ExportKeys(&user_key, &encryption_key, &mac_key); + linked_ptr<Nigori> nigori_copy(new Nigori()); + nigori_copy->InitByImport(user_key, encryption_key, mac_key); + nigoris_.insert(std::make_pair(it->first, nigori_copy)); + } + + if (other.pending_keys_) { + pending_keys_.reset(new sync_pb::EncryptedData(*(other.pending_keys_))); + } +} + Cryptographer::~Cryptographer() {} diff --git a/sync/util/cryptographer.h b/sync/util/cryptographer.h index 9876f83..530995c 100644 --- a/sync/util/cryptographer.h +++ b/sync/util/cryptographer.h @@ -51,6 +51,7 @@ class SYNC_EXPORT Cryptographer { public: // Does not take ownership of |encryptor|. explicit Cryptographer(Encryptor* encryptor); + explicit Cryptographer(const Cryptographer& other); ~Cryptographer(); // |restored_bootstrap_token| can be provided via this method to bootstrap @@ -206,13 +207,14 @@ class SYNC_EXPORT Cryptographer { // The Nigoris we know about, mapped by key name. NigoriMap nigoris_; + // The key name associated with the default nigori. If non-empty, must // correspond to a nigori within |nigoris_|. std::string default_nigori_name_; scoped_ptr<sync_pb::EncryptedData> pending_keys_; - DISALLOW_COPY_AND_ASSIGN(Cryptographer); + DISALLOW_ASSIGN(Cryptographer); }; } // namespace syncer diff --git a/sync/util/cryptographer_unittest.cc b/sync/util/cryptographer_unittest.cc index 3719db3..94a20c8 100644 --- a/sync/util/cryptographer_unittest.cc +++ b/sync/util/cryptographer_unittest.cc @@ -201,4 +201,67 @@ TEST_F(CryptographerTest, Bootstrap) { EXPECT_TRUE(cryptographer_.CanDecryptUsingDefaultKey(encrypted)); } +// Verifies that copied cryptographers are just as good as the original. +// +// Encrypt an item using the original cryptographer and two different sets of +// keys. Verify that it can decrypt them. +// +// Then copy the original cryptographer and ensure it can also decrypt these +// items and encrypt them with the most recent key. +TEST_F(CryptographerTest, CopyConstructor) { + sync_pb::PasswordSpecificsData original; + original.set_origin("http://example.com"); + original.set_username_value("luser"); + original.set_password_value("p4ssw0rd"); + + // Start by testing the original cryptogprapher. + KeyParams params1 = {"localhost", "dummy", "dummy"}; + EXPECT_TRUE(cryptographer_.AddKey(params1)); + EXPECT_TRUE(cryptographer_.is_ready()); + + sync_pb::EncryptedData encrypted_k1; + EXPECT_TRUE(cryptographer_.Encrypt(original, &encrypted_k1)); + + KeyParams params2 = {"localhost", "fatuous", "fatuous"}; + EXPECT_TRUE(cryptographer_.AddKey(params2)); + EXPECT_TRUE(cryptographer_.is_ready()); + + sync_pb::EncryptedData encrypted_k2; + EXPECT_TRUE(cryptographer_.Encrypt(original, &encrypted_k2)); + + sync_pb::PasswordSpecificsData decrypted_k1; + sync_pb::PasswordSpecificsData decrypted_k2; + EXPECT_TRUE(cryptographer_.Decrypt(encrypted_k1, &decrypted_k1)); + EXPECT_TRUE(cryptographer_.Decrypt(encrypted_k2, &decrypted_k2)); + + EXPECT_EQ(original.SerializeAsString(), decrypted_k1.SerializeAsString()); + EXPECT_EQ(original.SerializeAsString(), decrypted_k2.SerializeAsString()); + + // Clone the cryptographer and test that it behaves the same. + Cryptographer cryptographer_clone(cryptographer_); + + // The clone should be able to decrypt with old and new keys. + sync_pb::PasswordSpecificsData decrypted_k1_clone; + sync_pb::PasswordSpecificsData decrypted_k2_clone; + EXPECT_TRUE(cryptographer_clone.Decrypt(encrypted_k1, &decrypted_k1_clone)); + EXPECT_TRUE(cryptographer_clone.Decrypt(encrypted_k2, &decrypted_k2_clone)); + + EXPECT_EQ(original.SerializeAsString(), + decrypted_k1_clone.SerializeAsString()); + EXPECT_EQ(original.SerializeAsString(), + decrypted_k2_clone.SerializeAsString()); + + // The old cryptographer should be able to decrypt things encrypted by the + // new. + sync_pb::EncryptedData encrypted_c; + EXPECT_TRUE(cryptographer_clone.Encrypt(original, &encrypted_c)); + + sync_pb::PasswordSpecificsData decrypted_c; + EXPECT_TRUE(cryptographer_.Decrypt(encrypted_c, &decrypted_c)); + EXPECT_EQ(original.SerializeAsString(), decrypted_c.SerializeAsString()); + + // The cloned cryptographer should be using the latest key. + EXPECT_EQ(encrypted_c.key_name(), encrypted_k2.key_name()); +} + } // namespace syncer |