aws_request_signing: extend api to allow header exclusion

api/envoy/extensions/filters/http/aws_request_signing/v3/BUILD

@@ -5,5 +5,8 @@ load("@envoy_api//bazel:api_build_system.bzl", "api_proto_package")
 licenses(["notice"])  # Apache 2
 
 api_proto_package(
-    deps = ["@com_github_cncf_udpa//udpa/annotations:pkg"],
+    deps = [
+        "//envoy/type/matcher/v3:pkg",
+        "@com_github_cncf_udpa//udpa/annotations:pkg",
+    ],
 )

api/envoy/extensions/filters/http/aws_request_signing/v3/aws_request_signing.proto

@@ -2,6 +2,8 @@ syntax = "proto3";
 
 package envoy.extensions.filters.http.aws_request_signing.v3;
 
+import "envoy/type/matcher/v3/string.proto";
+
 import "udpa/annotations/status.proto";
 import "udpa/annotations/versioning.proto";
 import "validate/validate.proto";
@@ -16,6 +18,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // [#extension: envoy.filters.http.aws_request_signing]
 
 // Top level configuration for the AWS request signing filter.
+// [#next-free-field: 6]
 message AwsRequestSigning {
   option (udpa.annotations.versioning).previous_message_type =
       "envoy.config.filter.http.aws_request_signing.v2alpha.AwsRequestSigning";
@@ -48,4 +51,14 @@ message AwsRequestSigning {
   // to calculate the payload hash. Not all services support this option. See the `S3
   // <https://docs.aws.amazon.com/AmazonS3/latest/API/sig-v4-header-based-auth.html>`_ policy for details.
   bool use_unsigned_payload = 4;
+
+  // A list of request header string matchers that will be excluded from signing. The excluded header can be matched by
+  // any patterns defined in the StringMatcher proto (e.g. exact string, prefix, regex, etc).
+  // Example:
+  // match_excluded_headers:
+  //  - prefix: x-envoy
+  //  - exact: foo
+  //  - exact: bar
+  // When applied, all headers that start with "x-envoy" and headers "foo" and "bar" will not be signed.
+  repeated type.matcher.v3.StringMatcher match_excluded_headers = 5;
 }

docs/root/configuration/http/http_filters/aws_request_signing_filter.rst

@@ -25,6 +25,10 @@ When :ref:`use_unsigned_payload <envoy_v3_api_field_extensions.filters.http.aws_
 is false (the default), requests which exceed the configured buffer limit will receive a 413 response. See the
 ref:`flow control docs <faq_flow_control>` for details.
 
+The :ref:`match_excluded_headers <envoy_v3_api_field_extensions.filters.http.aws_request_signing.v3.AwsRequestSigning.match_excluded_headers>`
+option allows excluding certain request headers from being signed. This usually applies to headers that are likely to mutate or
+are added later such as in retries.
+
 Example configuration
 ---------------------
 
@@ -38,6 +42,10 @@ Example filter configuration:
     service_name: s3
     region: us-west-2
     use_unsigned_payload: true
+    match_excluded_headers:
+        - prefix: x-envoy
+        - prefix: x-forwarded
+        - exact: x-amzn-trace-id
 
 
 Statistics

source/extensions/common/aws/BUILD

@@ -62,6 +62,7 @@ envoy_cc_library(
     external_deps = ["curl"],
     deps = [
         "//source/common/common:empty_string",
+        "//source/common/common:matchers_lib",
         "//source/common/common:utility_lib",
         "//source/common/http:headers_lib",
     ],

source/extensions/common/aws/signer_impl.cc

@@ -57,7 +57,7 @@ void SignerImpl::sign(Http::RequestHeaderMap& headers, const std::string& conten
   const auto short_date = short_date_formatter_.now(time_source_);
   headers.addCopy(SignatureHeaders::get().Date, long_date);
   // Phase 1: Create a canonical request
-  const auto canonical_headers = Utility::canonicalizeHeaders(headers);
+  const auto canonical_headers = Utility::canonicalizeHeaders(headers, excluded_header_matchers_);
   const auto canonical_request = Utility::createCanonicalRequest(
       service_name_, method_header->value().getStringView(), path_header->value().getStringView(),
       canonical_headers, content_hash);

source/extensions/common/aws/signer_impl.h

@@ -1,7 +1,11 @@
 #pragma once
 
+#include <utility>
+
 #include "source/common/common/logger.h"
+#include "source/common/common/matchers.h"
 #include "source/common/common/utility.h"
+#include "source/common/http/headers.h"
 #include "source/common/singleton/const_singleton.h"
 #include "source/extensions/common/aws/credentials_provider.h"
 #include "source/extensions/common/aws/signer.h"
@@ -38,17 +42,26 @@ class SignatureConstantValues {
 
 using SignatureConstants = ConstSingleton<SignatureConstantValues>;
 
+using AwsSigV4HeaderExclusionVector = std::vector<envoy::type::matcher::v3::StringMatcher>;
+
 /**
  * Implementation of the Signature V4 signing process.
  * See https://docs.aws.amazon.com/general/latest/gr/signature-version-4.html
  */
 class SignerImpl : public Signer, public Logger::Loggable<Logger::Id::http> {
 public:
   SignerImpl(absl::string_view service_name, absl::string_view region,
-             const CredentialsProviderSharedPtr& credentials_provider, TimeSource& time_source)
+             const CredentialsProviderSharedPtr& credentials_provider, TimeSource& time_source,
+             const AwsSigV4HeaderExclusionVector& matcher_config)
       : service_name_(service_name), region_(region), credentials_provider_(credentials_provider),
         time_source_(time_source), long_date_formatter_(SignatureConstants::get().LongDateFormat),
-        short_date_formatter_(SignatureConstants::get().ShortDateFormat) {}
+        short_date_formatter_(SignatureConstants::get().ShortDateFormat) {
+    for (const auto& matcher : matcher_config) {
+      excluded_header_matchers_.emplace_back(
+          std::make_unique<Matchers::StringMatcherImpl<envoy::type::matcher::v3::StringMatcher>>(
+              matcher));
+    }
+  }
 
   void sign(Http::RequestMessage& message, bool sign_body = false) override;
   void sign(Http::RequestHeaderMap& headers, const std::string& content_hash) override;
@@ -71,9 +84,24 @@ class SignerImpl : public Signer, public Logger::Loggable<Logger::Id::http> {
                                         const std::map<std::string, std::string>& canonical_headers,
                                         absl::string_view signature) const;
 
+  std::vector<Matchers::StringMatcherPtr> defaultMatchers() const {
+    std::vector<Matchers::StringMatcherPtr> matcher_ptrs{};
+    for (const auto& header : default_excluded_headers_) {
+      envoy::type::matcher::v3::StringMatcher m;
+      m.set_exact(header);
+      matcher_ptrs.emplace_back(
+          std::make_unique<Matchers::StringMatcherImpl<envoy::type::matcher::v3::StringMatcher>>(
+              m));
+    }
+    return matcher_ptrs;
+  }
+
   const std::string service_name_;
   const std::string region_;
-
+  const std::vector<std::string> default_excluded_headers_ = {
+      Http::Headers::get().ForwardedFor.get(), Http::Headers::get().ForwardedProto.get(),
+      "x-amzn-trace-id"};
+  std::vector<Matchers::StringMatcherPtr> excluded_header_matchers_ = defaultMatchers();
   CredentialsProviderSharedPtr credentials_provider_;
   TimeSource& time_source_;
   DateFormatter long_date_formatter_;

source/extensions/common/aws/utility.cc

@@ -24,36 +24,39 @@ const std::string URI_ENCODE = "%{:02X}";
 const std::string URI_DOUBLE_ENCODE = "%25{:02X}";
 
 std::map<std::string, std::string>
-Utility::canonicalizeHeaders(const Http::RequestHeaderMap& headers) {
+Utility::canonicalizeHeaders(const Http::RequestHeaderMap& headers,
+                             const std::vector<Matchers::StringMatcherPtr>& excluded_headers) {
   std::map<std::string, std::string> out;
-  headers.iterate([&out](const Http::HeaderEntry& entry) -> Http::HeaderMap::Iterate {
-    // Skip empty headers
-    if (entry.key().empty() || entry.value().empty()) {
-      return Http::HeaderMap::Iterate::Continue;
-    }
-    // Pseudo-headers should not be canonicalized
-    if (!entry.key().getStringView().empty() && entry.key().getStringView()[0] == ':') {
-      return Http::HeaderMap::Iterate::Continue;
-    }
-    // Skip headers that are likely to mutate, when crossing proxies
-    const auto key = entry.key().getStringView();
-    if (key == Http::Headers::get().ForwardedFor.get() ||
-        key == Http::Headers::get().ForwardedProto.get() || key == "x-amzn-trace-id") {
-      return Http::HeaderMap::Iterate::Continue;
-    }
+  headers.iterate(
+      [&out, &excluded_headers](const Http::HeaderEntry& entry) -> Http::HeaderMap::Iterate {
+        // Skip empty headers
+        if (entry.key().empty() || entry.value().empty()) {
+          return Http::HeaderMap::Iterate::Continue;
+        }
+        // Pseudo-headers should not be canonicalized
+        if (!entry.key().getStringView().empty() && entry.key().getStringView()[0] == ':') {
+          return Http::HeaderMap::Iterate::Continue;
+        }
+        const auto key = entry.key().getStringView();
+        if (std::any_of(excluded_headers.begin(), excluded_headers.end(),
+                        [&key](const Matchers::StringMatcherPtr& matcher) {
+                          return matcher->match(key);
+                        })) {
+          return Http::HeaderMap::Iterate::Continue;
+        }
 
-    std::string value(entry.value().getStringView());
-    // Remove leading, trailing, and deduplicate repeated ascii spaces
-    absl::RemoveExtraAsciiWhitespace(&value);
-    const auto iter = out.find(std::string(entry.key().getStringView()));
-    // If the entry already exists, append the new value to the end
-    if (iter != out.end()) {
-      iter->second += fmt::format(",{}", value);
-    } else {
-      out.emplace(std::string(entry.key().getStringView()), value);
-    }
-    return Http::HeaderMap::Iterate::Continue;
-  });
+        std::string value(entry.value().getStringView());
+        // Remove leading, trailing, and deduplicate repeated ascii spaces
+        absl::RemoveExtraAsciiWhitespace(&value);
+        const auto iter = out.find(std::string(entry.key().getStringView()));
+        // If the entry already exists, append the new value to the end
+        if (iter != out.end()) {
+          iter->second += fmt::format(",{}", value);
+        } else {
+          out.emplace(std::string(entry.key().getStringView()), value);
+        }
+        return Http::HeaderMap::Iterate::Continue;
+      });
   // The AWS SDK has a quirk where it removes "default ports" (80, 443) from the host headers
   // Additionally, we canonicalize the :authority header as "host"
   // TODO(lavignes): This may need to be tweaked to canonicalize :authority for HTTP/2 requests

source/extensions/common/aws/utility.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "source/common/common/matchers.h"
 #include "source/common/http/headers.h"
 
 namespace Envoy {
@@ -13,10 +14,12 @@ class Utility {
    * Creates a canonicalized header map used in creating a AWS Signature V4 canonical request.
    * See https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html
    * @param headers a header map to canonicalize.
+   * @param excluded_headers a list of string matchers to exclude a given header from signing.
    * @return a std::map of canonicalized headers to be used in building a canonical request.
    */
   static std::map<std::string, std::string>
-  canonicalizeHeaders(const Http::RequestHeaderMap& headers);
+  canonicalizeHeaders(const Http::RequestHeaderMap& headers,
+                      const std::vector<Matchers::StringMatcherPtr>& excluded_headers);
 
   /**
    * Creates an AWS Signature V4 canonical request string.

source/extensions/filters/http/aws_lambda/config.cc

@@ -48,7 +48,8 @@ Http::FilterFactoryCb AwsLambdaFilterFactory::createFilterFactoryFromProtoTyped(
   const std::string region = arn->region();
   auto signer = std::make_shared<Extensions::Common::Aws::SignerImpl>(
       service_name, region, std::move(credentials_provider),
-      context.mainThreadDispatcher().timeSource());
+      context.mainThreadDispatcher().timeSource(),
+      Extensions::Common::Aws::AwsSigV4HeaderExclusionVector{});
 
   FilterSettings filter_settings{*arn, getInvocationMode(proto_config),
                                  proto_config.payload_passthrough()};

source/extensions/filters/http/aws_request_signing/BUILD

@@ -32,6 +32,7 @@ envoy_cc_extension(
     deps = [
         ":aws_request_signing_filter_lib",
         "//envoy/registry",
+        "//source/common/common:matchers_lib",
         "//source/extensions/common/aws:credentials_provider_impl_lib",
         "//source/extensions/common/aws:signer_impl_lib",
         "//source/extensions/filters/http/common:factory_base_lib",

source/extensions/filters/http/aws_request_signing/config.cc

@@ -21,10 +21,11 @@ Http::FilterFactoryCb AwsRequestSigningFilterFactory::createFilterFactoryFromPro
   auto credentials_provider =
       std::make_shared<Extensions::Common::Aws::DefaultCredentialsProviderChain>(
           context.api(), Extensions::Common::Aws::Utility::metadataFetcher);
+  const auto matcher_config = Extensions::Common::Aws::AwsSigV4HeaderExclusionVector(
+      config.match_excluded_headers().begin(), config.match_excluded_headers().end());
   auto signer = std::make_unique<Extensions::Common::Aws::SignerImpl>(
       config.service_name(), config.region(), credentials_provider,
-      context.mainThreadDispatcher().timeSource());
-
+      context.mainThreadDispatcher().timeSource(), matcher_config);
   auto filter_config =
       std::make_shared<FilterConfigImpl>(std::move(signer), stats_prefix, context.scope(),
                                          config.host_rewrite(), config.use_unsigned_payload());

source/extensions/grpc_credentials/aws_iam/config.cc

@@ -47,7 +47,8 @@ std::shared_ptr<grpc::ChannelCredentials> AwsIamGrpcCredentialsFactory::getChann
         auto credentials_provider = std::make_shared<Common::Aws::DefaultCredentialsProviderChain>(
             api, Common::Aws::Utility::metadataFetcher);
         auto signer = std::make_unique<Common::Aws::SignerImpl>(
-            config.service_name(), getRegion(config), credentials_provider, api.timeSource());
+            config.service_name(), getRegion(config), credentials_provider, api.timeSource(),
+            Common::Aws::AwsSigV4HeaderExclusionVector{});
         std::shared_ptr<grpc::CallCredentials> new_call_creds = grpc::MetadataCredentialsFromPlugin(
             std::make_unique<AwsIamHeaderAuthenticator>(std::move(signer)));
         if (call_creds == nullptr) {

test/extensions/common/aws/signer_impl_test.cc

@@ -22,7 +22,7 @@ class SignerImplTest : public testing::Test {
       : credentials_provider_(new NiceMock<MockCredentialsProvider>()),
         message_(new Http::RequestMessageImpl()),
         signer_("service", "region", CredentialsProviderSharedPtr{credentials_provider_},
-                time_system_),
+                time_system_, Extensions::Common::Aws::AwsSigV4HeaderExclusionVector{}),
         credentials_("akid", "secret"), token_credentials_("akid", "secret", "token") {
     // 20180102T030405Z
     time_system_.setSystemTime(std::chrono::milliseconds(1514862245000));
@@ -48,7 +48,7 @@ class SignerImplTest : public testing::Test {
     headers.addCopy(Http::LowerCaseString("host"), "www.example.com");
 
     SignerImpl signer(service_name, "region", CredentialsProviderSharedPtr{credentials_provider},
-                      time_system_);
+                      time_system_, Extensions::Common::Aws::AwsSigV4HeaderExclusionVector{});
     if (use_unsigned_payload) {
       signer.signUnsignedPayload(headers);
     } else {

test/extensions/common/aws/utility_test.cc

@@ -19,7 +19,8 @@ TEST(UtilityTest, CanonicalizeHeadersInAlphabeticalOrder) {
       {"d", "d_value"}, {"f", "f_value"}, {"b", "b_value"},
       {"e", "e_value"}, {"c", "c_value"}, {"a", "a_value"},
   };
-  const auto map = Utility::canonicalizeHeaders(headers);
+  std::vector<Matchers::StringMatcherPtr> exclusion_list = {};
+  const auto map = Utility::canonicalizeHeaders(headers, exclusion_list);
   EXPECT_THAT(map, ElementsAre(Pair("a", "a_value"), Pair("b", "b_value"), Pair("c", "c_value"),
                                Pair("d", "d_value"), Pair("e", "e_value"), Pair("f", "f_value")));
 }
@@ -31,7 +32,8 @@ TEST(UtilityTest, CanonicalizeHeadersSkippingPseudoHeaders) {
       {":method", "GET"},
       {"normal", "normal_value"},
   };
-  const auto map = Utility::canonicalizeHeaders(headers);
+  std::vector<Envoy::Matchers::StringMatcherPtr> exclusion_list = {};
+  const auto map = Utility::canonicalizeHeaders(headers, exclusion_list);
   EXPECT_THAT(map, ElementsAre(Pair("normal", "normal_value")));
 }
 
@@ -42,7 +44,8 @@ TEST(UtilityTest, CanonicalizeHeadersJoiningDuplicatesWithCommas) {
       {"a", "a_value2"},
       {"a", "a_value3"},
   };
-  const auto map = Utility::canonicalizeHeaders(headers);
+  std::vector<Matchers::StringMatcherPtr> exclusion_list = {};
+  const auto map = Utility::canonicalizeHeaders(headers, exclusion_list);
   EXPECT_THAT(map, ElementsAre(Pair("a", "a_value1,a_value2,a_value3")));
 }
 
@@ -51,7 +54,8 @@ TEST(UtilityTest, CanonicalizeHeadersAuthorityToHost) {
   Http::TestRequestHeaderMapImpl headers{
       {":authority", "authority_value"},
   };
-  const auto map = Utility::canonicalizeHeaders(headers);
+  std::vector<Matchers::StringMatcherPtr> exclusion_list = {};
+  const auto map = Utility::canonicalizeHeaders(headers, exclusion_list);
   EXPECT_THAT(map, ElementsAre(Pair("host", "authority_value")));
 }
 
@@ -60,13 +64,14 @@ TEST(UtilityTest, CanonicalizeHeadersRemovingDefaultPortsFromHost) {
   Http::TestRequestHeaderMapImpl headers_port80{
       {":authority", "example.com:80"},
   };
-  const auto map_port80 = Utility::canonicalizeHeaders(headers_port80);
+  std::vector<Matchers::StringMatcherPtr> exclusion_list = {};
+  const auto map_port80 = Utility::canonicalizeHeaders(headers_port80, exclusion_list);
   EXPECT_THAT(map_port80, ElementsAre(Pair("host", "example.com")));
 
   Http::TestRequestHeaderMapImpl headers_port443{
       {":authority", "example.com:443"},
   };
-  const auto map_port443 = Utility::canonicalizeHeaders(headers_port443);
+  const auto map_port443 = Utility::canonicalizeHeaders(headers_port443, exclusion_list);
   EXPECT_THAT(map_port443, ElementsAre(Pair("host", "example.com")));
 }
 
@@ -78,20 +83,39 @@ TEST(UtilityTest, CanonicalizeHeadersTrimmingWhitespace) {
       {"internal", "internal    value"},
       {"all", "    all    value    "},
   };
-  const auto map = Utility::canonicalizeHeaders(headers);
+  std::vector<Matchers::StringMatcherPtr> exclusion_list = {};
+  const auto map = Utility::canonicalizeHeaders(headers, exclusion_list);
   EXPECT_THAT(map,
               ElementsAre(Pair("all", "all value"), Pair("internal", "internal value"),
                           Pair("leading", "leading value"), Pair("trailing", "trailing value")));
 }
 
-// Headers that are likely to mutate are not considered canonical
-TEST(UtilityTest, CanonicalizeHeadersDropMutatingHeaders) {
+// Headers in the exclusion list are not canonicalized
+TEST(UtilityTest, CanonicalizeHeadersDropExcludedMatchers) {
   Http::TestRequestHeaderMapImpl headers{
       {":authority", "example.com"},          {"x-forwarded-for", "1.2.3.4"},
       {"x-forwarded-proto", "https"},         {"x-amz-date", "20130708T220855Z"},
-      {"x-amz-content-sha256", "e3b0c44..."},
-  };
-  const auto map = Utility::canonicalizeHeaders(headers);
+      {"x-amz-content-sha256", "e3b0c44..."}, {"x-envoy-retry-on", "5xx,reset"},
+      {"x-envoy-max-retries", "3"},           {"x-amzn-trace-id", "0123456789"}};
+  std::vector<Matchers::StringMatcherPtr> exclusion_list = {};
+  std::vector<std::string> exact_matches = {"x-amzn-trace-id", "x-forwarded-for",
+                                            "x-forwarded-proto"};
+  for (auto& str : exact_matches) {
+    envoy::type::matcher::v3::StringMatcher config;
+    config.set_exact(str);
+    exclusion_list.emplace_back(
+        std::make_unique<Matchers::StringMatcherImpl<envoy::type::matcher::v3::StringMatcher>>(
+            config));
+  }
+  std::vector<std::string> prefixes = {"x-envoy"};
+  for (auto& match_str : prefixes) {
+    envoy::type::matcher::v3::StringMatcher config;
+    config.set_prefix(match_str);
+    exclusion_list.emplace_back(
+        std::make_unique<Matchers::StringMatcherImpl<envoy::type::matcher::v3::StringMatcher>>(
+            config));
+  }
+  const auto map = Utility::canonicalizeHeaders(headers, exclusion_list);
   EXPECT_THAT(map,
               ElementsAre(Pair("host", "example.com"), Pair("x-amz-content-sha256", "e3b0c44..."),
                           Pair("x-amz-date", "20130708T220855Z")));