From 3160005944221da55d287a9a0bb92bc545e9604f Mon Sep 17 00:00:00 2001 From: paulavidas <paulavidas@google.com> Date: Mon, 29 Jul 2019 01:24:21 -0700 Subject: [PATCH] Remove the MAC catalogue. We decided to not support the catalogues in the future. Instead, we will simply register the key managers directly in the config. PiperOrigin-RevId: 260450792 --- cc/BUILD.bazel | 3 +- cc/CMakeLists.txt | 3 +- cc/config.h | 1 - cc/core/config.cc | 7 ++- cc/mac/BUILD.bazel | 37 ++------------- cc/mac/CMakeLists.txt | 31 ++---------- cc/mac/mac_catalogue.cc | 69 --------------------------- cc/mac/mac_catalogue.h | 42 ---------------- cc/mac/mac_catalogue_test.cc | 92 ------------------------------------ cc/mac/mac_config.cc | 21 +++++--- cc/mac/mac_config_test.cc | 30 ------------ 11 files changed, 29 insertions(+), 307 deletions(-) delete mode 100644 cc/mac/mac_catalogue.cc delete mode 100644 cc/mac/mac_catalogue.h delete mode 100644 cc/mac/mac_catalogue_test.cc diff --git a/cc/BUILD.bazel b/cc/BUILD.bazel index ff9773871..96b72e3f0 100644 --- a/cc/BUILD.bazel +++ b/cc/BUILD.bazel @@ -382,7 +382,6 @@ cc_library( ":hybrid_decrypt", ":hybrid_encrypt", ":key_manager", - ":mac", ":public_key_sign", ":public_key_verify", ":registry", @@ -391,7 +390,7 @@ cc_library( "//cc/daead:deterministic_aead_wrapper", "//cc/hybrid:hybrid_decrypt_wrapper", "//cc/hybrid:hybrid_encrypt_wrapper", - "//cc/mac:mac_wrapper", + "//cc/mac:mac_config", "//cc/signature:public_key_sign_wrapper", "//cc/signature:public_key_verify_wrapper", "//cc/streamingaead:streaming_aead_wrapper", diff --git a/cc/CMakeLists.txt b/cc/CMakeLists.txt index 40e5236d0..a36e79bd2 100644 --- a/cc/CMakeLists.txt +++ b/cc/CMakeLists.txt @@ -322,7 +322,6 @@ tink_cc_library( tink::core::hybrid_decrypt tink::core::hybrid_encrypt tink::core::key_manager - tink::core::mac tink::core::public_key_sign tink::core::public_key_verify tink::core::registry @@ -331,7 +330,7 @@ tink_cc_library( tink::daead::deterministic_aead_wrapper tink::hybrid::hybrid_decrypt_wrapper tink::hybrid::hybrid_encrypt_wrapper - tink::mac::mac_wrapper + tink::mac::mac_config tink::signature::public_key_sign_wrapper tink::signature::public_key_verify_wrapper tink::streamingaead::streaming_aead_wrapper diff --git a/cc/config.h b/cc/config.h index cc5684887..d847e6842 100644 --- a/cc/config.h +++ b/cc/config.h @@ -23,7 +23,6 @@ #include "tink/hybrid_encrypt.h" #include "tink/hybrid_decrypt.h" #include "tink/key_manager.h" -#include "tink/mac.h" #include "tink/registry.h" #include "tink/util/errors.h" #include "tink/util/status.h" diff --git a/cc/core/config.cc b/cc/core/config.cc index 395409e73..c343a8d4f 100644 --- a/cc/core/config.cc +++ b/cc/core/config.cc @@ -25,8 +25,7 @@ #include "tink/hybrid/hybrid_encrypt_wrapper.h" #include "tink/hybrid_decrypt.h" #include "tink/hybrid_encrypt.h" -#include "tink/mac.h" -#include "tink/mac/mac_wrapper.h" +#include "tink/mac/mac_config.h" #include "tink/public_key_sign.h" #include "tink/public_key_verify.h" #include "tink/signature/public_key_sign_wrapper.h" @@ -85,7 +84,7 @@ util::Status Config::Register( std::string primitive_name = absl::AsciiStrToLower(entry.primitive_name()); if (primitive_name == "mac") { - status = Register<Mac>(entry); + status = MacConfig::Register(); } else if (primitive_name == "aead") { status = Register<Aead>(entry); } else if (primitive_name == "deterministicaead") { @@ -119,7 +118,7 @@ util::Status Config::Register( util::Status Config::RegisterWrapper( absl::string_view lowercase_primitive_name) { if (lowercase_primitive_name == "mac") { - return Registry::RegisterPrimitiveWrapper(absl::make_unique<MacWrapper>()); + return MacConfig::Register(); } else if (lowercase_primitive_name == "aead") { return Registry::RegisterPrimitiveWrapper(absl::make_unique<AeadWrapper>()); } else if (lowercase_primitive_name == "deterministicaead") { diff --git a/cc/mac/BUILD.bazel b/cc/mac/BUILD.bazel index 5a137fa04..f0ddef3b4 100644 --- a/cc/mac/BUILD.bazel +++ b/cc/mac/BUILD.bazel @@ -27,26 +27,15 @@ cc_library( include_prefix = "tink", strip_include_prefix = "/cc", visibility = ["//visibility:public"], - deps = [ - ":mac_catalogue", - "//cc:config", - "//cc/util:status", - "//proto:config_cc_proto", - "@com_google_absl//absl/memory", - ], -) - -cc_library( - name = "mac_catalogue", - srcs = ["mac_catalogue.cc"], - hdrs = ["mac_catalogue.h"], - include_prefix = "tink", - strip_include_prefix = "/cc", deps = [ ":aes_cmac_key_manager", ":hmac_key_manager", - "//cc:catalogue", + ":mac_wrapper", + "//cc:registry", + "//cc/config:config_util", "//cc/util:status", + "//proto:config_cc_proto", + "@com_google_absl//absl/memory", ], ) @@ -151,22 +140,6 @@ cc_test( ], ) -cc_test( - name = "mac_catalogue_test", - size = "small", - srcs = ["mac_catalogue_test.cc"], - copts = ["-Iexternal/gtest/include"], - deps = [ - ":mac_catalogue", - ":mac_config", - "//cc:catalogue", - "//cc:config", - "//cc/util:status", - "//cc/util:statusor", - "@com_google_googletest//:gtest_main", - ], -) - cc_test( name = "mac_config_test", size = "small", diff --git a/cc/mac/CMakeLists.txt b/cc/mac/CMakeLists.txt index 182cd9e74..e379a8ff6 100644 --- a/cc/mac/CMakeLists.txt +++ b/cc/mac/CMakeLists.txt @@ -21,24 +21,15 @@ tink_cc_library( SRCS mac_config.cc mac_config.h - DEPS - tink::mac::mac_catalogue - tink::core::config - tink::util::status - tink::proto::config_cc_proto - absl::memory -) - -tink_cc_library( - NAME mac_catalogue - SRCS - mac_catalogue.cc - mac_catalogue.h DEPS tink::mac::aes_cmac_key_manager tink::mac::hmac_key_manager - tink::core::catalogue + tink::mac::mac_wrapper + tink::config::config_util + tink::core::registry tink::util::status + tink::proto::config_cc_proto + absl::memory ) tink_cc_library( @@ -129,18 +120,6 @@ tink_cc_test( tink::proto::tink_cc_proto ) -tink_cc_test( - NAME mac_catalogue_test - SRCS mac_catalogue_test.cc - DEPS - tink::mac::mac_catalogue - tink::mac::mac_config - tink::core::catalogue - tink::core::config - tink::util::status - tink::util::statusor -) - tink_cc_test( NAME mac_config_test SRCS mac_config_test.cc diff --git a/cc/mac/mac_catalogue.cc b/cc/mac/mac_catalogue.cc deleted file mode 100644 index 71d3a6d37..000000000 --- a/cc/mac/mac_catalogue.cc +++ /dev/null @@ -1,69 +0,0 @@ -// Copyright 2017 Google Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -/////////////////////////////////////////////////////////////////////////////// - -#include "tink/mac/mac_catalogue.h" - -#include "absl/strings/ascii.h" -#include "tink/catalogue.h" -#include "tink/key_manager.h" -#include "tink/mac/aes_cmac_key_manager.h" -#include "tink/mac/hmac_key_manager.h" -#include "tink/util/status.h" -#include "tink/util/statusor.h" - -namespace crypto { -namespace tink { - -namespace { - -crypto::tink::util::StatusOr<std::unique_ptr<KeyManager<Mac>>> CreateKeyManager( - const std::string& type_url) { - if (type_url == HmacKeyManager::static_key_type()) { - std::unique_ptr<KeyManager<Mac>> manager(new HmacKeyManager()); - return std::move(manager); - } - if (type_url == AesCmacKeyManager::static_key_type()) { - std::unique_ptr<KeyManager<Mac>> manager(new AesCmacKeyManager()); - return std::move(manager); - } - return ToStatusF(crypto::tink::util::error::NOT_FOUND, - "No key manager for type_url '%s'.", type_url.c_str()); -} - -} // anonymous namespace - -crypto::tink::util::StatusOr<std::unique_ptr<KeyManager<Mac>>> -MacCatalogue::GetKeyManager(const std::string& type_url, - const std::string& primitive_name, - uint32_t min_version) const { - if (!(absl::AsciiStrToLower(primitive_name) == "mac")) { - return ToStatusF(crypto::tink::util::error::NOT_FOUND, - "This catalogue does not support primitive %s.", - primitive_name.c_str()); - } - auto manager_result = CreateKeyManager(type_url); - if (!manager_result.ok()) return manager_result; - if (manager_result.ValueOrDie()->get_version() < min_version) { - return ToStatusF( - crypto::tink::util::error::NOT_FOUND, - "No key manager for type_url '%s' with version at least %d.", - type_url.c_str(), min_version); - } - return std::move(manager_result.ValueOrDie()); -} - -} // namespace tink -} // namespace crypto diff --git a/cc/mac/mac_catalogue.h b/cc/mac/mac_catalogue.h deleted file mode 100644 index 76403e6a9..000000000 --- a/cc/mac/mac_catalogue.h +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 2017 Google Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -/////////////////////////////////////////////////////////////////////////////// - -#ifndef TINK_MAC_MAC_CATALOGUE_H_ -#define TINK_MAC_MAC_CATALOGUE_H_ - -#include "tink/catalogue.h" -#include "tink/key_manager.h" -#include "tink/mac.h" -#include "tink/util/statusor.h" - -namespace crypto { -namespace tink { - -/////////////////////////////////////////////////////////////////////////////// -// A catalogue of Tink Mac key mangers. -class MacCatalogue : public Catalogue<Mac> { - public: - MacCatalogue() {} - - crypto::tink::util::StatusOr<std::unique_ptr<KeyManager<Mac>>> GetKeyManager( - const std::string& type_url, const std::string& primitive_name, - uint32_t min_version) const override; -}; - -} // namespace tink -} // namespace crypto - -#endif // TINK_MAC_MAC_CATALOGUE_H_ diff --git a/cc/mac/mac_catalogue_test.cc b/cc/mac/mac_catalogue_test.cc deleted file mode 100644 index 9e96aa38b..000000000 --- a/cc/mac/mac_catalogue_test.cc +++ /dev/null @@ -1,92 +0,0 @@ -// Copyright 2017 Google Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -//////////////////////////////////////////////////////////////////////////////// - -#include "tink/mac/mac_catalogue.h" - -#include "tink/catalogue.h" -#include "tink/mac/mac_config.h" -#include "tink/util/status.h" -#include "tink/util/statusor.h" -#include "gtest/gtest.h" - -namespace crypto { -namespace tink { -namespace { - -class MacCatalogueTest : public ::testing::Test { -}; - -TEST_F(MacCatalogueTest, testBasicHmac) { - std::string key_type = "type.googleapis.com/google.crypto.tink.HmacKey"; - MacCatalogue catalogue; - - { - auto manager_result = catalogue.GetKeyManager(key_type, "Mac", 0); - EXPECT_TRUE(manager_result.ok()) << manager_result.status(); - EXPECT_TRUE(manager_result.ValueOrDie()->DoesSupport(key_type)); - } - - { - auto manager_result = catalogue.GetKeyManager(key_type, "mAC", 0); - EXPECT_TRUE(manager_result.ok()) << manager_result.status(); - EXPECT_TRUE(manager_result.ValueOrDie()->DoesSupport(key_type)); - } - - { - auto manager_result = catalogue.GetKeyManager(key_type, "Aead", 0); - EXPECT_FALSE(manager_result.ok()); - EXPECT_EQ(util::error::NOT_FOUND, manager_result.status().error_code()); - } - - { - auto manager_result = catalogue.GetKeyManager(key_type, "Mac", 1); - EXPECT_FALSE(manager_result.ok()); - EXPECT_EQ(util::error::NOT_FOUND, manager_result.status().error_code()); - } -} - -TEST_F(MacCatalogueTest, testBasicAesCmac) { - std::string key_type = "type.googleapis.com/google.crypto.tink.AesCmacKey"; - MacCatalogue catalogue; - - { - auto manager_result = catalogue.GetKeyManager(key_type, "Mac", 0); - EXPECT_TRUE(manager_result.ok()) << manager_result.status(); - EXPECT_TRUE(manager_result.ValueOrDie()->DoesSupport(key_type)); - } - - { - auto manager_result = catalogue.GetKeyManager(key_type, "mAC", 0); - EXPECT_TRUE(manager_result.ok()) << manager_result.status(); - EXPECT_TRUE(manager_result.ValueOrDie()->DoesSupport(key_type)); - } - - { - auto manager_result = catalogue.GetKeyManager(key_type, "Aead", 0); - EXPECT_FALSE(manager_result.ok()); - EXPECT_EQ(util::error::NOT_FOUND, manager_result.status().error_code()); - } - - { - auto manager_result = catalogue.GetKeyManager(key_type, "Mac", 1); - EXPECT_FALSE(manager_result.ok()); - EXPECT_EQ(util::error::NOT_FOUND, manager_result.status().error_code()); - } -} - -} // namespace -} // namespace tink -} // namespace crypto diff --git a/cc/mac/mac_config.cc b/cc/mac/mac_config.cc index 1845b7cf3..be39102b7 100644 --- a/cc/mac/mac_config.cc +++ b/cc/mac/mac_config.cc @@ -17,9 +17,13 @@ #include "tink/mac/mac_config.h" #include "absl/memory/memory.h" -#include "tink/config.h" -#include "tink/mac/mac_catalogue.h" +#include "tink/config/config_util.h" +#include "tink/mac/aes_cmac_key_manager.h" +#include "tink/mac/hmac_key_manager.h" +#include "tink/mac/mac_wrapper.h" +#include "tink/registry.h" #include "tink/util/status.h" +#include "proto/config.pb.h" namespace crypto { namespace tink { @@ -29,10 +33,10 @@ namespace { google::crypto::tink::RegistryConfig* GenerateRegistryConfig() { google::crypto::tink::RegistryConfig* config = new google::crypto::tink::RegistryConfig(); - config->add_entry()->MergeFrom(*Config::GetTinkKeyTypeEntry( + config->add_entry()->MergeFrom(CreateTinkKeyTypeEntry( MacConfig::kCatalogueName, MacConfig::kPrimitiveName, "HmacKey", 0, true)); - config->add_entry()->MergeFrom(*Config::GetTinkKeyTypeEntry( + config->add_entry()->MergeFrom(CreateTinkKeyTypeEntry( MacConfig::kCatalogueName, MacConfig::kPrimitiveName, "AesCmacKey", 0, true)); config->set_config_name("TINK_MAC"); @@ -52,10 +56,13 @@ const google::crypto::tink::RegistryConfig& MacConfig::Latest() { // static util::Status MacConfig::Register() { - auto status = - Registry::AddCatalogue(kCatalogueName, absl::make_unique<MacCatalogue>()); + auto status = Registry::RegisterKeyManager( + absl::make_unique<HmacKeyManager>(), true); if (!status.ok()) return status; - return Config::Register(Latest()); + status = Registry::RegisterKeyManager( + absl::make_unique<AesCmacKeyManager>(), true); + if (!status.ok()) return status; + return Registry::RegisterPrimitiveWrapper(absl::make_unique<MacWrapper>()); } } // namespace tink diff --git a/cc/mac/mac_config_test.cc b/cc/mac/mac_config_test.cc index 17e59aa4e..1624089ed 100644 --- a/cc/mac/mac_config_test.cc +++ b/cc/mac/mac_config_test.cc @@ -92,36 +92,6 @@ TEST_F(MacConfigTest, testBasic) { EXPECT_TRUE(manager_result.ValueOrDie()->DoesSupport(aes_cmac_key_type)); } -TEST_F(MacConfigTest, testRegister) { - std::string key_type = "type.googleapis.com/google.crypto.tink.HmacKey"; - - // Try on empty registry. - auto status = Config::Register(MacConfig::Latest()); - EXPECT_FALSE(status.ok()); - EXPECT_EQ(util::error::NOT_FOUND, status.error_code()); - auto manager_result = Registry::get_key_manager<Mac>(key_type); - EXPECT_FALSE(manager_result.ok()); - - // Register and try again. - status = MacConfig::Register(); - EXPECT_TRUE(status.ok()) << status; - manager_result = Registry::get_key_manager<Mac>(key_type); - EXPECT_TRUE(manager_result.ok()) << manager_result.status(); - - // Try Register() again, should succeed (idempotence). - status = MacConfig::Register(); - EXPECT_TRUE(status.ok()) << status; - - // Reset the registry, and try overriding a catalogue with a different one. - Registry::Reset(); - status = - Registry::AddCatalogue("TinkMac", absl::make_unique<DummyMacCatalogue>()); - EXPECT_TRUE(status.ok()) << status; - status = MacConfig::Register(); - EXPECT_FALSE(status.ok()); - EXPECT_EQ(util::error::ALREADY_EXISTS, status.error_code()); -} - // Tests that the MacWrapper has been properly registered and we can wrap // primitives. TEST_F(MacConfigTest, WrappersRegistered) { -- GitLab