From e3046abcaa9bd2c5f43bdcd9a1accc883202e8c5 Mon Sep 17 00:00:00 2001 From: Ben Taussig Date: Tue, 22 Aug 2023 14:43:22 +0000 Subject: [PATCH 01/13] add new field to ServiceAccountCredentials for sts region --- api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto b/api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto index 4eb96f39..6607eb10 100644 --- a/api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto +++ b/api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto @@ -101,6 +101,8 @@ message AWSLambdaConfig { string uri = 2 [ (validate.rules).string.min_bytes = 1 ]; // timeout for the request google.protobuf.Duration timeout = 3; + // Region for the sts endpoint + string region = 4 [ (validate.rules).string.min_bytes = 1 ]; } // Send downstream path and method as `x-envoy-original-path` and From 17f64f179a4485b4c3d0b7af51143a9fc9982c22 Mon Sep 17 00:00:00 2001 From: Ben Taussig Date: Tue, 22 Aug 2023 14:47:11 +0000 Subject: [PATCH 02/13] support new field --- .../http/aws_lambda/sts_connection_pool.cc | 7 ++++++- .../filters/http/aws_lambda/sts_connection_pool.h | 1 + .../http/aws_lambda/sts_credentials_provider.cc | 12 ++++++++---- .../filters/http/aws_lambda/sts_fetcher.cc | 15 ++++++++++++--- .../filters/http/aws_lambda/sts_fetcher.h | 1 + test/extensions/filters/http/aws_lambda/mocks.h | 2 ++ 6 files changed, 30 insertions(+), 8 deletions(-) diff --git a/source/extensions/filters/http/aws_lambda/sts_connection_pool.cc b/source/extensions/filters/http/aws_lambda/sts_connection_pool.cc index 215342aa..e2612da2 100644 --- a/source/extensions/filters/http/aws_lambda/sts_connection_pool.cc +++ b/source/extensions/filters/http/aws_lambda/sts_connection_pool.cc @@ -28,6 +28,7 @@ class StsConnectionPoolImpl : public StsConnectionPool, ~StsConnectionPoolImpl(); void init(const envoy::config::core::v3::HttpUri &uri, + const std::string region, const absl::string_view web_token, StsCredentialsConstSharedPtr creds) override; void setInFlight() override; @@ -112,9 +113,10 @@ StsConnectionPoolImpl::~StsConnectionPoolImpl() { }; void StsConnectionPoolImpl::init(const envoy::config::core::v3::HttpUri &uri, + const std::string region, const absl::string_view web_token, StsCredentialsConstSharedPtr creds) { request_in_flight_ = true; - fetcher_->fetch(uri, role_arn_, web_token, creds, this); + fetcher_->fetch(uri, region, role_arn_, web_token, creds, this); } void StsConnectionPoolImpl::setInFlight() { request_in_flight_ = true; @@ -187,6 +189,9 @@ void StsConnectionPoolImpl::onSuccess(const absl::string_view body) { api_.timeSource().systemTime().time_since_epoch().count()); // Send result back to Credential Provider to store in cache // Report the existence of this credential to any pools that may be waitin + + std::cout << "sending result back to credential provider" << std::endl; + callbacks_->onResult(result, cache_key_arn_, chained_requests_); request_in_flight_ = false; diff --git a/source/extensions/filters/http/aws_lambda/sts_connection_pool.h b/source/extensions/filters/http/aws_lambda/sts_connection_pool.h index 84f25a32..33d857b9 100644 --- a/source/extensions/filters/http/aws_lambda/sts_connection_pool.h +++ b/source/extensions/filters/http/aws_lambda/sts_connection_pool.h @@ -106,6 +106,7 @@ class StsConnectionPool : public Logger::Loggable { using ContextPtr = std::unique_ptr; virtual void init(const envoy::config::core::v3::HttpUri &uri, + const std::string region, const absl::string_view web_token, StsCredentialsConstSharedPtr creds) PURE; virtual void setInFlight() PURE; virtual Context *add(StsConnectionPool::Context::Callbacks *callback) PURE; diff --git a/source/extensions/filters/http/aws_lambda/sts_credentials_provider.cc b/source/extensions/filters/http/aws_lambda/sts_credentials_provider.cc index 144daaec..a1777c5a 100644 --- a/source/extensions/filters/http/aws_lambda/sts_credentials_provider.cc +++ b/source/extensions/filters/http/aws_lambda/sts_credentials_provider.cc @@ -49,6 +49,7 @@ class StsCredentialsProviderImpl : public StsCredentialsProvider, std::string default_role_arn_; envoy::config::core::v3::HttpUri uri_; + std::string region_; StsConnectionPoolFactoryPtr conn_pool_factory_; // web_token set by AWS, will be auto-updated by StsCredentialsProvider @@ -71,6 +72,7 @@ StsCredentialsProviderImpl::StsCredentialsProviderImpl( uri_.set_cluster(config_.cluster()); uri_.set_uri(config_.uri()); + region_ = config_.region(); // TODO: Figure out how to get this to compile, timeout is not all that // important right now uri_.set_allocated_timeout(config_.mutable_timeout()) // Consider if this is still reasonable. We have a timeout configuration for @@ -98,7 +100,7 @@ void StsCredentialsProviderImpl::onResult( ENVOY_LOG(trace, "calling sts chained for {}", chained_role); auto conn_pool = connection_pools_.find(chained_role); if (conn_pool != connection_pools_.end()) { - conn_pool->second->init(uri_, "", result); + conn_pool->second->init(uri_, region_, "", result); } chained_requests.pop_back(); } @@ -180,7 +182,9 @@ StsConnectionPool::Context *StsCredentialsProviderImpl::find( // and use webtoken. if (role_arn == default_role_arn_ || disable_role_chaining) { // initialize the connection and subscribe to the callbacks - conn_pool->second->init(uri_, web_token_, NULL); + // we do not strictly need the region here, as this is not a + // chained call, but we will pass it in anyway. + conn_pool->second->init(uri_, region_, web_token_, NULL); return conn_pool->second->add(callbacks); } @@ -193,7 +197,7 @@ StsConnectionPool::Context *StsCredentialsProviderImpl::find( auto time_left = existing_base_token->second->expirationTime() - now; if (time_left > REFRESH_GRACE_PERIOD) { ENVOY_LOG(trace,"found base token with remaining time"); - conn_pool->second->init(uri_, web_token_, existing_base_token->second); + conn_pool->second->init(uri_, region_, web_token_, existing_base_token->second); return conn_pool->second->add(callbacks); } } @@ -208,7 +212,7 @@ StsConnectionPool::Context *StsCredentialsProviderImpl::find( } // only recreate base request if its not in flight if (!base_conn_pool->second->requestInFlight()) { - base_conn_pool->second->init(uri_, web_token_, NULL); + base_conn_pool->second->init(uri_, region_, web_token_, NULL); } base_conn_pool->second->addChained(role_arn); diff --git a/source/extensions/filters/http/aws_lambda/sts_fetcher.cc b/source/extensions/filters/http/aws_lambda/sts_fetcher.cc index 3a88dce5..75aa744b 100644 --- a/source/extensions/filters/http/aws_lambda/sts_fetcher.cc +++ b/source/extensions/filters/http/aws_lambda/sts_fetcher.cc @@ -34,6 +34,7 @@ class StsFetcherImpl : public StsFetcher, } void fetch(const envoy::config::core::v3::HttpUri &uri, + const std::string region, const absl::string_view role_arn, const absl::string_view web_token, StsCredentialsConstSharedPtr creds, @@ -97,9 +98,17 @@ class StsFetcherImpl : public StsFetcher, &creds->secretAccessKey().value(), &creds->sessionToken().value()); aws_authenticator.updatePayloadHash(message->body()); auto& hdrs = message->headers(); - // TODO(nfuden) allow for Region this to be overridable. - // DefaultRegion is gauranteed to be available but an override may be faster - aws_authenticator.sign(&hdrs, HeadersToSign, DefaultRegion); + // region should never be NULL at this point, but if it is, we can use the + // default region. + if (!region.empty()){ + ENVOY_LOG(debug, "assume chained role from [uri = {}]: region is {}", + uri_->uri(), region); + aws_authenticator.sign(&hdrs, HeadersToSign, region); + } else { + ENVOY_LOG(debug, "assume chained role from [uri = {}]: region is empty, using default region", + uri_->uri()); + aws_authenticator.sign(&hdrs, HeadersToSign, DefaultRegion); + } // Log the accessKey but not the secret. This is to show that we have valid // credentials but does not leak anything secret. This is due to our // sessions being diff --git a/source/extensions/filters/http/aws_lambda/sts_fetcher.h b/source/extensions/filters/http/aws_lambda/sts_fetcher.h index c5ed35fe..db32d548 100644 --- a/source/extensions/filters/http/aws_lambda/sts_fetcher.h +++ b/source/extensions/filters/http/aws_lambda/sts_fetcher.h @@ -98,6 +98,7 @@ class StsFetcher { * @param failure the cb called on failed role assumption */ virtual void fetch(const envoy::config::core::v3::HttpUri &uri, + const std::string region, const absl::string_view role_arn, const absl::string_view web_token, StsCredentialsConstSharedPtr creds, diff --git a/test/extensions/filters/http/aws_lambda/mocks.h b/test/extensions/filters/http/aws_lambda/mocks.h index 7eb873f9..3559fcdd 100644 --- a/test/extensions/filters/http/aws_lambda/mocks.h +++ b/test/extensions/filters/http/aws_lambda/mocks.h @@ -20,6 +20,7 @@ class MockStsFetcher : public StsFetcher { MOCK_METHOD(void, cancel, ()); MOCK_METHOD(void, fetch, (const envoy::config::core::v3::HttpUri &uri, + const std::string region, const absl::string_view role_arn, const absl::string_view web_token, StsCredentialsConstSharedPtr creds, @@ -92,6 +93,7 @@ class MockStsConnectionPool : public StsConnectionPool { (StsConnectionPool::Context::Callbacks * callback)); MOCK_METHOD(void, init, (const envoy::config::core::v3::HttpUri &uri, + const std::string region, const absl::string_view web_token, StsCredentialsConstSharedPtr creds)); MOCK_METHOD(void, addChained, (std::string role_arn)); From 7b5b15496baf6df78f5473747a8e5e11858c3f2d Mon Sep 17 00:00:00 2001 From: Ben Taussig Date: Tue, 22 Aug 2023 14:47:44 +0000 Subject: [PATCH 03/13] support new setting in tests --- .../sts_credentials_provider_test.cc | 19 +++++++++++------ .../http/aws_lambda/sts_fetcher_test.cc | 21 ++++++++++++------- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/test/extensions/filters/http/aws_lambda/sts_credentials_provider_test.cc b/test/extensions/filters/http/aws_lambda/sts_credentials_provider_test.cc index fd94ccc7..354def66 100644 --- a/test/extensions/filters/http/aws_lambda/sts_credentials_provider_test.cc +++ b/test/extensions/filters/http/aws_lambda/sts_credentials_provider_test.cc @@ -45,7 +45,8 @@ std::chrono::seconds expiry_time(4120492825); const std::string service_account_credentials_config = R"( cluster: test -uri: https://foo.com +uri: https://sts.us-east-1.amazonaws.com +region: us-east-1 timeout: 1s )"; @@ -97,13 +98,15 @@ TEST_F(StsCredentialsProviderTest, TestFullFlow) { return std::move(unique_pool); })); - EXPECT_CALL(*sts_connection_pool, init(_, _, _)) + EXPECT_CALL(*sts_connection_pool, init(_, _, _, _)) .WillOnce(Invoke([&](const envoy::config::core::v3::HttpUri &uri, + const std::string region, const absl::string_view web_token, StsCredentialsConstSharedPtr ) { EXPECT_EQ(web_token, token); EXPECT_EQ(uri.uri(), config_.uri()); EXPECT_EQ(uri.cluster(), config_.cluster()); + EXPECT_EQ(region, config_.region()); })); EXPECT_CALL(*sts_connection_pool, add(_)); @@ -156,7 +159,7 @@ TEST_F(StsCredentialsProviderTest, TestFullFlow) { TEST_F(StsCredentialsProviderTest, TestFullChainedFlow) { // Setup std::string base_role_arn = "test_arn"; - std::string role_arn = "test_arn_chained"; + std::string role_arn = "test_arn_chained";`` std::string token = "test_token"; std::unique_ptr> factory_ = std::move(sts_connection_pool_factory_); auto* factory = factory_.get(); @@ -193,13 +196,15 @@ TEST_F(StsCredentialsProviderTest, TestFullChainedFlow) { })); // expect the base pool to be initialized with fetch - EXPECT_CALL(*sts_connection_pool, init(_, _, _)) + EXPECT_CALL(*sts_connection_pool, init(_, _, _, _)) .WillOnce(Invoke([&](const envoy::config::core::v3::HttpUri &uri, + const std::string region, const absl::string_view web_token, StsCredentialsConstSharedPtr ) { EXPECT_EQ(web_token, token); EXPECT_EQ(uri.uri(), config_.uri()); EXPECT_EQ(uri.cluster(), config_.cluster()); + EXPECT_EQ(region, config_.region()); })); EXPECT_CALL(*chained_pool, setInFlight()); @@ -269,13 +274,15 @@ TEST_F(StsCredentialsProviderTest, TestUnchainedFlow) { return std::move(unique_pool); })); - EXPECT_CALL(*sts_connection_pool, init(_, _, _)) + EXPECT_CALL(*sts_connection_pool, init(_, _, _, _)) .WillOnce(Invoke([&](const envoy::config::core::v3::HttpUri &uri, + const std::string region, const absl::string_view web_token, StsCredentialsConstSharedPtr ) { EXPECT_EQ(web_token, token); EXPECT_EQ(uri.uri(), config_.uri()); EXPECT_EQ(uri.cluster(), config_.cluster()); + EXPECT_EQ(region, config_.region()); })); EXPECT_CALL(*sts_connection_pool, add(_)); @@ -310,4 +317,4 @@ TEST_F(StsCredentialsProviderTest, TestUnchainedFlow) { } // namespace AwsLambda } // namespace HttpFilters } // namespace Extensions -} // namespace Envoy +} // namespace Envoy \ No newline at end of file diff --git a/test/extensions/filters/http/aws_lambda/sts_fetcher_test.cc b/test/extensions/filters/http/aws_lambda/sts_fetcher_test.cc index 867f899d..cfd3badb 100644 --- a/test/extensions/filters/http/aws_lambda/sts_fetcher_test.cc +++ b/test/extensions/filters/http/aws_lambda/sts_fetcher_test.cc @@ -81,6 +81,7 @@ const std::string service_account_credentials_config = R"( cluster: test uri: https://foo.com timeout: 1s +region: us-east-1 )"; const std::string role_arn = "role_arn"; @@ -92,9 +93,15 @@ class StsFetcherTest : public testing::Test { void SetUp() override { mock_factory_ctx_.cluster_manager_.initializeClusters({"test"}, {}); mock_factory_ctx_.cluster_manager_.initializeThreadLocalClusters({"test"}); - TestUtility::loadFromYaml(service_account_credentials_config, uri_); + TestUtility::loadFromYaml(service_account_credentials_config, config_); + uri_.set_cluster(config_.cluster()); + uri_.set_uri(config_.uri()); + region_ = config_.region(); } + + envoy::config::filter::http::aws_lambda::v2::AWSLambdaConfig_ServiceAccountCredentials config_; HttpUri uri_; + std::string region_; testing::NiceMock mock_factory_ctx_; }; @@ -111,7 +118,7 @@ TEST_F(StsFetcherTest, TestGetSuccess) { testing::NiceMock callbacks; EXPECT_CALL(callbacks, onSuccess(valid_response)).Times(1); // Act - fetcher->fetch(uri_, role_arn, web_token, NULL, &callbacks); + fetcher->fetch(uri_, region_, role_arn, web_token, NULL, &callbacks); } TEST_F(StsFetcherTest, TestChainedSts) { @@ -135,7 +142,7 @@ TEST_F(StsFetcherTest, TestChainedSts) { access_key, secret_key, session_token, expiration_time); // Act - fetcher->fetch(uri_, to_chain_role_arn, "", sub_creds, &callbacks); + fetcher->fetch(uri_, region_, to_chain_role_arn, "", sub_creds, &callbacks); } TEST_F(StsFetcherTest, TestGet503) { @@ -149,7 +156,7 @@ TEST_F(StsFetcherTest, TestGet503) { EXPECT_CALL(callbacks, onFailure(CredentialsFailureStatus::Network)).Times(1); // Act - fetcher->fetch(uri_, role_arn, web_token, NULL, &callbacks); + fetcher->fetch(uri_, region_, role_arn, web_token, NULL, &callbacks); } TEST_F(StsFetcherTest, TestCredentialsExpired) { @@ -165,7 +172,7 @@ TEST_F(StsFetcherTest, TestCredentialsExpired) { .Times(1); // Act - fetcher->fetch(uri_, role_arn, web_token, NULL, &callbacks); + fetcher->fetch(uri_, region_, role_arn, web_token, NULL, &callbacks); } TEST_F(StsFetcherTest, TestHttpFailure) { @@ -180,7 +187,7 @@ TEST_F(StsFetcherTest, TestHttpFailure) { EXPECT_CALL(callbacks, onFailure(CredentialsFailureStatus::Network)).Times(1); // Act - fetcher->fetch(uri_, role_arn, web_token, NULL, &callbacks); + fetcher->fetch(uri_, region_, role_arn, web_token, NULL, &callbacks); } TEST_F(StsFetcherTest, TestCancel) { @@ -196,7 +203,7 @@ TEST_F(StsFetcherTest, TestCancel) { testing::NiceMock callbacks; // Act - fetcher->fetch(uri_, role_arn, web_token, NULL, &callbacks); + fetcher->fetch(uri_, region_, role_arn, web_token, NULL, &callbacks); // Proper cancel fetcher->cancel(); // Re-entrant cancel From d83abb72a52ef7531ed649104a31d87762ca853d Mon Sep 17 00:00:00 2001 From: Ben Taussig Date: Tue, 22 Aug 2023 14:52:04 +0000 Subject: [PATCH 04/13] add changelog entry --- changelog/v1.26.4-patch2/handle-sts-credentials-region.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/v1.26.4-patch2/handle-sts-credentials-region.yaml diff --git a/changelog/v1.26.4-patch2/handle-sts-credentials-region.yaml b/changelog/v1.26.4-patch2/handle-sts-credentials-region.yaml new file mode 100644 index 00000000..936cb88e --- /dev/null +++ b/changelog/v1.26.4-patch2/handle-sts-credentials-region.yaml @@ -0,0 +1,6 @@ +changelog: +- type: FIX + issueLink: https://github.com/solo-io/gloo/issues/8578 + resolvesIssue: false + description: > + Support role chaining using EKS ServiceAccounts outside of us-east-1 \ No newline at end of file From b9e80ac023c207b1dab1358d20f5be1d9c026348 Mon Sep 17 00:00:00 2001 From: Ben Taussig Date: Tue, 22 Aug 2023 15:22:10 +0000 Subject: [PATCH 05/13] remove typo ffrom sts_credentials_provider_test.cc --- .../filters/http/aws_lambda/sts_credentials_provider_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/extensions/filters/http/aws_lambda/sts_credentials_provider_test.cc b/test/extensions/filters/http/aws_lambda/sts_credentials_provider_test.cc index 354def66..f37e8dfc 100644 --- a/test/extensions/filters/http/aws_lambda/sts_credentials_provider_test.cc +++ b/test/extensions/filters/http/aws_lambda/sts_credentials_provider_test.cc @@ -159,7 +159,7 @@ TEST_F(StsCredentialsProviderTest, TestFullFlow) { TEST_F(StsCredentialsProviderTest, TestFullChainedFlow) { // Setup std::string base_role_arn = "test_arn"; - std::string role_arn = "test_arn_chained";`` + std::string role_arn = "test_arn_chained"; std::string token = "test_token"; std::unique_ptr> factory_ = std::move(sts_connection_pool_factory_); auto* factory = factory_.get(); From b76638f51eaa2fddf55e55ff39c372376103eb5d Mon Sep 17 00:00:00 2001 From: Ben Taussig Date: Tue, 22 Aug 2023 17:27:42 +0000 Subject: [PATCH 06/13] updae connection pool test --- .../aws_lambda/sts_connection_pool_test.cc | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/test/extensions/filters/http/aws_lambda/sts_connection_pool_test.cc b/test/extensions/filters/http/aws_lambda/sts_connection_pool_test.cc index af0133a2..d8f13cf4 100644 --- a/test/extensions/filters/http/aws_lambda/sts_connection_pool_test.cc +++ b/test/extensions/filters/http/aws_lambda/sts_connection_pool_test.cc @@ -50,17 +50,25 @@ const std::string service_account_credentials_config = R"( cluster: test uri: https://foo.com timeout: 1s +region: us-east-1 )"; class StsConnectionPoolTest : public testing::Test, public Event::TestUsingSimulatedTime { public: void SetUp() override { - TestUtility::loadFromYaml(service_account_credentials_config, uri_); + envoy::config::filter::http::aws_lambda::v2::AWSLambdaConfig_ServiceAccountCredentials + service_account_credentials; + TestUtility::loadFromYaml(service_account_credentials_config, service_account_credentials); + uri_.set_cluster(service_account_credentials.cluster()); + uri_.set_uri(service_account_credentials.uri()); + region_ = service_account_credentials.region(); + sts_fetcher_ = new testing::NiceMock(); } HttpUri uri_; + std::string region_; testing::NiceMock mock_factory_ctx_; testing::NiceMock *sts_fetcher_; @@ -79,8 +87,9 @@ TEST_F(StsConnectionPoolTest, TestSuccessfulCallback) { &pool_callbacks, std::move(unique_fetcher)); // Fetch credentials first call as they are not in the cache - EXPECT_CALL(*sts_fetcher_, fetch(_, _, _, _, _ )) + EXPECT_CALL(*sts_fetcher_, fetch(_, _, _, _, _, _ )) .WillOnce(Invoke([&](const envoy::config::core::v3::HttpUri &, + const std::string, const absl::string_view, const absl::string_view, StsCredentialsConstSharedPtr, StsFetcher::Callbacks *callbacks) -> void { @@ -112,7 +121,7 @@ TEST_F(StsConnectionPoolTest, TestSuccessfulCallback) { sts_conn_pool->add(&ctx_callbacks); - sts_conn_pool->init(uri_, web_token, NULL); + sts_conn_pool->init(uri_, region_, web_token, NULL); } TEST_F(StsConnectionPoolTest, TestPostInitAdd) { @@ -129,8 +138,9 @@ TEST_F(StsConnectionPoolTest, TestPostInitAdd) { StsFetcher::Callbacks *lambda_callbacks; // Fetch credentials first call as they are not in the cache - EXPECT_CALL(*sts_fetcher_, fetch(_, _, _, _, _ )) + EXPECT_CALL(*sts_fetcher_, fetch(_, _, _, _, _, _ )) .WillOnce(Invoke([&](const envoy::config::core::v3::HttpUri &, + const std::string, const absl::string_view, const absl::string_view, StsCredentialsConstSharedPtr, StsFetcher::Callbacks *callbacks) -> void { @@ -139,7 +149,7 @@ TEST_F(StsConnectionPoolTest, TestPostInitAdd) { sts_conn_pool->add(&ctx_callbacks); - sts_conn_pool->init(uri_, web_token, NULL); + sts_conn_pool->init(uri_, region_, web_token, NULL); auto context_1 = sts_conn_pool->add(&ctx_callbacks); @@ -187,8 +197,9 @@ TEST_F(StsConnectionPoolTest, TestFailure) { &pool_callbacks, std::move(unique_fetcher)); // Fetch credentials first call as they are not in the cache - EXPECT_CALL(*sts_fetcher_, fetch(_, _, _, _, _ )) + EXPECT_CALL(*sts_fetcher_, fetch(_, _, _, _, _, _ )) .WillOnce(Invoke([&](const envoy::config::core::v3::HttpUri &, + const std::string, const absl::string_view, const absl::string_view, StsCredentialsConstSharedPtr, StsFetcher::Callbacks *callbacks) -> void { @@ -207,7 +218,7 @@ TEST_F(StsConnectionPoolTest, TestFailure) { sts_conn_pool->add(&ctx_callbacks); - sts_conn_pool->init(uri_, web_token, NULL); + sts_conn_pool->init(uri_, region_, web_token, NULL); } } // namespace AwsLambda From 2d98c7a6d1db7f2e5704f25313abad6cbd536686 Mon Sep 17 00:00:00 2001 From: Ben Taussig Date: Tue, 22 Aug 2023 18:12:49 +0000 Subject: [PATCH 07/13] update lambda integration test --- test/integration/aws_lambda_filter_integration_test.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/test/integration/aws_lambda_filter_integration_test.cc b/test/integration/aws_lambda_filter_integration_test.cc index e2a709be..fb498dc6 100644 --- a/test/integration/aws_lambda_filter_integration_test.cc +++ b/test/integration/aws_lambda_filter_integration_test.cc @@ -31,6 +31,7 @@ name: io.solo.aws_lambda service_account_credentials: cluster: cluster_0 uri: https://foo.com + region: us-east-1 timeout: 1s )EOF"; From 643a909dc21eb8ec5b492320b3f806d5018a39c6 Mon Sep 17 00:00:00 2001 From: Ben Taussig Date: Thu, 7 Sep 2023 13:29:31 +0000 Subject: [PATCH 08/13] remove stray log messages --- .../extensions/filters/http/aws_lambda/sts_connection_pool.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/source/extensions/filters/http/aws_lambda/sts_connection_pool.cc b/source/extensions/filters/http/aws_lambda/sts_connection_pool.cc index e2612da2..8185c98e 100644 --- a/source/extensions/filters/http/aws_lambda/sts_connection_pool.cc +++ b/source/extensions/filters/http/aws_lambda/sts_connection_pool.cc @@ -189,9 +189,6 @@ void StsConnectionPoolImpl::onSuccess(const absl::string_view body) { api_.timeSource().systemTime().time_since_epoch().count()); // Send result back to Credential Provider to store in cache // Report the existence of this credential to any pools that may be waitin - - std::cout << "sending result back to credential provider" << std::endl; - callbacks_->onResult(result, cache_key_arn_, chained_requests_); request_in_flight_ = false; From e2af1f531541d75ef6a909fb2af13bd5cfdba451 Mon Sep 17 00:00:00 2001 From: Ben Taussig Date: Mon, 11 Sep 2023 14:27:19 -0400 Subject: [PATCH 09/13] relocate changelog entry --- .../handle-sts-credentials-region.yaml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/{v1.26.4-patch2 => v1.26.4-patch3}/handle-sts-credentials-region.yaml (100%) diff --git a/changelog/v1.26.4-patch2/handle-sts-credentials-region.yaml b/changelog/v1.26.4-patch3/handle-sts-credentials-region.yaml similarity index 100% rename from changelog/v1.26.4-patch2/handle-sts-credentials-region.yaml rename to changelog/v1.26.4-patch3/handle-sts-credentials-region.yaml From ca0f38a029a1f128fb417cfb6b5a0399d4558255 Mon Sep 17 00:00:00 2001 From: Ben Taussig Date: Tue, 12 Sep 2023 13:06:12 +0000 Subject: [PATCH 10/13] allow region to be unset --- api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto b/api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto index 6607eb10..597ab9b7 100644 --- a/api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto +++ b/api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto @@ -101,8 +101,8 @@ message AWSLambdaConfig { string uri = 2 [ (validate.rules).string.min_bytes = 1 ]; // timeout for the request google.protobuf.Duration timeout = 3; - // Region for the sts endpoint - string region = 4 [ (validate.rules).string.min_bytes = 1 ]; + // Region for the sts endpoint, defaults to us-east-1 + string region = 4; } // Send downstream path and method as `x-envoy-original-path` and From e11ac28a3271406b5abc4393738504cf4e687dd5 Mon Sep 17 00:00:00 2001 From: Ben Taussig Date: Tue, 12 Sep 2023 18:22:57 +0000 Subject: [PATCH 11/13] log credential scope mismatch on failure --- source/extensions/filters/http/aws_lambda/sts_fetcher.cc | 4 ++++ source/extensions/filters/http/aws_lambda/sts_fetcher.h | 2 ++ source/extensions/filters/http/aws_lambda/sts_status.h | 4 +++- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/source/extensions/filters/http/aws_lambda/sts_fetcher.cc b/source/extensions/filters/http/aws_lambda/sts_fetcher.cc index 75aa744b..925b9b09 100644 --- a/source/extensions/filters/http/aws_lambda/sts_fetcher.cc +++ b/source/extensions/filters/http/aws_lambda/sts_fetcher.cc @@ -149,6 +149,10 @@ class StsFetcherImpl : public StsFetcher, // TODO: cover more AWS error cases if (body.find(ExpiredTokenError) != std::string::npos) { callbacks_->onFailure(CredentialsFailureStatus::ExpiredToken); + } else if (body.find(SignaturedoesNotMatchError) != std::string::npos && + body.find(CredentialScopeMessage) != std::string::npos ) + { + callbacks_->onFailure(CredentialsFailureStatus::CredentialScopeMismatch); } else { callbacks_->onFailure(CredentialsFailureStatus::Network); } diff --git a/source/extensions/filters/http/aws_lambda/sts_fetcher.h b/source/extensions/filters/http/aws_lambda/sts_fetcher.h index db32d548..01fd62f6 100644 --- a/source/extensions/filters/http/aws_lambda/sts_fetcher.h +++ b/source/extensions/filters/http/aws_lambda/sts_fetcher.h @@ -25,6 +25,8 @@ namespace { "Version=2011-06-15"; constexpr char ExpiredTokenError[] = "ExpiredTokenException"; +constexpr char SignaturedoesNotMatchError[] = "SignatureDoesNotMatch"; +constexpr char CredentialScopeMessage[] = "Credential should be scoped to a valid region."; } // namespace class StsCredentials : public Extensions::Common::Aws::Credentials { diff --git a/source/extensions/filters/http/aws_lambda/sts_status.h b/source/extensions/filters/http/aws_lambda/sts_status.h index 40c7d66c..05a1ae3e 100644 --- a/source/extensions/filters/http/aws_lambda/sts_status.h +++ b/source/extensions/filters/http/aws_lambda/sts_status.h @@ -16,7 +16,9 @@ enum class CredentialsFailureStatus { /* Token is expired. */ ClusterNotFound, /* The filter is being destroyed, therefore the request should be cancelled */ - ContextCancelled + ContextCancelled, + /* The credentials computed by the filter are not valid in the current region */ + CredentialScopeMismatch, }; } // namespace AwsLambda From c48ddb6eefb9170e20237f515c61d074aab18035 Mon Sep 17 00:00:00 2001 From: Ben Taussig Date: Tue, 12 Sep 2023 18:32:01 +0000 Subject: [PATCH 12/13] add testing against credentialscopemismatch --- .../http/aws_lambda/sts_fetcher_test.cc | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/extensions/filters/http/aws_lambda/sts_fetcher_test.cc b/test/extensions/filters/http/aws_lambda/sts_fetcher_test.cc index cfd3badb..1d2da68f 100644 --- a/test/extensions/filters/http/aws_lambda/sts_fetcher_test.cc +++ b/test/extensions/filters/http/aws_lambda/sts_fetcher_test.cc @@ -52,6 +52,16 @@ const std::string expired_token_response = R"( )"; +const std::string credential_scope_mismatch = R"( + + + Sender + SignatureDoesNotMatch + Credential should be scoped to a valid region. + + +)"; + const std::string valid_chained_response = R"( @@ -175,6 +185,22 @@ TEST_F(StsFetcherTest, TestCredentialsExpired) { fetcher->fetch(uri_, region_, role_arn, web_token, NULL, &callbacks); } +TEST_F(StsFetcherTest, TestCredentialScopeMismatch) { + // Setup + MockUpstream mock_sts(mock_factory_ctx_.cluster_manager_, "401", + credential_scope_mismatch); + std::unique_ptr fetcher(StsFetcher::create( + mock_factory_ctx_.cluster_manager_, mock_factory_ctx_.api_)); + EXPECT_TRUE(fetcher != nullptr); + + testing::NiceMock callbacks; + EXPECT_CALL(callbacks, onFailure(CredentialsFailureStatus::CredentialScopeMismatch)) + .Times(1); + + // Act + fetcher->fetch(uri_, region_, role_arn, web_token, NULL, &callbacks); +} + TEST_F(StsFetcherTest, TestHttpFailure) { // Setup MockUpstream mock_sts(mock_factory_ctx_.cluster_manager_, From 571ad3930983ba28214295adc54467a8396cccb2 Mon Sep 17 00:00:00 2001 From: Ben Taussig Date: Tue, 12 Sep 2023 19:28:42 +0000 Subject: [PATCH 13/13] update proto comment for new field --- api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto b/api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto index 597ab9b7..0f879c34 100644 --- a/api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto +++ b/api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto @@ -101,7 +101,9 @@ message AWSLambdaConfig { string uri = 2 [ (validate.rules).string.min_bytes = 1 ]; // timeout for the request google.protobuf.Duration timeout = 3; - // Region for the sts endpoint, defaults to us-east-1 + // Region for the sts endpoint, defaults to us-east-1. + // This must be an enabled region https://docs.aws.amazon.com/IAM/latest/UserGuide/id_credentials_temp_enable-regions.html + // This should match the region specified in the uri. string region = 4; }