From 12f9970b0d0a270abeabfc277840a85b29efe251 Mon Sep 17 00:00:00 2001 From: tholenst <tholenst@google.com> Date: Tue, 20 Aug 2019 07:52:02 -0700 Subject: [PATCH] Migrate the ECIES AEAD HKDF Key Managers to KeyTypeManagers. This uses KeyTypeManagers to implement the ECIES Key Managers; making the code slightly simpler. The main simplification happens when we update the tests in a follow up cl. PiperOrigin-RevId: 264382557 --- cc/core/registry_test.cc | 15 ++- cc/hybrid/BUILD.bazel | 9 +- cc/hybrid/CMakeLists.txt | 8 +- .../ecies_aead_hkdf_private_key_manager.cc | 103 ++++-------------- .../ecies_aead_hkdf_private_key_manager.h | 59 ++++++---- ...cies_aead_hkdf_private_key_manager_test.cc | 91 +++++++++++----- .../ecies_aead_hkdf_public_key_manager.cc | 50 +-------- .../ecies_aead_hkdf_public_key_manager.h | 54 ++++----- ...ecies_aead_hkdf_public_key_manager_test.cc | 41 ++++--- cc/hybrid/hybrid_config.cc | 6 +- cc/hybrid/hybrid_decrypt_factory_test.cc | 6 +- cc/hybrid/hybrid_key_templates_test.cc | 13 +-- python/hybrid/hybrid_key_manager_test.py | 5 +- 13 files changed, 223 insertions(+), 237 deletions(-) diff --git a/cc/core/registry_test.cc b/cc/core/registry_test.cc index 451c37a6e..5aa7ae67c 100644 --- a/cc/core/registry_test.cc +++ b/cc/core/registry_test.cc @@ -517,8 +517,15 @@ TEST_F(RegistryTest, testNewKeyData) { TEST_F(RegistryTest, testGetPublicKeyData) { // Setup the registry. Registry::Reset(); + auto private_key_type_manager = + absl::make_unique<EciesAeadHkdfPrivateKeyManager>(); + auto public_key_type_manager = + absl::make_unique<EciesAeadHkdfPublicKeyManager>(); + auto status = Registry::RegisterKeyManager( - absl::make_unique<EciesAeadHkdfPrivateKeyManager>(), true); + internal::MakePrivateKeyManager<HybridDecrypt>( + private_key_type_manager.get(), public_key_type_manager.get()), + true); ASSERT_TRUE(status.ok()) << status; AesGcmKeyManager key_type_manager; status = Registry::RegisterKeyManager( @@ -532,11 +539,11 @@ TEST_F(RegistryTest, testGetPublicKeyData) { // Extract public key data and check. auto public_key_data_result = Registry::GetPublicKeyData( - EciesAeadHkdfPrivateKeyManager::static_key_type(), + EciesAeadHkdfPrivateKeyManager().get_key_type(), ecies_key.SerializeAsString()); EXPECT_TRUE(public_key_data_result.ok()) << public_key_data_result.status(); auto public_key_data = std::move(public_key_data_result.ValueOrDie()); - EXPECT_EQ(EciesAeadHkdfPublicKeyManager::static_key_type(), + EXPECT_EQ(EciesAeadHkdfPublicKeyManager().get_key_type(), public_key_data->type_url()); EXPECT_EQ(KeyData::ASYMMETRIC_PUBLIC, public_key_data->key_material_type()); EXPECT_EQ(ecies_key.public_key().SerializeAsString(), @@ -553,7 +560,7 @@ TEST_F(RegistryTest, testGetPublicKeyData) { // Try with a bad serialized key. auto bad_key_result = Registry::GetPublicKeyData( - EciesAeadHkdfPrivateKeyManager::static_key_type(), + EciesAeadHkdfPrivateKeyManager().get_key_type(), "some bad serialized key"); EXPECT_FALSE(bad_key_result.ok()); EXPECT_EQ(util::error::INVALID_ARGUMENT, diff --git a/cc/hybrid/BUILD.bazel b/cc/hybrid/BUILD.bazel index b6a40c9cb..2254ae6bd 100644 --- a/cc/hybrid/BUILD.bazel +++ b/cc/hybrid/BUILD.bazel @@ -195,10 +195,12 @@ cc_library( deps = [ ":ecies_aead_hkdf_hybrid_decrypt", ":ecies_aead_hkdf_public_key_manager", + "//cc:core/key_type_manager", + "//cc:core/private_key_type_manager", "//cc:hybrid_decrypt", "//cc:key_manager", - "//cc:key_manager_base", "//cc/subtle:subtle_util_boringssl", + "//cc/util:constants", "//cc/util:enums", "//cc/util:errors", "//cc/util:protobuf_helper", @@ -224,10 +226,11 @@ cc_library( ], deps = [ ":ecies_aead_hkdf_hybrid_encrypt", + "//cc:core/key_type_manager", "//cc:hybrid_encrypt", "//cc:key_manager", - "//cc:key_manager_base", "//cc:registry", + "//cc/util:constants", "//cc/util:protobuf_helper", "//cc/util:status", "//cc/util:statusor", @@ -409,6 +412,7 @@ cc_test( ":ecies_aead_hkdf_private_key_manager", ":ecies_aead_hkdf_public_key_manager", ":hybrid_key_templates", + "//cc:core/private_key_manager_impl", "//cc:hybrid_decrypt", "//cc/aead:aead_key_templates", "//cc/aead:aes_ctr_hmac_aead_key_manager", @@ -430,6 +434,7 @@ cc_test( srcs = ["ecies_aead_hkdf_public_key_manager_test.cc"], copts = ["-Iexternal/gtest/include"], deps = [ + ":ecies_aead_hkdf_private_key_manager", ":ecies_aead_hkdf_public_key_manager", "//cc:hybrid_encrypt", "//cc:registry", diff --git a/cc/hybrid/CMakeLists.txt b/cc/hybrid/CMakeLists.txt index d4e78b1e5..c6f323032 100644 --- a/cc/hybrid/CMakeLists.txt +++ b/cc/hybrid/CMakeLists.txt @@ -167,8 +167,10 @@ tink_cc_library( tink::hybrid::ecies_aead_hkdf_public_key_manager tink::core::hybrid_decrypt tink::core::key_manager - tink::core::key_manager_base + tink::core::key_type_manager + tink::core::private_key_type_manager tink::subtle::subtle_util_boringssl + tink::util::constants tink::util::enums tink::util::errors tink::util::protobuf_helper @@ -190,8 +192,9 @@ tink_cc_library( tink::hybrid::ecies_aead_hkdf_hybrid_encrypt tink::core::hybrid_encrypt tink::core::key_manager - tink::core::key_manager_base + tink::core::key_type_manager tink::core::registry + tink::util::constants tink::util::protobuf_helper tink::util::status tink::util::statusor @@ -339,6 +342,7 @@ tink_cc_test( tink::hybrid::ecies_aead_hkdf_public_key_manager tink::hybrid::hybrid_key_templates tink::core::hybrid_decrypt + tink::core::private_key_manager_impl tink::aead::aead_key_templates tink::aead::aes_ctr_hmac_aead_key_manager tink::aead::aes_gcm_key_manager diff --git a/cc/hybrid/ecies_aead_hkdf_private_key_manager.cc b/cc/hybrid/ecies_aead_hkdf_private_key_manager.cc index dfe2e09f9..03b41a57b 100644 --- a/cc/hybrid/ecies_aead_hkdf_private_key_manager.cc +++ b/cc/hybrid/ecies_aead_hkdf_private_key_manager.cc @@ -39,37 +39,20 @@ using crypto::tink::util::Status; using crypto::tink::util::StatusOr; using google::crypto::tink::EciesAeadHkdfKeyFormat; using google::crypto::tink::EciesAeadHkdfPrivateKey; +using google::crypto::tink::EciesAeadHkdfPublicKey; using google::crypto::tink::EciesHkdfKemParams; -using google::crypto::tink::KeyData; -class EciesAeadHkdfPrivateKeyFactory - : public PrivateKeyFactory, - public KeyFactoryBase<EciesAeadHkdfPrivateKey, - EciesAeadHkdfKeyFormat> { - public: - EciesAeadHkdfPrivateKeyFactory() {} - - KeyData::KeyMaterialType key_material_type() const override { - return KeyData::ASYMMETRIC_PRIVATE; +Status EciesAeadHkdfPrivateKeyManager::ValidateKeyFormat( + const EciesAeadHkdfKeyFormat& key_format) const { + if (!key_format.has_params()) { + return Status(util::error::INVALID_ARGUMENT, "Missing params."); } + return EciesAeadHkdfPublicKeyManager().ValidateParams(key_format.params()); +} - // Returns KeyData proto that contains EciesAeadHkdfPublicKey - // extracted from the given serialized_private_key, which must contain - // EciesAeadHkdfPrivateKey-proto. - crypto::tink::util::StatusOr<std::unique_ptr<google::crypto::tink::KeyData>> - GetPublicKeyData(absl::string_view serialized_private_key) const override; - - protected: - StatusOr<std::unique_ptr<EciesAeadHkdfPrivateKey>> NewKeyFromFormat( - const EciesAeadHkdfKeyFormat& ecies_key_format) const override; -}; - -StatusOr<std::unique_ptr<EciesAeadHkdfPrivateKey>> -EciesAeadHkdfPrivateKeyFactory::NewKeyFromFormat( +StatusOr<EciesAeadHkdfPrivateKey> +EciesAeadHkdfPrivateKeyManager::CreateKey( const EciesAeadHkdfKeyFormat& ecies_key_format) const { - Status status = EciesAeadHkdfPublicKeyManager::Validate(ecies_key_format); - if (!status.ok()) return status; - // Generate new EC key. const EciesHkdfKemParams& kem_params = ecies_key_format.params().kem_params(); auto ec_key_result = subtle::SubtleUtilBoringSSL::GetNewEcKey( @@ -78,72 +61,32 @@ EciesAeadHkdfPrivateKeyFactory::NewKeyFromFormat( auto ec_key = ec_key_result.ValueOrDie(); // Build EciesAeadHkdfPrivateKey. - std::unique_ptr<EciesAeadHkdfPrivateKey> ecies_private_key( - new EciesAeadHkdfPrivateKey()); - ecies_private_key->set_version(EciesAeadHkdfPrivateKeyManager::kVersion); - ecies_private_key->set_key_value(ec_key.priv); - auto ecies_public_key = ecies_private_key->mutable_public_key(); - ecies_public_key->set_version(EciesAeadHkdfPrivateKeyManager::kVersion); + EciesAeadHkdfPrivateKey ecies_private_key; + ecies_private_key.set_version(get_version()); + ecies_private_key.set_key_value(ec_key.priv); + auto ecies_public_key = ecies_private_key.mutable_public_key(); + ecies_public_key->set_version(get_version()); ecies_public_key->set_x(ec_key.pub_x); ecies_public_key->set_y(ec_key.pub_y); *(ecies_public_key->mutable_params()) = ecies_key_format.params(); - return absl::implicit_cast< - StatusOr<std::unique_ptr<EciesAeadHkdfPrivateKey>>>( - std::move(ecies_private_key)); -} - -StatusOr<std::unique_ptr<KeyData>> -EciesAeadHkdfPrivateKeyFactory::GetPublicKeyData( - absl::string_view serialized_private_key) const { - EciesAeadHkdfPrivateKey private_key; - if (!private_key.ParseFromString(std::string(serialized_private_key))) { - return ToStatusF(util::error::INVALID_ARGUMENT, - "Could not parse the passed string as proto '%s'.", - EciesAeadHkdfPrivateKeyManager::static_key_type().c_str()); - } - auto status = EciesAeadHkdfPrivateKeyManager::Validate(private_key); - if (!status.ok()) return status; - auto key_data = absl::make_unique<KeyData>(); - key_data->set_type_url(EciesAeadHkdfPublicKeyManager::static_key_type()); - key_data->set_value(private_key.public_key().SerializeAsString()); - key_data->set_key_material_type(KeyData:: ASYMMETRIC_PUBLIC); - return std::move(key_data); -} - -constexpr uint32_t EciesAeadHkdfPrivateKeyManager::kVersion; - -EciesAeadHkdfPrivateKeyManager::EciesAeadHkdfPrivateKeyManager() - : key_factory_(new EciesAeadHkdfPrivateKeyFactory()) { + return ecies_private_key; } -const KeyFactory& EciesAeadHkdfPrivateKeyManager::get_key_factory() const { - return *key_factory_; -} - -uint32_t EciesAeadHkdfPrivateKeyManager::get_version() const { - return kVersion; -} - -StatusOr<std::unique_ptr<HybridDecrypt>> -EciesAeadHkdfPrivateKeyManager::GetPrimitiveFromKey( - const EciesAeadHkdfPrivateKey& ecies_private_key) const { - Status status = Validate(ecies_private_key); - if (!status.ok()) return status; - auto ecies_result = EciesAeadHkdfHybridDecrypt::New(ecies_private_key); - if (!ecies_result.ok()) return ecies_result.status(); - return std::move(ecies_result.ValueOrDie()); +StatusOr<EciesAeadHkdfPublicKey> +EciesAeadHkdfPrivateKeyManager::GetPublicKey( + const EciesAeadHkdfPrivateKey& private_key) const { + return private_key.public_key(); } -// static -Status EciesAeadHkdfPrivateKeyManager::Validate( - const EciesAeadHkdfPrivateKey& key) { - Status status = ValidateVersion(key.version(), kVersion); +Status EciesAeadHkdfPrivateKeyManager::ValidateKey( + const EciesAeadHkdfPrivateKey& key) const { + Status status = ValidateVersion(key.version(), get_version()); if (!status.ok()) return status; if (!key.has_public_key()) { return Status(util::error::INVALID_ARGUMENT, "Missing public_key."); } - return EciesAeadHkdfPublicKeyManager::Validate(key.public_key()); + return EciesAeadHkdfPublicKeyManager().ValidateKey(key.public_key()); } } // namespace tink diff --git a/cc/hybrid/ecies_aead_hkdf_private_key_manager.h b/cc/hybrid/ecies_aead_hkdf_private_key_manager.h index 036f9122c..9c3cbabac 100644 --- a/cc/hybrid/ecies_aead_hkdf_private_key_manager.h +++ b/cc/hybrid/ecies_aead_hkdf_private_key_manager.h @@ -20,9 +20,12 @@ #include <vector> #include "absl/strings/string_view.h" -#include "tink/core/key_manager_base.h" +#include "tink/core/key_type_manager.h" +#include "tink/core/private_key_type_manager.h" +#include "tink/hybrid/ecies_aead_hkdf_hybrid_decrypt.h" #include "tink/hybrid_decrypt.h" #include "tink/key_manager.h" +#include "tink/util/constants.h" #include "tink/util/errors.h" #include "tink/util/protobuf_helper.h" #include "tink/util/status.h" @@ -34,34 +37,50 @@ namespace crypto { namespace tink { class EciesAeadHkdfPrivateKeyManager - : public KeyManagerBase<HybridDecrypt, - google::crypto::tink::EciesAeadHkdfPrivateKey> { + : public PrivateKeyTypeManager< + google::crypto::tink::EciesAeadHkdfPrivateKey, + google::crypto::tink::EciesAeadHkdfKeyFormat, + google::crypto::tink::EciesAeadHkdfPublicKey, List<HybridDecrypt>> { public: - static constexpr uint32_t kVersion = 0; + class HybridDecryptFactory : public PrimitiveFactory<HybridDecrypt> { + crypto::tink::util::StatusOr<std::unique_ptr<HybridDecrypt>> Create( + const google::crypto::tink::EciesAeadHkdfPrivateKey& ecies_private_key) + const override { + return EciesAeadHkdfHybridDecrypt::New(ecies_private_key); + } + }; - EciesAeadHkdfPrivateKeyManager(); + EciesAeadHkdfPrivateKeyManager() + : PrivateKeyTypeManager(absl::make_unique<HybridDecryptFactory>()) {} - // Returns the version of this key manager. - uint32_t get_version() const override; + uint32_t get_version() const override { return 0; } - // Returns a factory that generates keys of the key type - // handled by this manager. - const KeyFactory& get_key_factory() const override; + google::crypto::tink::KeyData::KeyMaterialType key_material_type() + const override { + return google::crypto::tink::KeyData::ASYMMETRIC_PRIVATE; + } - virtual ~EciesAeadHkdfPrivateKeyManager() {} + const std::string& get_key_type() const override { return key_type_; } - protected: - crypto::tink::util::StatusOr<std::unique_ptr<HybridDecrypt>> - GetPrimitiveFromKey(const google::crypto::tink::EciesAeadHkdfPrivateKey& - ecies_private_key) const override; + crypto::tink::util::Status ValidateKey( + const google::crypto::tink::EciesAeadHkdfPrivateKey& key) const override; - private: - friend class EciesAeadHkdfPrivateKeyFactory; + crypto::tink::util::Status ValidateKeyFormat( + const google::crypto::tink::EciesAeadHkdfKeyFormat& ecies_key_format) + const override; + + crypto::tink::util::StatusOr<google::crypto::tink::EciesAeadHkdfPrivateKey> + CreateKey(const google::crypto::tink::EciesAeadHkdfKeyFormat& key_format) + const override; - std::unique_ptr<KeyFactory> key_factory_; + crypto::tink::util::StatusOr<google::crypto::tink::EciesAeadHkdfPublicKey> + GetPublicKey(const google::crypto::tink::EciesAeadHkdfPrivateKey& private_key) + const override; - static crypto::tink::util::Status Validate( - const google::crypto::tink::EciesAeadHkdfPrivateKey& key); + private: + const std::string key_type_ = absl::StrCat( + kTypeGoogleapisCom, + google::crypto::tink::EciesAeadHkdfPrivateKey().GetTypeName()); }; } // namespace tink diff --git a/cc/hybrid/ecies_aead_hkdf_private_key_manager_test.cc b/cc/hybrid/ecies_aead_hkdf_private_key_manager_test.cc index 5b93760e5..d67efbd9f 100644 --- a/cc/hybrid/ecies_aead_hkdf_private_key_manager_test.cc +++ b/cc/hybrid/ecies_aead_hkdf_private_key_manager_test.cc @@ -16,6 +16,7 @@ #include "tink/hybrid/ecies_aead_hkdf_private_key_manager.h" +#include "tink/core/private_key_manager_impl.h" #include "tink/hybrid_decrypt.h" #include "tink/registry.h" #include "tink/aead/aead_key_templates.h" @@ -64,7 +65,9 @@ class EciesAeadHkdfPrivateKeyManagerTest : public ::testing::Test { // Checks whether given key is compatible with the given format. void CheckNewKey(const EciesAeadHkdfPrivateKey& ecies_key, const EciesAeadHkdfKeyFormat& key_format) { - EciesAeadHkdfPrivateKeyManager key_manager; + EciesAeadHkdfPrivateKeyManager key_type_manager; + auto key_manager = + internal::MakeKeyManager<HybridDecrypt>(&key_type_manager); EXPECT_EQ(0, ecies_key.version()); EXPECT_TRUE(ecies_key.has_public_key()); EXPECT_GT(ecies_key.key_value().length(), 0); @@ -73,28 +76,32 @@ void CheckNewKey(const EciesAeadHkdfPrivateKey& ecies_key, EXPECT_GT(ecies_key.public_key().y().length(), 0); EXPECT_EQ(ecies_key.public_key().params().SerializeAsString(), key_format.params().SerializeAsString()); - auto primitive_result = key_manager.GetPrimitive(ecies_key); + auto primitive_result = key_manager->GetPrimitive(ecies_key); EXPECT_TRUE(primitive_result.ok()) << primitive_result.status(); } TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testBasic) { - EciesAeadHkdfPrivateKeyManager key_manager; + EciesAeadHkdfPrivateKeyManager key_type_manager; + auto key_manager = + internal::MakeKeyManager<HybridDecrypt>(&key_type_manager); - EXPECT_EQ(0, key_manager.get_version()); + EXPECT_EQ(0, key_manager->get_version()); EXPECT_EQ("type.googleapis.com/google.crypto.tink.EciesAeadHkdfPrivateKey", - key_manager.get_key_type()); - EXPECT_TRUE(key_manager.DoesSupport(key_manager.get_key_type())); + key_manager->get_key_type()); + EXPECT_TRUE(key_manager->DoesSupport(key_manager->get_key_type())); } TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testKeyDataErrors) { - EciesAeadHkdfPrivateKeyManager key_manager; + EciesAeadHkdfPrivateKeyManager key_type_manager; + auto key_manager = + internal::MakeKeyManager<HybridDecrypt>(&key_type_manager); { // Bad key type. KeyData key_data; std::string bad_key_type = "type.googleapis.com/google.crypto.tink.SomeOtherKey"; key_data.set_type_url(bad_key_type); - auto result = key_manager.GetPrimitive(key_data); + auto result = key_manager->GetPrimitive(key_data); EXPECT_FALSE(result.ok()); EXPECT_EQ(util::error::INVALID_ARGUMENT, result.status().error_code()); EXPECT_PRED_FORMAT2(testing::IsSubstring, "not supported", @@ -107,7 +114,7 @@ TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testKeyDataErrors) { KeyData key_data; key_data.set_type_url(ecies_private_key_type); key_data.set_value("some bad serialized proto"); - auto result = key_manager.GetPrimitive(key_data); + auto result = key_manager->GetPrimitive(key_data); EXPECT_FALSE(result.ok()); EXPECT_EQ(util::error::INVALID_ARGUMENT, result.status().error_code()); EXPECT_PRED_FORMAT2(testing::IsSubstring, "not parse", @@ -120,7 +127,7 @@ TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testKeyDataErrors) { key.set_version(1); key_data.set_type_url(ecies_private_key_type); key_data.set_value(key.SerializeAsString()); - auto result = key_manager.GetPrimitive(key_data); + auto result = key_manager->GetPrimitive(key_data); EXPECT_FALSE(result.ok()); EXPECT_EQ(util::error::INVALID_ARGUMENT, result.status().error_code()); EXPECT_PRED_FORMAT2(testing::IsSubstring, "version", @@ -129,11 +136,13 @@ TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testKeyDataErrors) { } TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testKeyMessageErrors) { - EciesAeadHkdfPrivateKeyManager key_manager; + EciesAeadHkdfPrivateKeyManager key_type_manager; + auto key_manager = + internal::MakeKeyManager<HybridDecrypt>(&key_type_manager); { // Bad protobuffer. AesEaxKey key; - auto result = key_manager.GetPrimitive(key); + auto result = key_manager->GetPrimitive(key); EXPECT_FALSE(result.ok()); EXPECT_EQ(util::error::INVALID_ARGUMENT, result.status().error_code()); EXPECT_PRED_FORMAT2(testing::IsSubstring, "AesEaxKey", @@ -146,18 +155,24 @@ TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testKeyMessageErrors) { TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testPrimitives) { std::string plaintext = "some plaintext"; std::string context_info = "some context info"; - EciesAeadHkdfPublicKeyManager public_key_manager; - EciesAeadHkdfPrivateKeyManager private_key_manager; + EciesAeadHkdfPublicKeyManager public_key_type_manager; + EciesAeadHkdfPrivateKeyManager private_key_type_manager; + auto public_key_manager = + internal::MakeKeyManager<HybridEncrypt>(&public_key_type_manager); + auto private_key_manager = + internal::MakePrivateKeyManager<HybridDecrypt>(&private_key_type_manager, + &public_key_type_manager); + EciesAeadHkdfPrivateKey key = test::GetEciesAesGcmHkdfTestKey( EllipticCurveType::NIST_P256, EcPointFormat::UNCOMPRESSED, HashType::SHA256, 32); - auto hybrid_encrypt = std::move(public_key_manager.GetPrimitive( + auto hybrid_encrypt = std::move(public_key_manager->GetPrimitive( key.public_key()).ValueOrDie()); std::string ciphertext = hybrid_encrypt->Encrypt(plaintext, context_info).ValueOrDie(); { // Using Key proto. - auto result = private_key_manager.GetPrimitive(key); + auto result = private_key_manager->GetPrimitive(key); EXPECT_TRUE(result.ok()) << result.status(); auto hybrid_decrypt = std::move(result.ValueOrDie()); auto decrypt_result = hybrid_decrypt->Decrypt(ciphertext, context_info); @@ -168,7 +183,7 @@ TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testPrimitives) { KeyData key_data; key_data.set_type_url(ecies_private_key_type); key_data.set_value(key.SerializeAsString()); - auto result = private_key_manager.GetPrimitive(key_data); + auto result = private_key_manager->GetPrimitive(key_data); EXPECT_TRUE(result.ok()) << result.status(); auto hybrid_decrypt = std::move(result.ValueOrDie()); auto decrypt_result = hybrid_decrypt->Decrypt(ciphertext, context_info); @@ -177,8 +192,14 @@ TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testPrimitives) { } TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testNewKeyCreation) { - EciesAeadHkdfPrivateKeyManager key_manager; - const KeyFactory& key_factory = key_manager.get_key_factory(); + EciesAeadHkdfPublicKeyManager public_key_type_manager; + EciesAeadHkdfPrivateKeyManager private_key_type_manager; + auto public_key_manager = + internal::MakeKeyManager<HybridEncrypt>(&public_key_type_manager); + auto private_key_manager = + internal::MakePrivateKeyManager<HybridDecrypt>(&private_key_type_manager, + &public_key_type_manager); + const KeyFactory& key_factory = private_key_manager->get_key_factory(); { // Via NewKey(format_proto). EciesAeadHkdfKeyFormat key_format; @@ -253,9 +274,15 @@ TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testNewKeyCreation) { } TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testPublicKeyExtraction) { - EciesAeadHkdfPrivateKeyManager key_manager; + EciesAeadHkdfPublicKeyManager public_key_type_manager; + EciesAeadHkdfPrivateKeyManager private_key_type_manager; + auto public_key_manager = + internal::MakeKeyManager<HybridEncrypt>(&public_key_type_manager); + auto private_key_manager = + internal::MakePrivateKeyManager<HybridDecrypt>(&private_key_type_manager, + &public_key_type_manager); auto private_key_factory = dynamic_cast<const PrivateKeyFactory*>( - &(key_manager.get_key_factory())); + &(private_key_manager->get_key_factory())); ASSERT_NE(private_key_factory, nullptr); auto new_key_result = private_key_factory->NewKey( @@ -267,7 +294,7 @@ TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testPublicKeyExtraction) { new_key->SerializeAsString()); EXPECT_TRUE(public_key_data_result.ok()) << public_key_data_result.status(); auto public_key_data = std::move(public_key_data_result.ValueOrDie()); - EXPECT_EQ(EciesAeadHkdfPublicKeyManager::static_key_type(), + EXPECT_EQ(EciesAeadHkdfPublicKeyManager().get_key_type(), public_key_data->type_url()); EXPECT_EQ(KeyData::ASYMMETRIC_PUBLIC, public_key_data->key_material_type()); EXPECT_EQ(new_key->public_key().SerializeAsString(), @@ -275,9 +302,15 @@ TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testPublicKeyExtraction) { } TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testPublicKeyExtractionErrors) { - EciesAeadHkdfPrivateKeyManager key_manager; + EciesAeadHkdfPublicKeyManager public_key_type_manager; + EciesAeadHkdfPrivateKeyManager private_key_type_manager; + auto public_key_manager = + internal::MakeKeyManager<HybridEncrypt>(&public_key_type_manager); + auto private_key_manager = + internal::MakePrivateKeyManager<HybridDecrypt>(&private_key_type_manager, + &public_key_type_manager); auto private_key_factory = dynamic_cast<const PrivateKeyFactory*>( - &(key_manager.get_key_factory())); + &(private_key_manager->get_key_factory())); ASSERT_NE(private_key_factory, nullptr); AesCtrHmacAeadKeyManager aead_key_manager; @@ -297,8 +330,14 @@ TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testPublicKeyExtractionErrors) { } TEST_F(EciesAeadHkdfPrivateKeyManagerTest, testNewKeyErrors) { - EciesAeadHkdfPrivateKeyManager key_manager; - const KeyFactory& key_factory = key_manager.get_key_factory(); + EciesAeadHkdfPublicKeyManager public_key_type_manager; + EciesAeadHkdfPrivateKeyManager private_key_type_manager; + auto public_key_manager = + internal::MakeKeyManager<HybridEncrypt>(&public_key_type_manager); + auto private_key_manager = + internal::MakePrivateKeyManager<HybridDecrypt>(&private_key_type_manager, + &public_key_type_manager); + const KeyFactory& key_factory = private_key_manager->get_key_factory(); // Empty key format. EciesAeadHkdfKeyFormat key_format; diff --git a/cc/hybrid/ecies_aead_hkdf_public_key_manager.cc b/cc/hybrid/ecies_aead_hkdf_public_key_manager.cc index 9a0243e74..bdfa9a65e 100644 --- a/cc/hybrid/ecies_aead_hkdf_public_key_manager.cc +++ b/cc/hybrid/ecies_aead_hkdf_public_key_manager.cc @@ -33,43 +33,14 @@ namespace crypto { namespace tink { using crypto::tink::util::Status; -using crypto::tink::util::StatusOr; -using google::crypto::tink::EciesAeadHkdfKeyFormat; using google::crypto::tink::EciesAeadHkdfParams; using google::crypto::tink::EciesAeadHkdfPublicKey; using google::crypto::tink::EcPointFormat; using google::crypto::tink::EllipticCurveType; using google::crypto::tink::HashType; -constexpr uint32_t EciesAeadHkdfPublicKeyManager::kVersion; - -EciesAeadHkdfPublicKeyManager::EciesAeadHkdfPublicKeyManager() - : key_factory_(KeyFactory::AlwaysFailingFactory( - util::Status(util::error::UNIMPLEMENTED, - "Operation not supported for public keys, " - "please use EciesAeadHkdfPrivateKeyManager."))) {} - -const KeyFactory& EciesAeadHkdfPublicKeyManager::get_key_factory() const { - return *key_factory_; -} - -uint32_t EciesAeadHkdfPublicKeyManager::get_version() const { - return kVersion; -} - -StatusOr<std::unique_ptr<HybridEncrypt>> -EciesAeadHkdfPublicKeyManager::GetPrimitiveFromKey( - const EciesAeadHkdfPublicKey& recipient_key) const { - Status status = Validate(recipient_key); - if (!status.ok()) return status; - auto ecies_result = EciesAeadHkdfHybridEncrypt::New(recipient_key); - if (!ecies_result.ok()) return ecies_result.status(); - return std::move(ecies_result.ValueOrDie()); -} - -// static -Status EciesAeadHkdfPublicKeyManager::Validate( - const EciesAeadHkdfParams& params) { +Status EciesAeadHkdfPublicKeyManager::ValidateParams( + const EciesAeadHkdfParams& params) const { // Validate KEM params. if (!params.has_kem_params()) { return Status(util::error::INVALID_ARGUMENT, "Missing kem_params."); @@ -94,25 +65,16 @@ Status EciesAeadHkdfPublicKeyManager::Validate( return Status::OK; } -// static -Status EciesAeadHkdfPublicKeyManager::Validate( - const EciesAeadHkdfPublicKey& key) { - Status status = ValidateVersion(key.version(), kVersion); +Status EciesAeadHkdfPublicKeyManager::ValidateKey( + const EciesAeadHkdfPublicKey& key) const { + Status status = ValidateVersion(key.version(), get_version()); if (!status.ok()) return status; if (!key.has_params()) { return Status(util::error::INVALID_ARGUMENT, "Missing params."); } - return Validate(key.params()); + return ValidateParams(key.params()); } -// static -Status EciesAeadHkdfPublicKeyManager::Validate( - const EciesAeadHkdfKeyFormat& key_format) { - if (!key_format.has_params()) { - return Status(util::error::INVALID_ARGUMENT, "Missing params."); - } - return Validate(key_format.params()); -} } // namespace tink } // namespace crypto diff --git a/cc/hybrid/ecies_aead_hkdf_public_key_manager.h b/cc/hybrid/ecies_aead_hkdf_public_key_manager.h index e76389efa..d95fc7694 100644 --- a/cc/hybrid/ecies_aead_hkdf_public_key_manager.h +++ b/cc/hybrid/ecies_aead_hkdf_public_key_manager.h @@ -20,9 +20,11 @@ #include <vector> #include "absl/strings/string_view.h" -#include "tink/core/key_manager_base.h" +#include "tink/core/key_type_manager.h" +#include "tink/hybrid/ecies_aead_hkdf_hybrid_encrypt.h" #include "tink/hybrid_encrypt.h" #include "tink/key_manager.h" +#include "tink/util/constants.h" #include "tink/util/errors.h" #include "tink/util/protobuf_helper.h" #include "tink/util/status.h" @@ -34,40 +36,38 @@ namespace crypto { namespace tink { class EciesAeadHkdfPublicKeyManager - : public KeyManagerBase<HybridEncrypt, - google::crypto::tink::EciesAeadHkdfPublicKey> { + : public KeyTypeManager<google::crypto::tink::EciesAeadHkdfPublicKey, void, + List<HybridEncrypt>> { public: - static constexpr uint32_t kVersion = 0; + class HybridEncryptFactory : public PrimitiveFactory<HybridEncrypt> { + crypto::tink::util::StatusOr<std::unique_ptr<HybridEncrypt>> Create( + const google::crypto::tink::EciesAeadHkdfPublicKey& ecies_public_key) + const override { + return EciesAeadHkdfHybridEncrypt::New(ecies_public_key); + } + }; - EciesAeadHkdfPublicKeyManager(); + EciesAeadHkdfPublicKeyManager() + : KeyTypeManager(absl::make_unique<HybridEncryptFactory>()) {} - // Returns the version of this key manager. - uint32_t get_version() const override; + uint32_t get_version() const override { return 0; } - // Returns a factory that generates keys of the key type - // handled by this manager. - const KeyFactory& get_key_factory() const override; + google::crypto::tink::KeyData::KeyMaterialType key_material_type() + const override { + return google::crypto::tink::KeyData::ASYMMETRIC_PUBLIC; + } - virtual ~EciesAeadHkdfPublicKeyManager() {} + const std::string& get_key_type() const override { return key_type_; } - protected: - crypto::tink::util::StatusOr<std::unique_ptr<HybridEncrypt>> - GetPrimitiveFromKey(const google::crypto::tink::EciesAeadHkdfPublicKey& - recipient_key) const override; + crypto::tink::util::Status ValidateKey( + const google::crypto::tink::EciesAeadHkdfPublicKey& key) const override; + crypto::tink::util::Status ValidateParams( + const google::crypto::tink::EciesAeadHkdfParams& params) const; private: - // Friends that re-use proto validation helpers. - friend class EciesAeadHkdfPrivateKeyFactory; - friend class EciesAeadHkdfPrivateKeyManager; - - std::unique_ptr<KeyFactory> key_factory_; - - static crypto::tink::util::Status Validate( - const google::crypto::tink::EciesAeadHkdfParams& params); - static crypto::tink::util::Status Validate( - const google::crypto::tink::EciesAeadHkdfPublicKey& key); - static crypto::tink::util::Status Validate( - const google::crypto::tink::EciesAeadHkdfKeyFormat& key_format); + const std::string key_type_ = absl::StrCat( + kTypeGoogleapisCom, + google::crypto::tink::EciesAeadHkdfPublicKey().GetTypeName()); }; } // namespace tink diff --git a/cc/hybrid/ecies_aead_hkdf_public_key_manager_test.cc b/cc/hybrid/ecies_aead_hkdf_public_key_manager_test.cc index fce2be31b..4d38cb5e9 100644 --- a/cc/hybrid/ecies_aead_hkdf_public_key_manager_test.cc +++ b/cc/hybrid/ecies_aead_hkdf_public_key_manager_test.cc @@ -55,23 +55,26 @@ class EciesAeadHkdfPublicKeyManagerTest : public ::testing::Test { }; TEST_F(EciesAeadHkdfPublicKeyManagerTest, testBasic) { - EciesAeadHkdfPublicKeyManager key_manager; - - EXPECT_EQ(0, key_manager.get_version()); + EciesAeadHkdfPublicKeyManager key_type_manager; + auto key_manager = + internal::MakeKeyManager<HybridEncrypt>(&key_type_manager); + EXPECT_EQ(0, key_manager->get_version()); EXPECT_EQ("type.googleapis.com/google.crypto.tink.EciesAeadHkdfPublicKey", - key_manager.get_key_type()); - EXPECT_TRUE(key_manager.DoesSupport(key_manager.get_key_type())); + key_manager->get_key_type()); + EXPECT_TRUE(key_manager->DoesSupport(key_manager->get_key_type())); } TEST_F(EciesAeadHkdfPublicKeyManagerTest, testKeyDataErrors) { - EciesAeadHkdfPublicKeyManager key_manager; + EciesAeadHkdfPublicKeyManager key_type_manager; + auto key_manager = + internal::MakeKeyManager<HybridEncrypt>(&key_type_manager); { // Bad key type. KeyData key_data; std::string bad_key_type = "type.googleapis.com/google.crypto.tink.SomeOtherKey"; key_data.set_type_url(bad_key_type); - auto result = key_manager.GetPrimitive(key_data); + auto result = key_manager->GetPrimitive(key_data); EXPECT_FALSE(result.ok()); EXPECT_EQ(util::error::INVALID_ARGUMENT, result.status().error_code()); EXPECT_PRED_FORMAT2(testing::IsSubstring, "not supported", @@ -84,7 +87,7 @@ TEST_F(EciesAeadHkdfPublicKeyManagerTest, testKeyDataErrors) { KeyData key_data; key_data.set_type_url(ecies_public_key_type); key_data.set_value("some bad serialized proto"); - auto result = key_manager.GetPrimitive(key_data); + auto result = key_manager->GetPrimitive(key_data); EXPECT_FALSE(result.ok()); EXPECT_EQ(util::error::INVALID_ARGUMENT, result.status().error_code()); EXPECT_PRED_FORMAT2(testing::IsSubstring, "not parse", @@ -97,7 +100,7 @@ TEST_F(EciesAeadHkdfPublicKeyManagerTest, testKeyDataErrors) { key.set_version(1); key_data.set_type_url(ecies_public_key_type); key_data.set_value(key.SerializeAsString()); - auto result = key_manager.GetPrimitive(key_data); + auto result = key_manager->GetPrimitive(key_data); EXPECT_FALSE(result.ok()); EXPECT_EQ(util::error::INVALID_ARGUMENT, result.status().error_code()); EXPECT_PRED_FORMAT2(testing::IsSubstring, "version", @@ -106,11 +109,13 @@ TEST_F(EciesAeadHkdfPublicKeyManagerTest, testKeyDataErrors) { } TEST_F(EciesAeadHkdfPublicKeyManagerTest, testKeyMessageErrors) { - EciesAeadHkdfPublicKeyManager key_manager; + EciesAeadHkdfPublicKeyManager key_type_manager; + auto key_manager = + internal::MakeKeyManager<HybridEncrypt>(&key_type_manager); { // Bad protobuffer. AesEaxKey key; - auto result = key_manager.GetPrimitive(key); + auto result = key_manager->GetPrimitive(key); EXPECT_FALSE(result.ok()); EXPECT_EQ(util::error::INVALID_ARGUMENT, result.status().error_code()); EXPECT_PRED_FORMAT2(testing::IsSubstring, "AesEaxKey", @@ -123,13 +128,15 @@ TEST_F(EciesAeadHkdfPublicKeyManagerTest, testKeyMessageErrors) { TEST_F(EciesAeadHkdfPublicKeyManagerTest, testPrimitives) { std::string plaintext = "some plaintext"; std::string context_info = "some context info"; - EciesAeadHkdfPublicKeyManager key_manager; + EciesAeadHkdfPublicKeyManager key_type_manager; + auto key_manager = + internal::MakeKeyManager<HybridEncrypt>(&key_type_manager); EciesAeadHkdfPublicKey key = test::GetEciesAesGcmHkdfTestKey( EllipticCurveType::NIST_P256, EcPointFormat::UNCOMPRESSED, HashType::SHA256, 32).public_key(); { // Using Key proto. - auto result = key_manager.GetPrimitive(key); + auto result = key_manager->GetPrimitive(key); EXPECT_TRUE(result.ok()) << result.status(); auto hybrid_encrypt = std::move(result.ValueOrDie()); auto encrypt_result = hybrid_encrypt->Encrypt(plaintext, context_info); @@ -140,7 +147,7 @@ TEST_F(EciesAeadHkdfPublicKeyManagerTest, testPrimitives) { KeyData key_data; key_data.set_type_url(ecies_public_key_type); key_data.set_value(key.SerializeAsString()); - auto result = key_manager.GetPrimitive(key_data); + auto result = key_manager->GetPrimitive(key_data); EXPECT_TRUE(result.ok()) << result.status(); auto hybrid_encrypt = std::move(result.ValueOrDie()); auto encrypt_result = hybrid_encrypt->Encrypt(plaintext, context_info); @@ -149,8 +156,10 @@ TEST_F(EciesAeadHkdfPublicKeyManagerTest, testPrimitives) { } TEST_F(EciesAeadHkdfPublicKeyManagerTest, testNewKeyError) { - EciesAeadHkdfPublicKeyManager key_manager; - const KeyFactory& key_factory = key_manager.get_key_factory(); + EciesAeadHkdfPublicKeyManager key_type_manager; + auto key_manager = + internal::MakeKeyManager<HybridEncrypt>(&key_type_manager); + const KeyFactory& key_factory = key_manager->get_key_factory(); { // Via NewKey(format_proto). EciesAeadHkdfKeyFormat key_format; diff --git a/cc/hybrid/hybrid_config.cc b/cc/hybrid/hybrid_config.cc index cb4477490..4f44d8b58 100644 --- a/cc/hybrid/hybrid_config.cc +++ b/cc/hybrid/hybrid_config.cc @@ -44,10 +44,8 @@ util::Status HybridConfig::Register() { // Register key managers. if (!status.ok()) return status; - status = Registry::RegisterKeyManager( - absl::make_unique<EciesAeadHkdfPrivateKeyManager>(), true); - if (!status.ok()) return status; - status = Registry::RegisterKeyManager( + status = Registry::RegisterAsymmetricKeyManagers( + absl::make_unique<EciesAeadHkdfPrivateKeyManager>(), absl::make_unique<EciesAeadHkdfPublicKeyManager>(), true); if (!status.ok()) return status; diff --git a/cc/hybrid/hybrid_decrypt_factory_test.cc b/cc/hybrid/hybrid_decrypt_factory_test.cc index 67116ec0a..59ef20fd1 100644 --- a/cc/hybrid/hybrid_decrypt_factory_test.cc +++ b/cc/hybrid/hybrid_decrypt_factory_test.cc @@ -95,9 +95,11 @@ TEST_F(HybridDecryptFactoryTest, testPrimitive) { // Prepare HybridEncrypt-instances. auto ecies_key_manager = absl::make_unique<EciesAeadHkdfPublicKeyManager>(); std::unique_ptr<HybridEncrypt> ecies_1 = std::move( - ecies_key_manager->GetPrimitive(ecies_key_1.public_key()).ValueOrDie()); + ecies_key_manager->GetPrimitive<HybridEncrypt>(ecies_key_1.public_key()) + .ValueOrDie()); std::unique_ptr<HybridEncrypt> ecies_2 = std::move( - ecies_key_manager->GetPrimitive(ecies_key_2.public_key()).ValueOrDie()); + ecies_key_manager->GetPrimitive<HybridEncrypt>(ecies_key_2.public_key()) + .ValueOrDie()); // Create a KeysetHandle and use it with the factory. auto hybrid_decrypt_result = HybridDecryptFactory::GetPrimitive( diff --git a/cc/hybrid/hybrid_key_templates_test.cc b/cc/hybrid/hybrid_key_templates_test.cc index fe36c4c5b..71b6f4122 100644 --- a/cc/hybrid/hybrid_key_templates_test.cc +++ b/cc/hybrid/hybrid_key_templates_test.cc @@ -16,6 +16,7 @@ #include "tink/hybrid/hybrid_key_templates.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" #include "tink/aead/aead_key_templates.h" #include "tink/hybrid/ecies_aead_hkdf_private_key_manager.h" @@ -80,8 +81,7 @@ TEST_F(HybridKeyTemplatesTest, testEciesAeadHkdf) { // Check that the template works with the key manager. EciesAeadHkdfPrivateKeyManager key_manager; EXPECT_EQ(key_manager.get_key_type(), key_template.type_url()); - auto new_key_result = key_manager.get_key_factory().NewKey(key_format); - EXPECT_TRUE(new_key_result.ok()) << new_key_result.status(); + EXPECT_THAT(key_manager.ValidateKeyFormat(key_format), IsOk()); } { // Test EciesP256HkdfHmacSha256Aes128CtrHmacSha256(). @@ -115,8 +115,7 @@ TEST_F(HybridKeyTemplatesTest, testEciesAeadHkdf) { // Check that the template works with the key manager. EciesAeadHkdfPrivateKeyManager key_manager; EXPECT_EQ(key_manager.get_key_type(), key_template.type_url()); - auto new_key_result = key_manager.get_key_factory().NewKey(key_format); - EXPECT_TRUE(new_key_result.ok()) << new_key_result.status(); + EXPECT_THAT(key_manager.ValidateKeyFormat(key_format), IsOk()); } { // Test EciesP256CompressedHkdfHmacSha256Aes128Gcm(). @@ -147,8 +146,7 @@ TEST_F(HybridKeyTemplatesTest, testEciesAeadHkdf) { // Check that the template works with the key manager. EciesAeadHkdfPrivateKeyManager key_manager; EXPECT_EQ(key_manager.get_key_type(), key_template.type_url()); - auto new_key_result = key_manager.get_key_factory().NewKey(key_format); - EXPECT_TRUE(new_key_result.ok()) << new_key_result.status(); + EXPECT_THAT(key_manager.ValidateKeyFormat(key_format), IsOk()); } { // Test EciesP256CompressedHkdfHmacSha256Aes128CtrHmacSha256(). @@ -179,8 +177,7 @@ TEST_F(HybridKeyTemplatesTest, testEciesAeadHkdf) { // Check that the template works with the key manager. EciesAeadHkdfPrivateKeyManager key_manager; EXPECT_EQ(key_manager.get_key_type(), key_template.type_url()); - auto new_key_result = key_manager.get_key_factory().NewKey(key_format); - EXPECT_TRUE(new_key_result.ok()) << new_key_result.status(); + EXPECT_THAT(key_manager.ValidateKeyFormat(key_format), IsOk()); } } diff --git a/python/hybrid/hybrid_key_manager_test.py b/python/hybrid/hybrid_key_manager_test.py index a6ebadd8f..add29767f 100644 --- a/python/hybrid/hybrid_key_manager_test.py +++ b/python/hybrid/hybrid_key_manager_test.py @@ -96,8 +96,9 @@ class HybridKeyManagerTest(absltest.TestCase): 'type.googleapis.com/google.crypto.tink.EciesAeadHkdfPublicKey') key_template.value = key_format.SerializeToString() key_template.output_prefix_type = tink_pb2.TINK - with self.assertRaisesRegex(tink_error.TinkError, - 'Operation not supported for public keys'): + with self.assertRaisesRegex( + tink_error.TinkError, + 'Creating new keys is not supported for this key manager'): key_manager = _hybrid_encrypt_key_manager() key_manager.new_key_data(key_template) -- GitLab