Commit 8d0bfba2 authored by Mikel Astiz's avatar Mikel Astiz Committed by Commit Bot

Trivial fixes to Nigori USS codepath

One DCHECK is simply wrong (the Nigori sync entity is a folder) and the
bridge was lacking a call to ModelReadyToSync().

Bug: 922900
Change-Id: I529592e90aaae2ef12ca2fdde08984c0ddf835e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1620627
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Auto-Submit: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: default avatarMohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662627}
parent 8746b3a4
...@@ -322,7 +322,7 @@ void NigoriModelTypeProcessor::Put(std::unique_ptr<EntityData> entity_data) { ...@@ -322,7 +322,7 @@ void NigoriModelTypeProcessor::Put(std::unique_ptr<EntityData> entity_data) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
DCHECK(entity_data); DCHECK(entity_data);
DCHECK(!entity_data->is_deleted()); DCHECK(!entity_data->is_deleted());
DCHECK(!entity_data->is_folder); DCHECK(entity_data->is_folder);
DCHECK(!entity_data->non_unique_name.empty()); DCHECK(!entity_data->non_unique_name.empty());
DCHECK(!entity_data->specifics.has_encrypted()); DCHECK(!entity_data->specifics.has_encrypted());
DCHECK_EQ(NIGORI, GetModelTypeFromSpecifics(entity_data->specifics)); DCHECK_EQ(NIGORI, GetModelTypeFromSpecifics(entity_data->specifics));
......
...@@ -57,6 +57,7 @@ std::unique_ptr<syncer::UpdateResponseData> CreateDummyNigoriUpdateResponseData( ...@@ -57,6 +57,7 @@ std::unique_ptr<syncer::UpdateResponseData> CreateDummyNigoriUpdateResponseData(
const std::string keystore_decryptor_token_key_name, const std::string keystore_decryptor_token_key_name,
int response_version) { int response_version) {
auto entity_data = std::make_unique<syncer::EntityData>(); auto entity_data = std::make_unique<syncer::EntityData>();
entity_data->is_folder = true;
entity_data->id = kNigoriServerId; entity_data->id = kNigoriServerId;
sync_pb::NigoriSpecifics* nigori_specifics = sync_pb::NigoriSpecifics* nigori_specifics =
entity_data->specifics.mutable_nigori(); entity_data->specifics.mutable_nigori();
...@@ -195,6 +196,7 @@ TEST_F(NigoriModelTypeProcessorTest, ShouldIncrementSequenceNumberWhenPut) { ...@@ -195,6 +196,7 @@ TEST_F(NigoriModelTypeProcessorTest, ShouldIncrementSequenceNumberWhenPut) {
auto entity_data = std::make_unique<syncer::EntityData>(); auto entity_data = std::make_unique<syncer::EntityData>();
entity_data->specifics.mutable_nigori(); entity_data->specifics.mutable_nigori();
entity_data->non_unique_name = kNigoriNonUniqueName; entity_data->non_unique_name = kNigoriNonUniqueName;
entity_data->is_folder = true;
processor()->Put(std::move(entity_data)); processor()->Put(std::move(entity_data));
...@@ -221,6 +223,7 @@ TEST_F(NigoriModelTypeProcessorTest, ShouldGetLocalChangesWhenPut) { ...@@ -221,6 +223,7 @@ TEST_F(NigoriModelTypeProcessorTest, ShouldGetLocalChangesWhenPut) {
auto entity_data = std::make_unique<syncer::EntityData>(); auto entity_data = std::make_unique<syncer::EntityData>();
entity_data->specifics.mutable_nigori(); entity_data->specifics.mutable_nigori();
entity_data->non_unique_name = kNigoriNonUniqueName; entity_data->non_unique_name = kNigoriNonUniqueName;
entity_data->is_folder = true;
processor()->Put(std::move(entity_data)); processor()->Put(std::move(entity_data));
CommitRequestDataList commit_request; CommitRequestDataList commit_request;
...@@ -238,6 +241,7 @@ TEST_F(NigoriModelTypeProcessorTest, ...@@ -238,6 +241,7 @@ TEST_F(NigoriModelTypeProcessorTest,
auto entity_data = std::make_unique<syncer::EntityData>(); auto entity_data = std::make_unique<syncer::EntityData>();
entity_data->specifics.mutable_nigori(); entity_data->specifics.mutable_nigori();
entity_data->non_unique_name = kNigoriNonUniqueName; entity_data->non_unique_name = kNigoriNonUniqueName;
entity_data->is_folder = true;
processor()->Put(std::move(entity_data)); processor()->Put(std::move(entity_data));
CommitRequestDataList commit_request_list; CommitRequestDataList commit_request_list;
...@@ -273,6 +277,7 @@ TEST_F(NigoriModelTypeProcessorTest, ...@@ -273,6 +277,7 @@ TEST_F(NigoriModelTypeProcessorTest,
auto entity_data = std::make_unique<syncer::EntityData>(); auto entity_data = std::make_unique<syncer::EntityData>();
entity_data->specifics.mutable_nigori(); entity_data->specifics.mutable_nigori();
entity_data->non_unique_name = kNigoriNonUniqueName; entity_data->non_unique_name = kNigoriNonUniqueName;
entity_data->is_folder = true;
processor()->Put(std::move(entity_data)); processor()->Put(std::move(entity_data));
CommitRequestDataList commit_request_list; CommitRequestDataList commit_request_list;
...@@ -292,6 +297,7 @@ TEST_F(NigoriModelTypeProcessorTest, ...@@ -292,6 +297,7 @@ TEST_F(NigoriModelTypeProcessorTest,
auto entity_data = std::make_unique<syncer::EntityData>(); auto entity_data = std::make_unique<syncer::EntityData>();
entity_data->specifics.mutable_nigori(); entity_data->specifics.mutable_nigori();
entity_data->non_unique_name = kNigoriNonUniqueName; entity_data->non_unique_name = kNigoriNonUniqueName;
entity_data->is_folder = true;
return entity_data; return entity_data;
}); });
...@@ -312,6 +318,7 @@ TEST_F(NigoriModelTypeProcessorTest, ...@@ -312,6 +318,7 @@ TEST_F(NigoriModelTypeProcessorTest,
entity_data->specifics.mutable_nigori(); entity_data->specifics.mutable_nigori();
nigori_specifics->set_encrypt_bookmarks(true); nigori_specifics->set_encrypt_bookmarks(true);
entity_data->non_unique_name = kNigoriNonUniqueName; entity_data->non_unique_name = kNigoriNonUniqueName;
entity_data->is_folder = true;
processor()->Put(std::move(entity_data)); processor()->Put(std::move(entity_data));
CommitRequestDataList commit_request_list; CommitRequestDataList commit_request_list;
...@@ -331,6 +338,7 @@ TEST_F(NigoriModelTypeProcessorTest, ...@@ -331,6 +338,7 @@ TEST_F(NigoriModelTypeProcessorTest,
entity_data = std::make_unique<syncer::EntityData>(); entity_data = std::make_unique<syncer::EntityData>();
nigori_specifics = entity_data->specifics.mutable_nigori(); nigori_specifics = entity_data->specifics.mutable_nigori();
entity_data->non_unique_name = kNigoriNonUniqueName; entity_data->non_unique_name = kNigoriNonUniqueName;
entity_data->is_folder = true;
nigori_specifics->set_encrypt_preferences(true); nigori_specifics->set_encrypt_preferences(true);
processor()->Put(std::move(entity_data)); processor()->Put(std::move(entity_data));
...@@ -355,6 +363,7 @@ TEST_F(NigoriModelTypeProcessorTest, ...@@ -355,6 +363,7 @@ TEST_F(NigoriModelTypeProcessorTest,
auto entity_data = std::make_unique<syncer::EntityData>(); auto entity_data = std::make_unique<syncer::EntityData>();
entity_data->specifics.mutable_nigori(); entity_data->specifics.mutable_nigori();
entity_data->non_unique_name = kNigoriNonUniqueName; entity_data->non_unique_name = kNigoriNonUniqueName;
entity_data->is_folder = true;
processor()->Put(std::move(entity_data)); processor()->Put(std::move(entity_data));
...@@ -369,6 +378,7 @@ TEST_F(NigoriModelTypeProcessorTest, ShouldNudgeForCommitUponPutIfReadyToSync) { ...@@ -369,6 +378,7 @@ TEST_F(NigoriModelTypeProcessorTest, ShouldNudgeForCommitUponPutIfReadyToSync) {
auto entity_data = std::make_unique<syncer::EntityData>(); auto entity_data = std::make_unique<syncer::EntityData>();
entity_data->specifics.mutable_nigori(); entity_data->specifics.mutable_nigori();
entity_data->non_unique_name = kNigoriNonUniqueName; entity_data->non_unique_name = kNigoriNonUniqueName;
entity_data->is_folder = true;
EXPECT_CALL(*mock_commit_queue(), NudgeForCommit()); EXPECT_CALL(*mock_commit_queue(), NudgeForCommit());
processor()->Put(std::move(entity_data)); processor()->Put(std::move(entity_data));
......
...@@ -218,7 +218,9 @@ NigoriSyncBridgeImpl::NigoriSyncBridgeImpl( ...@@ -218,7 +218,9 @@ NigoriSyncBridgeImpl::NigoriSyncBridgeImpl(
: processor_(std::move(processor)), : processor_(std::move(processor)),
cryptographer_(encryptor), cryptographer_(encryptor),
passphrase_type_(NigoriSpecifics::UNKNOWN), passphrase_type_(NigoriSpecifics::UNKNOWN),
encrypt_everything_(false) {} encrypt_everything_(false) {
processor_->ModelReadyToSync(this, NigoriMetadataBatch());
}
NigoriSyncBridgeImpl::~NigoriSyncBridgeImpl() { NigoriSyncBridgeImpl::~NigoriSyncBridgeImpl() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
......
...@@ -55,6 +55,28 @@ MATCHER(HasKeystoreNigori, "") { ...@@ -55,6 +55,28 @@ MATCHER(HasKeystoreNigori, "") {
specifics.has_keystore_migration_time(); specifics.has_keystore_migration_time();
} }
MATCHER_P(CanDecryptWith, key_params, "") {
const Cryptographer& cryptographer = arg;
Nigori nigori;
nigori.InitByDerivation(key_params.derivation_params, key_params.password);
std::string nigori_name;
EXPECT_TRUE(
nigori.Permute(Nigori::Type::Password, kNigoriKeyName, &nigori_name));
const std::string unencrypted = "test";
sync_pb::EncryptedData encrypted;
encrypted.set_key_name(nigori_name);
EXPECT_TRUE(nigori.Encrypt(unencrypted, encrypted.mutable_blob()));
if (!cryptographer.CanDecrypt(encrypted)) {
return false;
}
std::string decrypted;
if (!cryptographer.DecryptToString(encrypted, &decrypted)) {
return false;
}
return decrypted == unencrypted;
}
KeyParams Pbkdf2KeyParams(std::string key) { KeyParams Pbkdf2KeyParams(std::string key) {
return {KeyDerivationParams::CreateForPbkdf2(), std::move(key)}; return {KeyDerivationParams::CreateForPbkdf2(), std::move(key)};
} }
...@@ -176,28 +198,6 @@ class NigoriSyncBridgeImplTest : public testing::Test { ...@@ -176,28 +198,6 @@ class NigoriSyncBridgeImplTest : public testing::Test {
testing::NiceMock<MockObserver> observer_; testing::NiceMock<MockObserver> observer_;
}; };
MATCHER_P(CanDecryptWith, key_params, "") {
const Cryptographer& cryptographer = arg;
Nigori nigori;
nigori.InitByDerivation(key_params.derivation_params, key_params.password);
std::string nigori_name;
EXPECT_TRUE(
nigori.Permute(Nigori::Type::Password, kNigoriKeyName, &nigori_name));
const std::string unencrypted = "test";
sync_pb::EncryptedData encrypted;
encrypted.set_key_name(nigori_name);
EXPECT_TRUE(nigori.Encrypt(unencrypted, encrypted.mutable_blob()));
if (!cryptographer.CanDecrypt(encrypted)) {
return false;
}
std::string decrypted;
if (!cryptographer.DecryptToString(encrypted, &decrypted)) {
return false;
}
return decrypted == unencrypted;
}
// Simplest case of keystore Nigori: we have only one keystore key and no old // Simplest case of keystore Nigori: we have only one keystore key and no old
// keys. This keystore key is encrypted in both encryption_keybag and // keys. This keystore key is encrypted in both encryption_keybag and
// keystore_decryptor_token. Client receives such Nigori if initialization of // keystore_decryptor_token. Client receives such Nigori if initialization of
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment