diff options
Diffstat (limited to 'sync/engine')
| -rw-r--r-- | sync/engine/cryptographer_provider.cc | 42 | ||||
| -rw-r--r-- | sync/engine/cryptographer_provider.h | 64 | ||||
| -rw-r--r-- | sync/engine/directory_cryptographer_provider.cc | 38 | ||||
| -rw-r--r-- | sync/engine/directory_cryptographer_provider.h | 51 | ||||
| -rw-r--r-- | sync/engine/model_type_entity.cc | 3 | ||||
| -rw-r--r-- | sync/engine/model_type_sync_proxy_impl.cc | 3 | ||||
| -rw-r--r-- | sync/engine/model_type_sync_worker_impl.cc | 123 | ||||
| -rw-r--r-- | sync/engine/model_type_sync_worker_impl.h | 25 | ||||
| -rw-r--r-- | sync/engine/model_type_sync_worker_impl_unittest.cc | 95 |
9 files changed, 151 insertions, 293 deletions
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. |