Skip to content

Commit

Permalink
[Nigori] Add PublicPrivateKeyInitializer to bootstrap key-pairs.
Browse files Browse the repository at this point in the history
Implement the basic block for bootstrapping a Public-private key-pair. Wiring of this logic will be implemented in a follow-up cl.
The key-bag will contain key-pairs keyed by their version. The version is the info that will be communicated by other clients to indicate Public-private key-pair intended for encryption/decryption.

Bug: 1445056
Change-Id: Ie71e8124a5fe0718a967246a8b2ec4d5c5dfa6f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4572816
Commit-Queue: Elias Khsheibun <eliaskh@chromium.org>
Reviewed-by: Maksim Moskvitin <mmoskvitin@google.com>
Cr-Commit-Position: refs/heads/main@{#1151764}
  • Loading branch information
Elias Khsheibun authored and Chromium LUCI CQ committed Jun 1, 2023
1 parent efeb081 commit 4350141
Show file tree
Hide file tree
Showing 9 changed files with 187 additions and 0 deletions.
9 changes: 9 additions & 0 deletions components/sync/nigori/cryptographer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ std::string CryptographerImpl::EmplaceKey(
Nigori::CreateByDerivation(derivation_params, passphrase));
}

void CryptographerImpl::EmplaceKeyPair(PublicPrivateKeyPair private_key,
uint32_t version) {
key_bag_.AddKeyPair(std::move(private_key), version);
}

void CryptographerImpl::EmplaceKeysFrom(const NigoriKeyBag& key_bag) {
key_bag_.AddAllUnknownKeysFrom(key_bag);
}
Expand Down Expand Up @@ -95,6 +100,10 @@ bool CryptographerImpl::HasKey(const std::string& key_name) const {
return key_bag_.HasKey(key_name);
}

bool CryptographerImpl::HasKeyPair(const uint32_t key_pair_version) const {
return key_bag_.HasKeyPair(key_pair_version);
}

sync_pb::NigoriKey CryptographerImpl::ExportDefaultKey() const {
DCHECK(CanEncrypt());
return key_bag_.ExportKey(default_encryption_key_name_);
Expand Down
8 changes: 8 additions & 0 deletions components/sync/nigori/cryptographer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "components/sync/engine/nigori/cryptographer.h"
#include "components/sync/engine/nigori/key_derivation_params.h"
#include "components/sync/engine/nigori/nigori.h"
#include "components/sync/engine/nigori/public_private_key_pair.h"
#include "components/sync/nigori/nigori_key_bag.h"
#include "components/sync/protocol/nigori_local_data.pb.h"

Expand Down Expand Up @@ -59,6 +60,9 @@ class CryptographerImpl : public Cryptographer {
// Does NOT set or change the default encryption key.
void EmplaceKeysFrom(const NigoriKeyBag& key_bag);

// Adds the given Public-private key-pair associated with |version|.
void EmplaceKeyPair(PublicPrivateKeyPair key_pair, uint32_t version);

// Sets or changes the default encryption key, which causes CanEncrypt() to
// return true. |key_name| must not be empty and must represent a known key.
void SelectDefaultEncryptionKey(const std::string& key_name);
Expand All @@ -82,6 +86,10 @@ class CryptographerImpl : public Cryptographer {
// Determines whether |key_name| represents a known key.
bool HasKey(const std::string& key_name) const;

// Determines whether |key_pair_version| represents a known Public-private
// key-pair.
bool HasKeyPair(const uint32_t key_pair_version) const;

// Returns a proto representation of the default encryption key. |*this| must
// have a default encryption key set, as reflected by CanEncrypt().
sync_pb::NigoriKey ExportDefaultKey() const;
Expand Down
28 changes: 28 additions & 0 deletions components/sync/nigori/cryptographer_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@

#include "components/sync/nigori/cryptographer_impl.h"

#include <utility>

#include "components/sync/engine/nigori/key_derivation_params.h"
#include "components/sync/engine/nigori/public_private_key_pair.h"
#include "components/sync/protocol/nigori_local_data.pb.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -154,4 +157,29 @@ TEST(CryptographerImplTest, ShouldExportDefaultKey) {
Eq(key_name));
}

TEST(CryptographerImplTest, ShouldEmplaceKeyPair) {
std::unique_ptr<CryptographerImpl> cryptographer =
CryptographerImpl::CreateEmpty();
ASSERT_THAT(cryptographer, NotNull());
PublicPrivateKeyPair key_pair = PublicPrivateKeyPair::GenerateNewKeyPair();
ASSERT_FALSE(cryptographer->HasKeyPair(0));

cryptographer->EmplaceKeyPair(std::move(key_pair), 0);

EXPECT_TRUE(cryptographer->HasKeyPair(0));
}

TEST(CryptographerImplTest, ShouldEmplaceExistingKeyPair) {
std::unique_ptr<CryptographerImpl> cryptographer =
CryptographerImpl::CreateEmpty();
ASSERT_THAT(cryptographer, NotNull());
ASSERT_FALSE(cryptographer->HasKeyPair(0));
cryptographer->EmplaceKeyPair(PublicPrivateKeyPair::GenerateNewKeyPair(), 0);
ASSERT_TRUE(cryptographer->HasKeyPair(0));

cryptographer->EmplaceKeyPair(PublicPrivateKeyPair::GenerateNewKeyPair(), 0);

EXPECT_TRUE(cryptographer->HasKeyPair(0));
}

} // namespace syncer
37 changes: 37 additions & 0 deletions components/sync/nigori/nigori_key_bag.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "components/sync/nigori/nigori_key_bag.h"

