Skip to content

Commit

Permalink
Make the SecretKeyAccessToken optional for the key parsers and serial…
Browse files Browse the repository at this point in the history
…izers.

PiperOrigin-RevId: 536703561
  • Loading branch information
ioannanedelcu authored and copybara-github committed May 31, 2023
1 parent 7c8b4e4 commit 578e002
Show file tree
Hide file tree
Showing 10 changed files with 77 additions and 24 deletions.
2 changes: 2 additions & 0 deletions cc/internal/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,7 @@ cc_library(
"@com_google_absl//absl/log",
"@com_google_absl//absl/status",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/types:optional",
],
)

Expand Down Expand Up @@ -799,6 +800,7 @@ cc_library(
"@com_google_absl//absl/functional:function_ref",
"@com_google_absl//absl/log",
"@com_google_absl//absl/status",
"@com_google_absl//absl/types:optional",
],
)

Expand Down
2 changes: 2 additions & 0 deletions cc/internal/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,7 @@ tink_cc_library(
absl::log
absl::status
absl::strings
absl::optional
tink::core::key
tink::core::secret_key_access_token
tink::util::status
Expand Down Expand Up @@ -753,6 +754,7 @@ tink_cc_library(
absl::function_ref
absl::log
absl::status
absl::optional
tink::core::key
tink::core::secret_key_access_token
tink::util::status
Expand Down
18 changes: 11 additions & 7 deletions cc/internal/key_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "absl/log/log.h"
#include "absl/status/status.h"
#include "absl/strings/string_view.h"
#include "absl/types/optional.h"
#include "tink/internal/parser_index.h"
#include "tink/internal/serialization.h"
#include "tink/key.h"
Expand All @@ -47,7 +48,8 @@ class KeyParser {
// value returned by `ObjectIdentifier()`. However, implementations should
// check that this is the case.
virtual util::StatusOr<std::unique_ptr<Key>> ParseKey(
const Serialization& serialization, SecretKeyAccessToken token) const = 0;
const Serialization& serialization,
absl::optional<SecretKeyAccessToken> token) const = 0;

// Returns the object identifier for `SerializationT`, which is only valid
// for the lifetime of this object.
Expand All @@ -74,15 +76,16 @@ class KeyParserImpl : public KeyParser {
public:
// Creates a key parser with `object_identifier` and parsing `function`. The
// referenced `function` should outlive the created key parser object.
explicit KeyParserImpl(absl::string_view object_identifier,
absl::FunctionRef<util::StatusOr<KeyT>(
SerializationT, SecretKeyAccessToken)>
function)
explicit KeyParserImpl(
absl::string_view object_identifier,
absl::FunctionRef<util::StatusOr<KeyT>(
SerializationT, absl::optional<SecretKeyAccessToken>)>
function)
: object_identifier_(object_identifier), function_(function) {}

util::StatusOr<std::unique_ptr<Key>> ParseKey(
const Serialization& serialization,
SecretKeyAccessToken token) const override {
absl::optional<SecretKeyAccessToken> token) const override {
if (serialization.ObjectIdentifier() != object_identifier_) {
return util::Status(absl::StatusCode::kInvalidArgument,
"Invalid object identifier for this key parser.");
Expand All @@ -108,7 +111,8 @@ class KeyParserImpl : public KeyParser {

private:
std::string object_identifier_;
std::function<util::StatusOr<KeyT>(SerializationT, SecretKeyAccessToken)>
std::function<util::StatusOr<KeyT>(SerializationT,
absl::optional<SecretKeyAccessToken>)>
function_;
};

Expand Down
15 changes: 15 additions & 0 deletions cc/internal/key_parser_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "tink/internal/key_parser.h"

#include <memory>
#include <optional>
#include <string_view>

#include "gmock/gmock.h"
Expand Down Expand Up @@ -67,6 +68,20 @@ TEST(KeyParserTest, ParseKey) {
EXPECT_THAT(**key, Eq(NoIdKey()));
}

TEST(KeyParserTest, ParsePublicKeyNoAccessToken) {
std::unique_ptr<KeyParser> parser =
absl::make_unique<KeyParserImpl<NoIdSerialization, NoIdKey>>(
kNoIdTypeUrl, ParseNoIdKey);

NoIdSerialization serialization;
util::StatusOr<std::unique_ptr<Key>> public_key =
parser->ParseKey(serialization, absl::nullopt);
ASSERT_THAT(public_key, IsOk());
EXPECT_THAT((*public_key)->GetIdRequirement(), Eq(absl::nullopt));
EXPECT_THAT((*public_key)->GetParameters(), Eq(NoIdParams()));
EXPECT_THAT(**public_key, Eq(NoIdKey()));
}

TEST(KeyParserTest, ParseKeyWithInvalidSerializationType) {
std::unique_ptr<KeyParser> parser =
absl::make_unique<KeyParserImpl<NoIdSerialization, NoIdKey>>(
Expand Down
11 changes: 7 additions & 4 deletions cc/internal/key_serializer.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "absl/functional/function_ref.h"
#include "absl/log/log.h"
#include "absl/status/status.h"
#include "absl/types/optional.h"
#include "tink/internal/serialization.h"
#include "tink/internal/serializer_index.h"
#include "tink/key.h"
Expand All @@ -41,7 +42,7 @@ class KeySerializer {
public:
// Returns the serialization of `key`.
virtual util::StatusOr<std::unique_ptr<Serialization>> SerializeKey(
const Key& key, SecretKeyAccessToken token) const = 0;
const Key& key, absl::optional<SecretKeyAccessToken> token) const = 0;

// Returns an index that can be used to look up the `KeySerializer`
// object registered for the `KeyT` type in a registry.
Expand All @@ -57,12 +58,13 @@ class KeySerializerImpl : public KeySerializer {
// Creates a key serializer with serialization `function`. The referenced
// `function` should outlive the created key serializer object.
explicit KeySerializerImpl(absl::FunctionRef<util::StatusOr<SerializationT>(
KeyT, SecretKeyAccessToken)>
KeyT, absl::optional<SecretKeyAccessToken>)>
function)
: function_(function) {}

util::StatusOr<std::unique_ptr<Serialization>> SerializeKey(
const Key& key, SecretKeyAccessToken token) const override {
const Key& key,
absl::optional<SecretKeyAccessToken> token) const override {
const KeyT* kt = dynamic_cast<const KeyT*>(&key);
if (kt == nullptr) {
return util::Status(absl::StatusCode::kInvalidArgument,
Expand All @@ -78,7 +80,8 @@ class KeySerializerImpl : public KeySerializer {
}

private:
std::function<util::StatusOr<SerializationT>(KeyT, SecretKeyAccessToken)>
std::function<util::StatusOr<SerializationT>(
KeyT, absl::optional<SecretKeyAccessToken>)>
function_;
};

Expand Down
12 changes: 12 additions & 0 deletions cc/internal/key_serializer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,18 @@ TEST(KeySerializerTest, SerializeKey) {
EXPECT_THAT((*serialization)->ObjectIdentifier(), Eq(kNoIdTypeUrl));
}

TEST(KeySerializerTest, SerializePublicKeyNoAccessToken) {
std::unique_ptr<KeySerializer> serializer =
absl::make_unique<KeySerializerImpl<NoIdKey, NoIdSerialization>>(
SerializeNoIdKey);

NoIdKey public_key;
util::StatusOr<std::unique_ptr<Serialization>> serialization =
serializer->SerializeKey(public_key, absl::nullopt);
ASSERT_THAT(serialization, IsOk());
EXPECT_THAT((*serialization)->ObjectIdentifier(), Eq(kNoIdTypeUrl));
}

TEST(KeySerializerTest, SerializeKeyWithInvalidKeyType) {
std::unique_ptr<KeySerializer> serializer =
absl::make_unique<KeySerializerImpl<NoIdKey, NoIdSerialization>>(
Expand Down
14 changes: 8 additions & 6 deletions cc/internal/serialization_test_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,26 +157,28 @@ inline util::StatusOr<IdParamsSerialization> SerializeIdParams(
}

// Parse `serialization` into a key without an ID requirement.
inline util::StatusOr<NoIdKey> ParseNoIdKey(NoIdSerialization serialization,
SecretKeyAccessToken token) {
inline util::StatusOr<NoIdKey> ParseNoIdKey(
NoIdSerialization serialization,
absl::optional<SecretKeyAccessToken> token) {
return NoIdKey();
}

// Parse `serialization` into a key with an ID requirement.
inline util::StatusOr<IdKey> ParseIdKey(IdKeySerialization serialization,
SecretKeyAccessToken token) {
inline util::StatusOr<IdKey> ParseIdKey(
IdKeySerialization serialization,
absl::optional<SecretKeyAccessToken> token) {
return IdKey(serialization.GetKeyId());
}

// Serialize `key` without an ID requirement.
inline util::StatusOr<NoIdSerialization> SerializeNoIdKey(
NoIdKey key, SecretKeyAccessToken token) {
NoIdKey key, absl::optional<SecretKeyAccessToken> token) {
return NoIdSerialization();
}

// Serialize `key` with an ID requirement.
inline util::StatusOr<IdKeySerialization> SerializeIdKey(
IdKey key, SecretKeyAccessToken token) {
IdKey key, absl::optional<SecretKeyAccessToken> token) {
return IdKeySerialization(key.GetIdRequirement().value());
}

Expand Down
1 change: 1 addition & 0 deletions cc/mac/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ cc_library(
"//util:status",
"//util:statusor",
"@com_google_absl//absl/status",
"@com_google_absl//absl/types:optional",
],
)

Expand Down
1 change: 1 addition & 0 deletions cc/mac/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ tink_cc_library(
tink::mac::aes_cmac_key
tink::mac::aes_cmac_parameters
absl::status
absl::optional
tink::core::partial_key_access
tink::core::restricted_data
tink::core::secret_key_access_token
Expand Down
25 changes: 18 additions & 7 deletions cc/mac/aes_cmac_proto_serialization.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <string>

#include "absl/status/status.h"
#include "absl/types/optional.h"
#include "tink/internal/key_parser.h"
#include "tink/internal/key_serializer.h"
#include "tink/internal/mutable_serialization_registry.h"
Expand Down Expand Up @@ -132,17 +133,22 @@ util::StatusOr<internal::ProtoParametersSerialization> SerializeParameters(
}

util::StatusOr<AesCmacKey> ParseKey(
internal::ProtoKeySerialization serialization, SecretKeyAccessToken token) {
internal::ProtoKeySerialization serialization,
absl::optional<SecretKeyAccessToken> token) {
if (serialization.TypeUrl() != kTypeUrl) {
return util::Status(absl::StatusCode::kInvalidArgument,
"Wrong type URL when parsing AesCmacKey.");
}

// TODO(ioannanedelcu): Add a test for this behaviour.
if (!token.has_value()) {
return util::Status(absl::StatusCode::kInvalidArgument,
"SecretKeyAccess is required");
}
google::crypto::tink::AesCmacKey proto_key;
RestrictedData restricted_data = serialization.SerializedKeyProto();
// OSS proto library complains if input is not converted to a string.
if (!proto_key.ParseFromString(
std::string(restricted_data.GetSecret(token)))) {
std::string(restricted_data.GetSecret(*token)))) {
return util::Status(absl::StatusCode::kInvalidArgument,
"Failed to parse AesCmacKey proto");
}
Expand All @@ -160,33 +166,38 @@ util::StatusOr<AesCmacKey> ParseKey(
if (!parameters.ok()) return parameters.status();

util::StatusOr<AesCmacKey> key = AesCmacKey::Create(
*parameters, RestrictedData(proto_key.key_value(), token),
*parameters, RestrictedData(proto_key.key_value(), *token),
serialization.IdRequirement(), GetPartialKeyAccess());
if (!key.ok()) return key.status();

return *key;
}

util::StatusOr<internal::ProtoKeySerialization> SerializeKey(
AesCmacKey key, SecretKeyAccessToken token) {
AesCmacKey key, absl::optional<SecretKeyAccessToken> token) {
util::StatusOr<RestrictedData> restricted_input =
key.GetKeyBytes(GetPartialKeyAccess());
if (!restricted_input.ok()) return restricted_input.status();
// TODO(ioannanedelcu): Add a test for this behaviour.
if (!token.has_value()) {
return util::Status(absl::StatusCode::kInvalidArgument,
"SecretKeyAccess is required");
}

AesCmacParams proto_params;
proto_params.set_tag_size(key.GetParameters().CryptographicTagSizeInBytes());
google::crypto::tink::AesCmacKey proto_key;
*proto_key.mutable_params() = proto_params;
proto_key.set_version(0);
// OSS proto library complains if input is not converted to a string.
proto_key.set_key_value(std::string(restricted_input->GetSecret(token)));
proto_key.set_key_value(std::string(restricted_input->GetSecret(*token)));

util::StatusOr<OutputPrefixType> output_prefix_type =
ToOutputPrefixType(key.GetParameters().GetVariant());
if (!output_prefix_type.ok()) return output_prefix_type.status();

RestrictedData restricted_output =
RestrictedData(proto_key.SerializeAsString(), token);
RestrictedData(proto_key.SerializeAsString(), *token);
return internal::ProtoKeySerialization::Create(
kTypeUrl, restricted_output, google::crypto::tink::KeyData::SYMMETRIC,
*output_prefix_type, key.GetIdRequirement());
Expand Down

0 comments on commit 578e002

Please sign in to comment.