summaryrefslogtreecommitdiffstats
path: root/sync
diff options
context:
space:
mode:
authorrlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-08-16 00:34:12 +0000
committerrlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-08-16 00:36:03 +0000
commit6777597b7d471a544472810b7594f9cf473415ea (patch)
tree6cf90ba020f9324f80983fcef38fefb9650bf44e /sync
parent947f48cf8ef031a2f126f7af5d1b4f4ba49b7973 (diff)
downloadchromium_src-6777597b7d471a544472810b7594f9cf473415ea.zip
chromium_src-6777597b7d471a544472810b7594f9cf473415ea.tar.gz
chromium_src-6777597b7d471a544472810b7594f9cf473415ea.tar.bz2
sync: Finish non-blocking type encryption support
Undoes some previous work towards encryption support. That approach suffered from some subtle deadlock issues that could not be easily worked around. The new approach involves less sharing and less locks. Gives the ModelTypeSyncWorker its own copy of the Cryptographer. By passing around copies, it no longer needs to worry about acquiring locks in order to access the Directory's cryptographer. This required a rewrite of some changes to the way the ModelTypeSyncWorker detects the current encryption state. Most notably, its Cryptographer is NULL if encryption is not enabled for its model type. Makes the ModelTypeSyncRegistry responsible for observing changes emitted by the SyncEncryptionHandler and forwarding them to the ModelTypeSyncWorkers. It should receive callbacks from the SyncEncryptionHandler during startup, so it does not need to cache or query any new data. Removes the CryptographerProviders. Since the ModelTypeSyncWorker no longer need to access the directory's cryptographer, it's no longer necessary. BUG=351005 Review URL: https://codereview.chromium.org/452283003 Cr-Commit-Position: refs/heads/master@{#290067} git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290067 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'sync')
-rw-r--r--sync/BUILD.gn6
-rw-r--r--sync/engine/cryptographer_provider.cc42
-rw-r--r--sync/engine/cryptographer_provider.h64
-rw-r--r--sync/engine/directory_cryptographer_provider.cc38
-rw-r--r--sync/engine/directory_cryptographer_provider.h51
-rw-r--r--sync/engine/model_type_entity.cc3
-rw-r--r--sync/engine/model_type_sync_proxy_impl.cc3
-rw-r--r--sync/engine/model_type_sync_worker_impl.cc123
-rw-r--r--sync/engine/model_type_sync_worker_impl.h25
-rw-r--r--sync/engine/model_type_sync_worker_impl_unittest.cc95
-rw-r--r--sync/internal_api/sync_manager_impl.cc5
-rw-r--r--sync/protocol/proto_value_conversions.cc1
-rw-r--r--sync/sessions/model_type_registry.cc54
-rw-r--r--sync/sessions/model_type_registry.h33
-rw-r--r--sync/sync.gyp4
-rw-r--r--sync/sync_tests.gypi2
-rw-r--r--sync/test/engine/simple_cryptographer_provider.cc35
-rw-r--r--sync/test/engine/simple_cryptographer_provider.h44
-rw-r--r--sync/util/cryptographer.cc18
-rw-r--r--sync/util/cryptographer.h4
-rw-r--r--sync/util/cryptographer_unittest.cc63
21 files changed, 320 insertions, 393 deletions
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