#include <utility>
#include <vector>

#include "base/logging.h"
#include "base/notreached.h"
Expand All @@ -27,6 +28,17 @@ sync_pb::NigoriKey NigoriToProto(const Nigori& nigori,
return proto;
}

sync_pb::PrivateKey KeyPairToPrivateKeyProto(
const uint32_t version,
const PublicPrivateKeyPair& key_pair) {
auto raw_private_key = key_pair.GetRawPrivateKey();
sync_pb::PrivateKey output;
output.set_version(version);
output.set_x25519_private_key(
std::string(std::begin(raw_private_key), std::end(raw_private_key)));
return output;
}

std::unique_ptr<Nigori> CloneNigori(const Nigori& nigori) {
std::string user_key;
std::string encryption_key;
Expand All @@ -39,6 +51,16 @@ std::unique_ptr<Nigori> CloneNigori(const Nigori& nigori) {
return nigori_copy;
}

PublicPrivateKeyPair CloneKeyPair(const PublicPrivateKeyPair& key_pair) {
const auto raw_private_key = key_pair.GetRawPrivateKey();
std::vector<uint8_t> key_for_import(raw_private_key.begin(),
raw_private_key.end());
absl::optional<PublicPrivateKeyPair> clone =
PublicPrivateKeyPair::CreateByImport(key_for_import);
CHECK(clone.has_value());
return std::move(clone.value());
}

} // namespace

// static
Expand Down Expand Up @@ -73,6 +95,10 @@ sync_pb::NigoriKeyBag NigoriKeyBag::ToProto() const {
for (const auto& [key_name, nigori] : nigori_map_) {
*output.add_key() = NigoriToProto(*nigori, key_name);
}
for (const auto& [key_version, key_pair] : key_pairs_map_) {
*output.add_private_key() = KeyPairToPrivateKeyProto(key_version, key_pair);
}

return output;
}

Expand All @@ -90,6 +116,10 @@ bool NigoriKeyBag::HasKey(const std::string& key_name) const {
return nigori_map_.count(key_name) != 0;
}

bool NigoriKeyBag::HasKeyPair(uint32_t key_pair_version) const {
return key_pairs_map_.contains(key_pair_version);
}

sync_pb::NigoriKey NigoriKeyBag::ExportKey(const std::string& key_name) const {
DCHECK(HasKey(key_name));
sync_pb::NigoriKey key =
Expand Down Expand Up @@ -132,6 +162,13 @@ void NigoriKeyBag::AddAllUnknownKeysFrom(const NigoriKeyBag& other) {
// Only use this key if we don't already know about it.
nigori_map_.emplace(key_name, CloneNigori(*nigori));
}
for (const auto& [public_key, key_pair] : other.key_pairs_map_) {
key_pairs_map_.emplace(public_key, CloneKeyPair(key_pair));
}
}

void NigoriKeyBag::AddKeyPair(PublicPrivateKeyPair key_pair, uint32_t version) {
key_pairs_map_.emplace(version, std::move(key_pair));
}

