From 929fc454083e137a7059263d5d508113e5ab9182 Mon Sep 17 00:00:00 2001 From: Scott LaVigne Date: Fri, 11 Jan 2019 17:21:30 -0800 Subject: [PATCH 1/6] Add AWS http request signer Signed-off-by: Scott LaVigne --- source/common/common/logger.h | 1 + .../extensions/filters/http/common/aws/BUILD | 47 ++++ .../http/common/aws/credentials_provider.h | 66 +++++ .../filters/http/common/aws/region_provider.h | 26 ++ .../filters/http/common/aws/signer.h | 29 ++ .../filters/http/common/aws/signer_impl.cc | 256 ++++++++++++++++++ .../filters/http/common/aws/signer_impl.h | 78 ++++++ test/extensions/filters/http/common/aws/BUILD | 34 +++ .../filters/http/common/aws/mocks.cc | 25 ++ .../filters/http/common/aws/mocks.h | 43 +++ .../http/common/aws/signer_impl_test.cc | 193 +++++++++++++ 11 files changed, 798 insertions(+) create mode 100644 source/extensions/filters/http/common/aws/BUILD create mode 100644 source/extensions/filters/http/common/aws/credentials_provider.h create mode 100644 source/extensions/filters/http/common/aws/region_provider.h create mode 100644 source/extensions/filters/http/common/aws/signer.h create mode 100644 source/extensions/filters/http/common/aws/signer_impl.cc create mode 100644 source/extensions/filters/http/common/aws/signer_impl.h create mode 100644 test/extensions/filters/http/common/aws/BUILD create mode 100644 test/extensions/filters/http/common/aws/mocks.cc create mode 100644 test/extensions/filters/http/common/aws/mocks.h create mode 100644 test/extensions/filters/http/common/aws/signer_impl_test.cc diff --git a/source/common/common/logger.h b/source/common/common/logger.h index 341caa20c8c96..f7a09fc9eacf1 100644 --- a/source/common/common/logger.h +++ b/source/common/common/logger.h @@ -22,6 +22,7 @@ namespace Logger { // clang-format off #define ALL_LOGGER_IDS(FUNCTION) \ FUNCTION(admin) \ + FUNCTION(aws) \ FUNCTION(assert) \ FUNCTION(backtrace) \ FUNCTION(client) \ diff --git a/source/extensions/filters/http/common/aws/BUILD b/source/extensions/filters/http/common/aws/BUILD new file mode 100644 index 0000000000000..37842b3a89764 --- /dev/null +++ b/source/extensions/filters/http/common/aws/BUILD @@ -0,0 +1,47 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_package", +) + +envoy_package() + +envoy_cc_library( + name = "signer_lib", + hdrs = ["signer.h"], + deps = [ + "//include/envoy/http:message_interface", + ], +) + +envoy_cc_library( + name = "signer_impl_lib", + srcs = ["signer_impl.cc"], + hdrs = ["signer_impl.h"], + external_deps = ["ssl"], + deps = [ + ":credentials_provider_lib", + ":region_provider_lib", + ":signer_lib", + "//source/common/buffer:buffer_lib", + "//source/common/common:assert_lib", + "//source/common/common:hex_lib", + "//source/common/common:logger_lib", + "//source/common/common:stack_array", + "//source/common/common:utility_lib", + "//source/common/http:headers_lib", + ], +) + +envoy_cc_library( + name = "credentials_provider_lib", + hdrs = ["credentials_provider.h"], + external_deps = ["abseil_optional"], +) + +envoy_cc_library( + name = "region_provider_lib", + hdrs = ["region_provider.h"], +) diff --git a/source/extensions/filters/http/common/aws/credentials_provider.h b/source/extensions/filters/http/common/aws/credentials_provider.h new file mode 100644 index 0000000000000..4caed11531f56 --- /dev/null +++ b/source/extensions/filters/http/common/aws/credentials_provider.h @@ -0,0 +1,66 @@ +#pragma once + +#include + +#include "envoy/common/pure.h" + +#include "absl/types/optional.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Common { +namespace Aws { + +class Credentials { +public: + Credentials() = default; + + ~Credentials() = default; + + Credentials(const std::string& access_key_id, const std::string& secret_access_key) + : access_key_id_(access_key_id), secret_access_key_(secret_access_key) {} + + Credentials(const std::string& access_key_id, const std::string& secret_access_key, + const std::string& session_token) + : access_key_id_(access_key_id), secret_access_key_(secret_access_key), + session_token_(session_token) {} + + void setAccessKeyId(const std::string& access_key_id) { + access_key_id_ = absl::optional(access_key_id); + } + + const absl::optional& accessKeyId() const { return access_key_id_; } + + void setSecretAccessKey(const std::string& secret_key) { + secret_access_key_ = absl::optional(secret_key); + } + + const absl::optional& secretAccessKey() const { return secret_access_key_; } + + void setSessionToken(const std::string& session_token) { + session_token_ = absl::optional(session_token); + } + + const absl::optional& sessionToken() const { return session_token_; } + +private: + absl::optional access_key_id_; + absl::optional secret_access_key_; + absl::optional session_token_; +}; + +class CredentialsProvider { +public: + virtual ~CredentialsProvider() = default; + + virtual Credentials getCredentials() PURE; +}; + +typedef std::shared_ptr CredentialsProviderSharedPtr; + +} // namespace Aws +} // namespace Common +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy \ No newline at end of file diff --git a/source/extensions/filters/http/common/aws/region_provider.h b/source/extensions/filters/http/common/aws/region_provider.h new file mode 100644 index 0000000000000..f218ada10ccc8 --- /dev/null +++ b/source/extensions/filters/http/common/aws/region_provider.h @@ -0,0 +1,26 @@ +#pragma once + +#include "envoy/common/pure.h" + +#include "absl/types/optional.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Common { +namespace Aws { + +class RegionProvider { +public: + virtual ~RegionProvider() = default; + + virtual absl::optional getRegion() PURE; +}; + +typedef std::shared_ptr RegionProviderSharedPtr; + +} // namespace Aws +} // namespace Common +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy \ No newline at end of file diff --git a/source/extensions/filters/http/common/aws/signer.h b/source/extensions/filters/http/common/aws/signer.h new file mode 100644 index 0000000000000..ce0df3d475b4e --- /dev/null +++ b/source/extensions/filters/http/common/aws/signer.h @@ -0,0 +1,29 @@ +#pragma once + +#include "envoy/common/pure.h" +#include "envoy/http/message.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Common { +namespace Aws { + +class Signer { +public: + virtual ~Signer() = default; + + /** + * Sign an AWS request. + * @param message an + */ + virtual void sign(Http::Message& message) const PURE; +}; + +typedef std::shared_ptr SignerSharedPtr; + +} // namespace Aws +} // namespace Common +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy \ No newline at end of file diff --git a/source/extensions/filters/http/common/aws/signer_impl.cc b/source/extensions/filters/http/common/aws/signer_impl.cc new file mode 100644 index 0000000000000..c2ae8828ac0ff --- /dev/null +++ b/source/extensions/filters/http/common/aws/signer_impl.cc @@ -0,0 +1,256 @@ +#include "extensions/filters/http/common/aws/signer_impl.h" + +#include "envoy/common/exception.h" + +#include "common/buffer/buffer_impl.h" +#include "common/common/assert.h" +#include "common/common/hex.h" +#include "common/common/stack_array.h" +#include "common/http/headers.h" + +#include "openssl/evp.h" +#include "openssl/hmac.h" +#include "openssl/sha.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Common { +namespace Aws { + +static const std::string AWS4{"AWS4"}; +static const std::string AWS4_HMAC_SHA256{"AWS4-HMAC-SHA256"}; +static const std::string AWS4_REQUEST{"aws4_request"}; +static const std::string CREDENTIAL{"Credential"}; +static const std::string SIGNED_HEADERS{"SignedHeaders"}; +static const std::string SIGNATURE{"Signature"}; +static const std::string HASHED_EMPTY_STRING{ + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}; + +DateFormatter SignerImpl::LONG_DATE_FORMATTER("%Y%m%dT%H%M00Z"); +DateFormatter SignerImpl::SHORT_DATE_FORMATTER("%Y%m%d"); +const Http::LowerCaseString SignerImpl::X_AMZ_SECURITY_TOKEN{"x-amz-security-token"}; +const Http::LowerCaseString SignerImpl::X_AMZ_DATE{"x-amz-date"}; +const Http::LowerCaseString SignerImpl::X_AMZ_CONTENT_SHA256{"x-amz-content-sha256"}; + +void SignerImpl::sign(Http::Message& message) const { + const auto& credentials = credentials_provider_->getCredentials(); + if (!credentials.accessKeyId() || !credentials.secretAccessKey()) { + return; + } + const auto& region = region_provider_->getRegion(); + if (!region) { + throw EnvoyException("Could not determine AWS region"); + } + auto& headers = message.headers(); + if (credentials.sessionToken()) { + headers.addCopy(X_AMZ_SECURITY_TOKEN, credentials.sessionToken().value()); + } + const auto long_date = LONG_DATE_FORMATTER.now(time_source_); + const auto short_date = SHORT_DATE_FORMATTER.now(time_source_); + headers.addCopy(X_AMZ_DATE, long_date); + const auto content_hash = createContentHash(message); + headers.addCopy(X_AMZ_CONTENT_SHA256, content_hash); + // Phase 1: Create a canonical request + const auto canonical_headers = canonicalizeHeaders(headers); + const auto signing_headers = createSigningHeaders(canonical_headers); + const auto canonical_request = + createCanonicalRequest(message, canonical_headers, signing_headers, content_hash); + ENVOY_LOG(debug, "Canonical request:\n{}", canonical_request); + // Phase 2: Create a string to sign + const auto credential_scope = createCredentialScope(short_date, region.value()); + const auto string_to_sign = createStringToSign(canonical_request, long_date, credential_scope); + ENVOY_LOG(debug, "String to sign:\n{}", string_to_sign); + // Phase 3: Create a signature + const auto signature = createSignature(credentials.secretAccessKey().value(), short_date, + region.value(), string_to_sign); + // Phase 4: Sign request + const auto authorization_header = createAuthorizationHeader( + credentials.accessKeyId().value(), credential_scope, signing_headers, signature); + ENVOY_LOG(debug, "Signing request with: {}", authorization_header); + headers.addCopy(Http::Headers::get().Authorization, authorization_header); +} + +std::string SignerImpl::createContentHash(Http::Message& message) const { + if (!message.body()) { + return HASHED_EMPTY_STRING; + } + return Hex::encode(hash(*message.body())); +} + +std::string SignerImpl::createCanonicalRequest( + Http::Message& message, const std::map& canonical_headers, + const std::string& signing_headers, const std::string& content_hash) const { + const auto& headers = message.headers(); + std::stringstream out; + // Http method + const auto* method_header = headers.Method(); + if (method_header == nullptr || method_header->value().empty()) { + throw EnvoyException("Message is missing :method header"); + } + out << method_header->value().c_str() << "\n"; + // Path + const auto* path_header = headers.Path(); + if (path_header == nullptr || path_header->value().empty()) { + throw EnvoyException("Message is missing :path header"); + } + const auto& path_value = path_header->value(); + const auto path = StringUtil::cropRight(path_value.getStringView(), "?"); + if (path.empty()) { + out << "/"; + } else { + out << path; + } + out << "\n"; + // Query string + const auto query = StringUtil::cropLeft(path_value.getStringView(), "?"); + if (query != path) { + out << query; + } + out << "\n"; + // Headers + for (const auto& header : canonical_headers) { + out << header.first << ":" << header.second << "\n"; + } + out << "\n" << signing_headers << "\n"; + // Content Hash + out << content_hash; + return out.str(); +} + +std::string SignerImpl::createSigningHeaders( + const std::map& canonical_headers) const { + std::vector keys; + keys.reserve(canonical_headers.size()); + for (const auto& header : canonical_headers) { + keys.emplace_back(header.first); + } + return StringUtil::join(keys, ";"); +} + +std::string SignerImpl::createCredentialScope(const std::string& short_date, + const std::string& region) const { + std::stringstream out; + out << short_date << "/" << region << "/" << service_name_ << "/" << AWS4_REQUEST; + return out.str(); +} + +std::string SignerImpl::createStringToSign(const std::string& canonical_request, + const std::string& long_date, + const std::string& credential_scope) const { + std::stringstream out; + out << AWS4_HMAC_SHA256 << "\n"; + out << long_date << "\n"; + out << credential_scope << "\n"; + out << Hex::encode(hash(Buffer::OwnedImpl(canonical_request))); + return out.str(); +} + +std::string SignerImpl::createSignature(const std::string& secret_access_key, + const std::string& short_date, const std::string& region, + const std::string& string_to_sign) const { + const auto k_secret = AWS4 + secret_access_key; + const auto k_date = hmac(std::vector(k_secret.begin(), k_secret.end()), short_date); + const auto k_region = hmac(k_date, region); + const auto k_service = hmac(k_region, service_name_); + const auto k_signing = hmac(k_service, AWS4_REQUEST); + return Hex::encode(hmac(k_signing, string_to_sign)); +} + +std::string SignerImpl::createAuthorizationHeader(const std::string& access_key_id, + const std::string& credential_scope, + const std::string& signing_headers, + const std::string& signature) const { + std::stringstream out; + out << AWS4_HMAC_SHA256 << " "; + out << CREDENTIAL << "=" << access_key_id << "/" << credential_scope << ", "; + out << SIGNED_HEADERS << "=" << signing_headers << ", "; + out << SIGNATURE << "=" << signature; + return out.str(); +} + +std::map +SignerImpl::canonicalizeHeaders(const Http::HeaderMap& headers) const { + std::map out; + headers.iterate( + [](const Http::HeaderEntry& entry, void* context) -> Http::HeaderMap::Iterate { + auto* map = static_cast*>(context); + const auto& key = entry.key().getStringView(); + // Pseudo-headers should not be canonicalized + if (key.empty() || key[0] == ':') { + return Http::HeaderMap::Iterate::Continue; + } + // Join multi-line headers with commas + std::vector lines; + for (const auto& line : StringUtil::splitToken(entry.value().getStringView(), "\n")) { + lines.emplace_back(StringUtil::trim(line)); + } + auto value = StringUtil::join(lines, ","); + // Remove duplicate spaces + const auto end = std::unique(value.begin(), value.end(), [](char lhs, char rhs) { + return (lhs == rhs) && (lhs == ' '); + }); + value.erase(end, value.end()); + map->emplace(entry.key().c_str(), value); + return Http::HeaderMap::Iterate::Continue; + }, + &out); + // 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" + const auto* authority_header = headers.Host(); + if (authority_header != nullptr && !authority_header->value().empty()) { + const auto& value = authority_header->value().getStringView(); + const auto parts = StringUtil::splitToken(value, ":"); + if (parts.size() > 1 && (parts[1] == "80" || parts[1] == "443")) { + out.emplace(Http::Headers::get().HostLegacy.get(), + std::string(parts[0].data(), parts[0].size())); + } else { + out.emplace(Http::Headers::get().HostLegacy.get(), std::string(value.data(), value.size())); + } + } + return out; +} + +std::vector SignerImpl::hash(const Buffer::Instance& buffer) const { + std::vector digest(SHA256_DIGEST_LENGTH); + EVP_MD_CTX ctx; + auto code = EVP_DigestInit(&ctx, EVP_sha256()); + RELEASE_ASSERT(code == 1, "Failed to init digest context"); + const auto num_slices = buffer.getRawSlices(nullptr, 0); + STACK_ARRAY(slices, Buffer::RawSlice, num_slices); + buffer.getRawSlices(slices.begin(), num_slices); + for (const auto& slice : slices) { + code = EVP_DigestUpdate(&ctx, slice.mem_, slice.len_); + RELEASE_ASSERT(code == 1, "Failed to update digest"); + } + unsigned int digest_length; + code = EVP_DigestFinal(&ctx, digest.data(), &digest_length); + RELEASE_ASSERT(code == 1, "Failed to finalize digest"); + RELEASE_ASSERT(digest_length == SHA256_DIGEST_LENGTH, "Digest length mismatch"); + return digest; +} + +std::vector SignerImpl::hmac(const std::vector& key, + const std::string& string) const { + std::vector mac(EVP_MAX_MD_SIZE); + HMAC_CTX ctx; + RELEASE_ASSERT(key.size() < std::numeric_limits::max(), "Hmac key is too long"); + HMAC_CTX_init(&ctx); + auto code = HMAC_Init_ex(&ctx, key.data(), static_cast(key.size()), EVP_sha256(), nullptr); + RELEASE_ASSERT(code == 1, "Failed to init hmac context"); + code = HMAC_Update(&ctx, reinterpret_cast(string.data()), string.size()); + RELEASE_ASSERT(code == 1, "Failed to update hmac"); + unsigned int len; + code = HMAC_Final(&ctx, mac.data(), &len); + RELEASE_ASSERT(code == 1, "Failed to finalize hmac"); + RELEASE_ASSERT(len <= EVP_MAX_MD_SIZE, "Hmac length too large"); + HMAC_CTX_cleanup(&ctx); + mac.resize(len); + return mac; +} + +} // namespace Aws +} // namespace Common +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy \ No newline at end of file diff --git a/source/extensions/filters/http/common/aws/signer_impl.h b/source/extensions/filters/http/common/aws/signer_impl.h new file mode 100644 index 0000000000000..007239376b4d9 --- /dev/null +++ b/source/extensions/filters/http/common/aws/signer_impl.h @@ -0,0 +1,78 @@ +#pragma once + +#include "common/common/logger.h" +#include "common/common/utility.h" + +#include "extensions/filters/http/common/aws/credentials_provider.h" +#include "extensions/filters/http/common/aws/region_provider.h" +#include "extensions/filters/http/common/aws/signer.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Common { +namespace Aws { + +/** + * 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 { +public: + SignerImpl(const std::string& service_name, + const CredentialsProviderSharedPtr& credentials_provider, + const RegionProviderSharedPtr& region_provider, TimeSource& time_source) + : service_name_(service_name), credentials_provider_(credentials_provider), + region_provider_(region_provider), time_source_(time_source) {} + + void sign(Http::Message& message) const override; + + static const Http::LowerCaseString X_AMZ_SECURITY_TOKEN; + static const Http::LowerCaseString X_AMZ_DATE; + static const Http::LowerCaseString X_AMZ_CONTENT_SHA256; + +private: + friend class SignerImplTest; + + static DateFormatter LONG_DATE_FORMATTER; + static DateFormatter SHORT_DATE_FORMATTER; + const std::string service_name_; + CredentialsProviderSharedPtr credentials_provider_; + RegionProviderSharedPtr region_provider_; + TimeSource& time_source_; + + std::string createContentHash(Http::Message& message) const; + + std::string createCanonicalRequest(Http::Message& message, + const std::map& canonical_headers, + const std::string& signing_headers, + const std::string& content_hash) const; + + std::string + createSigningHeaders(const std::map& canonical_headers) const; + + std::string createCredentialScope(const std::string& short_date, const std::string& region) const; + + std::string createStringToSign(const std::string& canonical_request, const std::string& long_date, + const std::string& credential_scope) const; + + std::string createSignature(const std::string& secret_access_key, const std::string& short_date, + const std::string& region, const std::string& string_to_sign) const; + + std::string createAuthorizationHeader(const std::string& access_key_id, + const std::string& credential_scope, + const std::string& signing_headers, + const std::string& signature) const; + + std::map canonicalizeHeaders(const Http::HeaderMap& headers) const; + + std::vector hash(const Buffer::Instance& buffer) const; + + std::vector hmac(const std::vector& key, const std::string& string) const; +}; + +} // namespace Aws +} // namespace Common +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy \ No newline at end of file diff --git a/test/extensions/filters/http/common/aws/BUILD b/test/extensions/filters/http/common/aws/BUILD new file mode 100644 index 0000000000000..8c3dfd92bee2f --- /dev/null +++ b/test/extensions/filters/http/common/aws/BUILD @@ -0,0 +1,34 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_mock", + "envoy_cc_test", + "envoy_package", +) + +envoy_package() + +envoy_cc_mock( + name = "aws_mocks", + srcs = ["mocks.cc"], + hdrs = ["mocks.h"], + deps = [ + "//source/extensions/filters/http/common/aws:credentials_provider_lib", + "//source/extensions/filters/http/common/aws:region_provider_lib", + "//source/extensions/filters/http/common/aws:signer_lib", + ], +) + +envoy_cc_test( + name = "signer_impl_test", + srcs = ["signer_impl_test.cc"], + deps = [ + "//source/common/buffer:buffer_lib", + "//source/common/http:message_lib", + "//source/extensions/filters/http/common/aws:signer_impl_lib", + "//test/extensions/filters/http/common/aws:aws_mocks", + "//test/test_common:simulated_time_system_lib", + "//test/test_common:utility_lib", + ], +) diff --git a/test/extensions/filters/http/common/aws/mocks.cc b/test/extensions/filters/http/common/aws/mocks.cc new file mode 100644 index 0000000000000..728cd9c275da2 --- /dev/null +++ b/test/extensions/filters/http/common/aws/mocks.cc @@ -0,0 +1,25 @@ +#include "test/extensions/filters/http/common/aws/mocks.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Common { +namespace Aws { + +MockCredentialsProvider::MockCredentialsProvider() {} + +MockCredentialsProvider::~MockCredentialsProvider() {} + +MockRegionProvider::MockRegionProvider() {} + +MockRegionProvider::~MockRegionProvider() {} + +MockSigner::MockSigner() {} + +MockSigner::~MockSigner() {} + +} // namespace Aws +} // namespace Common +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy \ No newline at end of file diff --git a/test/extensions/filters/http/common/aws/mocks.h b/test/extensions/filters/http/common/aws/mocks.h new file mode 100644 index 0000000000000..a632cf3a66fbc --- /dev/null +++ b/test/extensions/filters/http/common/aws/mocks.h @@ -0,0 +1,43 @@ +#pragma once + +#include "extensions/filters/http/common/aws/credentials_provider.h" +#include "extensions/filters/http/common/aws/region_provider.h" +#include "extensions/filters/http/common/aws/signer.h" + +#include "gmock/gmock.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Common { +namespace Aws { + +class MockCredentialsProvider : public CredentialsProvider { +public: + MockCredentialsProvider(); + ~MockCredentialsProvider(); + + MOCK_METHOD0(getCredentials, Credentials()); +}; + +class MockRegionProvider : public RegionProvider { +public: + MockRegionProvider(); + ~MockRegionProvider(); + + MOCK_METHOD0(getRegion, absl::optional()); +}; + +class MockSigner : public Signer { +public: + MockSigner(); + ~MockSigner(); + + MOCK_CONST_METHOD1(sign, void(Http::Message&)); +}; + +} // namespace Aws +} // namespace Common +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/filters/http/common/aws/signer_impl_test.cc b/test/extensions/filters/http/common/aws/signer_impl_test.cc new file mode 100644 index 0000000000000..deaa29d742b88 --- /dev/null +++ b/test/extensions/filters/http/common/aws/signer_impl_test.cc @@ -0,0 +1,193 @@ +#include "common/buffer/buffer_impl.h" +#include "common/http/message_impl.h" + +#include "extensions/filters/http/common/aws/signer_impl.h" + +#include "test/extensions/filters/http/common/aws/mocks.h" +#include "test/test_common/simulated_time_system.h" +#include "test/test_common/utility.h" + +using testing::NiceMock; +using testing::Return; + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Common { +namespace Aws { + +class SignerImplTest : public testing::Test { +public: + SignerImplTest() + : credentials_provider_(new NiceMock()), + region_provider_(new NiceMock()), + message_(new Http::RequestMessageImpl()), + signer_("service", CredentialsProviderSharedPtr{credentials_provider_}, + RegionProviderSharedPtr{region_provider_}, time_system_), + credentials_("akid", "secret"), token_credentials_("akid", "secret", "token"), + region_("region") { + // 20180102T030405Z + time_system_.setSystemTime(std::chrono::milliseconds(1514862245000)); + } + + void addMethod(const std::string& method) { message_->headers().insertMethod().value(method); } + + void addPath(const std::string& path) { message_->headers().insertPath().value(path); } + + void addHeader(const std::string& key, const std::string& value) { + message_->headers().addCopy(Http::LowerCaseString(key), value); + } + + void setBody(const std::string& body) { + message_->body() = std::make_unique(body); + } + + std::string canonicalRequest() { + const auto canonical_headers = signer_.canonicalizeHeaders(message_->headers()); + const auto signing_headers = signer_.createSigningHeaders(canonical_headers); + const auto content_hash = signer_.createContentHash(*message_); + return signer_.createCanonicalRequest(*message_, canonical_headers, signing_headers, + content_hash); + } + + NiceMock* credentials_provider_; + NiceMock* region_provider_; + Event::SimulatedTimeSystem time_system_; + Http::MessagePtr message_; + SignerImpl signer_; + Credentials credentials_; + Credentials token_credentials_; + absl::optional region_; +}; + +TEST_F(SignerImplTest, AnonymousCredentials) { + EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(Credentials())); + EXPECT_CALL(*region_provider_, getRegion()).Times(0); + signer_.sign(*message_); + EXPECT_EQ(nullptr, message_->headers().Authorization()); +} + +TEST_F(SignerImplTest, MissingRegionException) { + absl::optional no_region; + EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); + EXPECT_CALL(*region_provider_, getRegion()).WillOnce(Return(no_region)); + EXPECT_THROW_WITH_MESSAGE(signer_.sign(*message_), EnvoyException, + "Could not determine AWS region"); + EXPECT_EQ(nullptr, message_->headers().Authorization()); +} + +TEST_F(SignerImplTest, SecurityTokenHeader) { + EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(token_credentials_)); + EXPECT_CALL(*region_provider_, getRegion()).WillOnce(Return(region_)); + addMethod("GET"); + addPath("/"); + signer_.sign(*message_); + EXPECT_STREQ("token", message_->headers().get(SignerImpl::X_AMZ_SECURITY_TOKEN)->value().c_str()); + EXPECT_STREQ("AWS4-HMAC-SHA256 Credential=akid/20180102/region/service/aws4_request, " + "SignedHeaders=x-amz-content-sha256;x-amz-date;x-amz-security-token, " + "Signature=1d42526aabf7d8b6d7d33d9db43b03537300cc7e6bb2817e349749e0a08f5b5e", + message_->headers().Authorization()->value().c_str()); +} + +TEST_F(SignerImplTest, DateHeader) { + EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); + EXPECT_CALL(*region_provider_, getRegion()).WillOnce(Return(region_)); + addMethod("GET"); + addPath("/"); + signer_.sign(*message_); + EXPECT_STREQ("20180102T030400Z", + message_->headers().get(SignerImpl::X_AMZ_DATE)->value().c_str()); + EXPECT_STREQ("AWS4-HMAC-SHA256 Credential=akid/20180102/region/service/aws4_request, " + "SignedHeaders=x-amz-content-sha256;x-amz-date, " + "Signature=4ee6aa9355259c18133f150b139ea9aeb7969c9408ad361b2151f50a516afe42", + message_->headers().Authorization()->value().c_str()); +} + +TEST_F(SignerImplTest, EmptyContentHeader) { + EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); + EXPECT_CALL(*region_provider_, getRegion()).WillOnce(Return(region_)); + addMethod("GET"); + addPath("/empty?content=none"); + signer_.sign(*message_); + EXPECT_STREQ("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + message_->headers().get(SignerImpl::X_AMZ_CONTENT_SHA256)->value().c_str()); + EXPECT_STREQ("AWS4-HMAC-SHA256 Credential=akid/20180102/region/service/aws4_request, " + "SignedHeaders=x-amz-content-sha256;x-amz-date, " + "Signature=999e211bc7134cc685f830a332cf4d871b6d8bb8ced9367c1a0b59b95a03ee7d", + message_->headers().Authorization()->value().c_str()); +} + +TEST_F(SignerImplTest, ContentHeader) { + EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); + EXPECT_CALL(*region_provider_, getRegion()).WillOnce(Return(region_)); + addMethod("POST"); + addPath("/"); + setBody("test1234"); + signer_.sign(*message_); + EXPECT_STREQ("937e8d5fbb48bd4949536cd65b8d35c426b80d2f830c5c308e2cdec422ae2244", + message_->headers().get(SignerImpl::X_AMZ_CONTENT_SHA256)->value().c_str()); + EXPECT_STREQ("AWS4-HMAC-SHA256 Credential=akid/20180102/region/service/aws4_request, " + "SignedHeaders=x-amz-content-sha256;x-amz-date, " + "Signature=4eab89c36f45f2032d6010ba1adab93f8510ddd6afe540821f3a05bb0253e27b", + message_->headers().Authorization()->value().c_str()); +} + +TEST_F(SignerImplTest, MissingMethodException) { + EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); + EXPECT_CALL(*region_provider_, getRegion()).WillOnce(Return(region_)); + EXPECT_THROW_WITH_MESSAGE(signer_.sign(*message_), EnvoyException, + "Message is missing :method header"); + EXPECT_EQ(nullptr, message_->headers().Authorization()); +} + +TEST_F(SignerImplTest, MissingPathException) { + EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); + EXPECT_CALL(*region_provider_, getRegion()).WillOnce(Return(region_)); + addMethod("GET"); + EXPECT_THROW_WITH_MESSAGE(signer_.sign(*message_), EnvoyException, + "Message is missing :path header"); + EXPECT_EQ(nullptr, message_->headers().Authorization()); +} + +TEST_F(SignerImplTest, ComplexCanonicalRequest) { + addMethod("POST"); + addPath("/path/foo?bar=baz"); + setBody("test1234"); + addHeader(":authority", "example.com:80"); + addHeader("UpperCase", "uppercasevalue"); + addHeader("MultiLine", "hello\n\nworld\n\nline3\n"); + addHeader("Trimmable", " trim me "); + addHeader("EmptyOne", ""); + addHeader("Alphabetic", "abcd"); + EXPECT_EQ(R"(POST +/path/foo +bar=baz +alphabetic:abcd +emptyone: +host:example.com +multiline:hello,world,line3 +trimmable:trim me +uppercase:uppercasevalue + +alphabetic;emptyone;host;multiline;trimmable;uppercase +937e8d5fbb48bd4949536cd65b8d35c426b80d2f830c5c308e2cdec422ae2244)", + canonicalRequest()); +} + +TEST_F(SignerImplTest, EmptyCanonicalRequest) { + addMethod("POST"); + addPath("/hello"); + EXPECT_EQ(R"(POST +/hello + + + +e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855)", + canonicalRequest()); +} + +} // namespace Aws +} // namespace Common +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy \ No newline at end of file From e941344bef509969b8d1aa9e8a8e7281f1c770b1 Mon Sep 17 00:00:00 2001 From: Scott LaVigne Date: Tue, 15 Jan 2019 13:20:06 -0800 Subject: [PATCH 2/6] Address PR comments * Use absl::string_view more aggresively * Removing Aws::RegionProvider * Move SHA-256 Digest/HMAC utils to common/ssl/utility * Removed setters from Aws::Credentials * Remove usage of stringstream * Use ConstSingleton for string constants * Add test comments Signed-off-by: Scott LaVigne --- source/common/ssl/BUILD | 2 + source/common/ssl/utility.cc | 42 +++ source/common/ssl/utility.h | 17 ++ .../extensions/filters/http/common/aws/BUILD | 24 +- .../http/common/aws/credentials_provider.h | 21 +- .../filters/http/common/aws/region_provider.h | 26 -- .../filters/http/common/aws/signer.h | 7 +- .../filters/http/common/aws/signer_impl.cc | 247 +++++------------- .../filters/http/common/aws/signer_impl.h | 78 +++--- .../filters/http/common/aws/utility.cc | 54 ++++ .../filters/http/common/aws/utility.h | 26 ++ test/common/ssl/BUILD | 2 + test/common/ssl/utility_test.cc | 44 ++++ test/extensions/filters/http/common/aws/BUILD | 14 +- .../filters/http/common/aws/mocks.cc | 4 - .../filters/http/common/aws/mocks.h | 11 +- .../http/common/aws/signer_impl_test.cc | 116 ++++---- .../filters/http/common/aws/utility_test.cc | 89 +++++++ 18 files changed, 489 insertions(+), 335 deletions(-) delete mode 100644 source/extensions/filters/http/common/aws/region_provider.h create mode 100644 source/extensions/filters/http/common/aws/utility.cc create mode 100644 source/extensions/filters/http/common/aws/utility.h create mode 100644 test/extensions/filters/http/common/aws/utility_test.cc diff --git a/source/common/ssl/BUILD b/source/common/ssl/BUILD index 11fb5c9b61907..05577d32f5466 100644 --- a/source/common/ssl/BUILD +++ b/source/common/ssl/BUILD @@ -118,7 +118,9 @@ envoy_cc_library( "ssl", ], deps = [ + "//include/envoy/buffer:buffer_interface", "//source/common/common:assert_lib", + "//source/common/common:stack_array", "//source/common/common:utility_lib", ], ) diff --git a/source/common/ssl/utility.cc b/source/common/ssl/utility.cc index acf3583a920a5..5164dfb681b2a 100644 --- a/source/common/ssl/utility.cc +++ b/source/common/ssl/utility.cc @@ -1,8 +1,12 @@ #include "common/ssl/utility.h" #include "common/common/assert.h" +#include "common/common/stack_array.h" #include "absl/strings/str_join.h" +#include "openssl/evp.h" +#include "openssl/hmac.h" +#include "openssl/sha.h" #include "openssl/x509v3.h" namespace Envoy { @@ -101,5 +105,43 @@ SystemTime Utility::getExpirationTime(const X509& cert) { return std::chrono::system_clock::from_time_t(days * 24 * 60 * 60 + seconds); } +std::vector Utility::getSha256Digest(const Buffer::Instance& buffer) { + std::vector digest(SHA256_DIGEST_LENGTH); + EVP_MD_CTX ctx; + auto rc = EVP_DigestInit(&ctx, EVP_sha256()); + RELEASE_ASSERT(rc == 1, "Failed to init digest context"); + const auto num_slices = buffer.getRawSlices(nullptr, 0); + STACK_ARRAY(slices, Buffer::RawSlice, num_slices); + buffer.getRawSlices(slices.begin(), num_slices); + for (const auto& slice : slices) { + rc = EVP_DigestUpdate(&ctx, slice.mem_, slice.len_); + RELEASE_ASSERT(rc == 1, "Failed to update digest"); + } + unsigned int digest_length; + rc = EVP_DigestFinal(&ctx, digest.data(), &digest_length); + RELEASE_ASSERT(rc == 1, "Failed to finalize digest"); + RELEASE_ASSERT(digest_length == SHA256_DIGEST_LENGTH, "Digest length mismatch"); + return digest; +} + +std::vector Utility::getSha256Hmac(const std::vector& key, + absl::string_view string) { + std::vector mac(EVP_MAX_MD_SIZE); + HMAC_CTX ctx; + RELEASE_ASSERT(key.size() < std::numeric_limits::max(), "HMAC key is too long"); + HMAC_CTX_init(&ctx); + auto rc = HMAC_Init_ex(&ctx, key.data(), static_cast(key.size()), EVP_sha256(), nullptr); + RELEASE_ASSERT(rc == 1, "Failed to init HMAC context"); + rc = HMAC_Update(&ctx, reinterpret_cast(string.data()), string.size()); + RELEASE_ASSERT(rc == 1, "Failed to update HMAC"); + unsigned int len; + rc = HMAC_Final(&ctx, mac.data(), &len); + RELEASE_ASSERT(rc == 1, "Failed to finalize HMAC"); + RELEASE_ASSERT(len <= EVP_MAX_MD_SIZE, "HMAC length too large"); + HMAC_CTX_cleanup(&ctx); + mac.resize(len); + return mac; +} + } // namespace Ssl } // namespace Envoy diff --git a/source/common/ssl/utility.h b/source/common/ssl/utility.h index acab625a3f18e..468912752f6ec 100644 --- a/source/common/ssl/utility.h +++ b/source/common/ssl/utility.h @@ -3,6 +3,8 @@ #include #include +#include "envoy/buffer/buffer.h" + #include "common/common/utility.h" #include "openssl/ssl.h" @@ -56,6 +58,21 @@ SystemTime getValidFrom(const X509& cert); */ SystemTime getExpirationTime(const X509& cert); +/** + * Computes the SHA-256 digest of a buffer. + * @param buffer the buffer. + * @return a vector of bytes for the computed digest. + */ +std::vector getSha256Digest(const Buffer::Instance& buffer); + +/** + * Computes the SHA-256 HMAC for a given key and data. + * @param key the HMAC function key. + * @param string string data for the HMAC function. + * @return a vector of bytes for the computed HMAC. + */ +std::vector getSha256Hmac(const std::vector& key, absl::string_view string); + } // namespace Utility } // namespace Ssl } // namespace Envoy diff --git a/source/extensions/filters/http/common/aws/BUILD b/source/extensions/filters/http/common/aws/BUILD index 37842b3a89764..c73f0dbda1591 100644 --- a/source/extensions/filters/http/common/aws/BUILD +++ b/source/extensions/filters/http/common/aws/BUILD @@ -9,7 +9,7 @@ load( envoy_package() envoy_cc_library( - name = "signer_lib", + name = "signer_interface", hdrs = ["signer.h"], deps = [ "//include/envoy/http:message_interface", @@ -20,28 +20,32 @@ envoy_cc_library( name = "signer_impl_lib", srcs = ["signer_impl.cc"], hdrs = ["signer_impl.h"], - external_deps = ["ssl"], deps = [ - ":credentials_provider_lib", - ":region_provider_lib", - ":signer_lib", + ":credentials_provider_interface", + ":signer_interface", + ":utility_lib", "//source/common/buffer:buffer_lib", - "//source/common/common:assert_lib", "//source/common/common:hex_lib", "//source/common/common:logger_lib", - "//source/common/common:stack_array", "//source/common/common:utility_lib", "//source/common/http:headers_lib", + "//source/common/singleton:const_singleton", + "//source/common/ssl:utility_lib", ], ) envoy_cc_library( - name = "credentials_provider_lib", + name = "credentials_provider_interface", hdrs = ["credentials_provider.h"], external_deps = ["abseil_optional"], ) envoy_cc_library( - name = "region_provider_lib", - hdrs = ["region_provider.h"], + name = "utility_lib", + srcs = ["utility.cc"], + hdrs = ["utility.h"], + deps = [ + "//source/common/common:utility_lib", + "//source/common/http:headers_lib", + ], ) diff --git a/source/extensions/filters/http/common/aws/credentials_provider.h b/source/extensions/filters/http/common/aws/credentials_provider.h index 4caed11531f56..1afc551f0bf3c 100644 --- a/source/extensions/filters/http/common/aws/credentials_provider.h +++ b/source/extensions/filters/http/common/aws/credentials_provider.h @@ -4,6 +4,7 @@ #include "envoy/common/pure.h" +#include "absl/strings/string_view.h" #include "absl/types/optional.h" namespace Envoy { @@ -16,32 +17,18 @@ class Credentials { public: Credentials() = default; - ~Credentials() = default; - - Credentials(const std::string& access_key_id, const std::string& secret_access_key) + Credentials(absl::string_view access_key_id, absl::string_view secret_access_key) : access_key_id_(access_key_id), secret_access_key_(secret_access_key) {} - Credentials(const std::string& access_key_id, const std::string& secret_access_key, - const std::string& session_token) + Credentials(absl::string_view access_key_id, absl::string_view secret_access_key, + absl::string_view session_token) : access_key_id_(access_key_id), secret_access_key_(secret_access_key), session_token_(session_token) {} - void setAccessKeyId(const std::string& access_key_id) { - access_key_id_ = absl::optional(access_key_id); - } - const absl::optional& accessKeyId() const { return access_key_id_; } - void setSecretAccessKey(const std::string& secret_key) { - secret_access_key_ = absl::optional(secret_key); - } - const absl::optional& secretAccessKey() const { return secret_access_key_; } - void setSessionToken(const std::string& session_token) { - session_token_ = absl::optional(session_token); - } - const absl::optional& sessionToken() const { return session_token_; } private: diff --git a/source/extensions/filters/http/common/aws/region_provider.h b/source/extensions/filters/http/common/aws/region_provider.h deleted file mode 100644 index f218ada10ccc8..0000000000000 --- a/source/extensions/filters/http/common/aws/region_provider.h +++ /dev/null @@ -1,26 +0,0 @@ -#pragma once - -#include "envoy/common/pure.h" - -#include "absl/types/optional.h" - -namespace Envoy { -namespace Extensions { -namespace HttpFilters { -namespace Common { -namespace Aws { - -class RegionProvider { -public: - virtual ~RegionProvider() = default; - - virtual absl::optional getRegion() PURE; -}; - -typedef std::shared_ptr RegionProviderSharedPtr; - -} // namespace Aws -} // namespace Common -} // namespace HttpFilters -} // namespace Extensions -} // namespace Envoy \ No newline at end of file diff --git a/source/extensions/filters/http/common/aws/signer.h b/source/extensions/filters/http/common/aws/signer.h index ce0df3d475b4e..0e93b67d68689 100644 --- a/source/extensions/filters/http/common/aws/signer.h +++ b/source/extensions/filters/http/common/aws/signer.h @@ -9,18 +9,19 @@ namespace HttpFilters { namespace Common { namespace Aws { +// TODO(lavignes): Move this interface to include/envoy if this is needed elsewhere class Signer { public: virtual ~Signer() = default; /** * Sign an AWS request. - * @param message an + * @param message an AWS API request message. */ - virtual void sign(Http::Message& message) const PURE; + virtual void sign(Http::Message& message) PURE; }; -typedef std::shared_ptr SignerSharedPtr; +typedef std::unique_ptr SignerPtr; } // namespace Aws } // namespace Common diff --git a/source/extensions/filters/http/common/aws/signer_impl.cc b/source/extensions/filters/http/common/aws/signer_impl.cc index c2ae8828ac0ff..48908babe69ac 100644 --- a/source/extensions/filters/http/common/aws/signer_impl.cc +++ b/source/extensions/filters/http/common/aws/signer_impl.cc @@ -3,14 +3,14 @@ #include "envoy/common/exception.h" #include "common/buffer/buffer_impl.h" -#include "common/common/assert.h" +#include "common/common/fmt.h" #include "common/common/hex.h" -#include "common/common/stack_array.h" #include "common/http/headers.h" +#include "common/ssl/utility.h" -#include "openssl/evp.h" -#include "openssl/hmac.h" -#include "openssl/sha.h" +#include "extensions/filters/http/common/aws/utility.h" + +#include "absl/strings/str_join.h" namespace Envoy { namespace Extensions { @@ -18,52 +18,35 @@ namespace HttpFilters { namespace Common { namespace Aws { -static const std::string AWS4{"AWS4"}; -static const std::string AWS4_HMAC_SHA256{"AWS4-HMAC-SHA256"}; -static const std::string AWS4_REQUEST{"aws4_request"}; -static const std::string CREDENTIAL{"Credential"}; -static const std::string SIGNED_HEADERS{"SignedHeaders"}; -static const std::string SIGNATURE{"Signature"}; -static const std::string HASHED_EMPTY_STRING{ - "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}; - -DateFormatter SignerImpl::LONG_DATE_FORMATTER("%Y%m%dT%H%M00Z"); -DateFormatter SignerImpl::SHORT_DATE_FORMATTER("%Y%m%d"); -const Http::LowerCaseString SignerImpl::X_AMZ_SECURITY_TOKEN{"x-amz-security-token"}; -const Http::LowerCaseString SignerImpl::X_AMZ_DATE{"x-amz-date"}; -const Http::LowerCaseString SignerImpl::X_AMZ_CONTENT_SHA256{"x-amz-content-sha256"}; - -void SignerImpl::sign(Http::Message& message) const { +void SignerImpl::sign(Http::Message& message) { const auto& credentials = credentials_provider_->getCredentials(); if (!credentials.accessKeyId() || !credentials.secretAccessKey()) { + // Empty or "anonymous" credentials are a valid use-case for non-production environments. + // This behavior matches what the AWS SDK would do. return; } - const auto& region = region_provider_->getRegion(); - if (!region) { - throw EnvoyException("Could not determine AWS region"); - } auto& headers = message.headers(); if (credentials.sessionToken()) { - headers.addCopy(X_AMZ_SECURITY_TOKEN, credentials.sessionToken().value()); + headers.addCopy(SignatureHeaders::get().SecurityToken, credentials.sessionToken().value()); } - const auto long_date = LONG_DATE_FORMATTER.now(time_source_); - const auto short_date = SHORT_DATE_FORMATTER.now(time_source_); - headers.addCopy(X_AMZ_DATE, long_date); + const auto long_date = long_date_formatter_.now(time_source_); + const auto short_date = short_date_formatter_.now(time_source_); + headers.addCopy(SignatureHeaders::get().Date, long_date); const auto content_hash = createContentHash(message); - headers.addCopy(X_AMZ_CONTENT_SHA256, content_hash); + headers.addCopy(SignatureHeaders::get().ContentSha256, content_hash); // Phase 1: Create a canonical request - const auto canonical_headers = canonicalizeHeaders(headers); + const auto canonical_headers = Utility::canonicalizeHeaders(headers); const auto signing_headers = createSigningHeaders(canonical_headers); const auto canonical_request = createCanonicalRequest(message, canonical_headers, signing_headers, content_hash); ENVOY_LOG(debug, "Canonical request:\n{}", canonical_request); // Phase 2: Create a string to sign - const auto credential_scope = createCredentialScope(short_date, region.value()); + const auto credential_scope = createCredentialScope(short_date); const auto string_to_sign = createStringToSign(canonical_request, long_date, credential_scope); ENVOY_LOG(debug, "String to sign:\n{}", string_to_sign); // Phase 3: Create a signature - const auto signature = createSignature(credentials.secretAccessKey().value(), short_date, - region.value(), string_to_sign); + const auto signature = + createSignature(credentials.secretAccessKey().value(), short_date, string_to_sign); // Phase 4: Sign request const auto authorization_header = createAuthorizationHeader( credentials.accessKeyId().value(), credential_scope, signing_headers, signature); @@ -73,180 +56,86 @@ void SignerImpl::sign(Http::Message& message) const { std::string SignerImpl::createContentHash(Http::Message& message) const { if (!message.body()) { - return HASHED_EMPTY_STRING; + return SignatureConstants::get().HashedEmptyString; } - return Hex::encode(hash(*message.body())); + return Hex::encode(Ssl::Utility::getSha256Digest(*message.body())); } std::string SignerImpl::createCanonicalRequest( Http::Message& message, const std::map& canonical_headers, - const std::string& signing_headers, const std::string& content_hash) const { - const auto& headers = message.headers(); - std::stringstream out; - // Http method - const auto* method_header = headers.Method(); + absl::string_view signing_headers, absl::string_view content_hash) const { + std::vector parts; + const auto* method_header = message.headers().Method(); if (method_header == nullptr || method_header->value().empty()) { throw EnvoyException("Message is missing :method header"); } - out << method_header->value().c_str() << "\n"; - // Path - const auto* path_header = headers.Path(); + parts.emplace_back(method_header->value().getStringView()); + const auto* path_header = message.headers().Path(); if (path_header == nullptr || path_header->value().empty()) { throw EnvoyException("Message is missing :path header"); } - const auto& path_value = path_header->value(); - const auto path = StringUtil::cropRight(path_value.getStringView(), "?"); - if (path.empty()) { - out << "/"; - } else { - out << path; - } - out << "\n"; - // Query string - const auto query = StringUtil::cropLeft(path_value.getStringView(), "?"); - if (query != path) { - out << query; - } - out << "\n"; - // Headers + // don't include the query part of the path + const auto path = StringUtil::cropRight(path_header->value().getStringView(), "?"); + parts.emplace_back(path.empty() ? "/" : path); + const auto query = StringUtil::cropLeft(path_header->value().getStringView(), "?"); + // if query == path, then there is no query + parts.emplace_back(query == path ? "" : query); + std::vector formatted_headers; + formatted_headers.reserve(canonical_headers.size()); for (const auto& header : canonical_headers) { - out << header.first << ":" << header.second << "\n"; + formatted_headers.emplace_back(fmt::format("{}:{}", header.first, header.second)); + parts.emplace_back(formatted_headers.back()); } - out << "\n" << signing_headers << "\n"; - // Content Hash - out << content_hash; - return out.str(); + // need an extra blank space after the canonical headers + parts.emplace_back(""); + parts.emplace_back(signing_headers); + parts.emplace_back(content_hash); + return absl::StrJoin(parts, "\n"); } std::string SignerImpl::createSigningHeaders( const std::map& canonical_headers) const { - std::vector keys; + std::vector keys; keys.reserve(canonical_headers.size()); for (const auto& header : canonical_headers) { keys.emplace_back(header.first); } - return StringUtil::join(keys, ";"); -} - -std::string SignerImpl::createCredentialScope(const std::string& short_date, - const std::string& region) const { - std::stringstream out; - out << short_date << "/" << region << "/" << service_name_ << "/" << AWS4_REQUEST; - return out.str(); -} - -std::string SignerImpl::createStringToSign(const std::string& canonical_request, - const std::string& long_date, - const std::string& credential_scope) const { - std::stringstream out; - out << AWS4_HMAC_SHA256 << "\n"; - out << long_date << "\n"; - out << credential_scope << "\n"; - out << Hex::encode(hash(Buffer::OwnedImpl(canonical_request))); - return out.str(); + return absl::StrJoin(keys, ";"); } -std::string SignerImpl::createSignature(const std::string& secret_access_key, - const std::string& short_date, const std::string& region, - const std::string& string_to_sign) const { - const auto k_secret = AWS4 + secret_access_key; - const auto k_date = hmac(std::vector(k_secret.begin(), k_secret.end()), short_date); - const auto k_region = hmac(k_date, region); - const auto k_service = hmac(k_region, service_name_); - const auto k_signing = hmac(k_service, AWS4_REQUEST); - return Hex::encode(hmac(k_signing, string_to_sign)); +std::string SignerImpl::createCredentialScope(absl::string_view short_date) const { + return fmt::format(SignatureConstants::get().CredentialScopeFormat, short_date, region_, + service_name_); } -std::string SignerImpl::createAuthorizationHeader(const std::string& access_key_id, - const std::string& credential_scope, - const std::string& signing_headers, - const std::string& signature) const { - std::stringstream out; - out << AWS4_HMAC_SHA256 << " "; - out << CREDENTIAL << "=" << access_key_id << "/" << credential_scope << ", "; - out << SIGNED_HEADERS << "=" << signing_headers << ", "; - out << SIGNATURE << "=" << signature; - return out.str(); +std::string SignerImpl::createStringToSign(absl::string_view canonical_request, + absl::string_view long_date, + absl::string_view credential_scope) const { + return fmt::format( + SignatureConstants::get().StringToSignFormat, long_date, credential_scope, + Hex::encode(Ssl::Utility::getSha256Digest(Buffer::OwnedImpl(canonical_request)))); } -std::map -SignerImpl::canonicalizeHeaders(const Http::HeaderMap& headers) const { - std::map out; - headers.iterate( - [](const Http::HeaderEntry& entry, void* context) -> Http::HeaderMap::Iterate { - auto* map = static_cast*>(context); - const auto& key = entry.key().getStringView(); - // Pseudo-headers should not be canonicalized - if (key.empty() || key[0] == ':') { - return Http::HeaderMap::Iterate::Continue; - } - // Join multi-line headers with commas - std::vector lines; - for (const auto& line : StringUtil::splitToken(entry.value().getStringView(), "\n")) { - lines.emplace_back(StringUtil::trim(line)); - } - auto value = StringUtil::join(lines, ","); - // Remove duplicate spaces - const auto end = std::unique(value.begin(), value.end(), [](char lhs, char rhs) { - return (lhs == rhs) && (lhs == ' '); - }); - value.erase(end, value.end()); - map->emplace(entry.key().c_str(), value); - return Http::HeaderMap::Iterate::Continue; - }, - &out); - // 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" - const auto* authority_header = headers.Host(); - if (authority_header != nullptr && !authority_header->value().empty()) { - const auto& value = authority_header->value().getStringView(); - const auto parts = StringUtil::splitToken(value, ":"); - if (parts.size() > 1 && (parts[1] == "80" || parts[1] == "443")) { - out.emplace(Http::Headers::get().HostLegacy.get(), - std::string(parts[0].data(), parts[0].size())); - } else { - out.emplace(Http::Headers::get().HostLegacy.get(), std::string(value.data(), value.size())); - } - } - return out; -} - -std::vector SignerImpl::hash(const Buffer::Instance& buffer) const { - std::vector digest(SHA256_DIGEST_LENGTH); - EVP_MD_CTX ctx; - auto code = EVP_DigestInit(&ctx, EVP_sha256()); - RELEASE_ASSERT(code == 1, "Failed to init digest context"); - const auto num_slices = buffer.getRawSlices(nullptr, 0); - STACK_ARRAY(slices, Buffer::RawSlice, num_slices); - buffer.getRawSlices(slices.begin(), num_slices); - for (const auto& slice : slices) { - code = EVP_DigestUpdate(&ctx, slice.mem_, slice.len_); - RELEASE_ASSERT(code == 1, "Failed to update digest"); - } - unsigned int digest_length; - code = EVP_DigestFinal(&ctx, digest.data(), &digest_length); - RELEASE_ASSERT(code == 1, "Failed to finalize digest"); - RELEASE_ASSERT(digest_length == SHA256_DIGEST_LENGTH, "Digest length mismatch"); - return digest; +std::string SignerImpl::createSignature(absl::string_view secret_access_key, + absl::string_view short_date, + absl::string_view string_to_sign) const { + const auto secret_key = + absl::StrCat(SignatureConstants::get().SignatureVersion, secret_access_key); + const auto date_key = Ssl::Utility::getSha256Hmac( + std::vector(secret_key.begin(), secret_key.end()), short_date); + const auto region_key = Ssl::Utility::getSha256Hmac(date_key, region_); + const auto service_key = Ssl::Utility::getSha256Hmac(region_key, service_name_); + const auto signing_key = + Ssl::Utility::getSha256Hmac(service_key, SignatureConstants::get().Aws4Request); + return Hex::encode(Ssl::Utility::getSha256Hmac(signing_key, string_to_sign)); } -std::vector SignerImpl::hmac(const std::vector& key, - const std::string& string) const { - std::vector mac(EVP_MAX_MD_SIZE); - HMAC_CTX ctx; - RELEASE_ASSERT(key.size() < std::numeric_limits::max(), "Hmac key is too long"); - HMAC_CTX_init(&ctx); - auto code = HMAC_Init_ex(&ctx, key.data(), static_cast(key.size()), EVP_sha256(), nullptr); - RELEASE_ASSERT(code == 1, "Failed to init hmac context"); - code = HMAC_Update(&ctx, reinterpret_cast(string.data()), string.size()); - RELEASE_ASSERT(code == 1, "Failed to update hmac"); - unsigned int len; - code = HMAC_Final(&ctx, mac.data(), &len); - RELEASE_ASSERT(code == 1, "Failed to finalize hmac"); - RELEASE_ASSERT(len <= EVP_MAX_MD_SIZE, "Hmac length too large"); - HMAC_CTX_cleanup(&ctx); - mac.resize(len); - return mac; +std::string SignerImpl::createAuthorizationHeader(absl::string_view access_key_id, + absl::string_view credential_scope, + absl::string_view signing_headers, + absl::string_view signature) const { + return fmt::format(SignatureConstants::get().AuthorizationHeaderFormat, access_key_id, + credential_scope, signing_headers, signature); } } // namespace Aws diff --git a/source/extensions/filters/http/common/aws/signer_impl.h b/source/extensions/filters/http/common/aws/signer_impl.h index 007239376b4d9..c81f90cdefa52 100644 --- a/source/extensions/filters/http/common/aws/signer_impl.h +++ b/source/extensions/filters/http/common/aws/signer_impl.h @@ -2,9 +2,9 @@ #include "common/common/logger.h" #include "common/common/utility.h" +#include "common/singleton/const_singleton.h" #include "extensions/filters/http/common/aws/credentials_provider.h" -#include "extensions/filters/http/common/aws/region_provider.h" #include "extensions/filters/http/common/aws/signer.h" namespace Envoy { @@ -13,62 +13,78 @@ namespace HttpFilters { namespace Common { namespace Aws { +class SignatureHeaderValues { +public: + const Http::LowerCaseString ContentSha256{"x-amz-content-sha256"}; + const Http::LowerCaseString Date{"x-amz-date"}; + const Http::LowerCaseString SecurityToken{"x-amz-security-token"}; +}; + +typedef ConstSingleton SignatureHeaders; + +class SignatureConstantValues { +public: + const std::string Aws4Request{"aws4_request"}; + const std::string AuthorizationHeaderFormat{ + "AWS4-HMAC-SHA256 Credential={}/{}, SignedHeaders={}, Signature={}"}; + const std::string CredentialScopeFormat{"{}/{}/{}/aws4_request"}; + const std::string HashedEmptyString{ + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"}; + const std::string SignatureVersion{"AWS4"}; + const std::string StringToSignFormat{"AWS4-HMAC-SHA256\n{}\n{}\n{}"}; + + const std::string LongDateFormat{"%Y%m%dT%H%M00Z"}; + const std::string ShortDateFormat{"%Y%m%d"}; +}; + +typedef ConstSingleton SignatureConstants; + /** * 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 { public: - SignerImpl(const std::string& service_name, - const CredentialsProviderSharedPtr& credentials_provider, - const RegionProviderSharedPtr& region_provider, TimeSource& time_source) - : service_name_(service_name), credentials_provider_(credentials_provider), - region_provider_(region_provider), time_source_(time_source) {} - - void sign(Http::Message& message) const override; + SignerImpl(absl::string_view service_name, absl::string_view region, + const CredentialsProviderSharedPtr& credentials_provider, TimeSource& time_source) + : 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) {} - static const Http::LowerCaseString X_AMZ_SECURITY_TOKEN; - static const Http::LowerCaseString X_AMZ_DATE; - static const Http::LowerCaseString X_AMZ_CONTENT_SHA256; + void sign(Http::Message& message) override; private: friend class SignerImplTest; - static DateFormatter LONG_DATE_FORMATTER; - static DateFormatter SHORT_DATE_FORMATTER; const std::string service_name_; + const std::string region_; CredentialsProviderSharedPtr credentials_provider_; - RegionProviderSharedPtr region_provider_; TimeSource& time_source_; + DateFormatter long_date_formatter_; + DateFormatter short_date_formatter_; std::string createContentHash(Http::Message& message) const; std::string createCanonicalRequest(Http::Message& message, const std::map& canonical_headers, - const std::string& signing_headers, - const std::string& content_hash) const; + absl::string_view signing_headers, + absl::string_view content_hash) const; std::string createSigningHeaders(const std::map& canonical_headers) const; - std::string createCredentialScope(const std::string& short_date, const std::string& region) const; - - std::string createStringToSign(const std::string& canonical_request, const std::string& long_date, - const std::string& credential_scope) const; - - std::string createSignature(const std::string& secret_access_key, const std::string& short_date, - const std::string& region, const std::string& string_to_sign) const; - - std::string createAuthorizationHeader(const std::string& access_key_id, - const std::string& credential_scope, - const std::string& signing_headers, - const std::string& signature) const; + std::string createCredentialScope(absl::string_view short_date) const; - std::map canonicalizeHeaders(const Http::HeaderMap& headers) const; + std::string createStringToSign(absl::string_view canonical_request, absl::string_view long_date, + absl::string_view credential_scope) const; - std::vector hash(const Buffer::Instance& buffer) const; + std::string createSignature(absl::string_view secret_access_key, absl::string_view short_date, + absl::string_view string_to_sign) const; - std::vector hmac(const std::vector& key, const std::string& string) const; + std::string createAuthorizationHeader(absl::string_view access_key_id, + absl::string_view credential_scope, + absl::string_view signing_headers, + absl::string_view signature) const; }; } // namespace Aws diff --git a/source/extensions/filters/http/common/aws/utility.cc b/source/extensions/filters/http/common/aws/utility.cc new file mode 100644 index 0000000000000..226a6a0e2d7c8 --- /dev/null +++ b/source/extensions/filters/http/common/aws/utility.cc @@ -0,0 +1,54 @@ +#include "extensions/filters/http/common/aws/utility.h" + +#include "common/common/fmt.h" +#include "common/common/utility.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Common { +namespace Aws { + +std::map Utility::canonicalizeHeaders(const Http::HeaderMap& headers) { + std::map out; + headers.iterate( + [](const Http::HeaderEntry& entry, void* context) -> Http::HeaderMap::Iterate { + auto* map = static_cast*>(context); + // Pseudo-headers should not be canonicalized + if (entry.key().c_str()[0] == ':') { + 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 = map->find(entry.key().c_str()); + // If the entry already exists, append the new value to the end + if (iter != map->end()) { + iter->second += fmt::format(",{}", value); + } else { + map->emplace(entry.key().c_str(), value); + } + return Http::HeaderMap::Iterate::Continue; + }, + &out); + // 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" + const auto* authority_header = headers.Host(); + if (authority_header != nullptr && !authority_header->value().empty()) { + const auto& value = authority_header->value().getStringView(); + const auto parts = StringUtil::splitToken(value, ":"); + if (parts.size() > 1 && (parts[1] == "80" || parts[1] == "443")) { + // Has default port, so use only the host part + out.emplace(Http::Headers::get().HostLegacy.get(), std::string(parts[0])); + } else { + out.emplace(Http::Headers::get().HostLegacy.get(), std::string(value)); + } + } + return out; +} + +} // namespace Aws +} // namespace Common +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy \ No newline at end of file diff --git a/source/extensions/filters/http/common/aws/utility.h b/source/extensions/filters/http/common/aws/utility.h new file mode 100644 index 0000000000000..24e4de5c9de1a --- /dev/null +++ b/source/extensions/filters/http/common/aws/utility.h @@ -0,0 +1,26 @@ +#pragma once + +#include "common/http/headers.h" + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Common { +namespace Aws { + +class Utility { +public: + /** + * Creates a canonicalized header map according to + * https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html + * @param headers a header map to canonicalize. + * @return a map of header key value pairs used for signing requests. + */ + static std::map canonicalizeHeaders(const Http::HeaderMap& headers); +}; + +} // namespace Aws +} // namespace Common +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy \ No newline at end of file diff --git a/test/common/ssl/BUILD b/test/common/ssl/BUILD index 39fdbee27dcb0..9705eaf6aca06 100644 --- a/test/common/ssl/BUILD +++ b/test/common/ssl/BUILD @@ -88,6 +88,8 @@ envoy_cc_test( external_deps = ["ssl"], deps = [ ":ssl_test_utils", + "//source/common/buffer:buffer_lib", + "//source/common/common:hex_lib", "//source/common/ssl:utility_lib", "//test/common/ssl/test_data:cert_infos", "//test/test_common:environment_lib", diff --git a/test/common/ssl/utility_test.cc b/test/common/ssl/utility_test.cc index 207fe5cc1ffd6..56b3799a120a9 100644 --- a/test/common/ssl/utility_test.cc +++ b/test/common/ssl/utility_test.cc @@ -1,6 +1,8 @@ #include #include +#include "common/buffer/buffer_impl.h" +#include "common/common/hex.h" #include "common/ssl/utility.h" #include "test/common/ssl/ssl_test_utility.h" @@ -97,5 +99,47 @@ TEST(UtilityTest, TestExpirationTime) { ASSERT(len == sizeof(buffer) - 1); EXPECT_EQ(TEST_SAN_DNS_CERT_NOT_AFTER, std::string(buffer)); } + +TEST(UtilityTest, TestSha256Disgest) { + const Buffer::OwnedImpl buffer("test data"); + const auto digest = Utility::getSha256Digest(buffer); + EXPECT_EQ("916f0027a575074ce72a331777c3478d6513f786a591bd892da1a577bf2335f9", + Hex::encode(digest)); +} + +TEST(UtilityTest, TestSha256DisgestWithEmptyBuffer) { + const Buffer::OwnedImpl buffer; + const auto digest = Utility::getSha256Digest(buffer); + EXPECT_EQ("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + Hex::encode(digest)); +} + +TEST(UtilityTest, TestSha256DisgestGrowingBuffer) { + // Adding multiple slices to the buffer + Buffer::OwnedImpl buffer("slice 1"); + auto digest = Utility::getSha256Digest(buffer); + EXPECT_EQ("76571770bb46bdf51e1aba95b23c681fda27f6ae56a8a90898a4cb7556e19dcb", + Hex::encode(digest)); + buffer.add("slice 2"); + digest = Utility::getSha256Digest(buffer); + EXPECT_EQ("290b462b0fe5edcf6b8532de3ca70da8ab77937212042bb959192ec6c9f95b9a", + Hex::encode(digest)); + buffer.add("slice 3"); + digest = Utility::getSha256Digest(buffer); + EXPECT_EQ("29606bbf02fdc40007cdf799de36d931e3587dafc086937efd6599a4ea9397aa", + Hex::encode(digest)); +} + +TEST(UtilityTest, TestSha256Hmac) { + const std::string key = "key"; + auto hmac = Utility::getSha256Hmac(std::vector(key.begin(), key.end()), "test data"); + EXPECT_EQ("087d9eb992628854842ca4dbf790f8164c80355c1e78b72789d830334927a84c", Hex::encode(hmac)); +} + +TEST(UtilityTest, TestSha256HmacWithEmptyArguments) { + auto hmac = Utility::getSha256Hmac(std::vector(), ""); + EXPECT_EQ("b613679a0814d9ec772f95d778c35fc5ff1697c493715653c6c712144292c5ad", Hex::encode(hmac)); +} + } // namespace Ssl } // namespace Envoy diff --git a/test/extensions/filters/http/common/aws/BUILD b/test/extensions/filters/http/common/aws/BUILD index 8c3dfd92bee2f..1c5c0541e4ebf 100644 --- a/test/extensions/filters/http/common/aws/BUILD +++ b/test/extensions/filters/http/common/aws/BUILD @@ -14,9 +14,8 @@ envoy_cc_mock( srcs = ["mocks.cc"], hdrs = ["mocks.h"], deps = [ - "//source/extensions/filters/http/common/aws:credentials_provider_lib", - "//source/extensions/filters/http/common/aws:region_provider_lib", - "//source/extensions/filters/http/common/aws:signer_lib", + "//source/extensions/filters/http/common/aws:credentials_provider_interface", + "//source/extensions/filters/http/common/aws:signer_interface", ], ) @@ -32,3 +31,12 @@ envoy_cc_test( "//test/test_common:utility_lib", ], ) + +envoy_cc_test( + name = "utility_test", + srcs = ["utility_test.cc"], + deps = [ + "//source/extensions/filters/http/common/aws:utility_lib", + "//test/test_common:utility_lib", + ], +) diff --git a/test/extensions/filters/http/common/aws/mocks.cc b/test/extensions/filters/http/common/aws/mocks.cc index 728cd9c275da2..cdc43b1ed1c30 100644 --- a/test/extensions/filters/http/common/aws/mocks.cc +++ b/test/extensions/filters/http/common/aws/mocks.cc @@ -10,10 +10,6 @@ MockCredentialsProvider::MockCredentialsProvider() {} MockCredentialsProvider::~MockCredentialsProvider() {} -MockRegionProvider::MockRegionProvider() {} - -MockRegionProvider::~MockRegionProvider() {} - MockSigner::MockSigner() {} MockSigner::~MockSigner() {} diff --git a/test/extensions/filters/http/common/aws/mocks.h b/test/extensions/filters/http/common/aws/mocks.h index a632cf3a66fbc..5ca30056051fb 100644 --- a/test/extensions/filters/http/common/aws/mocks.h +++ b/test/extensions/filters/http/common/aws/mocks.h @@ -1,7 +1,6 @@ #pragma once #include "extensions/filters/http/common/aws/credentials_provider.h" -#include "extensions/filters/http/common/aws/region_provider.h" #include "extensions/filters/http/common/aws/signer.h" #include "gmock/gmock.h" @@ -20,20 +19,12 @@ class MockCredentialsProvider : public CredentialsProvider { MOCK_METHOD0(getCredentials, Credentials()); }; -class MockRegionProvider : public RegionProvider { -public: - MockRegionProvider(); - ~MockRegionProvider(); - - MOCK_METHOD0(getRegion, absl::optional()); -}; - class MockSigner : public Signer { public: MockSigner(); ~MockSigner(); - MOCK_CONST_METHOD1(sign, void(Http::Message&)); + MOCK_METHOD1(sign, void(Http::Message&)); }; } // namespace Aws diff --git a/test/extensions/filters/http/common/aws/signer_impl_test.cc b/test/extensions/filters/http/common/aws/signer_impl_test.cc index deaa29d742b88..b019b320cf123 100644 --- a/test/extensions/filters/http/common/aws/signer_impl_test.cc +++ b/test/extensions/filters/http/common/aws/signer_impl_test.cc @@ -2,6 +2,7 @@ #include "common/http/message_impl.h" #include "extensions/filters/http/common/aws/signer_impl.h" +#include "extensions/filters/http/common/aws/utility.h" #include "test/extensions/filters/http/common/aws/mocks.h" #include "test/test_common/simulated_time_system.h" @@ -20,12 +21,10 @@ class SignerImplTest : public testing::Test { public: SignerImplTest() : credentials_provider_(new NiceMock()), - region_provider_(new NiceMock()), message_(new Http::RequestMessageImpl()), - signer_("service", CredentialsProviderSharedPtr{credentials_provider_}, - RegionProviderSharedPtr{region_provider_}, time_system_), - credentials_("akid", "secret"), token_credentials_("akid", "secret", "token"), - region_("region") { + signer_("service", "region", CredentialsProviderSharedPtr{credentials_provider_}, + time_system_), + credentials_("akid", "secret"), token_credentials_("akid", "secret", "token") { // 20180102T030405Z time_system_.setSystemTime(std::chrono::milliseconds(1514862245000)); } @@ -43,7 +42,7 @@ class SignerImplTest : public testing::Test { } std::string canonicalRequest() { - const auto canonical_headers = signer_.canonicalizeHeaders(message_->headers()); + const auto canonical_headers = Utility::canonicalizeHeaders(message_->headers()); const auto signing_headers = signer_.createSigningHeaders(canonical_headers); const auto content_hash = signer_.createContentHash(*message_); return signer_.createCanonicalRequest(*message_, canonical_headers, signing_headers, @@ -51,7 +50,6 @@ class SignerImplTest : public testing::Test { } NiceMock* credentials_provider_; - NiceMock* region_provider_; Event::SimulatedTimeSystem time_system_; Http::MessagePtr message_; SignerImpl signer_; @@ -60,128 +58,142 @@ class SignerImplTest : public testing::Test { absl::optional region_; }; +// No authorization header should be present when the credentials are empty TEST_F(SignerImplTest, AnonymousCredentials) { EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(Credentials())); - EXPECT_CALL(*region_provider_, getRegion()).Times(0); signer_.sign(*message_); EXPECT_EQ(nullptr, message_->headers().Authorization()); } -TEST_F(SignerImplTest, MissingRegionException) { - absl::optional no_region; - EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); - EXPECT_CALL(*region_provider_, getRegion()).WillOnce(Return(no_region)); - EXPECT_THROW_WITH_MESSAGE(signer_.sign(*message_), EnvoyException, - "Could not determine AWS region"); - EXPECT_EQ(nullptr, message_->headers().Authorization()); -} - -TEST_F(SignerImplTest, SecurityTokenHeader) { +// Verify we sign the security token header if the token is present in the credentials +TEST_F(SignerImplTest, SignSecurityTokenHeader) { EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(token_credentials_)); - EXPECT_CALL(*region_provider_, getRegion()).WillOnce(Return(region_)); addMethod("GET"); addPath("/"); signer_.sign(*message_); - EXPECT_STREQ("token", message_->headers().get(SignerImpl::X_AMZ_SECURITY_TOKEN)->value().c_str()); + EXPECT_STREQ("token", + message_->headers().get(SignatureHeaders::get().SecurityToken)->value().c_str()); EXPECT_STREQ("AWS4-HMAC-SHA256 Credential=akid/20180102/region/service/aws4_request, " "SignedHeaders=x-amz-content-sha256;x-amz-date;x-amz-security-token, " "Signature=1d42526aabf7d8b6d7d33d9db43b03537300cc7e6bb2817e349749e0a08f5b5e", message_->headers().Authorization()->value().c_str()); } -TEST_F(SignerImplTest, DateHeader) { +// Verify we sign the date header +TEST_F(SignerImplTest, SignDateHeader) { EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); - EXPECT_CALL(*region_provider_, getRegion()).WillOnce(Return(region_)); addMethod("GET"); addPath("/"); signer_.sign(*message_); EXPECT_STREQ("20180102T030400Z", - message_->headers().get(SignerImpl::X_AMZ_DATE)->value().c_str()); + message_->headers().get(SignatureHeaders::get().Date)->value().c_str()); EXPECT_STREQ("AWS4-HMAC-SHA256 Credential=akid/20180102/region/service/aws4_request, " "SignedHeaders=x-amz-content-sha256;x-amz-date, " "Signature=4ee6aa9355259c18133f150b139ea9aeb7969c9408ad361b2151f50a516afe42", message_->headers().Authorization()->value().c_str()); } -TEST_F(SignerImplTest, EmptyContentHeader) { +// Verify we sign the content header as the hashed empty string if the body is empty +TEST_F(SignerImplTest, SignEmptyContentHeader) { EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); - EXPECT_CALL(*region_provider_, getRegion()).WillOnce(Return(region_)); addMethod("GET"); addPath("/empty?content=none"); signer_.sign(*message_); - EXPECT_STREQ("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", - message_->headers().get(SignerImpl::X_AMZ_CONTENT_SHA256)->value().c_str()); + EXPECT_EQ( + SignatureConstants::get().HashedEmptyString, + message_->headers().get(SignatureHeaders::get().ContentSha256)->value().getStringView()); EXPECT_STREQ("AWS4-HMAC-SHA256 Credential=akid/20180102/region/service/aws4_request, " "SignedHeaders=x-amz-content-sha256;x-amz-date, " "Signature=999e211bc7134cc685f830a332cf4d871b6d8bb8ced9367c1a0b59b95a03ee7d", message_->headers().Authorization()->value().c_str()); } -TEST_F(SignerImplTest, ContentHeader) { +// Verify we sign the content header correctly when we have a body +TEST_F(SignerImplTest, SignContentHeader) { EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); - EXPECT_CALL(*region_provider_, getRegion()).WillOnce(Return(region_)); addMethod("POST"); addPath("/"); setBody("test1234"); signer_.sign(*message_); EXPECT_STREQ("937e8d5fbb48bd4949536cd65b8d35c426b80d2f830c5c308e2cdec422ae2244", - message_->headers().get(SignerImpl::X_AMZ_CONTENT_SHA256)->value().c_str()); + message_->headers().get(SignatureHeaders::get().ContentSha256)->value().c_str()); EXPECT_STREQ("AWS4-HMAC-SHA256 Credential=akid/20180102/region/service/aws4_request, " "SignedHeaders=x-amz-content-sha256;x-amz-date, " "Signature=4eab89c36f45f2032d6010ba1adab93f8510ddd6afe540821f3a05bb0253e27b", message_->headers().Authorization()->value().c_str()); } +// HTTP :method header is required TEST_F(SignerImplTest, MissingMethodException) { EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); - EXPECT_CALL(*region_provider_, getRegion()).WillOnce(Return(region_)); EXPECT_THROW_WITH_MESSAGE(signer_.sign(*message_), EnvoyException, "Message is missing :method header"); EXPECT_EQ(nullptr, message_->headers().Authorization()); } +// HTTP :path header is required TEST_F(SignerImplTest, MissingPathException) { EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); - EXPECT_CALL(*region_provider_, getRegion()).WillOnce(Return(region_)); addMethod("GET"); EXPECT_THROW_WITH_MESSAGE(signer_.sign(*message_), EnvoyException, "Message is missing :path header"); EXPECT_EQ(nullptr, message_->headers().Authorization()); } -TEST_F(SignerImplTest, ComplexCanonicalRequest) { +// Verify that a correct number of blank lines exist in a minimalist canonical request +TEST_F(SignerImplTest, EmptyCanonicalRequest) { + addMethod("POST"); + addPath("/hello"); + EXPECT_EQ(R"(POST +/hello + + + +e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855)", + canonicalRequest()); +} + +// Verify that we add the query string to the canonical request +TEST_F(SignerImplTest, QueryStringInCanonicalRequest) { addMethod("POST"); addPath("/path/foo?bar=baz"); - setBody("test1234"); - addHeader(":authority", "example.com:80"); - addHeader("UpperCase", "uppercasevalue"); - addHeader("MultiLine", "hello\n\nworld\n\nline3\n"); - addHeader("Trimmable", " trim me "); - addHeader("EmptyOne", ""); - addHeader("Alphabetic", "abcd"); EXPECT_EQ(R"(POST /path/foo bar=baz -alphabetic:abcd -emptyone: -host:example.com -multiline:hello,world,line3 -trimmable:trim me -uppercase:uppercasevalue - -alphabetic;emptyone;host;multiline;trimmable;uppercase -937e8d5fbb48bd4949536cd65b8d35c426b80d2f830c5c308e2cdec422ae2244)", + + +e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855)", canonicalRequest()); } -TEST_F(SignerImplTest, EmptyCanonicalRequest) { +// Verify that we use an empty path if we only have a query string +TEST_F(SignerImplTest, OnlyQueryStringInCanonicalRequest) { addMethod("POST"); - addPath("/hello"); + addPath("?bar=baz"); EXPECT_EQ(R"(POST -/hello +/ +bar=baz +e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855)", + canonicalRequest()); +} + +// Verify that we insert an extra blank line after the canonical headers +TEST_F(SignerImplTest, ExtraNewlineAfterCanonicalizedHeaders) { + addMethod("POST"); + addPath("/?query"); + addHeader("header1", "header1 value"); + addHeader("header2", "header2 value"); + addHeader("header3", "header3 value"); + EXPECT_EQ(R"(POST +/ +query +header1:header1 value +header2:header2 value +header3:header3 value +header1;header2;header3 e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855)", canonicalRequest()); } diff --git a/test/extensions/filters/http/common/aws/utility_test.cc b/test/extensions/filters/http/common/aws/utility_test.cc new file mode 100644 index 0000000000000..54d012d676abf --- /dev/null +++ b/test/extensions/filters/http/common/aws/utility_test.cc @@ -0,0 +1,89 @@ +#include "extensions/filters/http/common/aws/utility.h" + +#include "test/test_common/utility.h" + +#include "gtest/gtest.h" + +using testing::ElementsAre; +using testing::Pair; + +namespace Envoy { +namespace Extensions { +namespace HttpFilters { +namespace Common { +namespace Aws { + +// Headers must be in alphabetical order by virtue of std::map +TEST(UtilityTest, CanonicalizeHeadersInAlphabeticalOrder) { + Http::TestHeaderMapImpl headers{ + {"d", "d_value"}, {"f", "f_value"}, {"b", "b_value"}, + {"e", "e_value"}, {"c", "c_value"}, {"a", "a_value"}, + }; + const auto map = Utility::canonicalizeHeaders(headers); + 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"))); +} + +// HTTP pseudo-headers should be ignored +TEST(UtilityTest, CanonicalizeHeadersSkippingPseudoHeaders) { + Http::TestHeaderMapImpl headers{ + {":path", "path_value"}, + {":method", "GET"}, + {"normal", "normal_value"}, + }; + const auto map = Utility::canonicalizeHeaders(headers); + EXPECT_THAT(map, ElementsAre(Pair("normal", "normal_value"))); +} + +// Repeated headers are joined with commas +TEST(UtilityTest, CanonicalizeHeadersJoiningDuplicatesWithCommas) { + Http::TestHeaderMapImpl headers{ + {"a", "a_value1"}, + {"a", "a_value2"}, + {"a", "a_value3"}, + }; + const auto map = Utility::canonicalizeHeaders(headers); + EXPECT_THAT(map, ElementsAre(Pair("a", "a_value1,a_value2,a_value3"))); +} + +// We canonicalize the :authority header as host +TEST(UtilityTest, CanonicalizeHeadersAuthorityToHost) { + Http::TestHeaderMapImpl headers{ + {":authority", "authority_value"}, + }; + const auto map = Utility::canonicalizeHeaders(headers); + EXPECT_THAT(map, ElementsAre(Pair("host", "authority_value"))); +} + +// Ports 80 and 443 are omitted from the host headers +TEST(UtilityTest, CanonicalizeHeadersRemovingDefaultPortsFromHost) { + Http::TestHeaderMapImpl headers_port80{ + {":authority", "example.com:80"}, + }; + const auto map_port80 = Utility::canonicalizeHeaders(headers_port80); + EXPECT_THAT(map_port80, ElementsAre(Pair("host", "example.com"))); + + Http::TestHeaderMapImpl headers_port443{ + {":authority", "example.com:443"}, + }; + const auto map_port443 = Utility::canonicalizeHeaders(headers_port443); + EXPECT_THAT(map_port443, ElementsAre(Pair("host", "example.com"))); +} + +// Whitespace is trimmed from headers +TEST(UtilityTest, CanonicalizeHeadersTrimmingWhitespace) { + Http::TestHeaderMapImpl headers{{"leading", " leading value"}, + {"trailing", "trailing value "}, + {"internal", "internal value"}, + {"all", " all value "}}; + const auto map = Utility::canonicalizeHeaders(headers); + EXPECT_THAT(map, + ElementsAre(Pair("all", "all value"), Pair("internal", "internal value"), + Pair("leading", "leading value"), Pair("trailing", "trailing value"))); +} + +} // namespace Aws +} // namespace Common +} // namespace HttpFilters +} // namespace Extensions +} // namespace Envoy \ No newline at end of file From 22ba065723721d9221d5cf15d6b21409e78810b7 Mon Sep 17 00:00:00 2001 From: Scott LaVigne Date: Wed, 16 Jan 2019 17:01:01 -0800 Subject: [PATCH 3/6] Address more PR comments * Remove Logging::Id::aws * Move crypto utils to Extensions::Common::Crypto * Make request body signing optional * Move request canonicalization method to Aws::Utility * Explicity check for empty headers when canonicalizing Signed-off-by: Scott LaVigne --- source/common/common/logger.h | 1 - source/common/ssl/BUILD | 2 - source/common/ssl/utility.cc | 42 ----- source/common/ssl/utility.h | 17 -- source/extensions/common/crypto/BUILD | 23 +++ source/extensions/common/crypto/utility.cc | 47 ++++++ source/extensions/common/crypto/utility.h | 32 ++++ .../extensions/filters/http/common/aws/BUILD | 2 +- .../filters/http/common/aws/signer.h | 4 +- .../filters/http/common/aws/signer_impl.cc | 108 +++++------- .../filters/http/common/aws/signer_impl.h | 32 ++-- .../filters/http/common/aws/utility.cc | 40 +++++ .../filters/http/common/aws/utility.h | 27 ++- test/common/ssl/BUILD | 2 - test/common/ssl/utility_test.cc | 44 ----- test/extensions/common/crypto/BUILD | 22 +++ test/extensions/common/crypto/utility_test.cc | 57 +++++++ .../filters/http/common/aws/mocks.h | 2 +- .../http/common/aws/signer_impl_test.cc | 154 +++++++----------- .../filters/http/common/aws/utility_test.cc | 72 +++++++- 20 files changed, 428 insertions(+), 302 deletions(-) create mode 100644 source/extensions/common/crypto/BUILD create mode 100644 source/extensions/common/crypto/utility.cc create mode 100644 source/extensions/common/crypto/utility.h create mode 100644 test/extensions/common/crypto/BUILD create mode 100644 test/extensions/common/crypto/utility_test.cc diff --git a/source/common/common/logger.h b/source/common/common/logger.h index f7a09fc9eacf1..341caa20c8c96 100644 --- a/source/common/common/logger.h +++ b/source/common/common/logger.h @@ -22,7 +22,6 @@ namespace Logger { // clang-format off #define ALL_LOGGER_IDS(FUNCTION) \ FUNCTION(admin) \ - FUNCTION(aws) \ FUNCTION(assert) \ FUNCTION(backtrace) \ FUNCTION(client) \ diff --git a/source/common/ssl/BUILD b/source/common/ssl/BUILD index 05577d32f5466..11fb5c9b61907 100644 --- a/source/common/ssl/BUILD +++ b/source/common/ssl/BUILD @@ -118,9 +118,7 @@ envoy_cc_library( "ssl", ], deps = [ - "//include/envoy/buffer:buffer_interface", "//source/common/common:assert_lib", - "//source/common/common:stack_array", "//source/common/common:utility_lib", ], ) diff --git a/source/common/ssl/utility.cc b/source/common/ssl/utility.cc index 5164dfb681b2a..acf3583a920a5 100644 --- a/source/common/ssl/utility.cc +++ b/source/common/ssl/utility.cc @@ -1,12 +1,8 @@ #include "common/ssl/utility.h" #include "common/common/assert.h" -#include "common/common/stack_array.h" #include "absl/strings/str_join.h" -#include "openssl/evp.h" -#include "openssl/hmac.h" -#include "openssl/sha.h" #include "openssl/x509v3.h" namespace Envoy { @@ -105,43 +101,5 @@ SystemTime Utility::getExpirationTime(const X509& cert) { return std::chrono::system_clock::from_time_t(days * 24 * 60 * 60 + seconds); } -std::vector Utility::getSha256Digest(const Buffer::Instance& buffer) { - std::vector digest(SHA256_DIGEST_LENGTH); - EVP_MD_CTX ctx; - auto rc = EVP_DigestInit(&ctx, EVP_sha256()); - RELEASE_ASSERT(rc == 1, "Failed to init digest context"); - const auto num_slices = buffer.getRawSlices(nullptr, 0); - STACK_ARRAY(slices, Buffer::RawSlice, num_slices); - buffer.getRawSlices(slices.begin(), num_slices); - for (const auto& slice : slices) { - rc = EVP_DigestUpdate(&ctx, slice.mem_, slice.len_); - RELEASE_ASSERT(rc == 1, "Failed to update digest"); - } - unsigned int digest_length; - rc = EVP_DigestFinal(&ctx, digest.data(), &digest_length); - RELEASE_ASSERT(rc == 1, "Failed to finalize digest"); - RELEASE_ASSERT(digest_length == SHA256_DIGEST_LENGTH, "Digest length mismatch"); - return digest; -} - -std::vector Utility::getSha256Hmac(const std::vector& key, - absl::string_view string) { - std::vector mac(EVP_MAX_MD_SIZE); - HMAC_CTX ctx; - RELEASE_ASSERT(key.size() < std::numeric_limits::max(), "HMAC key is too long"); - HMAC_CTX_init(&ctx); - auto rc = HMAC_Init_ex(&ctx, key.data(), static_cast(key.size()), EVP_sha256(), nullptr); - RELEASE_ASSERT(rc == 1, "Failed to init HMAC context"); - rc = HMAC_Update(&ctx, reinterpret_cast(string.data()), string.size()); - RELEASE_ASSERT(rc == 1, "Failed to update HMAC"); - unsigned int len; - rc = HMAC_Final(&ctx, mac.data(), &len); - RELEASE_ASSERT(rc == 1, "Failed to finalize HMAC"); - RELEASE_ASSERT(len <= EVP_MAX_MD_SIZE, "HMAC length too large"); - HMAC_CTX_cleanup(&ctx); - mac.resize(len); - return mac; -} - } // namespace Ssl } // namespace Envoy diff --git a/source/common/ssl/utility.h b/source/common/ssl/utility.h index 468912752f6ec..acab625a3f18e 100644 --- a/source/common/ssl/utility.h +++ b/source/common/ssl/utility.h @@ -3,8 +3,6 @@ #include #include -#include "envoy/buffer/buffer.h" - #include "common/common/utility.h" #include "openssl/ssl.h" @@ -58,21 +56,6 @@ SystemTime getValidFrom(const X509& cert); */ SystemTime getExpirationTime(const X509& cert); -/** - * Computes the SHA-256 digest of a buffer. - * @param buffer the buffer. - * @return a vector of bytes for the computed digest. - */ -std::vector getSha256Digest(const Buffer::Instance& buffer); - -/** - * Computes the SHA-256 HMAC for a given key and data. - * @param key the HMAC function key. - * @param string string data for the HMAC function. - * @return a vector of bytes for the computed HMAC. - */ -std::vector getSha256Hmac(const std::vector& key, absl::string_view string); - } // namespace Utility } // namespace Ssl } // namespace Envoy diff --git a/source/extensions/common/crypto/BUILD b/source/extensions/common/crypto/BUILD new file mode 100644 index 0000000000000..f802b7051e308 --- /dev/null +++ b/source/extensions/common/crypto/BUILD @@ -0,0 +1,23 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_package", +) + +envoy_package() + +envoy_cc_library( + name = "utility_lib", + srcs = ["utility.cc"], + hdrs = ["utility.h"], + external_deps = [ + "ssl", + ], + deps = [ + "//include/envoy/buffer:buffer_interface", + "//source/common/common:assert_lib", + "//source/common/common:stack_array", + ], +) diff --git a/source/extensions/common/crypto/utility.cc b/source/extensions/common/crypto/utility.cc new file mode 100644 index 0000000000000..5b568bcedb582 --- /dev/null +++ b/source/extensions/common/crypto/utility.cc @@ -0,0 +1,47 @@ +#include "extensions/common/crypto/utility.h" + +#include "common/common/assert.h" +#include "common/common/stack_array.h" + +#include "openssl/evp.h" +#include "openssl/hmac.h" +#include "openssl/sha.h" + +namespace Envoy { +namespace Extensions { +namespace Common { +namespace Crypto { + +std::vector Utility::getSha256Digest(const Buffer::Instance& buffer) { + std::vector digest(SHA256_DIGEST_LENGTH); + EVP_MD_CTX ctx; + auto rc = EVP_DigestInit(&ctx, EVP_sha256()); + RELEASE_ASSERT(rc == 1, "Failed to init digest context"); + const auto num_slices = buffer.getRawSlices(nullptr, 0); + STACK_ARRAY(slices, Buffer::RawSlice, num_slices); + buffer.getRawSlices(slices.begin(), num_slices); + for (const auto& slice : slices) { + rc = EVP_DigestUpdate(&ctx, slice.mem_, slice.len_); + RELEASE_ASSERT(rc == 1, "Failed to update digest"); + } + unsigned int len; + rc = EVP_DigestFinal(&ctx, digest.data(), &len); + RELEASE_ASSERT(rc == 1, "Failed to finalize digest"); + return digest; +} + +std::vector Utility::getSha256Hmac(const std::vector& key, + absl::string_view message) { + std::vector hmac(SHA256_DIGEST_LENGTH); + unsigned int len; + auto ret = + HMAC(EVP_sha256(), key.data(), key.size(), reinterpret_cast(message.data()), + message.size(), hmac.data(), &len); + RELEASE_ASSERT(ret != nullptr, "Failed to create HMAC"); + return hmac; +} + +} // namespace Crypto +} // namespace Common +} // namespace Extensions +} // namespace Envoy diff --git a/source/extensions/common/crypto/utility.h b/source/extensions/common/crypto/utility.h new file mode 100644 index 0000000000000..3e4771087fb34 --- /dev/null +++ b/source/extensions/common/crypto/utility.h @@ -0,0 +1,32 @@ +#pragma once + +#include "envoy/buffer/buffer.h" + +namespace Envoy { +namespace Extensions { +namespace Common { +namespace Crypto { + +class Utility { +public: + /** + * Computes the SHA-256 digest of a buffer. + * @param buffer the buffer. + * @return a vector of bytes for the computed digest. + */ + static std::vector getSha256Digest(const Buffer::Instance& buffer); + + /** + * Computes the SHA-256 HMAC for a given key and message. + * @param key the HMAC function key. + * @param message message data for the HMAC function. + * @return a vector of bytes for the computed HMAC. + */ + static std::vector getSha256Hmac(const std::vector& key, + absl::string_view message); +}; + +} // namespace Crypto +} // namespace Common +} // namespace Extensions +} // namespace Envoy \ No newline at end of file diff --git a/source/extensions/filters/http/common/aws/BUILD b/source/extensions/filters/http/common/aws/BUILD index c73f0dbda1591..3b79211956c81 100644 --- a/source/extensions/filters/http/common/aws/BUILD +++ b/source/extensions/filters/http/common/aws/BUILD @@ -30,7 +30,7 @@ envoy_cc_library( "//source/common/common:utility_lib", "//source/common/http:headers_lib", "//source/common/singleton:const_singleton", - "//source/common/ssl:utility_lib", + "//source/extensions/common/crypto:utility_lib", ], ) diff --git a/source/extensions/filters/http/common/aws/signer.h b/source/extensions/filters/http/common/aws/signer.h index 0e93b67d68689..bf504bc4356b1 100644 --- a/source/extensions/filters/http/common/aws/signer.h +++ b/source/extensions/filters/http/common/aws/signer.h @@ -17,8 +17,10 @@ class Signer { /** * Sign an AWS request. * @param message an AWS API request message. + * @param sign_body include the message body in the signature. The body must be fully buffered. + * @throws EnvoyException if the request cannot be signed. */ - virtual void sign(Http::Message& message) PURE; + virtual void sign(Http::Message& message, bool sign_body) PURE; }; typedef std::unique_ptr SignerPtr; diff --git a/source/extensions/filters/http/common/aws/signer_impl.cc b/source/extensions/filters/http/common/aws/signer_impl.cc index 48908babe69ac..dae01fa1fa25f 100644 --- a/source/extensions/filters/http/common/aws/signer_impl.cc +++ b/source/extensions/filters/http/common/aws/signer_impl.cc @@ -6,9 +6,9 @@ #include "common/common/fmt.h" #include "common/common/hex.h" #include "common/http/headers.h" -#include "common/ssl/utility.h" #include "extensions/filters/http/common/aws/utility.h" +#include "extensions/common/crypto/utility.h" #include "absl/strings/str_join.h" @@ -18,7 +18,7 @@ namespace HttpFilters { namespace Common { namespace Aws { -void SignerImpl::sign(Http::Message& message) { +void SignerImpl::sign(Http::Message& message, bool sign_body) { const auto& credentials = credentials_provider_->getCredentials(); if (!credentials.accessKeyId() || !credentials.secretAccessKey()) { // Empty or "anonymous" credentials are a valid use-case for non-production environments. @@ -26,19 +26,26 @@ void SignerImpl::sign(Http::Message& message) { return; } auto& headers = message.headers(); + const auto* method_header = headers.Method(); + if (method_header == nullptr) { + throw EnvoyException("Message is missing :method header"); + } + const auto* path_header = headers.Path(); + if (path_header == nullptr) { + throw EnvoyException("Message is missing :path header"); + } if (credentials.sessionToken()) { headers.addCopy(SignatureHeaders::get().SecurityToken, credentials.sessionToken().value()); } const auto long_date = long_date_formatter_.now(time_source_); const auto short_date = short_date_formatter_.now(time_source_); headers.addCopy(SignatureHeaders::get().Date, long_date); - const auto content_hash = createContentHash(message); - headers.addCopy(SignatureHeaders::get().ContentSha256, content_hash); + const auto content_hash = createContentHash(message, sign_body); // Phase 1: Create a canonical request const auto canonical_headers = Utility::canonicalizeHeaders(headers); - const auto signing_headers = createSigningHeaders(canonical_headers); - const auto canonical_request = - createCanonicalRequest(message, canonical_headers, signing_headers, content_hash); + const auto canonical_request = Utility::createCanonicalRequest( + method_header->value().getStringView(), path_header->value().getStringView(), + canonical_headers, content_hash); ENVOY_LOG(debug, "Canonical request:\n{}", canonical_request); // Phase 2: Create a string to sign const auto credential_scope = createCredentialScope(short_date); @@ -49,58 +56,21 @@ void SignerImpl::sign(Http::Message& message) { createSignature(credentials.secretAccessKey().value(), short_date, string_to_sign); // Phase 4: Sign request const auto authorization_header = createAuthorizationHeader( - credentials.accessKeyId().value(), credential_scope, signing_headers, signature); + credentials.accessKeyId().value(), credential_scope, canonical_headers, signature); ENVOY_LOG(debug, "Signing request with: {}", authorization_header); headers.addCopy(Http::Headers::get().Authorization, authorization_header); } -std::string SignerImpl::createContentHash(Http::Message& message) const { - if (!message.body()) { +std::string SignerImpl::createContentHash(Http::Message& message, bool sign_body) const { + if (!sign_body) { return SignatureConstants::get().HashedEmptyString; } - return Hex::encode(Ssl::Utility::getSha256Digest(*message.body())); -} - -std::string SignerImpl::createCanonicalRequest( - Http::Message& message, const std::map& canonical_headers, - absl::string_view signing_headers, absl::string_view content_hash) const { - std::vector parts; - const auto* method_header = message.headers().Method(); - if (method_header == nullptr || method_header->value().empty()) { - throw EnvoyException("Message is missing :method header"); - } - parts.emplace_back(method_header->value().getStringView()); - const auto* path_header = message.headers().Path(); - if (path_header == nullptr || path_header->value().empty()) { - throw EnvoyException("Message is missing :path header"); - } - // don't include the query part of the path - const auto path = StringUtil::cropRight(path_header->value().getStringView(), "?"); - parts.emplace_back(path.empty() ? "/" : path); - const auto query = StringUtil::cropLeft(path_header->value().getStringView(), "?"); - // if query == path, then there is no query - parts.emplace_back(query == path ? "" : query); - std::vector formatted_headers; - formatted_headers.reserve(canonical_headers.size()); - for (const auto& header : canonical_headers) { - formatted_headers.emplace_back(fmt::format("{}:{}", header.first, header.second)); - parts.emplace_back(formatted_headers.back()); - } - // need an extra blank space after the canonical headers - parts.emplace_back(""); - parts.emplace_back(signing_headers); - parts.emplace_back(content_hash); - return absl::StrJoin(parts, "\n"); -} - -std::string SignerImpl::createSigningHeaders( - const std::map& canonical_headers) const { - std::vector keys; - keys.reserve(canonical_headers.size()); - for (const auto& header : canonical_headers) { - keys.emplace_back(header.first); - } - return absl::StrJoin(keys, ";"); + const auto content_hash = + message.body() + ? Hex::encode(Extensions::Common::Crypto::Utility::getSha256Digest(*message.body())) + : SignatureConstants::get().HashedEmptyString; + message.headers().addCopy(SignatureHeaders::get().ContentSha256, content_hash); + return content_hash; } std::string SignerImpl::createCredentialScope(absl::string_view short_date) const { @@ -111,9 +81,9 @@ std::string SignerImpl::createCredentialScope(absl::string_view short_date) cons std::string SignerImpl::createStringToSign(absl::string_view canonical_request, absl::string_view long_date, absl::string_view credential_scope) const { - return fmt::format( - SignatureConstants::get().StringToSignFormat, long_date, credential_scope, - Hex::encode(Ssl::Utility::getSha256Digest(Buffer::OwnedImpl(canonical_request)))); + return fmt::format(SignatureConstants::get().StringToSignFormat, long_date, credential_scope, + Hex::encode(Extensions::Common::Crypto::Utility::getSha256Digest( + Buffer::OwnedImpl(canonical_request)))); } std::string SignerImpl::createSignature(absl::string_view secret_access_key, @@ -121,21 +91,25 @@ std::string SignerImpl::createSignature(absl::string_view secret_access_key, absl::string_view string_to_sign) const { const auto secret_key = absl::StrCat(SignatureConstants::get().SignatureVersion, secret_access_key); - const auto date_key = Ssl::Utility::getSha256Hmac( + const auto date_key = Extensions::Common::Crypto::Utility::getSha256Hmac( std::vector(secret_key.begin(), secret_key.end()), short_date); - const auto region_key = Ssl::Utility::getSha256Hmac(date_key, region_); - const auto service_key = Ssl::Utility::getSha256Hmac(region_key, service_name_); - const auto signing_key = - Ssl::Utility::getSha256Hmac(service_key, SignatureConstants::get().Aws4Request); - return Hex::encode(Ssl::Utility::getSha256Hmac(signing_key, string_to_sign)); + const auto region_key = Extensions::Common::Crypto::Utility::getSha256Hmac(date_key, region_); + const auto service_key = + Extensions::Common::Crypto::Utility::getSha256Hmac(region_key, service_name_); + const auto signing_key = Extensions::Common::Crypto::Utility::getSha256Hmac( + service_key, SignatureConstants::get().Aws4Request); + return Hex::encode( + Extensions::Common::Crypto::Utility::getSha256Hmac(signing_key, string_to_sign)); } -std::string SignerImpl::createAuthorizationHeader(absl::string_view access_key_id, - absl::string_view credential_scope, - absl::string_view signing_headers, - absl::string_view signature) const { +std::string +SignerImpl::createAuthorizationHeader(absl::string_view access_key_id, + absl::string_view credential_scope, + const std::map& canonical_headers, + absl::string_view signature) const { + const auto signed_headers = Utility::joinCanonicalHeaderNames(canonical_headers); return fmt::format(SignatureConstants::get().AuthorizationHeaderFormat, access_key_id, - credential_scope, signing_headers, signature); + credential_scope, signed_headers, signature); } } // namespace Aws diff --git a/source/extensions/filters/http/common/aws/signer_impl.h b/source/extensions/filters/http/common/aws/signer_impl.h index c81f90cdefa52..8a7dabfdcf765 100644 --- a/source/extensions/filters/http/common/aws/signer_impl.h +++ b/source/extensions/filters/http/common/aws/signer_impl.h @@ -43,7 +43,7 @@ typedef ConstSingleton SignatureConstants; * 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 { +class SignerImpl : public Signer, public Logger::Loggable { public: SignerImpl(absl::string_view service_name, absl::string_view region, const CredentialsProviderSharedPtr& credentials_provider, TimeSource& time_source) @@ -51,27 +51,10 @@ class SignerImpl : public Signer, public Logger::Loggable { time_source_(time_source), long_date_formatter_(SignatureConstants::get().LongDateFormat), short_date_formatter_(SignatureConstants::get().ShortDateFormat) {} - void sign(Http::Message& message) override; + void sign(Http::Message& message, bool sign_body = false) override; private: - friend class SignerImplTest; - - const std::string service_name_; - const std::string region_; - CredentialsProviderSharedPtr credentials_provider_; - TimeSource& time_source_; - DateFormatter long_date_formatter_; - DateFormatter short_date_formatter_; - - std::string createContentHash(Http::Message& message) const; - - std::string createCanonicalRequest(Http::Message& message, - const std::map& canonical_headers, - absl::string_view signing_headers, - absl::string_view content_hash) const; - - std::string - createSigningHeaders(const std::map& canonical_headers) const; + std::string createContentHash(Http::Message& message, bool sign_body) const; std::string createCredentialScope(absl::string_view short_date) const; @@ -83,8 +66,15 @@ class SignerImpl : public Signer, public Logger::Loggable { std::string createAuthorizationHeader(absl::string_view access_key_id, absl::string_view credential_scope, - absl::string_view signing_headers, + const std::map& canonical_headers, absl::string_view signature) const; + + const std::string service_name_; + const std::string region_; + CredentialsProviderSharedPtr credentials_provider_; + TimeSource& time_source_; + DateFormatter long_date_formatter_; + DateFormatter short_date_formatter_; }; } // namespace Aws diff --git a/source/extensions/filters/http/common/aws/utility.cc b/source/extensions/filters/http/common/aws/utility.cc index 226a6a0e2d7c8..55f4a5aab08b4 100644 --- a/source/extensions/filters/http/common/aws/utility.cc +++ b/source/extensions/filters/http/common/aws/utility.cc @@ -3,6 +3,8 @@ #include "common/common/fmt.h" #include "common/common/utility.h" +#include "absl/strings/str_join.h" + namespace Envoy { namespace Extensions { namespace HttpFilters { @@ -14,6 +16,10 @@ std::map Utility::canonicalizeHeaders(const Http::Head headers.iterate( [](const Http::HeaderEntry& entry, void* context) -> Http::HeaderMap::Iterate { auto* map = static_cast*>(context); + // Skip empty headers + if (entry.key().empty() || entry.value().empty()) { + return Http::HeaderMap::Iterate::Continue; + } // Pseudo-headers should not be canonicalized if (entry.key().c_str()[0] == ':') { return Http::HeaderMap::Iterate::Continue; @@ -33,6 +39,7 @@ std::map Utility::canonicalizeHeaders(const Http::Head &out); // 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 const auto* authority_header = headers.Host(); if (authority_header != nullptr && !authority_header->value().empty()) { const auto& value = authority_header->value().getStringView(); @@ -47,6 +54,39 @@ std::map Utility::canonicalizeHeaders(const Http::Head return out; } +std::string +Utility::createCanonicalRequest(absl::string_view method, absl::string_view path, + const std::map& canonical_headers, + absl::string_view content_hash) { + std::vector parts; + parts.emplace_back(method); + // don't include the query part of the path + const auto path_part = StringUtil::cropRight(path, "?"); + parts.emplace_back(path_part.empty() ? "/" : path_part); + const auto query_part = StringUtil::cropLeft(path, "?"); + // if query_part == path_part, then there is no query + parts.emplace_back(query_part == path_part ? "" : query_part); + std::vector formatted_headers; + formatted_headers.reserve(canonical_headers.size()); + for (const auto& header : canonical_headers) { + formatted_headers.emplace_back(fmt::format("{}:{}", header.first, header.second)); + parts.emplace_back(formatted_headers.back()); + } + // need an extra blank space after the canonical headers + parts.emplace_back(""); + const auto signed_headers = Utility::joinCanonicalHeaderNames(canonical_headers); + parts.emplace_back(signed_headers); + parts.emplace_back(content_hash); + return absl::StrJoin(parts, "\n"); +} + +std::string +Utility::joinCanonicalHeaderNames(const std::map& canonical_headers) { + return absl::StrJoin(canonical_headers, ";", [](auto* out, const auto& pair) { + return absl::StrAppend(out, pair.first); + }); +} + } // namespace Aws } // namespace Common } // namespace HttpFilters diff --git a/source/extensions/filters/http/common/aws/utility.h b/source/extensions/filters/http/common/aws/utility.h index 24e4de5c9de1a..8626a40f14280 100644 --- a/source/extensions/filters/http/common/aws/utility.h +++ b/source/extensions/filters/http/common/aws/utility.h @@ -11,12 +11,33 @@ namespace Aws { class Utility { public: /** - * Creates a canonicalized header map according to - * https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html + * 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. - * @return a map of header key value pairs used for signing requests. + * @return a std::map of canonicalized headers to be used in building a canonical request. */ static std::map canonicalizeHeaders(const Http::HeaderMap& headers); + + /** + * Creates an AWS Signature V4 canonical request string. + * See https://docs.aws.amazon.com/general/latest/gr/sigv4-create-canonical-request.html + * @param method the HTTP request method. + * @param path the request path. + * @param canonical_headers the pre-canonicalized request headers. + * @param content_hash the hashed request body. + * @return the canonicalized request string. + */ + static std::string + createCanonicalRequest(absl::string_view method, absl::string_view path, + const std::map& canonical_headers, + absl::string_view content_hash); + + /** + * Get the semicolon-delimited string of canonical header names. + * @param canonical_headers the pre-canonicalized request headers. + * @return the header names as a semicolon-delimited string. + */ + static std::string joinCanonicalHeaderNames(const std::map& canonical_headers); }; } // namespace Aws diff --git a/test/common/ssl/BUILD b/test/common/ssl/BUILD index 9705eaf6aca06..39fdbee27dcb0 100644 --- a/test/common/ssl/BUILD +++ b/test/common/ssl/BUILD @@ -88,8 +88,6 @@ envoy_cc_test( external_deps = ["ssl"], deps = [ ":ssl_test_utils", - "//source/common/buffer:buffer_lib", - "//source/common/common:hex_lib", "//source/common/ssl:utility_lib", "//test/common/ssl/test_data:cert_infos", "//test/test_common:environment_lib", diff --git a/test/common/ssl/utility_test.cc b/test/common/ssl/utility_test.cc index 56b3799a120a9..207fe5cc1ffd6 100644 --- a/test/common/ssl/utility_test.cc +++ b/test/common/ssl/utility_test.cc @@ -1,8 +1,6 @@ #include #include -#include "common/buffer/buffer_impl.h" -#include "common/common/hex.h" #include "common/ssl/utility.h" #include "test/common/ssl/ssl_test_utility.h" @@ -99,47 +97,5 @@ TEST(UtilityTest, TestExpirationTime) { ASSERT(len == sizeof(buffer) - 1); EXPECT_EQ(TEST_SAN_DNS_CERT_NOT_AFTER, std::string(buffer)); } - -TEST(UtilityTest, TestSha256Disgest) { - const Buffer::OwnedImpl buffer("test data"); - const auto digest = Utility::getSha256Digest(buffer); - EXPECT_EQ("916f0027a575074ce72a331777c3478d6513f786a591bd892da1a577bf2335f9", - Hex::encode(digest)); -} - -TEST(UtilityTest, TestSha256DisgestWithEmptyBuffer) { - const Buffer::OwnedImpl buffer; - const auto digest = Utility::getSha256Digest(buffer); - EXPECT_EQ("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", - Hex::encode(digest)); -} - -TEST(UtilityTest, TestSha256DisgestGrowingBuffer) { - // Adding multiple slices to the buffer - Buffer::OwnedImpl buffer("slice 1"); - auto digest = Utility::getSha256Digest(buffer); - EXPECT_EQ("76571770bb46bdf51e1aba95b23c681fda27f6ae56a8a90898a4cb7556e19dcb", - Hex::encode(digest)); - buffer.add("slice 2"); - digest = Utility::getSha256Digest(buffer); - EXPECT_EQ("290b462b0fe5edcf6b8532de3ca70da8ab77937212042bb959192ec6c9f95b9a", - Hex::encode(digest)); - buffer.add("slice 3"); - digest = Utility::getSha256Digest(buffer); - EXPECT_EQ("29606bbf02fdc40007cdf799de36d931e3587dafc086937efd6599a4ea9397aa", - Hex::encode(digest)); -} - -TEST(UtilityTest, TestSha256Hmac) { - const std::string key = "key"; - auto hmac = Utility::getSha256Hmac(std::vector(key.begin(), key.end()), "test data"); - EXPECT_EQ("087d9eb992628854842ca4dbf790f8164c80355c1e78b72789d830334927a84c", Hex::encode(hmac)); -} - -TEST(UtilityTest, TestSha256HmacWithEmptyArguments) { - auto hmac = Utility::getSha256Hmac(std::vector(), ""); - EXPECT_EQ("b613679a0814d9ec772f95d778c35fc5ff1697c493715653c6c712144292c5ad", Hex::encode(hmac)); -} - } // namespace Ssl } // namespace Envoy diff --git a/test/extensions/common/crypto/BUILD b/test/extensions/common/crypto/BUILD new file mode 100644 index 0000000000000..2f79bdf1e0955 --- /dev/null +++ b/test/extensions/common/crypto/BUILD @@ -0,0 +1,22 @@ +licenses(["notice"]) # Apache 2 + +load( + "//bazel:envoy_build_system.bzl", + "envoy_cc_test", + "envoy_package", +) + +envoy_package() + +envoy_cc_test( + name = "utility_test", + srcs = [ + "utility_test.cc", + ], + external_deps = ["ssl"], + deps = [ + "//source/common/buffer:buffer_lib", + "//source/common/common:hex_lib", + "//source/extensions/common/crypto:utility_lib", + ], +) diff --git a/test/extensions/common/crypto/utility_test.cc b/test/extensions/common/crypto/utility_test.cc new file mode 100644 index 0000000000000..1022564163a0c --- /dev/null +++ b/test/extensions/common/crypto/utility_test.cc @@ -0,0 +1,57 @@ +#include "extensions/common/crypto/utility.h" + +#include "common/buffer/buffer_impl.h" +#include "common/common/hex.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Common { +namespace Crypto { + +TEST(UtilityTest, TestSha256Disgest) { + const Buffer::OwnedImpl buffer("test data"); + const auto digest = Utility::getSha256Digest(buffer); + EXPECT_EQ("916f0027a575074ce72a331777c3478d6513f786a591bd892da1a577bf2335f9", + Hex::encode(digest)); +} + +TEST(UtilityTest, TestSha256DisgestWithEmptyBuffer) { + const Buffer::OwnedImpl buffer; + const auto digest = Utility::getSha256Digest(buffer); + EXPECT_EQ("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + Hex::encode(digest)); +} + +TEST(UtilityTest, TestSha256DisgestGrowingBuffer) { + // Adding multiple slices to the buffer + Buffer::OwnedImpl buffer("slice 1"); + auto digest = Utility::getSha256Digest(buffer); + EXPECT_EQ("76571770bb46bdf51e1aba95b23c681fda27f6ae56a8a90898a4cb7556e19dcb", + Hex::encode(digest)); + buffer.add("slice 2"); + digest = Utility::getSha256Digest(buffer); + EXPECT_EQ("290b462b0fe5edcf6b8532de3ca70da8ab77937212042bb959192ec6c9f95b9a", + Hex::encode(digest)); + buffer.add("slice 3"); + digest = Utility::getSha256Digest(buffer); + EXPECT_EQ("29606bbf02fdc40007cdf799de36d931e3587dafc086937efd6599a4ea9397aa", + Hex::encode(digest)); +} + +TEST(UtilityTest, TestSha256Hmac) { + const std::string key = "key"; + auto hmac = Utility::getSha256Hmac(std::vector(key.begin(), key.end()), "test data"); + EXPECT_EQ("087d9eb992628854842ca4dbf790f8164c80355c1e78b72789d830334927a84c", Hex::encode(hmac)); +} + +TEST(UtilityTest, TestSha256HmacWithEmptyArguments) { + auto hmac = Utility::getSha256Hmac(std::vector(), ""); + EXPECT_EQ("b613679a0814d9ec772f95d778c35fc5ff1697c493715653c6c712144292c5ad", Hex::encode(hmac)); +} + +} // namespace Crypto +} // namespace Common +} // namespace Extensions +} // namespace Envoy diff --git a/test/extensions/filters/http/common/aws/mocks.h b/test/extensions/filters/http/common/aws/mocks.h index 5ca30056051fb..6b21e8f5c9581 100644 --- a/test/extensions/filters/http/common/aws/mocks.h +++ b/test/extensions/filters/http/common/aws/mocks.h @@ -24,7 +24,7 @@ class MockSigner : public Signer { MockSigner(); ~MockSigner(); - MOCK_METHOD1(sign, void(Http::Message&)); + MOCK_METHOD2(sign, void(Http::Message&,bool)); }; } // namespace Aws diff --git a/test/extensions/filters/http/common/aws/signer_impl_test.cc b/test/extensions/filters/http/common/aws/signer_impl_test.cc index b019b320cf123..7d8d7331bc3d5 100644 --- a/test/extensions/filters/http/common/aws/signer_impl_test.cc +++ b/test/extensions/filters/http/common/aws/signer_impl_test.cc @@ -41,14 +41,6 @@ class SignerImplTest : public testing::Test { message_->body() = std::make_unique(body); } - std::string canonicalRequest() { - const auto canonical_headers = Utility::canonicalizeHeaders(message_->headers()); - const auto signing_headers = signer_.createSigningHeaders(canonical_headers); - const auto content_hash = signer_.createContentHash(*message_); - return signer_.createCanonicalRequest(*message_, canonical_headers, signing_headers, - content_hash); - } - NiceMock* credentials_provider_; Event::SimulatedTimeSystem time_system_; Http::MessagePtr message_; @@ -65,18 +57,21 @@ TEST_F(SignerImplTest, AnonymousCredentials) { EXPECT_EQ(nullptr, message_->headers().Authorization()); } -// Verify we sign the security token header if the token is present in the credentials -TEST_F(SignerImplTest, SignSecurityTokenHeader) { - EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(token_credentials_)); +// HTTP :method header is required +TEST_F(SignerImplTest, MissingMethodException) { + EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); + EXPECT_THROW_WITH_MESSAGE(signer_.sign(*message_), EnvoyException, + "Message is missing :method header"); + EXPECT_EQ(nullptr, message_->headers().Authorization()); +} + +// HTTP :path header is required +TEST_F(SignerImplTest, MissingPathException) { + EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); addMethod("GET"); - addPath("/"); - signer_.sign(*message_); - EXPECT_STREQ("token", - message_->headers().get(SignatureHeaders::get().SecurityToken)->value().c_str()); - EXPECT_STREQ("AWS4-HMAC-SHA256 Credential=akid/20180102/region/service/aws4_request, " - "SignedHeaders=x-amz-content-sha256;x-amz-date;x-amz-security-token, " - "Signature=1d42526aabf7d8b6d7d33d9db43b03537300cc7e6bb2817e349749e0a08f5b5e", - message_->headers().Authorization()->value().c_str()); + EXPECT_THROW_WITH_MESSAGE(signer_.sign(*message_), EnvoyException, + "Message is missing :path header"); + EXPECT_EQ(nullptr, message_->headers().Authorization()); } // Verify we sign the date header @@ -85,11 +80,26 @@ TEST_F(SignerImplTest, SignDateHeader) { addMethod("GET"); addPath("/"); signer_.sign(*message_); + EXPECT_EQ(nullptr, message_->headers().get(SignatureHeaders::get().ContentSha256)); EXPECT_STREQ("20180102T030400Z", message_->headers().get(SignatureHeaders::get().Date)->value().c_str()); EXPECT_STREQ("AWS4-HMAC-SHA256 Credential=akid/20180102/region/service/aws4_request, " - "SignedHeaders=x-amz-content-sha256;x-amz-date, " - "Signature=4ee6aa9355259c18133f150b139ea9aeb7969c9408ad361b2151f50a516afe42", + "SignedHeaders=x-amz-date, " + "Signature=1310784f67248cab70d98b9404d601f30d8fe20bd1820560cce224f4131dc1cc", + message_->headers().Authorization()->value().c_str()); +} + +// Verify we sign the security token header if the token is present in the credentials +TEST_F(SignerImplTest, SignSecurityTokenHeader) { + EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(token_credentials_)); + addMethod("GET"); + addPath("/"); + signer_.sign(*message_); + EXPECT_STREQ("token", + message_->headers().get(SignatureHeaders::get().SecurityToken)->value().c_str()); + EXPECT_STREQ("AWS4-HMAC-SHA256 Credential=akid/20180102/region/service/aws4_request, " + "SignedHeaders=x-amz-date;x-amz-security-token, " + "Signature=ff1d9fa7e54a72677b5336df047bb1f1493f86b92099973bf62da3af852d1679", message_->headers().Authorization()->value().c_str()); } @@ -97,14 +107,13 @@ TEST_F(SignerImplTest, SignDateHeader) { TEST_F(SignerImplTest, SignEmptyContentHeader) { EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); addMethod("GET"); - addPath("/empty?content=none"); - signer_.sign(*message_); - EXPECT_EQ( - SignatureConstants::get().HashedEmptyString, - message_->headers().get(SignatureHeaders::get().ContentSha256)->value().getStringView()); + addPath("/"); + signer_.sign(*message_, true); + EXPECT_STREQ(SignatureConstants::get().HashedEmptyString.c_str(), + message_->headers().get(SignatureHeaders::get().ContentSha256)->value().c_str()); EXPECT_STREQ("AWS4-HMAC-SHA256 Credential=akid/20180102/region/service/aws4_request, " "SignedHeaders=x-amz-content-sha256;x-amz-date, " - "Signature=999e211bc7134cc685f830a332cf4d871b6d8bb8ced9367c1a0b59b95a03ee7d", + "Signature=4ee6aa9355259c18133f150b139ea9aeb7969c9408ad361b2151f50a516afe42", message_->headers().Authorization()->value().c_str()); } @@ -114,7 +123,7 @@ TEST_F(SignerImplTest, SignContentHeader) { addMethod("POST"); addPath("/"); setBody("test1234"); - signer_.sign(*message_); + signer_.sign(*message_, true); EXPECT_STREQ("937e8d5fbb48bd4949536cd65b8d35c426b80d2f830c5c308e2cdec422ae2244", message_->headers().get(SignatureHeaders::get().ContentSha256)->value().c_str()); EXPECT_STREQ("AWS4-HMAC-SHA256 Credential=akid/20180102/region/service/aws4_request, " @@ -123,79 +132,32 @@ TEST_F(SignerImplTest, SignContentHeader) { message_->headers().Authorization()->value().c_str()); } -// HTTP :method header is required -TEST_F(SignerImplTest, MissingMethodException) { +// Verify we sign some extra headers +TEST_F(SignerImplTest, SignExtraHeaders) { EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); - EXPECT_THROW_WITH_MESSAGE(signer_.sign(*message_), EnvoyException, - "Message is missing :method header"); - EXPECT_EQ(nullptr, message_->headers().Authorization()); + addMethod("GET"); + addPath("/"); + addHeader("a", "a_value"); + addHeader("b", "b_value"); + addHeader("c", "c_value"); + signer_.sign(*message_); + EXPECT_STREQ("AWS4-HMAC-SHA256 Credential=akid/20180102/region/service/aws4_request, " + "SignedHeaders=a;b;c;x-amz-date, " + "Signature=d5e025e1cf0d5af0d83110bc2ef1cafd2d9dca1dea9d7767f58308da64aa6558", + message_->headers().Authorization()->value().c_str()); } -// HTTP :path header is required -TEST_F(SignerImplTest, MissingPathException) { +// Verify signing a host header +TEST_F(SignerImplTest, SignHostHeader) { EXPECT_CALL(*credentials_provider_, getCredentials()).WillOnce(Return(credentials_)); addMethod("GET"); - EXPECT_THROW_WITH_MESSAGE(signer_.sign(*message_), EnvoyException, - "Message is missing :path header"); - EXPECT_EQ(nullptr, message_->headers().Authorization()); -} - -// Verify that a correct number of blank lines exist in a minimalist canonical request -TEST_F(SignerImplTest, EmptyCanonicalRequest) { - addMethod("POST"); - addPath("/hello"); - EXPECT_EQ(R"(POST -/hello - - - -e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855)", - canonicalRequest()); -} - -// Verify that we add the query string to the canonical request -TEST_F(SignerImplTest, QueryStringInCanonicalRequest) { - addMethod("POST"); - addPath("/path/foo?bar=baz"); - EXPECT_EQ(R"(POST -/path/foo -bar=baz - - -e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855)", - canonicalRequest()); -} - -// Verify that we use an empty path if we only have a query string -TEST_F(SignerImplTest, OnlyQueryStringInCanonicalRequest) { - addMethod("POST"); - addPath("?bar=baz"); - EXPECT_EQ(R"(POST -/ -bar=baz - - -e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855)", - canonicalRequest()); -} - -// Verify that we insert an extra blank line after the canonical headers -TEST_F(SignerImplTest, ExtraNewlineAfterCanonicalizedHeaders) { - addMethod("POST"); - addPath("/?query"); - addHeader("header1", "header1 value"); - addHeader("header2", "header2 value"); - addHeader("header3", "header3 value"); - EXPECT_EQ(R"(POST -/ -query -header1:header1 value -header2:header2 value -header3:header3 value - -header1;header2;header3 -e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855)", - canonicalRequest()); + addPath("/"); + addHeader("host", "www.example.com"); + signer_.sign(*message_); + EXPECT_STREQ("AWS4-HMAC-SHA256 Credential=akid/20180102/region/service/aws4_request, " + "SignedHeaders=host;x-amz-date, " + "Signature=60216ee44dd651322ea10cc6747308dd30e582aaa773f6c1b1354e486385c021", + message_->headers().Authorization()->value().c_str()); } } // namespace Aws diff --git a/test/extensions/filters/http/common/aws/utility_test.cc b/test/extensions/filters/http/common/aws/utility_test.cc index 54d012d676abf..802e06036d3f5 100644 --- a/test/extensions/filters/http/common/aws/utility_test.cc +++ b/test/extensions/filters/http/common/aws/utility_test.cc @@ -72,16 +72,80 @@ TEST(UtilityTest, CanonicalizeHeadersRemovingDefaultPortsFromHost) { // Whitespace is trimmed from headers TEST(UtilityTest, CanonicalizeHeadersTrimmingWhitespace) { - Http::TestHeaderMapImpl headers{{"leading", " leading value"}, - {"trailing", "trailing value "}, - {"internal", "internal value"}, - {"all", " all value "}}; + Http::TestHeaderMapImpl headers{ + {"leading", " leading value"}, + {"trailing", "trailing value "}, + {"internal", "internal value"}, + {"all", " all value "}, + }; const auto map = Utility::canonicalizeHeaders(headers); EXPECT_THAT(map, ElementsAre(Pair("all", "all value"), Pair("internal", "internal value"), Pair("leading", "leading value"), Pair("trailing", "trailing value"))); } +// Verify the format of a minimalist canonical request +TEST(UtilityTest, MinimalCanonicalRequest) { + std::map headers; + const auto request = Utility::createCanonicalRequest("GET", "", headers, "content-hash"); + EXPECT_EQ(R"(GET +/ + + + +content-hash)", + request); +} + +TEST(UtilityTest, CanonicalRequestWithQueryString) { + const std::map headers; + const auto request = Utility::createCanonicalRequest("GET", "?query", headers, "content-hash"); + EXPECT_EQ(R"(GET +/ +query + + +content-hash)", + request); +} + +TEST(UtilityTest, CanonicalRequestWithHeaders) { + const std::map headers = { + {"header1", "value1"}, + {"header2", "value2"}, + {"header3", "value3"}, + }; + const auto request = Utility::createCanonicalRequest("GET", "", headers, "content-hash"); + EXPECT_EQ(R"(GET +/ + +header1:value1 +header2:value2 +header3:value3 + +header1;header2;header3 +content-hash)", + request); +} + +// Verify headers are joined with ";" +TEST(UtilityTest, JoinCanonicalHeaderNames) { + std::map headers = { + {"header1", "value1"}, + {"header2", "value2"}, + {"header3", "value3"}, + }; + const auto names = Utility::joinCanonicalHeaderNames(headers); + EXPECT_EQ("header1;header2;header3", names); +} + +// Verify we return "" when there are no headers +TEST(UtilityTest, JoinCanonicalHeaderNamesWithEmptyMap) { + std::map headers; + const auto names = Utility::joinCanonicalHeaderNames(headers); + EXPECT_EQ("", names); +} + } // namespace Aws } // namespace Common } // namespace HttpFilters From 78d49c83eb7900fbbbd8a8542f775f00f2219715 Mon Sep 17 00:00:00 2001 From: Scott LaVigne Date: Wed, 16 Jan 2019 17:06:58 -0800 Subject: [PATCH 4/6] Formatting Signed-off-by: Scott LaVigne --- source/extensions/filters/http/common/aws/signer_impl.cc | 2 +- source/extensions/filters/http/common/aws/utility.h | 3 ++- test/extensions/common/crypto/utility_test.cc | 4 ++-- test/extensions/filters/http/common/aws/mocks.h | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/source/extensions/filters/http/common/aws/signer_impl.cc b/source/extensions/filters/http/common/aws/signer_impl.cc index dae01fa1fa25f..869fd8f7b1e67 100644 --- a/source/extensions/filters/http/common/aws/signer_impl.cc +++ b/source/extensions/filters/http/common/aws/signer_impl.cc @@ -7,8 +7,8 @@ #include "common/common/hex.h" #include "common/http/headers.h" -#include "extensions/filters/http/common/aws/utility.h" #include "extensions/common/crypto/utility.h" +#include "extensions/filters/http/common/aws/utility.h" #include "absl/strings/str_join.h" diff --git a/source/extensions/filters/http/common/aws/utility.h b/source/extensions/filters/http/common/aws/utility.h index 8626a40f14280..ead9339fb826b 100644 --- a/source/extensions/filters/http/common/aws/utility.h +++ b/source/extensions/filters/http/common/aws/utility.h @@ -37,7 +37,8 @@ class Utility { * @param canonical_headers the pre-canonicalized request headers. * @return the header names as a semicolon-delimited string. */ - static std::string joinCanonicalHeaderNames(const std::map& canonical_headers); + static std::string + joinCanonicalHeaderNames(const std::map& canonical_headers); }; } // namespace Aws diff --git a/test/extensions/common/crypto/utility_test.cc b/test/extensions/common/crypto/utility_test.cc index 1022564163a0c..5cf328856ca0a 100644 --- a/test/extensions/common/crypto/utility_test.cc +++ b/test/extensions/common/crypto/utility_test.cc @@ -1,8 +1,8 @@ -#include "extensions/common/crypto/utility.h" - #include "common/buffer/buffer_impl.h" #include "common/common/hex.h" +#include "extensions/common/crypto/utility.h" + #include "gtest/gtest.h" namespace Envoy { diff --git a/test/extensions/filters/http/common/aws/mocks.h b/test/extensions/filters/http/common/aws/mocks.h index 6b21e8f5c9581..857ed433ac2ba 100644 --- a/test/extensions/filters/http/common/aws/mocks.h +++ b/test/extensions/filters/http/common/aws/mocks.h @@ -24,7 +24,7 @@ class MockSigner : public Signer { MockSigner(); ~MockSigner(); - MOCK_METHOD2(sign, void(Http::Message&,bool)); + MOCK_METHOD2(sign, void(Http::Message&, bool)); }; } // namespace Aws From c8b887c460c7d7ec5693b1106677cf5bbb64bb1f Mon Sep 17 00:00:00 2001 From: Scott LaVigne Date: Mon, 21 Jan 2019 10:15:40 -0800 Subject: [PATCH 5/6] move Extensions::Common::Crypto to Envoy::Common::Crypto Signed-off-by: Scott LaVigne --- source/{extensions => }/common/crypto/BUILD | 0 .../{extensions => }/common/crypto/utility.cc | 6 ++---- .../{extensions => }/common/crypto/utility.h | 2 -- .../extensions/filters/http/common/aws/BUILD | 2 +- .../filters/http/common/aws/signer_impl.cc | 21 ++++++++----------- test/{extensions => }/common/crypto/BUILD | 2 +- .../common/crypto/utility_test.cc | 5 +---- 7 files changed, 14 insertions(+), 24 deletions(-) rename source/{extensions => }/common/crypto/BUILD (100%) rename source/{extensions => }/common/crypto/utility.cc (93%) rename source/{extensions => }/common/crypto/utility.h (94%) rename test/{extensions => }/common/crypto/BUILD (86%) rename test/{extensions => }/common/crypto/utility_test.cc (95%) diff --git a/source/extensions/common/crypto/BUILD b/source/common/crypto/BUILD similarity index 100% rename from source/extensions/common/crypto/BUILD rename to source/common/crypto/BUILD diff --git a/source/extensions/common/crypto/utility.cc b/source/common/crypto/utility.cc similarity index 93% rename from source/extensions/common/crypto/utility.cc rename to source/common/crypto/utility.cc index 5b568bcedb582..e5c5f80e72d40 100644 --- a/source/extensions/common/crypto/utility.cc +++ b/source/common/crypto/utility.cc @@ -1,4 +1,4 @@ -#include "extensions/common/crypto/utility.h" +#include "common/crypto/utility.h" #include "common/common/assert.h" #include "common/common/stack_array.h" @@ -8,7 +8,6 @@ #include "openssl/sha.h" namespace Envoy { -namespace Extensions { namespace Common { namespace Crypto { @@ -34,7 +33,7 @@ std::vector Utility::getSha256Hmac(const std::vector& key, absl::string_view message) { std::vector hmac(SHA256_DIGEST_LENGTH); unsigned int len; - auto ret = + const auto ret = HMAC(EVP_sha256(), key.data(), key.size(), reinterpret_cast(message.data()), message.size(), hmac.data(), &len); RELEASE_ASSERT(ret != nullptr, "Failed to create HMAC"); @@ -43,5 +42,4 @@ std::vector Utility::getSha256Hmac(const std::vector& key, } // namespace Crypto } // namespace Common -} // namespace Extensions } // namespace Envoy diff --git a/source/extensions/common/crypto/utility.h b/source/common/crypto/utility.h similarity index 94% rename from source/extensions/common/crypto/utility.h rename to source/common/crypto/utility.h index 3e4771087fb34..d89c51fc75be0 100644 --- a/source/extensions/common/crypto/utility.h +++ b/source/common/crypto/utility.h @@ -3,7 +3,6 @@ #include "envoy/buffer/buffer.h" namespace Envoy { -namespace Extensions { namespace Common { namespace Crypto { @@ -28,5 +27,4 @@ class Utility { } // namespace Crypto } // namespace Common -} // namespace Extensions } // namespace Envoy \ No newline at end of file diff --git a/source/extensions/filters/http/common/aws/BUILD b/source/extensions/filters/http/common/aws/BUILD index 3b79211956c81..e21ff2645279a 100644 --- a/source/extensions/filters/http/common/aws/BUILD +++ b/source/extensions/filters/http/common/aws/BUILD @@ -28,9 +28,9 @@ envoy_cc_library( "//source/common/common:hex_lib", "//source/common/common:logger_lib", "//source/common/common:utility_lib", + "//source/common/crypto:utility_lib", "//source/common/http:headers_lib", "//source/common/singleton:const_singleton", - "//source/extensions/common/crypto:utility_lib", ], ) diff --git a/source/extensions/filters/http/common/aws/signer_impl.cc b/source/extensions/filters/http/common/aws/signer_impl.cc index 869fd8f7b1e67..432396c85ce87 100644 --- a/source/extensions/filters/http/common/aws/signer_impl.cc +++ b/source/extensions/filters/http/common/aws/signer_impl.cc @@ -5,9 +5,9 @@ #include "common/buffer/buffer_impl.h" #include "common/common/fmt.h" #include "common/common/hex.h" +#include "common/crypto/utility.h" #include "common/http/headers.h" -#include "extensions/common/crypto/utility.h" #include "extensions/filters/http/common/aws/utility.h" #include "absl/strings/str_join.h" @@ -66,9 +66,8 @@ std::string SignerImpl::createContentHash(Http::Message& message, bool sign_body return SignatureConstants::get().HashedEmptyString; } const auto content_hash = - message.body() - ? Hex::encode(Extensions::Common::Crypto::Utility::getSha256Digest(*message.body())) - : SignatureConstants::get().HashedEmptyString; + message.body() ? Hex::encode(Envoy::Common::Crypto::Utility::getSha256Digest(*message.body())) + : SignatureConstants::get().HashedEmptyString; message.headers().addCopy(SignatureHeaders::get().ContentSha256, content_hash); return content_hash; } @@ -82,7 +81,7 @@ std::string SignerImpl::createStringToSign(absl::string_view canonical_request, absl::string_view long_date, absl::string_view credential_scope) const { return fmt::format(SignatureConstants::get().StringToSignFormat, long_date, credential_scope, - Hex::encode(Extensions::Common::Crypto::Utility::getSha256Digest( + Hex::encode(Envoy::Common::Crypto::Utility::getSha256Digest( Buffer::OwnedImpl(canonical_request)))); } @@ -91,15 +90,13 @@ std::string SignerImpl::createSignature(absl::string_view secret_access_key, absl::string_view string_to_sign) const { const auto secret_key = absl::StrCat(SignatureConstants::get().SignatureVersion, secret_access_key); - const auto date_key = Extensions::Common::Crypto::Utility::getSha256Hmac( + const auto date_key = Envoy::Common::Crypto::Utility::getSha256Hmac( std::vector(secret_key.begin(), secret_key.end()), short_date); - const auto region_key = Extensions::Common::Crypto::Utility::getSha256Hmac(date_key, region_); - const auto service_key = - Extensions::Common::Crypto::Utility::getSha256Hmac(region_key, service_name_); - const auto signing_key = Extensions::Common::Crypto::Utility::getSha256Hmac( + const auto region_key = Envoy::Common::Crypto::Utility::getSha256Hmac(date_key, region_); + const auto service_key = Envoy::Common::Crypto::Utility::getSha256Hmac(region_key, service_name_); + const auto signing_key = Envoy::Common::Crypto::Utility::getSha256Hmac( service_key, SignatureConstants::get().Aws4Request); - return Hex::encode( - Extensions::Common::Crypto::Utility::getSha256Hmac(signing_key, string_to_sign)); + return Hex::encode(Envoy::Common::Crypto::Utility::getSha256Hmac(signing_key, string_to_sign)); } std::string diff --git a/test/extensions/common/crypto/BUILD b/test/common/crypto/BUILD similarity index 86% rename from test/extensions/common/crypto/BUILD rename to test/common/crypto/BUILD index 2f79bdf1e0955..6cf18be53a67a 100644 --- a/test/extensions/common/crypto/BUILD +++ b/test/common/crypto/BUILD @@ -17,6 +17,6 @@ envoy_cc_test( deps = [ "//source/common/buffer:buffer_lib", "//source/common/common:hex_lib", - "//source/extensions/common/crypto:utility_lib", + "//source/common/crypto:utility_lib", ], ) diff --git a/test/extensions/common/crypto/utility_test.cc b/test/common/crypto/utility_test.cc similarity index 95% rename from test/extensions/common/crypto/utility_test.cc rename to test/common/crypto/utility_test.cc index 5cf328856ca0a..a23e1576ce326 100644 --- a/test/extensions/common/crypto/utility_test.cc +++ b/test/common/crypto/utility_test.cc @@ -1,12 +1,10 @@ #include "common/buffer/buffer_impl.h" #include "common/common/hex.h" - -#include "extensions/common/crypto/utility.h" +#include "common/crypto/utility.h" #include "gtest/gtest.h" namespace Envoy { -namespace Extensions { namespace Common { namespace Crypto { @@ -53,5 +51,4 @@ TEST(UtilityTest, TestSha256HmacWithEmptyArguments) { } // namespace Crypto } // namespace Common -} // namespace Extensions } // namespace Envoy From 0fd58cde8245d60639f6cb6908a23b8fda869a08 Mon Sep 17 00:00:00 2001 From: Scott LaVigne Date: Wed, 23 Jan 2019 11:02:11 -0800 Subject: [PATCH 6/6] Fix typo in crypto test names Signed-off-by: Scott LaVigne --- test/common/crypto/utility_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/common/crypto/utility_test.cc b/test/common/crypto/utility_test.cc index a23e1576ce326..46c6852102441 100644 --- a/test/common/crypto/utility_test.cc +++ b/test/common/crypto/utility_test.cc @@ -8,21 +8,21 @@ namespace Envoy { namespace Common { namespace Crypto { -TEST(UtilityTest, TestSha256Disgest) { +TEST(UtilityTest, TestSha256Digest) { const Buffer::OwnedImpl buffer("test data"); const auto digest = Utility::getSha256Digest(buffer); EXPECT_EQ("916f0027a575074ce72a331777c3478d6513f786a591bd892da1a577bf2335f9", Hex::encode(digest)); } -TEST(UtilityTest, TestSha256DisgestWithEmptyBuffer) { +TEST(UtilityTest, TestSha256DigestWithEmptyBuffer) { const Buffer::OwnedImpl buffer; const auto digest = Utility::getSha256Digest(buffer); EXPECT_EQ("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", Hex::encode(digest)); } -TEST(UtilityTest, TestSha256DisgestGrowingBuffer) { +TEST(UtilityTest, TestSha256DigestGrowingBuffer) { // Adding multiple slices to the buffer Buffer::OwnedImpl buffer("slice 1"); auto digest = Utility::getSha256Digest(buffer);