diff options
| -rw-r--r-- | sync/engine/apply_control_data_updates.cc | 98 | ||||
| -rw-r--r-- | sync/engine/apply_control_data_updates_unittest.cc | 622 | ||||
| -rw-r--r-- | sync/engine/syncer_unittest.cc | 124 | ||||
| -rw-r--r-- | sync/internal_api/public/test/test_entry_factory.h | 34 | ||||
| -rw-r--r-- | sync/internal_api/sync_encryption_handler_impl.cc | 7 | ||||
| -rw-r--r-- | sync/internal_api/sync_encryption_handler_impl.h | 2 | ||||
| -rw-r--r-- | sync/internal_api/sync_encryption_handler_impl_unittest.cc | 74 | ||||
| -rw-r--r-- | sync/internal_api/test/test_entry_factory.cc | 73 |
8 files changed, 816 insertions, 218 deletions
diff --git a/sync/engine/apply_control_data_updates.cc b/sync/engine/apply_control_data_updates.cc index 7a52e8c..aa1454e 100644 --- a/sync/engine/apply_control_data_updates.cc +++ b/sync/engine/apply_control_data_updates.cc @@ -57,6 +57,8 @@ bool ApplyNigoriUpdates(syncable::WriteTransaction* trans, return true; } + // We apply the nigori update regardless of whether there's a conflict or + // not in order to preserve any new encrypted types or encryption keys. const sync_pb::NigoriSpecifics& nigori = nigori_node.Get(SERVER_SPECIFICS).nigori(); trans->directory()->GetNigoriHandler()->ApplyNigoriUpdate(nigori, trans); @@ -83,45 +85,67 @@ bool ApplyNigoriUpdates(syncable::WriteTransaction* trans, if (!nigori_node.Get(IS_UNSYNCED)) { // Update only. UpdateLocalDataFromServerData(trans, &nigori_node); } else { // Conflict. - // Create a new set of specifics based on the server specifics (which - // preserves their encryption keys). - sync_pb::EntitySpecifics specifics = nigori_node.Get(SERVER_SPECIFICS); - sync_pb::NigoriSpecifics* server_nigori = specifics.mutable_nigori(); - // Store the merged set of encrypted types. - // (NigoriHandler::ApplyNigoriUpdate(..) will have merged the local types - // already). - trans->directory()->GetNigoriHandler()->UpdateNigoriFromEncryptedTypes( - server_nigori, - trans); - // The cryptographer has the both the local and remote encryption keys - // (added at NigoriHandler::ApplyNigoriUpdate(..) 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. + const sync_pb::EntitySpecifics& server_specifics = + nigori_node.Get(SERVER_SPECIFICS); + const sync_pb::NigoriSpecifics& server_nigori = server_specifics.nigori(); + const sync_pb::EntitySpecifics& local_specifics = + nigori_node.Get(SPECIFICS); + const sync_pb::NigoriSpecifics& local_nigori = local_specifics.nigori(); + + // We initialize the new nigori with the server state, and will override + // it as necessary below. + sync_pb::EntitySpecifics new_specifics = nigori_node.Get(SERVER_SPECIFICS); + sync_pb::NigoriSpecifics* new_nigori = new_specifics.mutable_nigori(); + + // If the cryptographer is not ready, another client set a new encryption + // passphrase. If we had migrated locally, we will re-migrate when the + // pending keys are provided. If we had set a new custom passphrase locally + // the user will have another chance to set a custom passphrase later + // (assuming they hadn't set a custom passphrase on the other client). + // Therefore, we only attempt to merge the nigori nodes if the cryptographer + // is ready. + // Note: we only update the encryption keybag if we're sure that we aren't + // invalidating the keystore_decryptor_token (i.e. we're either + // not migrated or we copying over all local state). if (cryptographer->is_ready()) { - cryptographer->GetKeys(server_nigori->mutable_encryption_keybag()); - server_nigori->set_keybag_is_frozen( - nigori_node.Get(SPECIFICS).nigori().keybag_is_frozen()); + if (local_nigori.has_passphrase_type() && + server_nigori.has_passphrase_type()) { + // They're both migrated, preserve the local nigori if the passphrase + // type is more conservative. + if (server_nigori.passphrase_type() == + sync_pb::NigoriSpecifics::KEYSTORE_PASSPHRASE && + local_nigori.passphrase_type() != + sync_pb::NigoriSpecifics::KEYSTORE_PASSPHRASE) { + DCHECK(local_nigori.passphrase_type() == + sync_pb::NigoriSpecifics::FROZEN_IMPLICIT_PASSPHRASE || + local_nigori.passphrase_type() == + sync_pb::NigoriSpecifics::CUSTOM_PASSPHRASE); + new_nigori->CopyFrom(local_nigori); + cryptographer->GetKeys(new_nigori->mutable_encryption_keybag()); + } + } else if (!local_nigori.has_passphrase_type() && + !server_nigori.has_passphrase_type()) { + // Set the explicit passphrase based on the local state. If the server + // had set an explict passphrase, we should have pending keys, so + // should not reach this code. + // Because neither side is migrated, we don't have to worry about the + // keystore decryptor token. + new_nigori->set_keybag_is_frozen(local_nigori.keybag_is_frozen()); + cryptographer->GetKeys(new_nigori->mutable_encryption_keybag()); + } else if (local_nigori.has_passphrase_type()) { + // Local is migrated but server is not. Copy over the local migrated + // data. + new_nigori->CopyFrom(local_nigori); + cryptographer->GetKeys(new_nigori->mutable_encryption_keybag()); + } // else leave the new nigori with the server state. } - nigori_node.Put(SPECIFICS, specifics); + + // Always update to the safest set of encrypted types. + trans->directory()->GetNigoriHandler()->UpdateNigoriFromEncryptedTypes( + new_nigori, + trans); + + nigori_node.Put(SPECIFICS, new_specifics); DVLOG(1) << "Resolving simple conflict, merging nigori nodes: " << nigori_node; diff --git a/sync/engine/apply_control_data_updates_unittest.cc b/sync/engine/apply_control_data_updates_unittest.cc index 7f1a9fa..aa00865 100644 --- a/sync/engine/apply_control_data_updates_unittest.cc +++ b/sync/engine/apply_control_data_updates_unittest.cc @@ -56,59 +56,17 @@ class ApplyControlDataUpdatesTest : public SyncerCommandTest { DISALLOW_COPY_AND_ASSIGN(ApplyControlDataUpdatesTest); }; +// Verify that applying a nigori node sets initial sync ended properly, +// updates the set of encrypted types, and updates the cryptographer. TEST_F(ApplyControlDataUpdatesTest, NigoriUpdate) { // Storing the cryptographer separately is bad, but for this test we // know it's safe. Cryptographer* cryptographer; ModelTypeSet encrypted_types; - encrypted_types.Put(PASSWORDS); + encrypted_types.PutAll(SyncEncryptionHandler::SensitiveTypes()); - // We start with initial_sync_ended == false. This is wrong, since would have - // no nigori node if that were the case. Howerver, it makes it easier to - // verify that ApplyControlDataUpdates sets initial_sync_ended correctly. - EXPECT_FALSE(directory()->initial_sync_ended_types().Has(NIGORI)); - - { - syncable::ReadTransaction trans(FROM_HERE, directory()); - cryptographer = directory()->GetCryptographer(&trans); - EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans) - .Equals(encrypted_types)); - } - - // Nigori node updates should update the Cryptographer. - Cryptographer other_cryptographer(cryptographer->encryptor()); - KeyParams params = {"localhost", "dummy", "foobar"}; - other_cryptographer.AddKey(params); - - sync_pb::EntitySpecifics specifics; - sync_pb::NigoriSpecifics* nigori = specifics.mutable_nigori(); - other_cryptographer.GetKeys(nigori->mutable_encryption_keybag()); - nigori->set_encrypt_everything(true); - entry_factory_->CreateUnappliedNewItem( - ModelTypeToRootTag(NIGORI), specifics, true); - EXPECT_FALSE(cryptographer->has_pending_keys()); - - ApplyControlDataUpdates(directory()); - - EXPECT_FALSE(cryptographer->is_ready()); - EXPECT_TRUE(cryptographer->has_pending_keys()); - { - syncable::ReadTransaction trans(FROM_HERE, directory()); - EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans) - .Equals(ModelTypeSet::All())); - EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI)); - } -} - -TEST_F(ApplyControlDataUpdatesTest, NigoriUpdateForDisabledTypes) { - // Storing the cryptographer separately is bad, but for this test we - // know it's safe. - Cryptographer* cryptographer; - ModelTypeSet encrypted_types; - encrypted_types.Put(PASSWORDS); - - // We start with initial_sync_ended == false. This is wrong, since would have - // no nigori node if that were the case. Howerver, it makes it easier to + // We start with initial_sync_ended == false. This is wrong, since we would + // have no nigori node if that were the case. However, it makes it easier to // verify that ApplyControlDataUpdates sets initial_sync_ended correctly. EXPECT_FALSE(directory()->initial_sync_ended_types().Has(NIGORI)); @@ -154,7 +112,7 @@ TEST_F(ApplyControlDataUpdatesTest, EncryptUnsyncedChanges) { // know it's safe. Cryptographer* cryptographer; ModelTypeSet encrypted_types; - encrypted_types.Put(PASSWORDS); + encrypted_types.PutAll(SyncEncryptionHandler::SensitiveTypes()); { syncable::ReadTransaction trans(FROM_HERE, directory()); cryptographer = directory()->GetCryptographer(&trans); @@ -258,12 +216,16 @@ TEST_F(ApplyControlDataUpdatesTest, EncryptUnsyncedChanges) { } } +// Create some local unsynced and unencrypted changes. Receive a new nigori +// node enabling their encryption but also introducing pending keys. Ensure +// we apply the update properly without encrypting the unsynced changes or +// breaking. TEST_F(ApplyControlDataUpdatesTest, CannotEncryptUnsyncedChanges) { // Storing the cryptographer separately is bad, but for this test we // know it's safe. Cryptographer* cryptographer; ModelTypeSet encrypted_types; - encrypted_types.Put(PASSWORDS); + encrypted_types.PutAll(SyncEncryptionHandler::SensitiveTypes()); { syncable::ReadTransaction trans(FROM_HERE, directory()); cryptographer = directory()->GetCryptographer(&trans); @@ -344,4 +306,566 @@ TEST_F(ApplyControlDataUpdatesTest, CannotEncryptUnsyncedChanges) { } } +// Verify we handle a nigori node conflict by merging encryption keys and +// types, but preserve the custom passphrase state of the server. +// Initial sync ended should be set. +TEST_F(ApplyControlDataUpdatesTest, + NigoriConflictPendingKeysServerEncryptEverythingCustom) { + Cryptographer* cryptographer; + ModelTypeSet encrypted_types(SyncEncryptionHandler::SensitiveTypes()); + KeyParams other_params = {"localhost", "dummy", "foobar"}; + KeyParams local_params = {"localhost", "dummy", "local"}; + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + cryptographer = directory()->GetCryptographer(&trans); + EXPECT_TRUE(encrypted_types.Equals( + directory()->GetNigoriHandler()->GetEncryptedTypes(&trans))); + } + + // Set up a temporary cryptographer to generate new keys with. + Cryptographer other_cryptographer(cryptographer->encryptor()); + other_cryptographer.AddKey(other_params); + + // Create server specifics with pending keys, new encrypted types, + // and a custom passphrase (unmigrated). + sync_pb::EntitySpecifics server_specifics; + sync_pb::NigoriSpecifics* server_nigori = server_specifics.mutable_nigori(); + other_cryptographer.GetKeys(server_nigori->mutable_encryption_keybag()); + server_nigori->set_encrypt_everything(true); + server_nigori->set_keybag_is_frozen(true); + int64 nigori_handle = + entry_factory_->CreateUnappliedNewItem(kNigoriTag, + server_specifics, + true); + + // Initialize the local cryptographer with the local keys. + cryptographer->AddKey(local_params); + EXPECT_TRUE(cryptographer->is_ready()); + + // Set up a local nigori with the local encryption keys and default encrypted + // types. + sync_pb::EntitySpecifics local_specifics; + sync_pb::NigoriSpecifics* local_nigori = local_specifics.mutable_nigori(); + cryptographer->GetKeys(local_nigori->mutable_encryption_keybag()); + local_nigori->set_encrypt_everything(false); + local_nigori->set_keybag_is_frozen(true); + ASSERT_TRUE(entry_factory_->SetLocalSpecificsForItem( + nigori_handle, local_specifics)); + // Apply the update locally so that UpdateFromEncryptedTypes knows what state + // to use. + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + cryptographer = directory()->GetCryptographer(&trans); + directory()->GetNigoriHandler()->ApplyNigoriUpdate( + *local_nigori, + &trans); + } + + EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); + EXPECT_TRUE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); + ApplyControlDataUpdates(directory()); + EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); + EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); + + EXPECT_FALSE(cryptographer->is_ready()); + EXPECT_TRUE(cryptographer->is_initialized()); + EXPECT_TRUE(cryptographer->has_pending_keys()); + EXPECT_TRUE(other_cryptographer.CanDecryptUsingDefaultKey( + entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().encryption_keybag())); + EXPECT_TRUE(entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().keybag_is_frozen()); + EXPECT_TRUE(entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().encrypt_everything()); + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans) + .Equals(ModelTypeSet::All())); + EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI)); + } +} + +// Verify we handle a nigori node conflict by merging encryption keys and +// types, but preserve the custom passphrase state of the server. +// Initial sync ended should be set. +TEST_F(ApplyControlDataUpdatesTest, + NigoriConflictPendingKeysLocalEncryptEverythingCustom) { + Cryptographer* cryptographer; + ModelTypeSet encrypted_types(SyncEncryptionHandler::SensitiveTypes()); + KeyParams other_params = {"localhost", "dummy", "foobar"}; + KeyParams local_params = {"localhost", "dummy", "local"}; + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + cryptographer = directory()->GetCryptographer(&trans); + EXPECT_TRUE(encrypted_types.Equals( + directory()->GetNigoriHandler()->GetEncryptedTypes(&trans))); + } + + // Set up a temporary cryptographer to generate new keys with. + Cryptographer other_cryptographer(cryptographer->encryptor()); + other_cryptographer.AddKey(other_params); + + // Create server specifics with pending keys, new encrypted types, + // and a custom passphrase (unmigrated). + sync_pb::EntitySpecifics server_specifics; + sync_pb::NigoriSpecifics* server_nigori = server_specifics.mutable_nigori(); + other_cryptographer.GetKeys(server_nigori->mutable_encryption_keybag()); + server_nigori->set_encrypt_everything(false); + server_nigori->set_keybag_is_frozen(false); + int64 nigori_handle = + entry_factory_->CreateUnappliedNewItem(kNigoriTag, + server_specifics, + true); + + // Initialize the local cryptographer with the local keys. + cryptographer->AddKey(local_params); + EXPECT_TRUE(cryptographer->is_ready()); + + // Set up a local nigori with the local encryption keys and default encrypted + // types. + sync_pb::EntitySpecifics local_specifics; + sync_pb::NigoriSpecifics* local_nigori = local_specifics.mutable_nigori(); + cryptographer->GetKeys(local_nigori->mutable_encryption_keybag()); + local_nigori->set_encrypt_everything(true); + local_nigori->set_keybag_is_frozen(true); + ASSERT_TRUE(entry_factory_->SetLocalSpecificsForItem( + nigori_handle, local_specifics)); + // Apply the update locally so that UpdateFromEncryptedTypes knows what state + // to use. + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + cryptographer = directory()->GetCryptographer(&trans); + directory()->GetNigoriHandler()->ApplyNigoriUpdate( + *local_nigori, + &trans); + } + + EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); + EXPECT_TRUE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); + ApplyControlDataUpdates(directory()); + EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); + EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); + + EXPECT_FALSE(cryptographer->is_ready()); + EXPECT_TRUE(cryptographer->is_initialized()); + EXPECT_TRUE(cryptographer->has_pending_keys()); + EXPECT_TRUE(other_cryptographer.CanDecryptUsingDefaultKey( + entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().encryption_keybag())); + EXPECT_FALSE(entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().keybag_is_frozen()); + EXPECT_TRUE(entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().encrypt_everything()); + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans) + .Equals(ModelTypeSet::All())); + EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI)); + } +} + +// If the conflicting nigori has a subset of the local keys, the conflict +// resolution should preserve the full local keys. Initial sync ended should be +// set. +TEST_F(ApplyControlDataUpdatesTest, + NigoriConflictOldKeys) { + Cryptographer* cryptographer; + ModelTypeSet encrypted_types(SyncEncryptionHandler::SensitiveTypes()); + KeyParams old_params = {"localhost", "dummy", "old"}; + KeyParams new_params = {"localhost", "dummy", "new"}; + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + cryptographer = directory()->GetCryptographer(&trans); + EXPECT_TRUE(encrypted_types.Equals( + directory()->GetNigoriHandler()->GetEncryptedTypes(&trans))); + } + + // Set up the cryptographer with old keys + cryptographer->AddKey(old_params); + + // Create server specifics with old keys and new encrypted types. + sync_pb::EntitySpecifics server_specifics; + sync_pb::NigoriSpecifics* server_nigori = server_specifics.mutable_nigori(); + cryptographer->GetKeys(server_nigori->mutable_encryption_keybag()); + server_nigori->set_encrypt_everything(true); + int64 nigori_handle = + entry_factory_->CreateUnappliedNewItem(kNigoriTag, + server_specifics, + true); + + // Add the new keys to the cryptogrpaher + cryptographer->AddKey(new_params); + EXPECT_TRUE(cryptographer->is_ready()); + + // Set up a local nigori with the superset of keys. + sync_pb::EntitySpecifics local_specifics; + sync_pb::NigoriSpecifics* local_nigori = local_specifics.mutable_nigori(); + cryptographer->GetKeys(local_nigori->mutable_encryption_keybag()); + local_nigori->set_encrypt_everything(false); + ASSERT_TRUE(entry_factory_->SetLocalSpecificsForItem( + nigori_handle, local_specifics)); + // Apply the update locally so that UpdateFromEncryptedTypes knows what state + // to use. + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + cryptographer = directory()->GetCryptographer(&trans); + directory()->GetNigoriHandler()->ApplyNigoriUpdate( + *local_nigori, + &trans); + } + + EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); + EXPECT_TRUE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); + ApplyControlDataUpdates(directory()); + EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); + EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); + + EXPECT_TRUE(cryptographer->is_ready()); + EXPECT_TRUE(cryptographer->CanDecryptUsingDefaultKey( + entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().encryption_keybag())); + EXPECT_FALSE(entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().keybag_is_frozen()); + EXPECT_TRUE(entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().encrypt_everything()); + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans) + .Equals(ModelTypeSet::All())); + EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI)); + } +} + +// If both nigoris are migrated, but we also set a custom passphrase locally, +// the local nigori should be preserved. +TEST_F(ApplyControlDataUpdatesTest, + NigoriConflictBothMigratedLocalCustom) { + Cryptographer* cryptographer; + ModelTypeSet encrypted_types(SyncEncryptionHandler::SensitiveTypes()); + KeyParams old_params = {"localhost", "dummy", "old"}; + KeyParams new_params = {"localhost", "dummy", "new"}; + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + cryptographer = directory()->GetCryptographer(&trans); + EXPECT_TRUE(encrypted_types.Equals( + directory()->GetNigoriHandler()->GetEncryptedTypes(&trans))); + } + + // Set up the cryptographer with new keys + Cryptographer other_cryptographer(cryptographer->encryptor()); + other_cryptographer.AddKey(old_params); + + // Create server specifics with a migrated keystore passphrase type. + sync_pb::EntitySpecifics server_specifics; + sync_pb::NigoriSpecifics* server_nigori = server_specifics.mutable_nigori(); + other_cryptographer.GetKeys(server_nigori->mutable_encryption_keybag()); + server_nigori->set_encrypt_everything(false); + server_nigori->set_keybag_is_frozen(true); + server_nigori->set_passphrase_type( + sync_pb::NigoriSpecifics::KEYSTORE_PASSPHRASE); + server_nigori->mutable_keystore_decryptor_token(); + int64 nigori_handle = + entry_factory_->CreateUnappliedNewItem(kNigoriTag, + server_specifics, + true); + + // Add the new keys to the cryptographer. + cryptographer->AddKey(old_params); + cryptographer->AddKey(new_params); + EXPECT_TRUE(cryptographer->is_ready()); + + // Set up a local nigori with a migrated custom passphrase type + sync_pb::EntitySpecifics local_specifics; + sync_pb::NigoriSpecifics* local_nigori = local_specifics.mutable_nigori(); + cryptographer->GetKeys(local_nigori->mutable_encryption_keybag()); + local_nigori->set_encrypt_everything(true); + local_nigori->set_keybag_is_frozen(true); + local_nigori->set_passphrase_type( + sync_pb::NigoriSpecifics::CUSTOM_PASSPHRASE); + ASSERT_TRUE(entry_factory_->SetLocalSpecificsForItem( + nigori_handle, local_specifics)); + // Apply the update locally so that UpdateFromEncryptedTypes knows what state + // to use. + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + cryptographer = directory()->GetCryptographer(&trans); + directory()->GetNigoriHandler()->ApplyNigoriUpdate( + *local_nigori, + &trans); + } + + EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); + EXPECT_TRUE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); + ApplyControlDataUpdates(directory()); + EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); + EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); + + EXPECT_TRUE(cryptographer->is_ready()); + EXPECT_TRUE(cryptographer->CanDecryptUsingDefaultKey( + entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().encryption_keybag())); + EXPECT_TRUE(entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().keybag_is_frozen()); + EXPECT_TRUE(entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().encrypt_everything()); + EXPECT_EQ(sync_pb::NigoriSpecifics::CUSTOM_PASSPHRASE, + entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().passphrase_type()); + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans) + .Equals(ModelTypeSet::All())); + EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI)); + } +} + +// If both nigoris are migrated, but a custom passphrase with a new key was +// set remotely, the remote nigori should be preserved. +TEST_F(ApplyControlDataUpdatesTest, + NigoriConflictBothMigratedServerCustom) { + Cryptographer* cryptographer; + ModelTypeSet encrypted_types(SyncEncryptionHandler::SensitiveTypes()); + KeyParams old_params = {"localhost", "dummy", "old"}; + KeyParams new_params = {"localhost", "dummy", "new"}; + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + cryptographer = directory()->GetCryptographer(&trans); + EXPECT_TRUE(encrypted_types.Equals( + directory()->GetNigoriHandler()->GetEncryptedTypes(&trans))); + } + + // Set up the cryptographer with both new keys and old keys. + Cryptographer other_cryptographer(cryptographer->encryptor()); + other_cryptographer.AddKey(old_params); + other_cryptographer.AddKey(new_params); + + // Create server specifics with a migrated custom passphrase type. + sync_pb::EntitySpecifics server_specifics; + sync_pb::NigoriSpecifics* server_nigori = server_specifics.mutable_nigori(); + other_cryptographer.GetKeys(server_nigori->mutable_encryption_keybag()); + server_nigori->set_encrypt_everything(true); + server_nigori->set_keybag_is_frozen(true); + server_nigori->set_passphrase_type( + sync_pb::NigoriSpecifics::CUSTOM_PASSPHRASE); + int64 nigori_handle = + entry_factory_->CreateUnappliedNewItem(kNigoriTag, + server_specifics, + true); + + // Add the old keys to the cryptographer. + cryptographer->AddKey(old_params); + EXPECT_TRUE(cryptographer->is_ready()); + + // Set up a local nigori with a migrated keystore passphrase type + sync_pb::EntitySpecifics local_specifics; + sync_pb::NigoriSpecifics* local_nigori = local_specifics.mutable_nigori(); + cryptographer->GetKeys(local_nigori->mutable_encryption_keybag()); + local_nigori->set_encrypt_everything(false); + local_nigori->set_keybag_is_frozen(true); + local_nigori->set_passphrase_type( + sync_pb::NigoriSpecifics::KEYSTORE_PASSPHRASE); + server_nigori->mutable_keystore_decryptor_token(); + ASSERT_TRUE(entry_factory_->SetLocalSpecificsForItem( + nigori_handle, local_specifics)); + // Apply the update locally so that UpdateFromEncryptedTypes knows what state + // to use. + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + cryptographer = directory()->GetCryptographer(&trans); + directory()->GetNigoriHandler()->ApplyNigoriUpdate( + *local_nigori, + &trans); + } + + EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); + EXPECT_TRUE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); + ApplyControlDataUpdates(directory()); + EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); + EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); + + EXPECT_TRUE(cryptographer->is_initialized()); + EXPECT_TRUE(cryptographer->has_pending_keys()); + EXPECT_TRUE(other_cryptographer.CanDecryptUsingDefaultKey( + entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().encryption_keybag())); + EXPECT_TRUE(entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().keybag_is_frozen()); + EXPECT_TRUE(entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().encrypt_everything()); + EXPECT_EQ(sync_pb::NigoriSpecifics::CUSTOM_PASSPHRASE, + entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().passphrase_type()); + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans) + .Equals(ModelTypeSet::All())); + EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI)); + } +} + +// If the local nigori is migrated but the server is not, preserve the local +// nigori. +TEST_F(ApplyControlDataUpdatesTest, + NigoriConflictLocalMigrated) { + Cryptographer* cryptographer; + ModelTypeSet encrypted_types(SyncEncryptionHandler::SensitiveTypes()); + KeyParams old_params = {"localhost", "dummy", "old"}; + KeyParams new_params = {"localhost", "dummy", "new"}; + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + cryptographer = directory()->GetCryptographer(&trans); + EXPECT_TRUE(encrypted_types.Equals( + directory()->GetNigoriHandler()->GetEncryptedTypes(&trans))); + } + + // Set up the cryptographer with both new keys and old keys. + Cryptographer other_cryptographer(cryptographer->encryptor()); + other_cryptographer.AddKey(old_params); + + // Create server specifics with an unmigrated implicit passphrase type. + sync_pb::EntitySpecifics server_specifics; + sync_pb::NigoriSpecifics* server_nigori = server_specifics.mutable_nigori(); + other_cryptographer.GetKeys(server_nigori->mutable_encryption_keybag()); + server_nigori->set_encrypt_everything(true); + server_nigori->set_keybag_is_frozen(false); + int64 nigori_handle = + entry_factory_->CreateUnappliedNewItem(kNigoriTag, + server_specifics, + true); + + // Add the old keys to the cryptographer. + cryptographer->AddKey(old_params); + cryptographer->AddKey(new_params); + EXPECT_TRUE(cryptographer->is_ready()); + + // Set up a local nigori with a migrated custom passphrase type + sync_pb::EntitySpecifics local_specifics; + sync_pb::NigoriSpecifics* local_nigori = local_specifics.mutable_nigori(); + cryptographer->GetKeys(local_nigori->mutable_encryption_keybag()); + local_nigori->set_encrypt_everything(true); + local_nigori->set_keybag_is_frozen(true); + local_nigori->set_passphrase_type( + sync_pb::NigoriSpecifics::CUSTOM_PASSPHRASE); + ASSERT_TRUE(entry_factory_->SetLocalSpecificsForItem( + nigori_handle, local_specifics)); + // Apply the update locally so that UpdateFromEncryptedTypes knows what state + // to use. + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + cryptographer = directory()->GetCryptographer(&trans); + directory()->GetNigoriHandler()->ApplyNigoriUpdate( + *local_nigori, + &trans); + } + + EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); + EXPECT_TRUE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); + ApplyControlDataUpdates(directory()); + EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); + EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); + + EXPECT_TRUE(cryptographer->is_ready()); + EXPECT_TRUE(cryptographer->CanDecryptUsingDefaultKey( + entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().encryption_keybag())); + EXPECT_TRUE(entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().keybag_is_frozen()); + EXPECT_TRUE(entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().encrypt_everything()); + EXPECT_EQ(sync_pb::NigoriSpecifics::CUSTOM_PASSPHRASE, + entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().passphrase_type()); + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + EXPECT_TRUE(directory()->GetNigoriHandler()->GetEncryptedTypes(&trans) + .Equals(ModelTypeSet::All())); + EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI)); + } +} + +// If the server nigori is migrated but the local is not, preserve the server +// nigori. +TEST_F(ApplyControlDataUpdatesTest, + NigoriConflictServerMigrated) { + Cryptographer* cryptographer; + ModelTypeSet encrypted_types(SyncEncryptionHandler::SensitiveTypes()); + KeyParams old_params = {"localhost", "dummy", "old"}; + KeyParams new_params = {"localhost", "dummy", "new"}; + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + cryptographer = directory()->GetCryptographer(&trans); + EXPECT_TRUE(encrypted_types.Equals( + directory()->GetNigoriHandler()->GetEncryptedTypes(&trans))); + } + + // Set up the cryptographer with both new keys and old keys. + Cryptographer other_cryptographer(cryptographer->encryptor()); + other_cryptographer.AddKey(old_params); + + // Create server specifics with an migrated keystore passphrase type. + sync_pb::EntitySpecifics server_specifics; + sync_pb::NigoriSpecifics* server_nigori = server_specifics.mutable_nigori(); + other_cryptographer.GetKeys(server_nigori->mutable_encryption_keybag()); + server_nigori->set_encrypt_everything(false); + server_nigori->set_keybag_is_frozen(true); + server_nigori->set_passphrase_type( + sync_pb::NigoriSpecifics::KEYSTORE_PASSPHRASE); + server_nigori->mutable_keystore_decryptor_token(); + int64 nigori_handle = + entry_factory_->CreateUnappliedNewItem(kNigoriTag, + server_specifics, + true); + + // Add the old keys to the cryptographer. + cryptographer->AddKey(old_params); + cryptographer->AddKey(new_params); + EXPECT_TRUE(cryptographer->is_ready()); + + // Set up a local nigori with a migrated custom passphrase type + sync_pb::EntitySpecifics local_specifics; + sync_pb::NigoriSpecifics* local_nigori = local_specifics.mutable_nigori(); + cryptographer->GetKeys(local_nigori->mutable_encryption_keybag()); + local_nigori->set_encrypt_everything(false); + local_nigori->set_keybag_is_frozen(false); + ASSERT_TRUE(entry_factory_->SetLocalSpecificsForItem( + nigori_handle, local_specifics)); + // Apply the update locally so that UpdateFromEncryptedTypes knows what state + // to use. + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + cryptographer = directory()->GetCryptographer(&trans); + directory()->GetNigoriHandler()->ApplyNigoriUpdate( + *local_nigori, + &trans); + } + + EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); + EXPECT_TRUE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); + ApplyControlDataUpdates(directory()); + EXPECT_TRUE(entry_factory_->GetIsUnsyncedForItem(nigori_handle)); + EXPECT_FALSE(entry_factory_->GetIsUnappliedForItem(nigori_handle)); + + EXPECT_TRUE(cryptographer->is_ready()); + // Note: we didn't overwrite the encryption keybag with the local keys. The + // sync encryption handler will do that when it detects that the new + // keybag is out of date (and update the keystore bootstrap if necessary). + EXPECT_FALSE(cryptographer->CanDecryptUsingDefaultKey( + entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().encryption_keybag())); + EXPECT_TRUE(cryptographer->CanDecrypt( + entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().encryption_keybag())); + EXPECT_TRUE(entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().keybag_is_frozen()); + EXPECT_TRUE(entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().has_keystore_decryptor_token()); + EXPECT_EQ(sync_pb::NigoriSpecifics::KEYSTORE_PASSPHRASE, + entry_factory_->GetLocalSpecificsForItem(nigori_handle). + nigori().passphrase_type()); + { + syncable::ReadTransaction trans(FROM_HERE, directory()); + EXPECT_TRUE(directory()->initial_sync_ended_types().Has(NIGORI)); + } +} + } // namespace syncer diff --git a/sync/engine/syncer_unittest.cc b/sync/engine/syncer_unittest.cc index 0165016..28650a6 100644 --- a/sync/engine/syncer_unittest.cc +++ b/sync/engine/syncer_unittest.cc @@ -978,130 +978,6 @@ TEST_F(SyncerTest, EncryptionAwareConflicts) { #undef VERIFY_ENTRY -// Resolve a confict between two nigori's with different encrypted types, -// and encryption keys (remote is explicit). Afterwards, the encrypted types -// should be unioned and the cryptographer should have both keys and be -// encrypting with the remote encryption key by default. -// TODO(zea): Test conflicts with keystore migration. -TEST_F(SyncerTest, NigoriConflicts) { - KeyParams local_key_params = {"localhost", "dummy", "blargle"}; - KeyParams other_key_params = {"localhost", "dummy", "foobar"}; - Cryptographer other_cryptographer(&encryptor_); - other_cryptographer.AddKey(other_key_params); - ModelTypeSet encrypted_types(PASSWORDS, NIGORI); - sync_pb::EntitySpecifics initial_nigori_specifics; - initial_nigori_specifics.mutable_nigori(); - mock_server_->SetNigori(1, 10, 10, initial_nigori_specifics); - - // Data for testing encryption/decryption. - 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"); - - // Receive the initial nigori node. - SyncShareNudge(); - encrypted_types = ModelTypeSet::All(); - { - // Local changes with different passphrase, different types. - WriteTransaction wtrans(FROM_HERE, UNITTEST, directory()); - sync_pb::EntitySpecifics specifics; - sync_pb::NigoriSpecifics* nigori = specifics.mutable_nigori(); - GetCryptographer(&wtrans)->AddKey(local_key_params); - GetCryptographer(&wtrans)->Encrypt( - our_encrypted_specifics, - our_encrypted_specifics.mutable_encrypted()); - GetCryptographer(&wtrans)->GetKeys( - nigori->mutable_encryption_keybag()); - dir_maker_.encryption_handler()->EnableEncryptEverything(); - directory()->GetNigoriHandler()->UpdateNigoriFromEncryptedTypes( - nigori, - &wtrans); - MutableEntry nigori_entry(&wtrans, GET_BY_SERVER_TAG, - ModelTypeToRootTag(NIGORI)); - ASSERT_TRUE(nigori_entry.good()); - nigori_entry.Put(SPECIFICS, specifics); - nigori_entry.Put(IS_UNSYNCED, true); - EXPECT_FALSE(GetCryptographer(&wtrans)->has_pending_keys()); - EXPECT_TRUE(encrypted_types.Equals( - directory()->GetNigoriHandler()->GetEncryptedTypes(&wtrans))); - } - { - sync_pb::EntitySpecifics specifics; - sync_pb::NigoriSpecifics* nigori = specifics.mutable_nigori(); - other_cryptographer.GetKeys(nigori->mutable_encryption_keybag()); - nigori->set_encrypt_bookmarks(true); - nigori->set_encrypt_preferences(true); - nigori->set_encrypt_everything(false); - nigori->set_keybag_is_frozen(true); - mock_server_->SetNigori(1, 20, 20, specifics); - } - - // Will result in downloading the server nigori, which puts the local nigori - // in a state of conflict. This is resolved by merging the local and server - // data (with priority given to the server's encryption keys if they are - // undecryptable), which we then commit. The cryptographer should have pending - // keys and merge the set of encrypted types. - SyncShareNudge(); // Resolve conflict in this cycle. - SyncShareNudge(); // Commit local change in this cycle. - { - // Ensure the nigori data merged (encrypted types). - WriteTransaction wtrans(FROM_HERE, UNITTEST, directory()); - MutableEntry nigori_entry(&wtrans, GET_BY_SERVER_TAG, - ModelTypeToRootTag(NIGORI)); - ASSERT_TRUE(nigori_entry.good()); - EXPECT_FALSE(nigori_entry.Get(IS_UNAPPLIED_UPDATE)); - EXPECT_FALSE(nigori_entry.Get(IS_UNSYNCED)); - sync_pb::EntitySpecifics specifics = nigori_entry.Get(SPECIFICS); - ASSERT_TRUE(GetCryptographer(&wtrans)->has_pending_keys()); - EXPECT_TRUE(encrypted_types.Equals( - directory()->GetNigoriHandler()->GetEncryptedTypes(&wtrans))); - EXPECT_TRUE(dir_maker_.encryption_handler()->EncryptEverythingEnabled()); - EXPECT_TRUE(specifics.nigori().keybag_is_frozen()); - // 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. - EXPECT_TRUE( - GetCryptographer(&wtrans)->DecryptPendingKeys(other_key_params)); - EXPECT_FALSE(GetCryptographer(&wtrans)->has_pending_keys()); - sync_pb::NigoriSpecifics* nigori = specifics.mutable_nigori(); - GetCryptographer(&wtrans)->GetKeys(nigori->mutable_encryption_keybag()); - directory()->GetNigoriHandler()->UpdateNigoriFromEncryptedTypes( - nigori, - &wtrans); - // Normally this would be written as part of SetPassphrase, but we do it - // manually for the test. - nigori_entry.Put(SPECIFICS, specifics); - nigori_entry.Put(IS_UNSYNCED, true); - } - - SyncShareNudge(); - { - // Ensure everything is committed and stable now. The cryptographer - // should be able to decrypt both sets of keys, and the encrypted types - // should have been unioned. - WriteTransaction wtrans(FROM_HERE, UNITTEST, directory()); - MutableEntry nigori_entry(&wtrans, GET_BY_SERVER_TAG, - ModelTypeToRootTag(NIGORI)); - ASSERT_TRUE(nigori_entry.good()); - EXPECT_FALSE(nigori_entry.Get(IS_UNAPPLIED_UPDATE)); - EXPECT_FALSE(nigori_entry.Get(IS_UNSYNCED)); - EXPECT_TRUE(GetCryptographer(&wtrans)->CanDecrypt( - our_encrypted_specifics.encrypted())); - EXPECT_FALSE(GetCryptographer(&wtrans)-> - CanDecryptUsingDefaultKey(our_encrypted_specifics.encrypted())); - EXPECT_TRUE(GetCryptographer(&wtrans)->CanDecrypt( - other_encrypted_specifics.encrypted())); - EXPECT_TRUE(GetCryptographer(&wtrans)-> - CanDecryptUsingDefaultKey(other_encrypted_specifics.encrypted())); - EXPECT_TRUE(nigori_entry.Get(SPECIFICS).nigori(). - keybag_is_frozen()); - } -} - TEST_F(SyncerTest, TestGetUnsyncedAndSimpleCommit) { { WriteTransaction wtrans(FROM_HERE, UNITTEST, directory()); diff --git a/sync/internal_api/public/test/test_entry_factory.h b/sync/internal_api/public/test/test_entry_factory.h index 83efe26..1661a33 100644 --- a/sync/internal_api/public/test/test_entry_factory.h +++ b/sync/internal_api/public/test/test_entry_factory.h @@ -30,9 +30,9 @@ class TestEntryFactory { const std::string& parent_id); // Create a new unapplied update without a parent. - void CreateUnappliedNewItem(const std::string& item_id, - const sync_pb::EntitySpecifics& specifics, - bool is_unique); + int64 CreateUnappliedNewItem(const std::string& item_id, + const sync_pb::EntitySpecifics& specifics, + bool is_unique); // Create an unsynced item in the database. If item_id is a local ID, it will // be treated as a create-new. Otherwise, if it's a server ID, we'll fake the @@ -56,6 +56,34 @@ class TestEntryFactory { int64 CreateSyncedItem(const std::string& name, ModelType model_type, bool is_folder); + // Looks up the item referenced by |meta_handle|. If successful, overwrites + // the server specifics with |specifics|, sets + // IS_UNAPPLIED_UPDATES/IS_UNSYNCED appropriately, and returns true. + // Else, return false. + bool SetServerSpecificsForItem(int64 meta_handle, + const sync_pb::EntitySpecifics specifics); + + // Looks up the item referenced by |meta_handle|. If successful, overwrites + // the local specifics with |specifics|, sets + // IS_UNAPPLIED_UPDATES/IS_UNSYNCED appropriately, and returns true. + // Else, return false. + bool SetLocalSpecificsForItem(int64 meta_handle, + const sync_pb::EntitySpecifics specifics); + + // Looks up the item referenced by |meta_handle|. If successful, stores + // the server specifics into |specifics| and returns true. Else, return false. + const sync_pb::EntitySpecifics& GetServerSpecificsForItem( + int64 meta_handle) const; + + // Looks up the item referenced by |meta_handle|. If successful, stores + // the local specifics into |specifics| and returns true. Else, return false. + const sync_pb::EntitySpecifics& GetLocalSpecificsForItem( + int64 meta_handle) const; + + // Getters for IS_UNSYNCED and IS_UNAPPLIED_UPDATE bit fields. + bool GetIsUnsyncedForItem(int64 meta_handle) const; + bool GetIsUnappliedForItem(int64 meta_handle) const; + int64 GetNextRevision(); private: diff --git a/sync/internal_api/sync_encryption_handler_impl.cc b/sync/internal_api/sync_encryption_handler_impl.cc index f0bb6f87..5617df9 100644 --- a/sync/internal_api/sync_encryption_handler_impl.cc +++ b/sync/internal_api/sync_encryption_handler_impl.cc @@ -1104,6 +1104,12 @@ bool SyncEncryptionHandlerImpl::ShouldTriggerMigration( } else if (passphrase_type_ == KEYSTORE_PASSPHRASE && encrypt_everything_) { return true; + } else if ( + cryptographer.is_ready() && + !cryptographer.CanDecryptUsingDefaultKey(nigori.encryption_keybag())) { + // We need to overwrite the keybag. This might involve overwriting the + // keystore decryptor too. + return true; } else { return false; } @@ -1148,7 +1154,6 @@ bool SyncEncryptionHandlerImpl::AttemptToMigrateNigoriToKeystore( new_encrypt_everything = true; migrated_nigori.clear_keystore_decryptor_token(); } else { - DCHECK_EQ(passphrase_type_, IMPLICIT_PASSPHRASE); DCHECK(!encrypt_everything_); new_passphrase_type = KEYSTORE_PASSPHRASE; DVLOG(1) << "Switching to keystore passphrase state."; diff --git a/sync/internal_api/sync_encryption_handler_impl.h b/sync/internal_api/sync_encryption_handler_impl.h index 8c9edd1..f54055b 100644 --- a/sync/internal_api/sync_encryption_handler_impl.h +++ b/sync/internal_api/sync_encryption_handler_impl.h @@ -116,6 +116,8 @@ class SyncEncryptionHandlerImpl SetImplicitPassAfterMigrationNoKeystoreKey); FRIEND_TEST_ALL_PREFIXES(SyncEncryptionHandlerImplTest, MigrateOnEncryptEverythingKeystorePassphrase); + FRIEND_TEST_ALL_PREFIXES(SyncEncryptionHandlerImplTest, + ReceiveMigratedNigoriWithOldPassphrase); // Container for members that require thread safety protection. All members // that can be accessed from more than one thread should be held here and diff --git a/sync/internal_api/sync_encryption_handler_impl_unittest.cc b/sync/internal_api/sync_encryption_handler_impl_unittest.cc index f237556..996394e 100644 --- a/sync/internal_api/sync_encryption_handler_impl_unittest.cc +++ b/sync/internal_api/sync_encryption_handler_impl_unittest.cc @@ -1603,4 +1603,78 @@ TEST_F(SyncEncryptionHandlerImplTest, EXPECT_TRUE(GetCryptographer()->CanDecrypt(keystore_encrypted)); } +// If we receive a nigori migrated and with a KEYSTORE_PASSPHRASE type, but +// using an old default key (i.e. old GAIA password), we should overwrite the +// nigori, updating the keybag and keystore decryptor. +TEST_F(SyncEncryptionHandlerImplTest, + ReceiveMigratedNigoriWithOldPassphrase) { + const char kOldKey[] = "old"; + const char kCurKey[] = "cur"; + sync_pb::EncryptedData encrypted; + KeyParams old_key = {"localhost", "dummy", kOldKey}; + KeyParams cur_key = {"localhost", "dummy", kCurKey}; + KeyParams keystore_key = {"localhost", "dummy", kKeystoreKey}; + GetCryptographer()->AddKey(old_key); + GetCryptographer()->AddKey(cur_key); + + Cryptographer other_cryptographer(GetCryptographer()->encryptor()); + other_cryptographer.AddKey(old_key); + EXPECT_TRUE(other_cryptographer.is_ready()); + + EXPECT_CALL(*observer(), + OnCryptographerStateChanged(_)); + EXPECT_CALL(*observer(), + OnEncryptedTypesChanged(_, false)); + EXPECT_CALL(*observer(), + OnEncryptionComplete()); + encryption_handler()->Init(); + EXPECT_TRUE(GetCryptographer()->is_ready()); + EXPECT_FALSE(encryption_handler()->EncryptEverythingEnabled()); + + { + EXPECT_CALL(*observer(), + OnBootstrapTokenUpdated(_, KEYSTORE_BOOTSTRAP_TOKEN)); + ReadTransaction trans(FROM_HERE, user_share()); + encryption_handler()->SetKeystoreKey(kKeystoreKey, trans.GetWrappedTrans()); + } + EXPECT_CALL(*observer(), + OnPassphraseTypeChanged(KEYSTORE_PASSPHRASE)); + PumpLoop(); + Mock::VerifyAndClearExpectations(observer()); + EXPECT_TRUE(encryption_handler()->MigratedToKeystore()); + EXPECT_EQ(encryption_handler()->GetPassphraseType(), KEYSTORE_PASSPHRASE); + VerifyMigratedNigori(KEYSTORE_PASSPHRASE, kCurKey); + + // Now build an old keystore passphrase nigori node. + EXPECT_CALL(*observer(), + OnCryptographerStateChanged(_)); + { + WriteTransaction trans(FROM_HERE, user_share()); + WriteNode nigori_node(&trans); + ASSERT_EQ(nigori_node.InitByTagLookup(kNigoriTag), BaseNode::INIT_OK); + sync_pb::NigoriSpecifics nigori; + Cryptographer other_cryptographer(GetCryptographer()->encryptor()); + other_cryptographer.AddKey(old_key); + encryption_handler()->GetKeystoreDecryptor( + other_cryptographer, + kKeystoreKey, + nigori.mutable_keystore_decryptor_token()); + other_cryptographer.GetKeys(nigori.mutable_encryption_keybag()); + nigori.set_keybag_is_frozen(true); + nigori.set_encrypt_everything(false); + nigori.set_passphrase_type(sync_pb::NigoriSpecifics::KEYSTORE_PASSPHRASE); + nigori.set_keystore_migration_time(1); + encryption_handler()->ApplyNigoriUpdate(nigori, trans.GetWrappedTrans()); + nigori_node.SetNigoriSpecifics(nigori); + } + PumpLoop(); + + // Verify we're still migrated and have proper encryption state. + EXPECT_TRUE(encryption_handler()->MigratedToKeystore()); + EXPECT_TRUE(GetCryptographer()->is_ready()); + EXPECT_EQ(encryption_handler()->GetPassphraseType(), KEYSTORE_PASSPHRASE); + EXPECT_FALSE(encryption_handler()->EncryptEverythingEnabled()); + VerifyMigratedNigori(KEYSTORE_PASSPHRASE, kCurKey); +} + } // namespace syncer diff --git a/sync/internal_api/test/test_entry_factory.cc b/sync/internal_api/test/test_entry_factory.cc index 512fe55..3fa0d67 100644 --- a/sync/internal_api/test/test_entry_factory.cc +++ b/sync/internal_api/test/test_entry_factory.cc @@ -5,7 +5,9 @@ #include "sync/internal_api/public/test/test_entry_factory.h" #include "sync/syncable/directory.h" +#include "sync/syncable/entry.h" #include "sync/syncable/mutable_entry.h" +#include "sync/syncable/read_transaction.h" #include "sync/syncable/syncable_id.h" #include "sync/syncable/write_transaction.h" #include "sync/test/engine/test_id_factory.h" @@ -42,10 +44,10 @@ void TestEntryFactory::CreateUnappliedNewItemWithParent( entry.Put(syncable::SERVER_SPECIFICS, specifics); } -void TestEntryFactory::CreateUnappliedNewItem( - const string& item_id, - const sync_pb::EntitySpecifics& specifics, - bool is_unique) { +int64 TestEntryFactory::CreateUnappliedNewItem( + const string& item_id, + const sync_pb::EntitySpecifics& specifics, + bool is_unique) { WriteTransaction trans(FROM_HERE, UNITTEST, directory_); MutableEntry entry(&trans, syncable::CREATE_NEW_UPDATE_ITEM, Id::CreateFromServerId(item_id)); @@ -58,6 +60,7 @@ void TestEntryFactory::CreateUnappliedNewItem( entry.Put(syncable::SERVER_SPECIFICS, specifics); if (is_unique) // For top-level nodes. entry.Put(syncable::UNIQUE_SERVER_TAG, item_id); + return entry.Get(syncable::META_HANDLE); } void TestEntryFactory::CreateUnsyncedItem( @@ -159,6 +162,68 @@ int64 TestEntryFactory::CreateSyncedItem( return entry.Get(syncable::META_HANDLE); } +bool TestEntryFactory::SetServerSpecificsForItem( + int64 meta_handle, + const sync_pb::EntitySpecifics specifics) { + WriteTransaction trans(FROM_HERE, UNITTEST, directory_); + MutableEntry entry(&trans, syncable::GET_BY_HANDLE, meta_handle); + if (!entry.good()) { + return false; + } + entry.Put(syncable::SERVER_SPECIFICS, specifics); + entry.Put(syncable::IS_UNAPPLIED_UPDATE, true); + return true; +} + +bool TestEntryFactory::SetLocalSpecificsForItem( + int64 meta_handle, + const sync_pb::EntitySpecifics specifics) { + WriteTransaction trans(FROM_HERE, UNITTEST, directory_); + MutableEntry entry(&trans, syncable::GET_BY_HANDLE, meta_handle); + if (!entry.good()) { + return false; + } + entry.Put(syncable::SPECIFICS, specifics); + entry.Put(syncable::IS_UNSYNCED, true); + return true; +} + +const sync_pb::EntitySpecifics& TestEntryFactory::GetServerSpecificsForItem( + int64 meta_handle) const { + syncable::ReadTransaction trans(FROM_HERE, directory_); + syncable::Entry entry(&trans, syncable::GET_BY_HANDLE, meta_handle); + DCHECK(entry.good()); + return entry.Get(syncable::SERVER_SPECIFICS); +} + +const sync_pb::EntitySpecifics& TestEntryFactory::GetLocalSpecificsForItem( + int64 meta_handle) const { + syncable::ReadTransaction trans(FROM_HERE, directory_); + syncable::Entry entry(&trans, syncable::GET_BY_HANDLE, meta_handle); + DCHECK(entry.good()); + return entry.Get(syncable::SPECIFICS); +} + +bool TestEntryFactory::GetIsUnsyncedForItem(int64 meta_handle) const { + syncable::ReadTransaction trans(FROM_HERE, directory_); + syncable::Entry entry(&trans, syncable::GET_BY_HANDLE, meta_handle); + if (!entry.good()) { + NOTREACHED(); + return false; + } + return entry.Get(syncable::IS_UNSYNCED); +} + +bool TestEntryFactory::GetIsUnappliedForItem(int64 meta_handle) const { + syncable::ReadTransaction trans(FROM_HERE, directory_); + syncable::Entry entry(&trans, syncable::GET_BY_HANDLE, meta_handle); + if (!entry.good()) { + NOTREACHED(); + return false; + } + return entry.Get(syncable::IS_UNAPPLIED_UPDATE); +} + int64 TestEntryFactory::GetNextRevision() { return next_revision_++; } |