Skip to content

Commit

Permalink
Handle STS Credentials Region (#267)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
ben-taussig-solo committed Sep 12, 2023
1 parent 4b5b068 commit 1d40d7b
Show file tree
Hide file tree
Showing 13 changed files with 117 additions and 28 deletions.
4 changes: 4 additions & 0 deletions api/envoy/config/filter/http/aws_lambda/v2/aws_lambda.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions changelog/v1.26.4-patch3/handle-sts-credentials-region.yaml
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
}
}
Expand All @@ -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);

Expand Down
19 changes: 16 additions & 3 deletions source/extensions/filters/http/aws_lambda/sts_fetcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down
3 changes: 3 additions & 0 deletions source/extensions/filters/http/aws_lambda/sts_fetcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion source/extensions/filters/http/aws_lambda/sts_status.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions test/extensions/filters/http/aws_lambda/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand All @@ -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 {
Expand Down Expand Up @@ -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) {
Expand All @@ -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 {
Expand All @@ -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);

Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
)";

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

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

Expand Down Expand Up @@ -310,4 +317,4 @@ TEST_F(StsCredentialsProviderTest, TestUnchainedFlow) {
} // namespace AwsLambda
} // namespace HttpFilters
} // namespace Extensions
} // namespace Envoy
} // namespace Envoy
Loading

0 comments on commit 1d40d7b

Please sign in to comment.