From 8e4b5c700262227c90938ee266209d3594b98bdd Mon Sep 17 00:00:00 2001 From: Charles Lee Date: Thu, 6 Feb 2020 22:11:04 -0800 Subject: [PATCH] Encrypt only DEK bytes for envelope encryption. This changes the envelope encryption in C++ to solely encrypt the key bytes of the DEK instead of the full KeyData proto. This reduces the overhead in the envelope encryption and fixes an issue with incompatible format between C++ and the envelope encryption in Java and Go. PiperOrigin-RevId: 292406414 (cherry picked from commit 02bdc3e37818d2367ee75e7835f0eb8129f857d5) --- cc/aead/BUILD.bazel | 2 + cc/aead/CMakeLists.txt | 2 + cc/aead/kms_envelope_aead.cc | 44 ++++++------------ cc/aead/kms_envelope_aead_test.cc | 75 ++++++++++++++++++++++--------- 4 files changed, 72 insertions(+), 51 deletions(-) diff --git a/cc/aead/BUILD.bazel b/cc/aead/BUILD.bazel index 0df838929b..6bb5813283 100644 --- a/cc/aead/BUILD.bazel +++ b/cc/aead/BUILD.bazel @@ -235,6 +235,7 @@ cc_library( "//cc/util:status", "//cc/util:statusor", "//proto:tink_cc_proto", + "@com_google_absl//absl/base:endian", "@com_google_absl//absl/strings", ], ) @@ -482,6 +483,7 @@ cc_test( "//cc/util:test_matchers", "//cc/util:test_util", "//proto:tink_cc_proto", + "@com_google_absl//absl/base:endian", "@com_google_absl//absl/memory", "@com_google_absl//absl/strings", "@com_google_googletest//:gtest_main", diff --git a/cc/aead/CMakeLists.txt b/cc/aead/CMakeLists.txt index a92bbbe803..3b252300f2 100644 --- a/cc/aead/CMakeLists.txt +++ b/cc/aead/CMakeLists.txt @@ -210,6 +210,7 @@ tink_cc_library( tink::util::statusor tink::proto::tink_cc_proto absl::strings + absl::base ) tink_cc_library( @@ -403,6 +404,7 @@ tink_cc_test( NAME kms_envelope_aead_test SRCS kms_envelope_aead_test.cc DEPS + absl::base absl::memory absl::strings tink::aead::aead_config diff --git a/cc/aead/kms_envelope_aead.cc b/cc/aead/kms_envelope_aead.cc index 25522bb1f1..47dc5d3a90 100644 --- a/cc/aead/kms_envelope_aead.cc +++ b/cc/aead/kms_envelope_aead.cc @@ -16,6 +16,7 @@ #include +#include "absl/base/internal/endian.h" #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "tink/aead.h" @@ -25,7 +26,6 @@ #include "tink/util/statusor.h" #include "proto/tink.pb.h" - namespace crypto { namespace tink { @@ -34,20 +34,6 @@ namespace { const int kEncryptedDekPrefixSize = 4; const char* kEmptyAssociatedData = ""; -void BigEndianStore32(uint8_t dst[4], uint32_t val) { - dst[0] = (val >> 24) & 0xff; - dst[1] = (val >> 16) & 0xff; - dst[2] = (val >> 8) & 0xff; - dst[3] = val & 0xff; -} - -int32_t BigEndianLoad32(const uint8_t src[4]) { - return static_cast(src[3]) - | (static_cast(src[2]) << 8) - | (static_cast(src[1]) << 16) - | (static_cast(src[0]) << 24); -} - // Constructs a ciphertext of KMS envelope encryption. // The format of the ciphertext is the following: // 4-byte-prefix | encrypted_dek | encrypted_plaintext @@ -56,7 +42,7 @@ int32_t BigEndianLoad32(const uint8_t src[4]) { std::string GetEnvelopeCiphertext(absl::string_view encrypted_dek, absl::string_view encrypted_plaintext) { uint8_t enc_dek_size[kEncryptedDekPrefixSize]; - BigEndianStore32(enc_dek_size, encrypted_dek.size()); + absl::big_endian::Store32(enc_dek_size, encrypted_dek.size()); return absl::StrCat(std::string(reinterpret_cast(enc_dek_size), kEncryptedDekPrefixSize), encrypted_dek, encrypted_plaintext); @@ -64,11 +50,10 @@ std::string GetEnvelopeCiphertext(absl::string_view encrypted_dek, } // namespace - // static util::StatusOr> KmsEnvelopeAead::New( - const google::crypto::tink::KeyTemplate& dek_template, - std::unique_ptr remote_aead) { + const google::crypto::tink::KeyTemplate& dek_template, + std::unique_ptr remote_aead) { if (remote_aead == nullptr) { return util::Status(util::error::INVALID_ARGUMENT, "remote_aead must be non-null"); @@ -87,9 +72,9 @@ util::StatusOr KmsEnvelopeAead::Encrypt( if (!dek_result.ok()) return dek_result.status(); auto dek = std::move(dek_result.ValueOrDie()); - // Wrap DEK with remote. - auto dek_encrypt_result = remote_aead_->Encrypt( - dek->SerializeAsString(), kEmptyAssociatedData); + // Wrap DEK key values with remote. + auto dek_encrypt_result = + remote_aead_->Encrypt(dek->value(), kEmptyAssociatedData); if (!dek_encrypt_result.ok()) return dek_encrypt_result.status(); // Encrypt plaintext using DEK. @@ -108,15 +93,13 @@ util::StatusOr KmsEnvelopeAead::Decrypt( absl::string_view ciphertext, absl::string_view associated_data) const { // Parse the ciphertext. if (ciphertext.size() < kEncryptedDekPrefixSize) { - return util::Status(util::error::INVALID_ARGUMENT, - "ciphertext too short"); + return util::Status(util::error::INVALID_ARGUMENT, "ciphertext too short"); } - auto enc_dek_size = BigEndianLoad32( + auto enc_dek_size = absl::big_endian::Load32( reinterpret_cast(ciphertext.data())); if (enc_dek_size > ciphertext.size() - kEncryptedDekPrefixSize || enc_dek_size < 0) { - return util::Status(util::error::INVALID_ARGUMENT, - "invalid ciphertext"); + return util::Status(util::error::INVALID_ARGUMENT, "invalid ciphertext"); } // Decrypt the DEK with remote. auto dek_decrypt_result = remote_aead_->Decrypt( @@ -131,10 +114,9 @@ util::StatusOr KmsEnvelopeAead::Decrypt( // Create AEAD from DEK. google::crypto::tink::KeyData dek; - if (!dek.ParseFromString(dek_decrypt_result.ValueOrDie())) { - return util::Status(util::error::INVALID_ARGUMENT, - "invalid ciphertext"); - } + dek.set_type_url(dek_template_.type_url()); + dek.set_value(dek_decrypt_result.ValueOrDie()); + dek.set_key_material_type(google::crypto::tink::KeyData::SYMMETRIC); // Encrypt plaintext using DEK. auto aead_result = Registry::GetPrimitive(dek); diff --git a/cc/aead/kms_envelope_aead_test.cc b/cc/aead/kms_envelope_aead_test.cc index d0efc38c21..3911b3bdf9 100644 --- a/cc/aead/kms_envelope_aead_test.cc +++ b/cc/aead/kms_envelope_aead_test.cc @@ -17,28 +17,30 @@ #include #include +#include "gtest/gtest.h" +#include "absl/base/internal/endian.h" #include "absl/memory/memory.h" #include "absl/strings/str_cat.h" -#include "tink/registry.h" #include "tink/aead/aead_config.h" #include "tink/aead/aead_key_templates.h" #include "tink/mac/mac_key_templates.h" +#include "tink/registry.h" #include "tink/util/status.h" #include "tink/util/statusor.h" -#include "tink/util/test_util.h" #include "tink/util/test_matchers.h" -#include "gtest/gtest.h" - +#include "tink/util/test_util.h" +#include "proto/aes_gcm.pb.h" namespace crypto { namespace tink { namespace { +using crypto::tink::test::DummyAead; using crypto::tink::test::IsOk; using crypto::tink::test::StatusIs; -using crypto::tink::test::DummyAead; using testing::HasSubstr; + TEST(KmsEnvelopeAeadTest, BasicEncryptDecrypt) { EXPECT_THAT(AeadConfig::Register(), IsOk()); @@ -61,8 +63,8 @@ TEST(KmsEnvelopeAeadTest, BasicEncryptDecrypt) { TEST(KmsEnvelopeAeadTest, NullAead) { auto dek_template = AeadKeyTemplates::Aes128Eax(); auto aead_result = KmsEnvelopeAead::New(dek_template, nullptr); - EXPECT_THAT(aead_result.status(), StatusIs(util::error::INVALID_ARGUMENT, - HasSubstr("non-null"))); + EXPECT_THAT(aead_result.status(), + StatusIs(util::error::INVALID_ARGUMENT, HasSubstr("non-null"))); } TEST(KmsEnvelopeAeadTest, MissingDekKeyManager) { @@ -71,8 +73,8 @@ TEST(KmsEnvelopeAeadTest, MissingDekKeyManager) { std::string remote_aead_name = "kms-backed-aead"; auto remote_aead = absl::make_unique(remote_aead_name); auto aead_result = KmsEnvelopeAead::New(dek_template, std::move(remote_aead)); - EXPECT_THAT(aead_result.status(), StatusIs(util::error::NOT_FOUND, - HasSubstr("AesEaxKey"))); + EXPECT_THAT(aead_result.status(), + StatusIs(util::error::NOT_FOUND, HasSubstr("AesEaxKey"))); } TEST(KmsEnvelopeAeadTest, WrongDekPrimitive) { @@ -104,33 +106,66 @@ TEST(KmsEnvelopeAeadTest, DecryptionErrors) { // Empty ciphertext. auto decrypt_result = aead->Decrypt("", aad); - EXPECT_THAT(decrypt_result.status(), StatusIs(util::error::INVALID_ARGUMENT, - HasSubstr("too short"))); + EXPECT_THAT(decrypt_result.status(), + StatusIs(util::error::INVALID_ARGUMENT, HasSubstr("too short"))); // Short ciphertext. decrypt_result = aead->Decrypt("sh", aad); - EXPECT_THAT(decrypt_result.status(), StatusIs(util::error::INVALID_ARGUMENT, - HasSubstr("too short"))); + EXPECT_THAT(decrypt_result.status(), + StatusIs(util::error::INVALID_ARGUMENT, HasSubstr("too short"))); // Truncated ciphertext. decrypt_result = aead->Decrypt(ct.substr(2), aad); - EXPECT_THAT(decrypt_result.status(), StatusIs(util::error::INVALID_ARGUMENT, - HasSubstr("invalid"))); + EXPECT_THAT(decrypt_result.status(), + StatusIs(util::error::INVALID_ARGUMENT, HasSubstr("invalid"))); // Corrupted ciphertext. auto ct_copy = ct; ct_copy[4] = 'a'; // corrupt serialized DEK. decrypt_result = aead->Decrypt(ct_copy, aad); - EXPECT_THAT(decrypt_result.status(), StatusIs(util::error::INVALID_ARGUMENT, - HasSubstr("invalid"))); + EXPECT_THAT(decrypt_result.status(), + StatusIs(util::error::INVALID_ARGUMENT, HasSubstr("invalid"))); // Wrong associated data. decrypt_result = aead->Decrypt(ct, "wrong aad"); - EXPECT_THAT(decrypt_result.status(), - StatusIs(util::error::INTERNAL, - HasSubstr("Authentication failed"))); + EXPECT_THAT( + decrypt_result.status(), + StatusIs(util::error::INTERNAL, HasSubstr("Authentication failed"))); } +TEST(KmsEnvelopeAeadTest, KeyFormat) { + EXPECT_THAT(AeadConfig::Register(), IsOk()); + + auto dek_template = AeadKeyTemplates::Aes128Gcm(); + + // Construct a remote AEAD which uses same key template for this test. + std::string remote_aead_name = "kms-backed-aead"; + auto remote_aead = absl::make_unique(remote_aead_name); + + // Create envelope AEAD and encrypt some data. + auto aead_result = KmsEnvelopeAead::New(dek_template, std::move(remote_aead)); + EXPECT_THAT(aead_result.status(), IsOk()); + + auto aead = std::move(aead_result.ValueOrDie()); + std::string message = "Some data to encrypt."; + std::string aad = "Some data to authenticate."; + auto encrypt_result = aead->Encrypt(message, aad); + EXPECT_THAT(encrypt_result.status(), IsOk()); + auto ct = encrypt_result.ValueOrDie(); + + // Recover DEK from ciphertext + auto enc_dek_size = + absl::big_endian::Load32(reinterpret_cast(ct.data())); + + remote_aead = absl::make_unique(remote_aead_name); + auto dek_decrypt_result = + remote_aead->Decrypt(ct.substr(4, enc_dek_size), ""); + + // Check if we can deserialize a GCM key proto from the decrypted DEK. + google::crypto::tink::AesGcmKey key; + EXPECT_THAT(key.ParseFromString(dek_decrypt_result.ValueOrDie()), true); + EXPECT_THAT(key.key_value().size(), testing::Eq(16)); +} } // namespace } // namespace tink