feat: Updates to the AWS Encryption SDK

This change includes fixes for issues that were reported by Thai Duong from Google's Security team, and
for issues that were identified by AWS Cryptography.

See: https://docs.aws.amazon.com/encryption-sdk/latest/developer-guide/migration.html

Author: alex-chew

CMakeLists.txt

@@ -50,7 +50,7 @@ set(PROJECT_NAME aws-encryption-sdk)
 
 # Version number of the SDK to be consumed by C code and Doxygen
 set(MAJOR 1)
-set(MINOR 1)
+set(MINOR 7)
 set(PATCH 0)
 
 # Compiler feature tests and feature flags
@@ -161,6 +161,7 @@ target_link_libraries(aws-encryption-sdk-test PRIVATE ${PLATFORM_LIBS} ${OPENSSL
 target_link_libraries(aws-encryption-sdk-test PUBLIC AWS::aws-c-common)
 target_compile_definitions(aws-encryption-sdk-test PRIVATE AWS_CRYPTOSDK_TEST_STATIC=)
 target_compile_definitions(aws-encryption-sdk-test PUBLIC AWS_ENCRYPTION_SDK_FORCE_STATIC)
+target_compile_definitions(aws-encryption-sdk-test PRIVATE UNIT_TEST_ONLY_ALLOW_ENCRYPT_WITH_COMMITMENT)
 
 include(CodeCoverageTargets)
 

aws-encryption-sdk-cpp/CMakeLists.txt

@@ -13,6 +13,8 @@
 # limitations under the License.
 #
 
+set(TEST_DATA ${CMAKE_CURRENT_SOURCE_DIR}/tests/data)
+
 file(GLOB AWS_CRYPTOSDK_CPP_HEADERS
     # Headers subject to API/ABI stability guarantees
     "${CMAKE_CURRENT_SOURCE_DIR}/include/aws/cryptosdk/cpp/*.h"
@@ -72,6 +74,16 @@ if (AWS_ENC_SDK_END_TO_END_TESTS)
         )
     set_target_properties(t_integration_kms_keyring PROPERTIES CXX_STANDARD 11 C_STANDARD 99)
     aws_add_test(integration_kms_mk ${VALGRIND} ${CMAKE_CURRENT_BINARY_DIR}/t_integration_kms_keyring)
+
+    add_executable(t_commitment_known_answer tests/integration/t_commitment_known_answer.cpp)
+    target_link_libraries(t_commitment_known_answer testlibcpp)
+    target_include_directories(t_commitment_known_answer PUBLIC ${PROJECT_SOURCE_DIR}/tests/lib
+        ${PROJECT_SOURCE_DIR}/tests/unit
+        ${PROJECT_SOURCE_DIR}/tests/integration
+        $<INSTALL_INTERFACE:include>
+        )
+    set_target_properties(t_commitment_known_answer PROPERTIES CXX_STANDARD 11 C_STANDARD 99)
+    aws_add_test(commitment_known_answer ${VALGRIND} ${CMAKE_CURRENT_BINARY_DIR}/t_commitment_known_answer ${TEST_DATA}/commitment_known_answer_tests.json)
 else()
     message(STATUS "End to end tests off")
 endif()

aws-encryption-sdk-cpp/include/aws/cryptosdk/cpp/kms_keyring.h

@@ -20,6 +20,7 @@
 #include <aws/core/Aws.h>
 #include <aws/core/utils/Outcome.h>
 #include <aws/core/utils/memory/stl/AWSMap.h>
+#include <aws/core/utils/memory/stl/AWSSet.h>
 #include <aws/core/utils/memory/stl/AWSString.h>
 #include <aws/core/utils/memory/stl/AWSVector.h>
 #include <aws/cryptosdk/materials.h>
@@ -31,6 +32,7 @@ namespace Aws {
 namespace Cryptosdk {
 namespace KmsKeyring {
 class ClientSupplier;
+class DiscoveryFilter;
 
 /**
  * @defgroup kms_keyring KMS keyring (AWS SDK for C++)
@@ -144,6 +146,28 @@ class AWS_CRYPTOSDK_CPP_API Builder {
      */
     aws_cryptosdk_keyring *BuildDiscovery() const;
 
+    /**
+     * Creates a new KmsKeyring object in discovery mode (i.e., no KMS keys
+     * configured) but with a DiscoveryFilter. This means the following:
+     *
+     * (1) As in discovery mode without a DiscoveryFilter, this KmsKeyring will
+     * not do anything on encryption attempts.
+     *
+     * (2) On attempts to decrypt, the AWS Encryption SDK will attempt KMS
+     *     DecryptDataKey calls for every KMS key that was used to encrypt the
+     *     data until it finds one that:
+     *
+     *       (a) you have permission to use, and
+     *       (b) that is in an account specified by the DiscoveryFilter.
+     *
+     *     This may include calls to any region, unless prevented by policies
+     *     on the IAM user or role.
+     *
+     * The discovery_filter argument must not be nullptr, or else this function
+     * fails and returns nullptr.
+     */
+    aws_cryptosdk_keyring *BuildDiscovery(std::shared_ptr<KmsKeyring::DiscoveryFilter> discovery_filter) const;
+
    private:
     std::shared_ptr<KMS::KMSClient> kms_client;
     Aws::Vector<Aws::String> grant_tokens;
@@ -213,6 +237,73 @@ class AWS_CRYPTOSDK_CPP_API SingleClientSupplier : public ClientSupplier {
     std::shared_ptr<KMS::KMSClient> kms_client;
 };
 
+static const char *AWS_CRYPTO_SDK_DISCOVERY_FILTER_CLASS_TAG = "DiscoveryFilter";
+
+/**
+ * Builder for DiscoveryFilter objects.
+ */
+class AWS_CRYPTOSDK_CPP_API DiscoveryFilterBuilder {
+   public:
+    /**
+     * Constructs a builder with no authorized account IDs.
+     */
+    DiscoveryFilterBuilder(Aws::String partition) : partition(partition) {}
+
+    /**
+     * Adds an account ID to the set of authorized account IDs.
+     */
+    DiscoveryFilterBuilder &AddAccount(const Aws::String &account_id);
+
+    /**
+     * Adds account IDs to the set of authorized account IDs.
+     */
+    DiscoveryFilterBuilder &AddAccounts(const Aws::Vector<Aws::String> &account_ids);
+
+    /**
+     * Replaces the set of authorized account IDs.
+     */
+    DiscoveryFilterBuilder &WithAccounts(const Aws::Vector<Aws::String> &account_ids);
+
+    /**
+     * Creates a new DiscoveryFilter object, or returns NULL if no account
+     * IDs have been added.
+     */
+    std::shared_ptr<DiscoveryFilter> Build() const;
+
+   private:
+    Aws::String partition;
+    Aws::Set<Aws::String> account_ids;
+};
+
+/**
+ * The KmsKeyring can be configured with a DiscoveryFilter in order to limit
+ * the key ARNs used to decrypt EDKs in discovery mode.
+ */
+class AWS_CRYPTOSDK_CPP_API DiscoveryFilter {
+   public:
+    /**
+     * Returns true if the given key ARN is authorized for usage by a
+     * KmsKeyring in discovery mode, according to the configured partition and
+     * account IDs, or false otherwise.
+     */
+    bool IsAuthorized(const Aws::String &key_arn) const;
+
+    DiscoveryFilter() = delete;
+
+    /**
+     * Constructs a DiscoveryFilterBuilder with no authorized account IDs and no partition.
+     */
+    static DiscoveryFilterBuilder Builder(Aws::String partition);
+
+   protected:
+    DiscoveryFilter(Aws::String partition, Aws::Set<Aws::String> account_ids)
+        : partition(partition), account_ids(account_ids) {}
+
+   private:
+    Aws::String partition;
+    Aws::Set<Aws::String> account_ids;
+};
+
 /** @} */  // doxygen group kms_keyring
 
 }  // namespace KmsKeyring

aws-encryption-sdk-cpp/include/aws/cryptosdk/private/kms_keyring.h

@@ -15,6 +15,7 @@
 #ifndef AWS_ENCRYPTION_SDK_PRIVATE_KMS_KEYRING_H
 #define AWS_ENCRYPTION_SDK_PRIVATE_KMS_KEYRING_H
 
+#include <assert.h>
 #include <aws/cryptosdk/cpp/kms_keyring.h>
 
 namespace Aws {
@@ -47,6 +48,25 @@ class AWS_CRYPTOSDK_CPP_API KmsKeyringImpl : public aws_cryptosdk_keyring {
         const Aws::Vector<Aws::String> &grant_tokens,
         std::shared_ptr<Aws::Cryptosdk::KmsKeyring::ClientSupplier> supplier);
 
+    /**
+     * Constructor of KmsKeyring for internal use only. Use KmsKeyring::Builder to make a new KmsKeyring.
+     *
+     * @param key_ids List of KMS customer master keys (CMK)
+     * @param grant_tokens A list of grant tokens.
+     * @param supplier Object that supplies the KMSClient instances to use for each region.
+     * @param discovery_filter DiscoveryFilter specifying authorized partition
+     *        and account IDs. The stored pointer must not be nullptr.
+     */
+    KmsKeyringImpl(
+        const Aws::Vector<Aws::String> &key_ids,
+        const Aws::Vector<Aws::String> &grant_tokens,
+        std::shared_ptr<Aws::Cryptosdk::KmsKeyring::ClientSupplier> supplier,
+        std::shared_ptr<Aws::Cryptosdk::KmsKeyring::DiscoveryFilter> discovery_filter)
+        : KmsKeyringImpl(key_ids, grant_tokens, supplier) {
+        assert((bool)discovery_filter);
+        this->discovery_filter = discovery_filter;
+    }
+
     /**
      * Returns the KMS Client for a specific key ID
      */
@@ -57,6 +77,23 @@ class AWS_CRYPTOSDK_CPP_API KmsKeyringImpl : public aws_cryptosdk_keyring {
 
     Aws::Vector<Aws::String> grant_tokens;
     Aws::Vector<Aws::String> key_ids;
+
+    /**
+     * This is nullptr if and only if no DiscoveryFilter is configured during
+     * construction.
+     */
+    std::shared_ptr<KmsKeyring::DiscoveryFilter> discovery_filter;
+};
+
+/**
+ * This just serves to provide a public KmsKeyring::Discovery
+ * (derived-class-)constructor for use with Aws::MakeShared, without exposing a
+ * constructor in the public API.
+ */
+class AWS_CRYPTOSDK_CPP_API DiscoveryFilterImpl : public KmsKeyring::DiscoveryFilter {
+   public:
+    DiscoveryFilterImpl(Aws::String partition, Aws::Set<Aws::String> account_ids)
+        : DiscoveryFilter(partition, account_ids) {}
 };
 
 }  // namespace Private

aws-encryption-sdk-cpp/source/kms_keyring.cpp

@@ -14,6 +14,7 @@
  */
 #include <aws/cryptosdk/private/kms_keyring.h>
 
+#include <aws/core/utils/ARN.h>
 #include <aws/core/utils/Outcome.h>
 #include <aws/core/utils/logging/LogMacros.h>
 #include <aws/core/utils/memory/MemorySystemInterface.h>
@@ -78,16 +79,26 @@ static int OnDecrypt(
         const Aws::String key_arn = Private::aws_string_from_c_aws_byte_buf(&edk->provider_info);
 
         /* If there are no key IDs in the list, keyring is in "discovery" mode and will attempt KMS calls with
-         * every ARN it comes across in the message. If there are key IDs in the list, it will cross check the
-         * ARN it reads with that list before attempting KMS calls. Note that if caller provided key IDs in
-         * anything other than a CMK ARN format, the SDK will not attempt to decrypt those data keys, because
-         * the EDK data format always specifies the CMK with the full (non-alias) ARN.
+         * every key ARN it comes across in the message, so long as the key ARN is authorized by the
+         * DiscoveryFilter (matches the partition and an account ID).
+         *
+         * If there are key IDs in the list, it will cross check the ARN it reads with that list
+         * before attempting KMS calls. Note that if caller provided key IDs in anything other than
+         * a CMK ARN format, the SDK will not attempt to decrypt those data keys, because the EDK
+         * data format always specifies the CMK with the full (non-alias) ARN.
          */
         if (self->key_ids.size() &&
             std::find(self->key_ids.begin(), self->key_ids.end(), key_arn) == self->key_ids.end()) {
             // This keyring does not have access to the CMK used to encrypt this data key. Skip.
             continue;
         }
+        // self->discovery_filter is non-null only if self was constructed via BuildDiscovery, which
+        // in turn implies discovery mode
+        if (self->discovery_filter && !self->discovery_filter->IsAuthorized(key_arn)) {
+            // The DiscoveryFilter blocks the CMK used to encrypt this data key. Skip.
+            continue;
+        }
+
         Aws::String kms_region = Private::parse_region_from_kms_key_arn(key_arn);
         if (kms_region.empty()) {
             error_buf << "Error: Malformed ciphertext. Provider ID field of KMS EDK is invalid KMS CMK ARN: " << key_arn
@@ -104,6 +115,7 @@ static int OnDecrypt(
 
         Aws::KMS::Model::DecryptRequest kms_request;
         kms_request.WithGrantTokens(self->grant_tokens)
+            .WithKeyId(key_arn)
             .WithCiphertextBlob(aws_utils_byte_buffer_from_c_aws_byte_buf(&edk->ciphertext))
             .WithEncryptionContext(enc_ctx_cpp);
 
@@ -131,6 +143,10 @@ static int OnDecrypt(
                     AWS_CRYPTOSDK_WRAPPING_KEY_DECRYPTED_DATA_KEY | AWS_CRYPTOSDK_WRAPPING_KEY_VERIFIED_ENC_CTX);
             }
             return ret;
+        } else {
+            // Since we specified the key ARN explicitly in the request,
+            // KMS had better use that key to decrypt
+            return aws_raise_error(AWS_ERROR_INVALID_STATE);
         }
     }
 
@@ -419,6 +435,20 @@ aws_cryptosdk_keyring *KmsKeyring::Builder::BuildDiscovery() const {
         BuildClientSupplier(empty_key_ids_list, kms_client, client_supplier));
 }
 
+aws_cryptosdk_keyring *KmsKeyring::Builder::BuildDiscovery(std::shared_ptr<DiscoveryFilter> discovery_filter) const {
+    if (!discovery_filter) {
+        return nullptr;
+    }
+
+    Aws::Vector<Aws::String> empty_key_ids_list;
+    return Aws::New<Private::KmsKeyringImpl>(
+        AWS_CRYPTO_SDK_KMS_CLASS_TAG,
+        empty_key_ids_list,
+        grant_tokens,
+        BuildClientSupplier(empty_key_ids_list, kms_client, client_supplier),
+        discovery_filter);
+}
+
 KmsKeyring::Builder &KmsKeyring::Builder::WithGrantTokens(const Aws::Vector<Aws::String> &grant_tokens) {
     this->grant_tokens.insert(this->grant_tokens.end(), grant_tokens.begin(), grant_tokens.end());
     return *this;
@@ -440,5 +470,59 @@ KmsKeyring::Builder &KmsKeyring::Builder::WithKmsClient(const std::shared_ptr<KM
     return *this;
 }
 
+bool KmsKeyring::DiscoveryFilter::IsAuthorized(const Aws::String &key_arn) const {
+    Utils::ARN arn(key_arn);
+    if (!arn) {
+        return false;
+    }
+
+    bool matching_partition = arn.GetPartition() == partition;
+    bool matching_account   = account_ids.find(arn.GetAccountId()) != account_ids.end();
+    return matching_partition && matching_account;
+}
+
+KmsKeyring::DiscoveryFilterBuilder KmsKeyring::DiscoveryFilter::Builder(Aws::String partition) {
+    KmsKeyring::DiscoveryFilterBuilder builder(partition);
+    return builder;
+}
+
+KmsKeyring::DiscoveryFilterBuilder &KmsKeyring::DiscoveryFilterBuilder::AddAccount(const Aws::String &account_id) {
+    this->account_ids.insert(account_id);
+    return *this;
+}
+
+KmsKeyring::DiscoveryFilterBuilder &KmsKeyring::DiscoveryFilterBuilder::AddAccounts(
+    const Aws::Vector<Aws::String> &account_ids) {
+    this->account_ids.insert(account_ids.begin(), account_ids.end());
+    return *this;
+}
+
+KmsKeyring::DiscoveryFilterBuilder &KmsKeyring::DiscoveryFilterBuilder::WithAccounts(
+    const Aws::Vector<Aws::String> &account_ids) {
+    this->account_ids.clear();
+    return this->AddAccounts(account_ids);
+}
+
+std::shared_ptr<KmsKeyring::DiscoveryFilter> KmsKeyring::DiscoveryFilterBuilder::Build() const {
+    // Must have at least one account ID, and partition and account IDs cannot be the empty string
+    if (account_ids.empty()) {
+        AWS_LOGSTREAM_ERROR(
+            AWS_CRYPTO_SDK_KMS_CLASS_TAG, "Invalid DiscoveryFilterBuilder: account IDs cannot be empty");
+        return nullptr;
+    }
+    if (partition.empty()) {
+        AWS_LOGSTREAM_ERROR(AWS_CRYPTO_SDK_KMS_CLASS_TAG, "Invalid DiscoveryFilterBuilder: partition cannot be blank");
+        return nullptr;
+    }
+    if (account_ids.find("") != account_ids.end()) {
+        AWS_LOGSTREAM_ERROR(
+            AWS_CRYPTO_SDK_KMS_CLASS_TAG, "Invalid DiscoveryFilterBuilder: account IDs cannot be blank");
+        return nullptr;
+    }
+
+    return Aws::MakeShared<Private::DiscoveryFilterImpl>(
+        KmsKeyring::AWS_CRYPTO_SDK_DISCOVERY_FILTER_CLASS_TAG, partition, account_ids);
+}
+
 }  // namespace Cryptosdk
 }  // namespace Aws