bool NigoriKeyBag::EncryptWithKey(
Expand Down
9 changes: 9 additions & 0 deletions components/sync/nigori/nigori_key_bag.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include <memory>
#include <string>

#include "components/sync/engine/nigori/public_private_key_pair.h"

namespace sync_pb {
class EncryptedData;
class NigoriKey;
Expand Down Expand Up @@ -42,6 +44,7 @@ class NigoriKeyBag {

size_t size() const;
bool HasKey(const std::string& key_name) const;
bool HasKeyPair(const uint32_t key_version) const;

// |key_name| must exist in this keybag.
sync_pb::NigoriKey ExportKey(const std::string& key_name) const;
Expand All @@ -58,6 +61,9 @@ class NigoriKeyBag {
// don't know about.
void AddAllUnknownKeysFrom(const NigoriKeyBag& other);

// Adds a Public-private key-pair to the keybag associated with |version|.
void AddKeyPair(PublicPrivateKeyPair key_pair, uint32_t version);

// Encryption of strings (possibly binary). Returns true if success.
// |key_name| must be known. |encrypted_output| must not be null.
bool EncryptWithKey(const std::string& key_name,
Expand All @@ -77,6 +83,9 @@ class NigoriKeyBag {

// The Nigoris we know about, mapped by key name.
std::map<std::string, std::unique_ptr<const Nigori>> nigori_map_;

// Public-private key-pairs we know about, mapped by version.
std::map<uint32_t, const PublicPrivateKeyPair> key_pairs_map_;
};

} // namespace syncer
Expand Down
40 changes: 40 additions & 0 deletions components/sync/nigori/nigori_key_bag_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@

#include "components/sync/nigori/nigori_key_bag.h"

#include <vector>

#include "components/sync/engine/nigori/key_derivation_params.h"
#include "components/sync/engine/nigori/nigori.h"
#include "components/sync/engine/nigori/public_private_key_pair.h"
#include "components/sync/protocol/nigori_specifics.pb.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -136,6 +139,23 @@ TEST(NigoriKeyBagTest, ShouldClone) {
EXPECT_TRUE(cloned_key_bag.HasKey(key_name2));
}

TEST(NigoriKeyBagTest, ShouldCloneWithKeyPairs) {
NigoriKeyBag original_key_bag = NigoriKeyBag::CreateEmpty();
const std::string key_name1 =
original_key_bag.AddKey(CreateTestNigori("password1"));
const std::string key_name2 =
original_key_bag.AddKey(CreateTestNigori("password2"));
original_key_bag.AddKeyPair(PublicPrivateKeyPair::GenerateNewKeyPair(), 0);
original_key_bag.AddKeyPair(PublicPrivateKeyPair::GenerateNewKeyPair(), 1);

const NigoriKeyBag cloned_key_bag = original_key_bag.Clone();

EXPECT_TRUE(cloned_key_bag.HasKey(key_name1));
EXPECT_TRUE(cloned_key_bag.HasKey(key_name2));
EXPECT_TRUE(cloned_key_bag.HasKeyPair(0));
EXPECT_TRUE(cloned_key_bag.HasKeyPair(1));
}

// This holds true for M79 and above, but older clients rely on the field being
// set.
TEST(NigoriKeyBagTest, ShouldIgnoreDeprecatedKeyNameProtoField) {
Expand All @@ -157,5 +177,25 @@ TEST(NigoriKeyBagTest, ShouldIgnoreDeprecatedKeyNameProtoField) {
EXPECT_FALSE(restored_key_bag.HasKey(actual_key_name_in_proto));
}

TEST(NigoriKeyBagTest, ShouldAddKeyPair) {
NigoriKeyBag key_bag = NigoriKeyBag::CreateEmpty();

key_bag.AddKeyPair(PublicPrivateKeyPair::GenerateNewKeyPair(), 0);

EXPECT_TRUE(key_bag.HasKeyPair(0));
}

TEST(NigoriKeyBagTest, ShouldAddMultipleKeyPairs) {
NigoriKeyBag key_bag = NigoriKeyBag::CreateEmpty();

key_bag.AddKeyPair(PublicPrivateKeyPair::GenerateNewKeyPair(), 0);
key_bag.AddKeyPair(PublicPrivateKeyPair::GenerateNewKeyPair(), 1);
key_bag.AddKeyPair(PublicPrivateKeyPair::GenerateNewKeyPair(), 3);

EXPECT_TRUE(key_bag.HasKeyPair(0));
EXPECT_TRUE(key_bag.HasKeyPair(1));
EXPECT_TRUE(key_bag.HasKeyPair(3));
}

} // namespace
} // namespace syncer
4 changes: 4 additions & 0 deletions components/sync/nigori/nigori_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "components/sync/base/model_type.h"
#include "components/sync/engine/nigori/key_derivation_params.h"
#include "components/sync/engine/nigori/nigori.h"
#include "components/sync/engine/nigori/public_key.h"
#include "components/sync/protocol/encryption.pb.h"
#include "components/sync/protocol/nigori_specifics.pb.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
Expand Down Expand Up @@ -93,6 +94,9 @@ struct NigoriState {

// Some debug-only fields for passphrase type TRUSTED_VAULT_PASSPHRASE.
sync_pb::NigoriSpecifics::TrustedVaultDebugInfo trusted_vault_debug_info;

// Current Public-key.
absl::optional<PublicKey> public_key;
};

} // namespace syncer
Expand Down
49 changes: 49 additions & 0 deletions components/sync/nigori/pending_local_nigori_commit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,20 @@

#include "components/sync/nigori/pending_local_nigori_commit.h"

#include <utility>

#include "base/feature_list.h"
#include "base/logging.h"
#include "base/metrics/histogram_macros.h"
#include "base/time/time.h"
#include "components/sync/engine/nigori/key_derivation_params.h"
#include "components/sync/engine/nigori/nigori.h"
#include "components/sync/engine/nigori/public_key.h"
#include "components/sync/engine/nigori/public_private_key_pair.h"
#include "components/sync/nigori/cryptographer_impl.h"
#include "components/sync/nigori/keystore_keys_cryptographer.h"
#include "components/sync/nigori/nigori_state.h"
#include "components/sync/protocol/nigori_specifics.pb.h"

namespace syncer {

Expand Down Expand Up @@ -116,6 +121,9 @@ class KeystoreInitializer : public PendingLocalNigoriCommit {
state->cryptographer->EmplaceKeysAndSelectDefaultKeyFrom(*cryptographer);
state->passphrase_type = NigoriSpecifics::KEYSTORE_PASSPHRASE;
state->keystore_migration_time = base::Time::Now();

// TODO(crbug.com/1445056): handle creation of Public-private key pair for
// new sync users.
return true;
}

Expand Down Expand Up @@ -160,6 +168,41 @@ class KeystoreReencryptor : public PendingLocalNigoriCommit {
void OnFailure(SyncEncryptionHandler::Observer* observer) override {}
};

class PublicPrivateKeyInitializer : public PendingLocalNigoriCommit {
public:
PublicPrivateKeyInitializer() = default;

PublicPrivateKeyInitializer(const PublicPrivateKeyInitializer&) = delete;
PublicPrivateKeyInitializer& operator=(const PublicPrivateKeyInitializer&) =
delete;

~PublicPrivateKeyInitializer() override = default;

bool TryApply(NigoriState* state) const override {
// It is not safe to commit while we have pending keys.
// Also, there is no work to do if a public-key already exists.
if (state->pending_keys.has_value() || state->public_key.has_value()) {
return false;
}

PublicPrivateKeyPair key_pair = PublicPrivateKeyPair::GenerateNewKeyPair();
state->public_key = PublicKey::CreateByImport(key_pair.GetRawPublicKey());
state->cryptographer->EmplaceKeyPair(std::move(key_pair), 0);
return true;
}

void OnSuccess(const NigoriState& state,
SyncEncryptionHandler::Observer* observer) override {
observer->OnCryptographerStateChanged(state.cryptographer.get(),
/*has_pending_keys=*/false);
}

void OnFailure(SyncEncryptionHandler::Observer* observer) override {
// TODO(crbug.com/1445056): handle rejection of Public-private key, can be
// due to already existing key.
}
};

} // namespace

// static
Expand All @@ -183,4 +226,10 @@ PendingLocalNigoriCommit::ForKeystoreReencryption() {
return std::make_unique<KeystoreReencryptor>();
}

// static
std::unique_ptr<PendingLocalNigoriCommit>
PendingLocalNigoriCommit::ForPublicPrivateKeyInitialization() {
return std::make_unique<PublicPrivateKeyInitializer>();
}

} // namespace syncer
3 changes: 3 additions & 0 deletions components/sync/nigori/pending_local_nigori_commit.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ class PendingLocalNigoriCommit {

static std::unique_ptr<PendingLocalNigoriCommit> ForKeystoreReencryption();

static std::unique_ptr<PendingLocalNigoriCommit>
ForPublicPrivateKeyInitialization();

PendingLocalNigoriCommit() = default;

PendingLocalNigoriCommit(const PendingLocalNigoriCommit&) = delete;
Expand Down

0 comments on commit 4350141

Please sign in to comment.