Skip to content

Commit

Permalink
Replace unique_ptr with shared_ptr in KeysetHandleBuilder::Entry API.
Browse files Browse the repository at this point in the history 
Also, return shared_ptr in KeysetHandle::Entry::GetKey().

PiperOrigin-RevId: 524063105
  • Loading branch information
@ise-crypto @copybara-github
ise-crypto authored and copybara-github committed Apr 13, 2023
1 parent c7c1d53 commit 7e08b9a
Show file tree
Hide file tree
Showing 12 changed files with 122 additions and 309 deletions.
14 changes: 2 additions & 12 deletions cc/core/cleartext_keyset_handle.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include <memory>
#include <string>
#include <utility>
#include <vector>

#include "absl/container/flat_hash_map.h"
#include "absl/status/status.h"
Expand All @@ -42,23 +41,14 @@ util::StatusOr<std::unique_ptr<KeysetHandle>> CleartextKeysetHandle::Read(
std::unique_ptr<KeysetReader> reader,
const absl::flat_hash_map<std::string, std::string>&
monitoring_annotations) {
util::StatusOr<std::unique_ptr<Keyset>> keyset_result = reader->Read();
auto keyset_result = reader->Read();
if (!keyset_result.ok()) {
return ToStatusF(absl::StatusCode::kInvalidArgument,
"Error reading keyset data: %s",
keyset_result.status().message());
}
util::StatusOr<std::vector<std::shared_ptr<const KeysetHandle::Entry>>>
entries = KeysetHandle::GetEntriesFromKeyset(**keyset_result);
if (!entries.ok()) {
return entries.status();
}
if (entries->size() != (*keyset_result)->key_size()) {
return util::Status(absl::StatusCode::kInternal,
"Error converting keyset proto into key entries.");
}
std::unique_ptr<KeysetHandle> handle(new KeysetHandle(
std::move(keyset_result.value()), *entries, monitoring_annotations));
std::move(keyset_result.value()), monitoring_annotations));
return std::move(handle);
}

