Fix HMAC key parsing bug and add a test for this use case and also fo…

…r unknown output prefix. PiperOrigin-RevId: 586643536 Change-Id: I1ca7d454badff450ad5580796bc6554dff54c0a2
tink-crypto · Nov 30, 2023 · 36d195b · 36d195b
1 parent e23456d
commit 36d195b
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 1 deletion.
diff --git a/tink/mac/hmac_proto_serialization.cc b/tink/mac/hmac_proto_serialization.cc
@@ -214,7 +214,7 @@ util::StatusOr<HmacKey> ParseKey(
   if (!variant.ok()) return variant.status();
   util::StatusOr<HmacParameters::HashType> hash_type =
       ToHashType(proto_key.params().hash());
-  if (!hash_type.ok()) return variant.status();
+  if (!hash_type.ok()) return hash_type.status();
 
   util::StatusOr<HmacParameters> parameters = HmacParameters::Create(
       proto_key.key_value().length(), proto_key.params().tag_size(), *hash_type,

diff --git a/tink/mac/hmac_proto_serialization_test.cc b/tink/mac/hmac_proto_serialization_test.cc
@@ -336,6 +336,56 @@ TEST_F(HmacProtoSerializationTest, ParseKeyWithInvalidVersion) {
   ASSERT_THAT(key.status(), StatusIs(absl::StatusCode::kInvalidArgument));
 }
 
+TEST_F(HmacProtoSerializationTest, ParseKeyWithUnknownOutputPrefixType) {
+  ASSERT_THAT(RegisterHmacProtoSerialization(), IsOk());
+
+  std::string raw_key_bytes = Random::GetRandomBytes(16);
+  google::crypto::tink::HmacKey key_proto;
+  key_proto.set_version(0);
+  key_proto.set_key_value(raw_key_bytes);
+  key_proto.mutable_params()->set_tag_size(10);
+  key_proto.mutable_params()->set_hash(HashType::SHA256);
+  RestrictedData serialized_key = RestrictedData(
+      key_proto.SerializeAsString(), InsecureSecretKeyAccess::Get());
+
+  util::StatusOr<internal::ProtoKeySerialization> serialization =
+      internal::ProtoKeySerialization::Create(
+          "type.googleapis.com/google.crypto.tink.HmacKey", serialized_key,
+          KeyData::SYMMETRIC, OutputPrefixType::UNKNOWN_PREFIX,
+          /*id_requirement=*/0x23456789);
+  ASSERT_THAT(serialization, IsOk());
+
+  util::StatusOr<std::unique_ptr<Key>> key =
+      internal::MutableSerializationRegistry::GlobalInstance().ParseKey(
+          *serialization, InsecureSecretKeyAccess::Get());
+  ASSERT_THAT(key.status(), StatusIs(absl::StatusCode::kInvalidArgument));
+}
+
+TEST_F(HmacProtoSerializationTest, ParseKeyWithUnknownHashType) {
+  ASSERT_THAT(RegisterHmacProtoSerialization(), IsOk());
+
+  std::string raw_key_bytes = Random::GetRandomBytes(16);
+  google::crypto::tink::HmacKey key_proto;
+  key_proto.set_version(0);
+  key_proto.set_key_value(raw_key_bytes);
+  key_proto.mutable_params()->set_tag_size(10);
+  key_proto.mutable_params()->set_hash(HashType::UNKNOWN_HASH);
+  RestrictedData serialized_key = RestrictedData(
+      key_proto.SerializeAsString(), InsecureSecretKeyAccess::Get());
+
+  util::StatusOr<internal::ProtoKeySerialization> serialization =
+      internal::ProtoKeySerialization::Create(
+          "type.googleapis.com/google.crypto.tink.HmacKey", serialized_key,
+          KeyData::SYMMETRIC, OutputPrefixType::TINK,
+          /*id_requirement=*/0x23456789);
+  ASSERT_THAT(serialization, IsOk());
+
+  util::StatusOr<std::unique_ptr<Key>> key =
+      internal::MutableSerializationRegistry::GlobalInstance().ParseKey(
+          *serialization, InsecureSecretKeyAccess::Get());
+  ASSERT_THAT(key.status(), StatusIs(absl::StatusCode::kInvalidArgument));
+}
+
 TEST_F(HmacProtoSerializationTest, ParseKeyWithoutSecretKeyAccess) {
   ASSERT_THAT(RegisterHmacProtoSerialization(), IsOk());