Commit 847553ad authored by Maksim Moskvitin's avatar Maksim Moskvitin Committed by Commit Bot

[Sync] Verify passphrase type before triggering keystore initialization

Passphrase type is an important setting, that should be not downgraded
even in case of data corruption. This patch changes the behavior of
NigoriSyncBridgeImpl in a way it checks not only |encryption_keybag|
emptiness, but also that |passphrase_type| is the default one (i.e.
IMPLICIT_PASSPHRASE).

Bug: 1141410
Change-Id: I1329fc3cdb4d89b73b8620db8bf389d7dd38e00e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2494810Reviewed-by: default avatarMikel Astiz <mastiz@chromium.org>
Commit-Queue: Maksim Moskvitin <mmoskvitin@google.com>
Auto-Submit: Maksim Moskvitin <mmoskvitin@google.com>
Cr-Commit-Position: refs/heads/master@{#820299}
parent d9915169
......@@ -728,7 +728,9 @@ base::Optional<ModelError> NigoriSyncBridgeImpl::MergeSyncData(
}
DCHECK(data->specifics.has_nigori());
if (!data->specifics.nigori().encryption_keybag().blob().empty()) {
const NigoriSpecifics& specifics = data->specifics.nigori();
if (specifics.passphrase_type() != NigoriSpecifics::IMPLICIT_PASSPHRASE ||
!specifics.encryption_keybag().blob().empty()) {
// We received regular Nigori.
return UpdateLocalState(data->specifics.nigori());
}
......
......@@ -615,6 +615,27 @@ TEST_F(NigoriSyncBridgeImplTest,
EXPECT_THAT(cryptographer, HasDefaultKeyDerivedFrom(kKeystoreKeyParams));
}
// Tests that upon receiving Nigori corrupted due to absence of
// |encryption_keybag|, bridge respect its passphrase type and doesn't attempt
// to trigger keystore initialization.
TEST_F(NigoriSyncBridgeImplTest,
ShouldNotTriggerKeystoreInitializationForCorruptedCustomPassphrase) {
const KeyParams kKeystoreKeyParams = KeystoreKeyParams(kRawKeystoreKey);
EntityData entity_data;
*entity_data.specifics.mutable_nigori() =
sync_pb::NigoriSpecifics::default_instance();
entity_data.specifics.mutable_nigori()->set_passphrase_type(
sync_pb::NigoriSpecifics::CUSTOM_PASSPHRASE);
EXPECT_TRUE(bridge()->SetKeystoreKeys({kRawKeystoreKey}));
// There should be no commits.
EXPECT_CALL(*processor(), Put(_)).Times(0);
// Model error should be reported, because there is no |encryption_keybag|.
EXPECT_THAT(bridge()->MergeSyncData(std::move(entity_data)),
Ne(base::nullopt));
}
TEST_F(NigoriSyncBridgeImplTest, ShouldRotateKeystoreKey) {
const std::vector<uint8_t> kRawKeystoreKey1{kRawKeystoreKey};
const KeyParams kKeystoreKeyParams1 = KeystoreKeyParams(kRawKeystoreKey1);
......
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