Commit 2a878884 authored by Maksim Moskvitin's avatar Maksim Moskvitin Committed by Commit Bot

[Sync:USS] Persist explicit passphrase key in prefs

Once explicit passphrase is known by the client, it should be persisted
somewhere to not ask user to provide it on every Chrome startup. While
for USS implementation we're going to use different way for key
persistence, we still store it in preferences, because:

1. It allows to start experiments without complete implementation of
new persistence approach.

2. It allows to disable USS Nigori implementation and continue using
Directory implementation without asking user to enter passphrase again.

Bug: 922900
Change-Id: I2295872752b23901c7f0915b1d156e28dc54d961
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1709326
Commit-Queue: Maksim Moskvitin <mmoskvitin@google.com>
Reviewed-by: default avatarMikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680048}
parent 6c7fa9ea
......@@ -348,8 +348,8 @@ void SyncEngineBackend::DoInitialize(SyncEngine::InitParams params) {
nigori_controller_ = std::make_unique<ModelTypeController>(
NIGORI, std::make_unique<ForwardingModelTypeControllerDelegate>(
nigori_processor->GetControllerDelegate().get()));
sync_encryption_handler_ =
std::make_unique<NigoriSyncBridgeImpl>(std::move(nigori_processor));
sync_encryption_handler_ = std::make_unique<NigoriSyncBridgeImpl>(
std::move(nigori_processor), &encryptor_);
} else {
sync_encryption_handler_ = std::make_unique<SyncEncryptionHandlerImpl>(
&user_share_, &encryptor_, params.restored_key_for_bootstrapping,
......
......@@ -180,7 +180,8 @@ class Cryptographer {
std::string GetDefaultNigoriKeyName() const;
// Returns a serialized sync_pb::NigoriKey version of current default
// encryption key.
// encryption key. Returns empty string if Cryptographer is not initialized
// or protobuf serialization error occurs.
std::string GetDefaultNigoriKeyData() const;
// Generates a new Nigori from |serialized_nigori_key|, and if successful
......
......@@ -8,6 +8,7 @@
#include "base/base64.h"
#include "base/location.h"
#include "components/sync/base/encryptor.h"
#include "components/sync/base/passphrase_enums.h"
#include "components/sync/base/sync_base_switches.h"
#include "components/sync/base/time.h"
......@@ -337,13 +338,39 @@ void UpdateNigoriSpecificsFromEncryptedTypes(
specifics->set_encrypt_web_apps(encrypted_types.Has(WEB_APPS));
}
// Packs explicit passphrase key in order to persist it. Should be aligned with
// Directory implementation (Cryptographer::GetBootstrapToken()) unless it is
// removed. Returns empty string in case of errors.
std::string PackExplicitPassphraseKey(const Encryptor& encryptor,
const Cryptographer& cryptographer) {
// Explicit passphrase key should always be default one.
std::string serialized_key = cryptographer.GetDefaultNigoriKeyData();
if (serialized_key.empty()) {
DLOG(ERROR) << "Failed to serialize explicit passphrase key.";
return std::string();
}
std::string encrypted_key;
if (!encryptor.EncryptString(serialized_key, &encrypted_key)) {
DLOG(ERROR) << "Failed to encrypt explicit passphrase key.";
return std::string();
}
std::string encoded_key;
base::Base64Encode(encrypted_key, &encoded_key);
return encoded_key;
}
} // namespace
NigoriSyncBridgeImpl::NigoriSyncBridgeImpl(
std::unique_ptr<NigoriLocalChangeProcessor> processor)
: processor_(std::move(processor)),
std::unique_ptr<NigoriLocalChangeProcessor> processor,
const Encryptor* encryptor)
: encryptor_(encryptor),
processor_(std::move(processor)),
passphrase_type_(NigoriSpecifics::UNKNOWN),
encrypt_everything_(false) {
DCHECK(encryptor);
processor_->ModelReadyToSync(this, NigoriMetadataBatch());
}
......@@ -427,9 +454,9 @@ void NigoriSyncBridgeImpl::SetEncryptionPassphrase(
observer.OnEncryptedTypesChanged(EncryptableUserTypes(),
encrypt_everything_);
}
MaybeNotifyBootstrapTokenUpdated();
// OnLocalSetPassphraseEncryption() is intentionally not called here, because
// it's needed only for the Directory implementation unit tests.
// TODO(crbug.com/922900): persist |passphrase| in corresponding storage.
// TODO(crbug.com/922900): support SCRYPT key derivation method.
NOTIMPLEMENTED();
}
......@@ -437,8 +464,8 @@ void NigoriSyncBridgeImpl::SetEncryptionPassphrase(
void NigoriSyncBridgeImpl::SetDecryptionPassphrase(
const std::string& passphrase) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// |passphrase| should be a valid one already (verified by the UI part, using
// pending keys exposed by OnPassphraseRequired()).
// |passphrase| should be a valid one already (verified by SyncServiceCrypto,
// using pending keys exposed by OnPassphraseRequired()).
DCHECK(!passphrase.empty());
DCHECK(cryptographer_.has_pending_keys());
KeyParams key_params = {GetKeyDerivationParamsForPendingKeys(), passphrase};
......@@ -466,7 +493,7 @@ void NigoriSyncBridgeImpl::SetDecryptionPassphrase(
for (auto& observer : observers_) {
observer.OnPassphraseAccepted();
}
// TODO(crbug.com/922900): persist |passphrase| in corresponding storage.
MaybeNotifyBootstrapTokenUpdated();
// TODO(crbug.com/922900): we may need to rewrite encryption_keybag in Nigori
// node in case we have some keys in |cryptographer_| which is not stored in
// encryption_keybag yet.
......@@ -831,4 +858,29 @@ KeyDerivationParams NigoriSyncBridgeImpl::GetKeyDerivationParamsForPendingKeys()
}
}
void NigoriSyncBridgeImpl::MaybeNotifyBootstrapTokenUpdated() const {
switch (passphrase_type_) {
case NigoriSpecifics::UNKNOWN:
case NigoriSpecifics::IMPLICIT_PASSPHRASE:
NOTREACHED();
return;
case NigoriSpecifics::KEYSTORE_PASSPHRASE:
// TODO(crbug.com/922900): notify about keystore bootstrap token updates.
NOTIMPLEMENTED();
return;
case NigoriSpecifics::FROZEN_IMPLICIT_PASSPHRASE:
case NigoriSpecifics::CUSTOM_PASSPHRASE:
// |packed_custom_passphrase_key| will be empty in case serialization or
// encryption error occurs.
std::string packed_custom_passphrase_key =
PackExplicitPassphraseKey(*encryptor_, cryptographer_);
if (!packed_custom_passphrase_key.empty()) {
for (auto& observer : observers_) {
observer.OnBootstrapTokenUpdated(packed_custom_passphrase_key,
PASSPHRASE_BOOTSTRAP_TOKEN);
}
}
}
}
} // namespace syncer
......@@ -24,6 +24,8 @@
namespace syncer {
class Encryptor;
// USS implementation of SyncEncryptionHandler.
// This class holds the current Nigori state and processes incoming changes and
// queries:
......@@ -36,8 +38,9 @@ class NigoriSyncBridgeImpl : public KeystoreKeysHandler,
public NigoriSyncBridge,
public SyncEncryptionHandler {
public:
explicit NigoriSyncBridgeImpl(
std::unique_ptr<NigoriLocalChangeProcessor> processor);
// |encryptor| must be not null and must outlive this object.
NigoriSyncBridgeImpl(std::unique_ptr<NigoriLocalChangeProcessor> processor,
const Encryptor* encryptor);
~NigoriSyncBridgeImpl() override;
// SyncEncryptionHandler implementation.
......@@ -90,6 +93,13 @@ class NigoriSyncBridgeImpl : public KeystoreKeysHandler,
// |passphrase_type_| is an explicit passphrase.
KeyDerivationParams GetKeyDerivationParamsForPendingKeys() const;
// Persists Nigori derived from explicit passphrase into preferences, in case
// error occurs during serialization/encryption, corresponding preference
// just won't be updated.
void MaybeNotifyBootstrapTokenUpdated() const;
const Encryptor* const encryptor_;
const std::unique_ptr<NigoriLocalChangeProcessor> processor_;
// Base64 encoded keystore keys. The last element is the current keystore
......
......@@ -7,6 +7,7 @@
#include <utility>
#include "base/base64.h"
#include "components/sync/base/fake_encryptor.h"
#include "components/sync/base/time.h"
#include "components/sync/model/entity_data.h"
#include "testing/gmock/include/gmock/gmock.h"
......@@ -152,7 +153,8 @@ class NigoriSyncBridgeImplTest : public testing::Test {
auto processor =
std::make_unique<testing::NiceMock<MockNigoriLocalChangeProcessor>>();
processor_ = processor.get();
bridge_ = std::make_unique<NigoriSyncBridgeImpl>(std::move(processor));
bridge_ = std::make_unique<NigoriSyncBridgeImpl>(std::move(processor),
&encryptor_);
bridge_->AddObserver(&observer_);
}
......@@ -238,6 +240,7 @@ class NigoriSyncBridgeImplTest : public testing::Test {
}
private:
const FakeEncryptor encryptor_;
std::unique_ptr<NigoriSyncBridgeImpl> bridge_;
// Ownership transferred to |bridge_|.
testing::NiceMock<MockNigoriLocalChangeProcessor>* processor_;
......@@ -500,6 +503,8 @@ TEST_P(NigoriSyncBridgeImplTestWithOptionalScryptDerivation,
EXPECT_CALL(*observer(), OnPassphraseAccepted());
EXPECT_CALL(*observer(), OnCryptographerStateChanged(NotNull()));
EXPECT_CALL(*observer(), OnBootstrapTokenUpdated(Ne(std::string()),
PASSPHRASE_BOOTSTRAP_TOKEN));
bridge()->SetDecryptionPassphrase(passphrase_key_params.password);
const Cryptographer& cryptographer = bridge()->GetCryptographerForTesting();
......@@ -537,6 +542,8 @@ TEST_F(NigoriSyncBridgeImplTest,
EXPECT_CALL(*observer(),
OnPassphraseTypeChanged(PassphraseType::CUSTOM_PASSPHRASE,
/*passphrase_time=*/NotNullTime()));
EXPECT_CALL(*observer(), OnBootstrapTokenUpdated(Ne(std::string()),
PASSPHRASE_BOOTSTRAP_TOKEN));
EXPECT_CALL(*processor(), Put(HasCustomPassphraseNigori()));
bridge()->SetEncryptionPassphrase(kPassphraseKeyParams.password);
EXPECT_THAT(bridge()->GetData(), HasCustomPassphraseNigori());
......
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