Handle STS Credentials Region (#267)

* add new field to ServiceAccountCredentials for sts region

* support new field

* support new setting in tests

* add changelog entry

* remove typo ffrom sts_credentials_provider_test.cc

* updae connection pool test

* update lambda integration test

* remove stray log messages

* relocate changelog entry

* allow region to be unset

* log credential scope mismatch on failure

* add testing against credentialscopemismatch

* update proto comment for new field

api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto

@@ -101,6 +101,10 @@ 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. 
+    // 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;
   }
 
   // Send downstream path and method as `x-envoy-original-path` and

changelog/v1.26.4-patch3/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

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;

source/extensions/filters/http/aws_lambda/sts_connection_pool.h

@@ -106,6 +106,7 @@ class StsConnectionPool :  public Logger::Loggable<Logger::Id::aws> {
   using ContextPtr = std::unique_ptr<Context>;
 
   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;

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);
 

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 
@@ -140,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);
         }

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 {
@@ -98,6 +100,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,

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

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));

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<MockStsFetcher>();
   }
 
   HttpUri uri_;
+  std::string region_;
   testing::NiceMock<Server::Configuration::MockFactoryContext>
       mock_factory_ctx_;
   testing::NiceMock<MockStsFetcher> *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

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(_));
 
@@ -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