Commit 628a21af authored by Andy Paicu's avatar Andy Paicu Committed by Commit Bot

Revert "Add test for keystore key rotation"

This reverts commit b9c8d354.

Reason for revert: SingleClientNigoriSyncTestWithUssTests.ShouldRotateKeystoreKey fails consistently on various builders

First builds
* linux-chromeos-google-rel:
https://ci.chromium.org/p/chrome/builders/ci/linux-chromeos-google-rel/8654
* Win 7 Tests x64 (1):
https://ci.chromium.org/p/chromium/builders/ci/Win%207%20Tests%20x64%20%281%29/60958
* linux-chromeos-rel:
https://ci.chromium.org/p/chromium/builders/ci/linux-chromeos-rel/30949
* Linux TSan Tests:
https://ci.chromium.org/p/chromium/builders/ci/Linux%20TSan%20Tests/46542


Original change's description:
> Add test for keystore key rotation
> 
> New test mimics server-side behavior for triggering keystore key
> rotation: generation of new keystore key with touching the Nigori
> node. After that client should receive updated keystore keys and the
> same Nigori node as it already has (modulo metadata update). The client
> should commit the new Nigori node with rotated keystore key and this
> verified by checking |encryption_keybag| encryption key name.
> 
> Bug: 922900
> Change-Id: I91b86bbb0f68a55fdcc10049fa90afa70dc32155
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1887611
> Reviewed-by: Mikel Astiz <mastiz@chromium.org>
> Commit-Queue: Maksim Moskvitin <mmoskvitin@google.com>
> Auto-Submit: Maksim Moskvitin <mmoskvitin@google.com>
> Cr-Commit-Position: refs/heads/master@{#712502}

TBR=mastiz@chromium.org,mmoskvitin@google.com

Change-Id: Iad001f70868944270cfbc54f77a5e68f852dc019
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 922900
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1899495Reviewed-by: default avatarAndy Paicu <andypaicu@chromium.org>
Commit-Queue: Andy Paicu <andypaicu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712521}
parent 30dd4a76
......@@ -161,28 +161,6 @@ bool ServerNigoriChecker::IsExitConditionSatisfied(std::ostream* os) {
expected_passphrase_type_;
}
ServerNigoriKeyNameChecker::ServerNigoriKeyNameChecker(
const std::string& expected_key_name,
syncer::ProfileSyncService* service,
fake_server::FakeServer* fake_server)
: SingleClientStatusChangeChecker(service),
fake_server_(fake_server),
expected_key_name_(expected_key_name) {}
bool ServerNigoriKeyNameChecker::IsExitConditionSatisfied(std::ostream* os) {
std::vector<sync_pb::SyncEntity> nigori_entities =
fake_server_->GetPermanentSyncEntitiesByModelType(syncer::NIGORI);
DCHECK_EQ(nigori_entities.size(), 1U);
const std::string given_key_name =
nigori_entities[0].specifics().nigori().encryption_keybag().key_name();
*os << "Waiting for a Nigori node with proper key bag encryption key name ("
<< expected_key_name_ << ") to become available on the server."
<< "The server key bag encryption key name is " << given_key_name << ".";
return given_key_name == expected_key_name_;
}
PassphraseRequiredStateChecker::PassphraseRequiredStateChecker(
syncer::ProfileSyncService* service,
bool desired_state)
......
......@@ -64,21 +64,6 @@ class ServerNigoriChecker : public SingleClientStatusChangeChecker {
const syncer::PassphraseType expected_passphrase_type_;
};
// Checker used to block until a Nigori with a given keybag encryption key name
// is available on the server.
class ServerNigoriKeyNameChecker : public SingleClientStatusChangeChecker {
public:
ServerNigoriKeyNameChecker(const std::string& expected_key_name,
syncer::ProfileSyncService* service,
fake_server::FakeServer* fake_server);
bool IsExitConditionSatisfied(std::ostream* os) override;
private:
fake_server::FakeServer* const fake_server_;
const std::string expected_key_name_;
};
// Checker used to block until Sync requires or stops requiring a passphrase.
class PassphraseRequiredStateChecker : public SingleClientStatusChangeChecker {
public:
......
......@@ -69,14 +69,6 @@ KeyParams KeystoreKeyParams(const std::string& key) {
std::move(encoded_key)};
}
std::string ComputeKeyName(const KeyParams& key_params) {
std::string key_name;
syncer::Nigori::CreateByDerivation(key_params.derivation_params,
key_params.password)
->Permute(syncer::Nigori::Password, syncer::kNigoriKeyName, &key_name);
return key_name;
}
// Builds NigoriSpecifics with following fields:
// 1. encryption_keybag contains all keys derived from |keybag_keys_params|
// and encrypted with a key derived from |keybag_decryptor_params|.
......@@ -336,22 +328,6 @@ IN_PROC_BROWSER_TEST_P(SingleClientNigoriSyncTestWithUssTests,
EXPECT_TRUE(WaitForPasswordForms({password_form}));
}
IN_PROC_BROWSER_TEST_P(SingleClientNigoriSyncTestWithUssTests,
ShouldRotateKeystoreKey) {
ASSERT_TRUE(SetupSync());
GetFakeServer()->TriggerKeystoreKeyRotation();
const std::vector<std::string>& keystore_keys =
GetFakeServer()->GetKeystoreKeys();
ASSERT_THAT(keystore_keys, SizeIs(2));
const KeyParams new_keystore_key_params = KeystoreKeyParams(keystore_keys[1]);
const std::string expected_key_bag_key_name =
ComputeKeyName(new_keystore_key_params);
EXPECT_TRUE(ServerNigoriKeyNameChecker(expected_key_bag_key_name,
GetSyncService(0), GetFakeServer())
.Wait());
}
IN_PROC_BROWSER_TEST_P(SingleClientNigoriSyncTestWithUssTests,
ShouldExposeExperimentalAuthenticationKey) {
const std::vector<std::string>& keystore_keys =
......
......@@ -374,10 +374,6 @@ void LoopbackServer::EnableStrongConsistencyWithConflictDetectionModel() {
strong_consistency_model_enabled_ = true;
}
void LoopbackServer::AddNewKeystoreKeyForTesting() {
keystore_keys_.push_back(GenerateNewKeystoreKey());
}
bool LoopbackServer::HandleGetUpdatesRequest(
const sync_pb::GetUpdatesMessage& get_updates,
const std::string& store_birthday,
......
......@@ -78,8 +78,6 @@ class LoopbackServer {
return keystore_keys_;
}
void AddNewKeystoreKeyForTesting();
private:
// Allow the FakeServer decorator to inspect the internals of this class.
friend class fake_server::FakeServer;
......
......@@ -364,19 +364,6 @@ const std::vector<std::string>& FakeServer::GetKeystoreKeys() const {
return loopback_server_->GetKeystoreKeysForTesting();
}
void FakeServer::TriggerKeystoreKeyRotation() {
DCHECK(thread_checker_.CalledOnValidThread());
loopback_server_->AddNewKeystoreKeyForTesting();
std::vector<sync_pb::SyncEntity> nigori_entities =
loopback_server_->GetPermanentSyncEntitiesByModelType(syncer::NIGORI);
DCHECK_EQ(nigori_entities.size(), 1U);
DCHECK(ModifyEntitySpecifics(
loopback_server_->GetTopLevelPermanentItemId(syncer::NIGORI),
nigori_entities[0].specifics()));
}
void FakeServer::InjectEntity(std::unique_ptr<LoopbackServerEntity> entity) {
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK(entity->GetModelType() != syncer::AUTOFILL_WALLET_DATA)
......
......@@ -100,10 +100,6 @@ class FakeServer : public syncer::LoopbackServer::ObserverForTests {
// Returns all keystore keys from the server.
const std::vector<std::string>& GetKeystoreKeys() const;
// Triggers the keystore key rotation events on the server side: generating
// new keystore key and touching the Nigori node.
void TriggerKeystoreKeyRotation();
// Adds |entity| to the server's collection of entities. This method makes no
// guarantees that the added entity will result in successful server
// operations.
......
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