summaryrefslogtreecommitdiffstats
path: root/sync
diff options
context:
space:
mode:
Diffstat (limited to 'sync')
-rw-r--r--sync/engine/conflict_resolver.cc31
-rw-r--r--sync/engine/syncer_unittest.cc92
-rw-r--r--sync/util/cryptographer.cc30
-rw-r--r--sync/util/cryptographer.h26
4 files changed, 152 insertions, 27 deletions
diff --git a/sync/engine/conflict_resolver.cc b/sync/engine/conflict_resolver.cc
index 17f76a4..a365e1c 100644
--- a/sync/engine/conflict_resolver.cc
+++ b/sync/engine/conflict_resolver.cc
@@ -231,13 +231,34 @@ ConflictResolver::ProcessSimpleConflict(WriteTransaction* trans,
// Store the merged set of encrypted types (cryptographer->Update(..) will
// have merged the local types already).
cryptographer->UpdateNigoriFromEncryptedTypes(server_nigori);
- // The local set of keys is already merged with the server's set within
- // the cryptographer. If we don't have pending keys we can store the
- // merged set back immediately. Else we preserve the server keys and will
- // update the nigori when the user provides the pending passphrase via
- // SetDecryptionPassphrase(..).
+ // The cryptographer has the both the local and remote encryption keys
+ // (added at cryptographer->Update(..) time).
+ // If the cryptographer is ready, then it already merged both sets of keys
+ // and we can store them back in. In that case, the remote key was already
+ // part of the local keybag, so we preserve the local key as the default
+ // (including whether it's an explicit key).
+ // If the cryptographer is not ready, then the user will have to provide
+ // the passphrase to decrypt the pending keys. When they do so, the
+ // SetDecryptionPassphrase code will act based on whether the server
+ // update has an explicit passphrase or not.
+ // - If the server had an explicit passphrase, that explicit passphrase
+ // will be preserved as the default encryption key.
+ // - If the server did not have an explicit passphrase, we assume the
+ // local passphrase is the most up to date and preserve the local
+ // default encryption key marked as an implicit passphrase.
+ // This works fine except for the case where we had locally set an
+ // explicit passphrase. In that case the nigori node will have the default
+ // key based on the local explicit passphassphrase, but will not have it
+ // marked as explicit. To fix this we'd have to track whether we have a
+ // explicit passphrase or not separate from the nigori, which would
+ // introduce even more complexity, so we leave it up to the user to
+ // reset that passphrase as an explicit one via settings. The goal here
+ // is to ensure both sets of encryption keys are preserved.
if (cryptographer->is_ready()) {
cryptographer->GetKeys(server_nigori->mutable_encrypted());
+ server_nigori->set_using_explicit_passphrase(
+ entry.Get(syncable::SPECIFICS).nigori().
+ using_explicit_passphrase());
}
// TODO(zea): Find a better way of doing this. As it stands, we have to
// update this code whenever we add a new non-cryptographer related field
diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc
index 9a66d98..56908df 100644
--- a/sync/engine/syncer_unittest.cc
+++ b/sync/engine/syncer_unittest.cc
@@ -943,6 +943,94 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) {
#undef VERIFY_ENTRY
+// Receive an old nigori with old encryption keys and encrypted types. We should
+// not revert our default key or encrypted types.
+TEST_F(SyncerTest, ReceiveOldNigori) {
+ KeyParams old_key = {"localhost", "dummy", "old"};
+ KeyParams current_key = {"localhost", "dummy", "cur"};
+
+ // Data for testing encryption/decryption.
+ browser_sync::Cryptographer other_cryptographer(&encryptor_);
+ other_cryptographer.AddKey(old_key);
+ sync_pb::EntitySpecifics other_encrypted_specifics;
+ other_encrypted_specifics.mutable_bookmark()->set_title("title");
+ other_cryptographer.Encrypt(
+ other_encrypted_specifics,
+ other_encrypted_specifics.mutable_encrypted());
+ sync_pb::EntitySpecifics our_encrypted_specifics;
+ our_encrypted_specifics.mutable_bookmark()->set_title("title2");
+ syncable::ModelTypeSet encrypted_types = syncable::ModelTypeSet::All();
+
+
+ // Receive the initial nigori node.
+ sync_pb::EntitySpecifics initial_nigori_specifics;
+ initial_nigori_specifics.mutable_nigori();
+ mock_server_->SetNigori(1, 10, 10, initial_nigori_specifics);
+ SyncShareAsDelegate();
+
+ {
+ // Set up the current nigori node (containing both keys and encrypt
+ // everything).
+ WriteTransaction wtrans(FROM_HERE, UNITTEST, directory());
+ sync_pb::EntitySpecifics specifics;
+ sync_pb::NigoriSpecifics* nigori = specifics.mutable_nigori();
+ cryptographer(&wtrans)->AddKey(old_key);
+ cryptographer(&wtrans)->AddKey(current_key);
+ cryptographer(&wtrans)->Encrypt(
+ our_encrypted_specifics,
+ our_encrypted_specifics.mutable_encrypted());
+ cryptographer(&wtrans)->GetKeys(
+ nigori->mutable_encrypted());
+ cryptographer(&wtrans)->set_encrypt_everything();
+ cryptographer(&wtrans)->UpdateNigoriFromEncryptedTypes(nigori);
+ MutableEntry nigori_entry(&wtrans, GET_BY_SERVER_TAG,
+ syncable::ModelTypeToRootTag(syncable::NIGORI));
+ ASSERT_TRUE(nigori_entry.good());
+ nigori_entry.Put(SPECIFICS, specifics);
+ nigori_entry.Put(IS_UNSYNCED, true);
+ EXPECT_FALSE(cryptographer(&wtrans)->has_pending_keys());
+ EXPECT_TRUE(encrypted_types.Equals(
+ cryptographer(&wtrans)->GetEncryptedTypes()));
+ }
+
+ SyncShareAsDelegate(); // Commit it.
+
+ // Now set up the old nigori node and add it as a server update.
+ sync_pb::EntitySpecifics old_nigori_specifics;
+ sync_pb::NigoriSpecifics *old_nigori = old_nigori_specifics.mutable_nigori();
+ other_cryptographer.GetKeys(old_nigori->mutable_encrypted());
+ other_cryptographer.UpdateNigoriFromEncryptedTypes(old_nigori);
+ mock_server_->SetNigori(1, 30, 30, old_nigori_specifics);
+
+ SyncShareAsDelegate(); // Download the old nigori and apply it.
+
+ {
+ // Ensure everything is committed and stable now. The cryptographer
+ // should be able to decrypt both sets of keys and still be encrypting with
+ // the newest, and the encrypted types should be the most recent
+ ReadTransaction trans(FROM_HERE, directory());
+ Entry nigori_entry(&trans, GET_BY_SERVER_TAG,
+ syncable::ModelTypeToRootTag(syncable::NIGORI));
+ ASSERT_TRUE(nigori_entry.good());
+ EXPECT_FALSE(nigori_entry.Get(IS_UNAPPLIED_UPDATE));
+ EXPECT_FALSE(nigori_entry.Get(IS_UNSYNCED));
+ const sync_pb::NigoriSpecifics& nigori =
+ nigori_entry.Get(SPECIFICS).nigori();
+ EXPECT_TRUE(cryptographer(&trans)->CanDecryptUsingDefaultKey(
+ our_encrypted_specifics.encrypted()));
+ EXPECT_TRUE(cryptographer(&trans)->CanDecrypt(
+ other_encrypted_specifics.encrypted()));
+ EXPECT_TRUE(cryptographer(&trans)->CanDecrypt(
+ nigori.encrypted()));
+ EXPECT_TRUE(cryptographer(&trans)->encrypt_everything());
+ }
+}
+
+// Resolve a confict between two nigori's with different encrypted types,
+// sync_tabs bits, and encryption keys (remote is explicit). Afterwards, the
+// encrypted types should be unioned, the sync_tab bit should be true, and the
+// cryptographer should have both keys and be encrypting with the remote
+// encryption key by default.
TEST_F(SyncerTest, NigoriConflicts) {
KeyParams local_key_params = {"localhost", "dummy", "blargle"};
KeyParams other_key_params = {"localhost", "dummy", "foobar"};
@@ -995,6 +1083,7 @@ TEST_F(SyncerTest, NigoriConflicts) {
nigori->set_encrypt_bookmarks(true);
nigori->set_encrypt_preferences(true);
nigori->set_encrypt_everything(false);
+ nigori->set_using_explicit_passphrase(true);
mock_server_->SetNigori(1, 20, 20, specifics);
}
@@ -1019,6 +1108,7 @@ TEST_F(SyncerTest, NigoriConflicts) {
cryptographer(&wtrans)->GetEncryptedTypes()));
EXPECT_TRUE(cryptographer(&wtrans)->encrypt_everything());
EXPECT_TRUE(specifics.nigori().sync_tabs());
+ EXPECT_TRUE(specifics.nigori().using_explicit_passphrase());
// Supply the pending keys. Afterwards, we should be able to decrypt both
// our own encrypted data and data encrypted by the other cryptographer,
// but the key provided by the other cryptographer should be the default.
@@ -1053,6 +1143,8 @@ TEST_F(SyncerTest, NigoriConflicts) {
EXPECT_TRUE(cryptographer(&wtrans)->
CanDecryptUsingDefaultKey(other_encrypted_specifics.encrypted()));
EXPECT_TRUE(nigori_entry.Get(SPECIFICS).nigori().sync_tabs());
+ EXPECT_TRUE(nigori_entry.Get(SPECIFICS).nigori().
+ using_explicit_passphrase());
}
}
diff --git a/sync/util/cryptographer.cc b/sync/util/cryptographer.cc
index b3d1833..e2ab5e2 100644
--- a/sync/util/cryptographer.cc
+++ b/sync/util/cryptographer.cc
@@ -168,15 +168,13 @@ bool Cryptographer::AddKeyImpl(Nigori* initialized_nigori) {
return true;
}
-bool Cryptographer::SetKeys(const sync_pb::EncryptedData& encrypted) {
+void Cryptographer::InstallKeys(const sync_pb::EncryptedData& encrypted) {
DCHECK(CanDecrypt(encrypted));
sync_pb::NigoriKeyBag bag;
- if (!Decrypt(encrypted, &bag)) {
- return false;
- }
- InstallKeys(encrypted.key_name(), bag);
- return true;
+ if (!Decrypt(encrypted, &bag))
+ return;
+ InstallKeyBag(bag);
}
void Cryptographer::SetPendingKeys(const sync_pb::EncryptedData& encrypted) {
@@ -207,7 +205,10 @@ bool Cryptographer::DecryptPendingKeys(const KeyParams& params) {
NOTREACHED();
return false;
}
- InstallKeys(pending_keys_->key_name(), bag);
+ InstallKeyBag(bag);
+ const std::string& new_default_key_name = pending_keys_->key_name();
+ DCHECK(nigoris_.end() != nigoris_.find(new_default_key_name));
+ default_nigori_ = &*nigoris_.find(new_default_key_name);
pending_keys_.reset();
return true;
}
@@ -289,7 +290,15 @@ Cryptographer::UpdateResult Cryptographer::Update(
UpdateEncryptedTypesFromNigori(nigori);
if (!nigori.encrypted().blob().empty()) {
if (CanDecrypt(nigori.encrypted())) {
- SetKeys(nigori.encrypted());
+ InstallKeys(nigori.encrypted());
+ // We only update the default passphrase if this was a new explicit
+ // passphrase. Else, since it was decryptable, it must not have been a new
+ // key.
+ if (nigori.using_explicit_passphrase()) {
+ std::string new_default_key_name = nigori.encrypted().key_name();
+ DCHECK(nigoris_.end() != nigoris_.find(new_default_key_name));
+ default_nigori_ = &*nigoris_.find(new_default_key_name);
+ }
return Cryptographer::SUCCESS;
} else {
SetPendingKeys(nigori.encrypted());
@@ -423,8 +432,7 @@ void Cryptographer::EmitEncryptedTypesChangedNotification() {
OnEncryptedTypesChanged(encrypted_types_, encrypt_everything_));
}
-void Cryptographer::InstallKeys(const std::string& default_key_name,
- const sync_pb::NigoriKeyBag& bag) {
+void Cryptographer::InstallKeyBag(const sync_pb::NigoriKeyBag& bag) {
int key_size = bag.key_size();
for (int i = 0; i < key_size; ++i) {
const sync_pb::NigoriKey key = bag.key(i);
@@ -440,8 +448,6 @@ void Cryptographer::InstallKeys(const std::string& default_key_name,
nigoris_[key.name()] = make_linked_ptr(new_nigori.release());
}
}
- DCHECK(nigoris_.end() != nigoris_.find(default_key_name));
- default_nigori_ = &*nigoris_.find(default_key_name);
}
} // namespace browser_sync
diff --git a/sync/util/cryptographer.h b/sync/util/cryptographer.h
index af7b272..12f3412 100644
--- a/sync/util/cryptographer.h
+++ b/sync/util/cryptographer.h
@@ -139,11 +139,6 @@ class Cryptographer {
// with a cryptographer that has already been initialized.
bool AddKeyFromBootstrapToken(const std::string restored_bootstrap_token);
- // Decrypts |encrypted| and uses its contents to initialize Nigori instances.
- // Returns true unless decryption of |encrypted| fails. The caller is
- // responsible for checking that CanDecrypt(encrypted) == true.
- bool SetKeys(const sync_pb::EncryptedData& encrypted);
-
// Makes a local copy of |encrypted| to later be decrypted by
// DecryptPendingKeys. This should only be used if CanDecrypt(encrypted) ==
// false.
@@ -157,7 +152,8 @@ class Cryptographer {
// Attempts to decrypt the set of keys that was copied in the previous call to
// SetPendingKeys using |params|. Returns true if the pending keys were
- // successfully decrypted and installed.
+ // successfully decrypted and installed. If successful, the default key
+ // is updated.
bool DecryptPendingKeys(const KeyParams& params);
bool is_initialized() const { return !nigoris_.empty() && default_nigori_; }
@@ -178,6 +174,10 @@ class Cryptographer {
// This updates both the encryption keys and the set of encrypted types.
// Returns NEEDS_PASSPHRASE if was unable to decrypt the pending keys,
// SUCCESS otherwise.
+ // Note: will not change the default key. If the nigori's keybag
+ // is decryptable, all keys are added to the local keybag and the current
+ // default is preserved. If the nigori's keybag is not decryptable, it is
+ // stored in the |pending_keys_|.
UpdateResult Update(const sync_pb::NigoriSpecifics& nigori);
// The set of types that are always encrypted.
@@ -214,12 +214,18 @@ class Cryptographer {
void EmitEncryptedTypesChangedNotification();
+ // Decrypts |encrypted| and uses its contents to initialize Nigori instances.
+ // Returns true unless decryption of |encrypted| fails. The caller is
+ // responsible for checking that CanDecrypt(encrypted) == true.
+ // Does not update the default nigori.
+ void InstallKeys(const sync_pb::EncryptedData& encrypted);
+
// Helper method to instantiate Nigori instances for each set of key
- // parameters in |bag| and setting the default encryption key to
- // |default_key_name|.
- void InstallKeys(const std::string& default_key_name,
- const sync_pb::NigoriKeyBag& bag);
+ // parameters in |bag|.
+ // Does not update the default nigori.
+ void InstallKeyBag(const sync_pb::NigoriKeyBag& bag);
+ // Helper method to add a nigori as the new default nigori.
bool AddKeyImpl(Nigori* nigori);
// Functions to serialize + encrypt a Nigori object in an opaque format for
generated by cgit v1.1 at 2025-02-14 01:33:17 +0000