Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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";
Expand Down Expand Up @@ -48,4 +51,15 @@ 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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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. By default, the headers ``x-forwarded-for``, ``x-forwarded-proto``, and ``x-amzn-trace-id`` are always excluded.

Example configuration
---------------------

Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ New Features
------------
* access log: added :ref:`grpc_stream_retry_policy <envoy_v3_api_field_extensions.access_loggers.grpc.v3.CommonGrpcAccessLogConfig.grpc_stream_retry_policy>` to the gRPC logger to reconnect when a connection fails to be established.
* api: added support for *xds.type.v3.TypedStruct* in addition to the now-deprecated *udpa.type.v1.TypedStruct* proto message, which is a wrapper proto used to encode typed JSON data in a *google.protobuf.Any* field.
* aws_request_signing_filter: added :ref:`match_excluded_headers <envoy_v3_api_field_extensions.filters.http.aws_request_signing.v3.AwsRequestSigning.match_excluded_headers>` to the signing filter to optionally exclude request headers from signing.
* bootstrap: added :ref:`typed_dns_resolver_config <envoy_v3_api_field_config.bootstrap.v3.Bootstrap.typed_dns_resolver_config>` in the bootstrap to support DNS resolver as an extension.
* cluster: added :ref:`typed_dns_resolver_config <envoy_v3_api_field_config.cluster.v3.Cluster.typed_dns_resolver_config>` in the cluster to support DNS resolver as an extension.
* config: added :ref:`environment_variable <envoy_v3_api_field_config.core.v3.datasource.environment_variable>` to the :ref:`DataSource <envoy_v3_api_msg_config.core.v3.datasource>`.
Expand Down
1 change: 1 addition & 0 deletions source/extensions/common/aws/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
Expand Down
2 changes: 1 addition & 1 deletion source/extensions/common/aws/signer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
34 changes: 31 additions & 3 deletions source/extensions/common/aws/signer_impl.h
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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;
Expand All @@ -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();
Comment on lines +101 to +104
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see these defaults are pre-existing, but are they documented anywhere? Perhaps make it more clear in the API docs what the defaults are if not set?

Copy link
Copy Markdown
Contributor Author

@rexnp rexnp Nov 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Matt! Yes, I'll list them out in the doc.

CredentialsProviderSharedPtr credentials_provider_;
TimeSource& time_source_;
DateFormatter long_date_formatter_;
Expand Down
59 changes: 31 additions & 28 deletions source/extensions/common/aws/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 4 additions & 1 deletion source/extensions/common/aws/utility.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include "source/common/common/matchers.h"
#include "source/common/http/headers.h"

namespace Envoy {
Expand All @@ -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.
Expand Down
5 changes: 4 additions & 1 deletion source/extensions/filters/http/aws_lambda/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ 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(),
// TODO: extend API to allow specifying header exclusion. ref:
// https://github.com/envoyproxy/envoy/pull/18998
Extensions::Common::Aws::AwsSigV4HeaderExclusionVector{});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a TODO here for allow configuration of this? Wouldn't the same problem exist for this filter?

Copy link
Copy Markdown
Contributor Author

@rexnp rexnp Nov 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think we'd eventually want the same changes for the lambda and grpc iam filters. I'll add a TODO.


FilterSettings filter_settings{*arn, getInvocationMode(proto_config),
proto_config.payload_passthrough()};
Expand Down
1 change: 1 addition & 0 deletions source/extensions/filters/http/aws_request_signing/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
5 changes: 4 additions & 1 deletion source/extensions/grpc_credentials/aws_iam/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ 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(),
// TODO: extend API to allow specifying header exclusion. ref:
// https://github.com/envoyproxy/envoy/pull/18998
Common::Aws::AwsSigV4HeaderExclusionVector{});
std::shared_ptr<grpc::CallCredentials> new_call_creds = grpc::MetadataCredentialsFromPlugin(
std::make_unique<AwsIamHeaderAuthenticator>(std::move(signer)));
if (call_creds == nullptr) {
Expand Down
4 changes: 2 additions & 2 deletions test/extensions/common/aws/signer_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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 {
Expand Down
Loading