-
Notifications
You must be signed in to change notification settings - Fork 7
Role-based S3 access (AssumeRole) #875
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: antalya-25.6
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,9 @@ namespace S3 | |
# include <aws/core/utils/UUID.h> | ||
# include <aws/core/http/HttpClientFactory.h> | ||
|
||
# include <aws/sts/STSClient.h> | ||
# include <aws/identity-management/auth/STSAssumeRoleCredentialsProvider.h> | ||
|
||
# include <aws/core/utils/HashingUtils.h> | ||
# include <aws/core/platform/FileSystem.h> | ||
|
||
|
@@ -560,6 +563,86 @@ void AwsAuthSTSAssumeRoleWebIdentityCredentialsProvider::refreshIfExpired() | |
Reload(); | ||
} | ||
|
||
AWSInstanceMetadataAssumeRoleCredentialsProvider::AWSInstanceMetadataAssumeRoleCredentialsProvider( | ||
const Aws::String & role_arn_, | ||
const Aws::String & session_name_, | ||
DB::S3::PocoHTTPClientConfiguration aws_client_configuration, | ||
uint64_t expiration_window_seconds_) | ||
: role_arn(role_arn_) | ||
, session_name(session_name_.empty() ? Aws::String(Aws::Utils::UUID::RandomUUID()) : session_name_) | ||
, logger(getLogger("AWSInstanceMetadataAssumeRoleCredentialsProvider")) | ||
, expiration_window_seconds(expiration_window_seconds_) | ||
{ | ||
// Create metadata credentials provider | ||
auto ec2_metadata_client = createEC2MetadataClient(aws_client_configuration); | ||
auto config_loader = std::make_shared<AWSEC2InstanceProfileConfigLoader>(ec2_metadata_client, true); | ||
metadata_provider = std::make_shared<AWSInstanceProfileCredentialsProvider>(config_loader); | ||
|
||
aws_client_configuration.scheme = Aws::Http::Scheme::HTTPS; | ||
|
||
std::vector<Aws::String> retryable_errors; | ||
retryable_errors.push_back("IDPCommunicationError"); | ||
retryable_errors.push_back("InvalidIdentityToken"); | ||
aws_client_configuration.retryStrategy = std::make_shared<Aws::Client::SpecifiedRetryableErrorsRetryStrategy>( | ||
retryable_errors, /*maxRetries=*/ 3); | ||
|
||
sts_client = std::make_unique<Aws::STS::STSClient>(metadata_provider, aws_client_configuration); | ||
|
||
LOG_INFO(logger, "Created STS AssumeRole provider using EC2 instance metadata"); | ||
} | ||
|
||
Aws::Auth::AWSCredentials AWSInstanceMetadataAssumeRoleCredentialsProvider::GetAWSCredentials() | ||
{ | ||
refreshIfExpired(); | ||
Aws::Utils::Threading::ReaderLockGuard guard(m_reloadLock); | ||
return credentials; | ||
} | ||
|
||
void AWSInstanceMetadataAssumeRoleCredentialsProvider::Reload() | ||
{ | ||
// Get fresh metadata credentials | ||
auto metadata_creds = metadata_provider->GetAWSCredentials(); | ||
if (metadata_creds.IsEmpty()) | ||
{ | ||
LOG_ERROR(logger, "Failed to obtain instance metadata credentials"); | ||
return; | ||
} | ||
|
||
// Perform AssumeRole | ||
Aws::STS::Model::AssumeRoleRequest request; | ||
request.SetRoleArn(role_arn); | ||
request.SetRoleSessionName(session_name); | ||
|
||
auto outcome = sts_client->AssumeRole(request); | ||
if (!outcome.IsSuccess()) | ||
{ | ||
LOG_ERROR(logger, "Failed to assume role: {}", outcome.GetError().GetMessage()); | ||
return; | ||
} | ||
|
||
const auto & result = outcome.GetResult().GetCredentials(); | ||
credentials = Aws::Auth::AWSCredentials( | ||
result.GetAccessKeyId(), | ||
result.GetSecretAccessKey(), | ||
result.GetSessionToken(), | ||
result.GetExpiration().SecondsWithMSPrecision()); | ||
|
||
LOG_TRACE(logger, "Successfully assumed role using metadata credentials"); | ||
} | ||
|
||
void AWSInstanceMetadataAssumeRoleCredentialsProvider::refreshIfExpired() | ||
{ | ||
Aws::Utils::Threading::ReaderLockGuard guard(m_reloadLock); | ||
if (!areCredentialsEmptyOrExpired(credentials, expiration_window_seconds)) | ||
return; | ||
|
||
guard.UpgradeToWriterLock(); | ||
if (!areCredentialsEmptyOrExpired(credentials, expiration_window_seconds)) | ||
return; | ||
|
||
Reload(); | ||
} | ||
|
||
|
||
SSOCredentialsProvider::SSOCredentialsProvider(DB::S3::PocoHTTPClientConfiguration aws_client_configuration_, uint64_t expiration_window_seconds_) | ||
: profile_to_use(Aws::Auth::GetConfigProfileName()) | ||
|
@@ -704,11 +787,64 @@ S3CredentialsProviderChain::S3CredentialsProviderChain( | |
if (credentials_configuration.no_sign_request) | ||
return; | ||
|
||
/// add explicit credentials to the front of the chain | ||
/// because it's manually defined by the user | ||
if (!credentials.IsEmpty()) | ||
if (credentials_configuration.role_arn.empty()) | ||
{ | ||
AddProvider(std::make_shared<Aws::Auth::SimpleAWSCredentialsProvider>(credentials)); | ||
if (!credentials.IsEmpty()) | ||
{ | ||
AddProvider(std::make_shared<Aws::Auth::SimpleAWSCredentialsProvider>(credentials)); | ||
return; | ||
} | ||
} | ||
else | ||
{ | ||
if (!credentials.IsEmpty()) | ||
{ | ||
auto sts_client_config = Aws::STS::STSClientConfiguration(); | ||
|
||
if (!credentials_configuration.sts_endpoint_override.empty()) | ||
{ | ||
auto endpoint_uri = Poco::URI(credentials_configuration.sts_endpoint_override); | ||
|
||
String url_without_scheme = endpoint_uri.getHost(); | ||
if (endpoint_uri.getPort() != 0) | ||
url_without_scheme += ":" + std::to_string(endpoint_uri.getPort()); | ||
|
||
sts_client_config.endpointOverride = url_without_scheme; | ||
sts_client_config.scheme = endpoint_uri.getScheme() == "https" ? Aws::Http::Scheme::HTTPS : Aws::Http::Scheme::HTTP; | ||
} | ||
|
||
AddProvider(std::make_shared<Aws::Auth::STSAssumeRoleCredentialsProvider>( | ||
credentials_configuration.role_arn, | ||
/* sessionName */ credentials_configuration.role_session_name, | ||
/* externalId */ Aws::String(), | ||
/* loadFrequency */ Aws::Auth::DEFAULT_CREDS_LOAD_FREQ_SECONDS, | ||
std::make_shared<Aws::STS::STSClient>(credentials, | ||
/* endpointProvider */ Aws::MakeShared<Aws::STS::STSEndpointProvider>(Aws::STS::STSClient::ALLOCATION_TAG), | ||
/* clientConfiguration */ sts_client_config) | ||
) | ||
); | ||
} | ||
else | ||
{ | ||
DB::S3::PocoHTTPClientConfiguration aws_client_configuration = DB::S3::ClientFactory::instance().createClientConfiguration( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is not DB::S3 a current namespace? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed it is. But I guess it does not actually matter here -- it is done similarly in other places. Maybe some ambiguity can arise without explicitly specifying this... |
||
configuration.region, | ||
configuration.remote_host_filter, | ||
configuration.s3_max_redirects, | ||
configuration.s3_retry_attempts, | ||
configuration.s3_slow_all_threads_after_network_error, | ||
configuration.enable_s3_requests_logging, | ||
configuration.for_disk_s3, | ||
configuration.get_request_throttler, | ||
configuration.put_request_throttler); | ||
|
||
// Use metadata credentials for AssumeRole | ||
AddProvider(std::make_shared<AWSInstanceMetadataAssumeRoleCredentialsProvider>( | ||
Aws::String(credentials_configuration.role_arn), | ||
Aws::String(credentials_configuration.role_session_name), | ||
std::move(aws_client_configuration), | ||
credentials_configuration.expiration_window_seconds) | ||
); | ||
} | ||
return; | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, explain how does hardcoded maxRetries correspond with configuration.s3_retry_attempts ?
It existed before your change (e.g. in AwsAuthSTSAssumeRoleWebIdentityCredentialsProvider ctor) , but the question remains.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a complete stranger in s3 secure world, but is my understanding correct that there is no audit facilities?
The credential machine is not trivial after AssumeRole is introduced, so may be we should start from making log
a bit more verbose and increase its severity?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, all other similar logs are no higher than trace. There can be thousands of those lines per second, and they are basically meaningless, aren't they?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I understand, this "3 retries" policy is specific for those retryable errors listed above (and those are errors in communicating with STS, which is in general a standalone thing not related to S3), this
s3_retry_attempts
-- it is a general setting for S3.I took this from existing code (
AwsAuthSTSAssumeRoleWebIdentityCredentialsProvider
), but now I think about it and I do not know why they decided to set this to 3....There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds a bit worrying.
Does it mean that we are requesting thousands of tokens per a second?
They are meaningless if there is a way (e.g. using AWS logs) to clarify how exactly access was obtained.
Anyway, audit should be out of the scope of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, my bad -- of course, they are reused after obtaining them.