Expand Down
8 changes: 4 additions & 4 deletions cc/core/cleartext_keyset_handle_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ class CleartextKeysetHandleTest : public ::testing::Test {
TEST_F(CleartextKeysetHandleTest, testRead) {
Keyset keyset;
Keyset::Key key;
AddTinkKey("some_key_type", 42, key, KeyStatusType::ENABLED,
AddTinkKey("some key type", 42, key, KeyStatusType::ENABLED,
KeyData::SYMMETRIC, &keyset);
AddRawKey("some_other_key_type", 711, key, KeyStatusType::ENABLED,
AddRawKey("some other key type", 711, key, KeyStatusType::ENABLED,
KeyData::SYMMETRIC, &keyset);
keyset.set_primary_key_id(42);
{ // Reader that reads a valid keyset.
Expand All @@ -75,9 +75,9 @@ TEST_F(CleartextKeysetHandleTest, testRead) {
TEST_F(CleartextKeysetHandleTest, testWrite) {
Keyset keyset;
Keyset::Key key;
AddTinkKey("some_key_type", 42, key, KeyStatusType::ENABLED,
AddTinkKey("some key type", 42, key, KeyStatusType::ENABLED,
KeyData::SYMMETRIC, &keyset);
AddRawKey("some_other_key_type", 711, key, KeyStatusType::ENABLED,
AddRawKey("some other key type", 711, key, KeyStatusType::ENABLED,
KeyData::SYMMETRIC, &keyset);
keyset.set_primary_key_id(42);

Expand Down
117 changes: 22 additions & 95 deletions cc/core/keyset_handle.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,10 @@
///////////////////////////////////////////////////////////////////////////////
#include "tink/keyset_handle.h"

#include <cstdint>
#include <iostream>
#include <memory>
#include <string>
#include <utility>
#include <vector>

#include "absl/container/flat_hash_map.h"
#include "absl/log/check.h"
Expand Down Expand Up @@ -180,57 +179,29 @@ KeysetHandle::Entry KeysetHandle::operator[](int index) const {
CHECK(index >= 0 && index < size())
<< "Invalid index " << index << " for keyset of size " << size();

if (!entries_.empty() && entries_.size() > index) {
return *entries_[index];
}
// Since `entries_` has not been populated, the entry must be created on
// demand from the key proto entry at `index` in `keyset_`. This special
// case will no longer be necessary after `keyset_` has been removed from the
// `KeysetHandle` class.
//
// TODO(b/277792846): Remove after transition to rely solely on
// `KeysetHandle::Entry`.
return CreateEntryAt(index);
}

KeysetHandle::Entry KeysetHandle::CreateEntryAt(int index) const {
CHECK(index >= 0 && index < size())
<< "Invalid index " << index << " for keyset of size " << size();

util::Status validation = ValidateAt(index);
CHECK_OK(validation);

Keyset keyset = get_keyset();
util::StatusOr<Entry> entry =
CreateEntry(keyset.key(index), keyset.primary_key_id());
// Status should be OK since this keyset handle has been validated.
CHECK_OK(entry.status());
return *entry;
}
const Keyset::Key& proto_key = get_keyset().key(index);
int id = proto_key.key_id();

util::StatusOr<KeysetHandle::Entry> KeysetHandle::CreateEntry(
const Keyset::Key& proto_key, uint32_t primary_key_id) {
util::StatusOr<internal::ProtoKeySerialization> serialization =
ToProtoKeySerialization(proto_key);
if (!serialization.ok()) {
return serialization.status();
}
// Status should be OK since this keyset handle has been validated.
CHECK_OK(serialization.status());

util::StatusOr<std::shared_ptr<const Key>> key =
util::StatusOr<std::unique_ptr<Key>> key =
internal::MutableSerializationRegistry::GlobalInstance()
.ParseKeyWithLegacyFallback(*serialization);
if (!key.ok()) {
return key.status();
}
CHECK_OK(key.status());

util::StatusOr<KeyStatus> key_status =
internal::FromKeyStatusType(proto_key.status());
if (!key_status.ok()) {
return key_status.status();
}
// Status should be OK since this keyset handle has been validated.
CHECK_OK(key_status.status());

return Entry(*std::move(key), *key_status, proto_key.key_id(),
proto_key.key_id() == primary_key_id);
return Entry(*std::move(key), *key_status, id,
id == get_keyset().primary_key_id());
}

util::StatusOr<std::unique_ptr<KeysetHandle>> KeysetHandle::Read(
Expand Down Expand Up @@ -262,17 +233,8 @@ KeysetHandle::ReadWithAssociatedData(
"Error decrypting encrypted keyset: %s",
keyset_result.status().message());
}
util::StatusOr<std::vector<std::shared_ptr<const Entry>>> entries =
GetEntriesFromKeyset(**keyset_result);
if (!entries.ok()) {
return entries.status();
}
if (entries->size() != (*keyset_result)->key_size()) {
return util::Status(absl::StatusCode::kInternal,
"Error converting keyset proto into key entries.");
}
return absl::WrapUnique(new KeysetHandle(*std::move(keyset_result), *entries,
monitoring_annotations));
return absl::WrapUnique(
new KeysetHandle(*std::move(keyset_result), monitoring_annotations));
}

util::StatusOr<std::unique_ptr<KeysetHandle>> KeysetHandle::ReadNoSecret(
Expand All @@ -288,17 +250,8 @@ util::StatusOr<std::unique_ptr<KeysetHandle>> KeysetHandle::ReadNoSecret(
if (!validation.ok()) {
return validation;
}
util::StatusOr<std::vector<std::shared_ptr<const Entry>>> entries =
GetEntriesFromKeyset(keyset);
if (!entries.ok()) {
return entries.status();
}
if (entries->size() != keyset.key_size()) {
return util::Status(absl::StatusCode::kInternal,
"Error converting keyset proto into key entries.");
}
return absl::WrapUnique(
new KeysetHandle(std::move(keyset), *entries, monitoring_annotations));
new KeysetHandle(std::move(keyset), monitoring_annotations));
}

util::Status KeysetHandle::Write(KeysetWriter* writer,
Expand Down Expand Up @@ -372,17 +325,8 @@ KeysetHandle::GetPublicKeysetHandle() const {
public_keyset->add_key()->Swap(public_key_result.value().get());
}
public_keyset->set_primary_key_id(get_keyset().primary_key_id());
util::StatusOr<std::vector<std::shared_ptr<const Entry>>> entries =
GetEntriesFromKeyset(*public_keyset);
if (!entries.ok()) {
return entries.status();
}
if (entries->size() != public_keyset->key_size()) {
return util::Status(absl::StatusCode::kInternal,
"Error converting keyset proto into key entries.");
}
std::unique_ptr<KeysetHandle> handle(
new KeysetHandle(std::move(public_keyset), *entries));
new KeysetHandle(std::move(public_keyset)));
return std::move(handle);
}

Expand Down Expand Up @@ -411,36 +355,19 @@ crypto::tink::util::StatusOr<uint32_t> KeysetHandle::AddToKeyset(

crypto::tink::util::StatusOr<uint32_t> KeysetHandle::AddKey(
const google::crypto::tink::KeyTemplate& key_template, bool as_primary) {
util::StatusOr<uint32_t> id = AddToKeyset(key_template, as_primary, &keyset_);
if (!id.ok()) {
return id.status();
}
util::StatusOr<const Entry> entry = CreateEntry(
keyset_.key(keyset_.key_size() - 1), keyset_.primary_key_id());
if (!entry.ok()) {
return entry.status();
}
entries_.push_back(std::make_shared<const Entry>(*entry));
return *id;
return AddToKeyset(key_template, as_primary, &keyset_);
}

KeysetInfo KeysetHandle::GetKeysetInfo() const {
return KeysetInfoFromKeyset(get_keyset());
}

util::StatusOr<std::vector<std::shared_ptr<const KeysetHandle::Entry>>>
KeysetHandle::GetEntriesFromKeyset(const Keyset& keyset) {
std::vector<std::shared_ptr<const Entry>> entries;
for (const Keyset::Key& key : keyset.key()) {
util::StatusOr<const Entry> entry =
CreateEntry(key, keyset.primary_key_id());
if (!entry.ok()) {
return entry.status();
}
entries.push_back(std::make_shared<const Entry>(*entry));
}
return entries;
}
KeysetHandle::KeysetHandle(Keyset keyset) : keyset_(std::move(keyset)) {}

KeysetHandle::KeysetHandle(std::unique_ptr<Keyset> keyset)
: keyset_(std::move(*keyset)) {}

const Keyset& KeysetHandle::get_keyset() const { return keyset_; }

} // namespace tink
} // namespace crypto
10 changes: 4 additions & 6 deletions cc/core/keyset_handle_builder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ KeysetHandleBuilder::KeysetHandleBuilder(const KeysetHandle& handle) {
}

KeysetHandleBuilder::Entry KeysetHandleBuilder::Entry::CreateFromKey(
std::shared_ptr<const Key> key, KeyStatus status, bool is_primary) {
std::unique_ptr<Key> key, KeyStatus status, bool is_primary) {
absl::optional<int> id_requirement = key->GetIdRequirement();
auto imported_entry = absl::make_unique<internal::KeyEntry>(std::move(key));
KeysetHandleBuilder::Entry entry(std::move(imported_entry));
Expand All @@ -76,8 +76,8 @@ KeysetHandleBuilder::Entry KeysetHandleBuilder::Entry::CreateFromKey(
}

KeysetHandleBuilder::Entry KeysetHandleBuilder::Entry::CreateFromParams(
std::shared_ptr<const Parameters> parameters, KeyStatus status,
bool is_primary, absl::optional<int> id) {
std::unique_ptr<Parameters> parameters, KeyStatus status, bool is_primary,
absl::optional<int> id) {
auto generated_entry =
absl::make_unique<internal::ParametersEntry>(std::move(parameters));
KeysetHandleBuilder::Entry entry(std::move(generated_entry));
Expand Down Expand Up @@ -189,9 +189,7 @@ util::StatusOr<KeysetHandle> KeysetHandleBuilder::Build() {
"No primary set in this keyset.");
}
keyset.set_primary_key_id(*primary_id);
util::StatusOr<std::vector<std::shared_ptr<const KeysetHandle::Entry>>>
entries = KeysetHandle::GetEntriesFromKeyset(keyset);
return KeysetHandle(keyset, *std::move(entries));
return KeysetHandle(keyset);
}

} // namespace tink
Expand Down
Loading

0 comments on commit 7e08b9a

Please sign in to comment.