Skip to content

Commit

Permalink
Ecs credentials provider fixes (#132)
Browse files Browse the repository at this point in the history 
* Fix ecs credentials provider by massaging request construction:
* * Add Host and Accept-Encoding headers
* * Modify Accept header although it's probably not an issue
* * Always add Authorization header if the environment variable is set (matches other implementations)
* * Don't add keep alive header
* * Fix memory leak in ecs code path due to not cleaning up uri object
  • Loading branch information
@bretambrose
bretambrose committed May 7, 2021
1 parent a5dbb0c commit c74534c
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 31 deletions.
37 changes: 21 additions & 16 deletions source/credentials_provider_default_chain.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,26 +41,41 @@ static struct aws_credentials_provider *s_aws_credentials_provider_new_ecs_or_im
struct aws_client_bootstrap *bootstrap,
struct aws_tls_ctx *tls_ctx) {

struct aws_byte_cursor auth_token_cursor;
AWS_ZERO_STRUCT(auth_token_cursor);

struct aws_credentials_provider *ecs_or_imds_provider = NULL;
struct aws_string *ecs_relative_uri = NULL;
struct aws_string *ecs_full_uri = NULL;
struct aws_string *ec2_imds_disable = NULL;
struct aws_string *ecs_token = NULL;

if (aws_get_environment_value(allocator, s_ecs_creds_env_relative_uri, &ecs_relative_uri) != AWS_OP_SUCCESS ||
aws_get_environment_value(allocator, s_ecs_creds_env_full_uri, &ecs_full_uri) != AWS_OP_SUCCESS ||
aws_get_environment_value(allocator, s_ec2_creds_env_disable, &ec2_imds_disable) != AWS_OP_SUCCESS) {
aws_get_environment_value(allocator, s_ec2_creds_env_disable, &ec2_imds_disable) != AWS_OP_SUCCESS ||
aws_get_environment_value(allocator, s_ecs_creds_env_token, &ecs_token) != AWS_OP_SUCCESS) {
AWS_LOGF_ERROR(
AWS_LS_AUTH_CREDENTIALS_PROVIDER,
"Failed reading envrionment variables during default credentials provider chain initialization.");
"Failed reading environment variables during default credentials provider chain initialization.");
goto clean_up;
}

if (ecs_token && ecs_token->len) {
auth_token_cursor = aws_byte_cursor_from_string(ecs_token);
}

/*
* ToDo: the uri choice logic should be done in the ecs provider init logic. As it stands, it's a nightmare
* to try and use the ecs provider anywhere outside the default chain.
*/
if (ecs_relative_uri && ecs_relative_uri->len) {
struct aws_credentials_provider_ecs_options ecs_options = {
.shutdown_options = *shutdown_options,
.bootstrap = bootstrap,
.host = aws_byte_cursor_from_string(s_ecs_host),
.path_and_query = aws_byte_cursor_from_string(ecs_relative_uri),
.tls_ctx = NULL,
.auth_token = auth_token_cursor,
};
ecs_or_imds_provider = aws_credentials_provider_new_ecs(allocator, &ecs_options);

Expand All @@ -71,29 +86,17 @@ static struct aws_credentials_provider *s_aws_credentials_provider_new_ecs_or_im
goto clean_up;
}

struct aws_string *ecs_token = NULL;
if (aws_get_environment_value(allocator, s_ecs_creds_env_token, &ecs_token) != AWS_OP_SUCCESS) {
AWS_LOGF_WARN(
AWS_LS_AUTH_CREDENTIALS_PROVIDER,
"Failed reading ECS Token environment variable during ECS creds provider initialization.");
goto clean_up;
}

struct aws_byte_cursor nullify_cursor;
AWS_ZERO_STRUCT(nullify_cursor);

struct aws_credentials_provider_ecs_options ecs_options = {
.shutdown_options = *shutdown_options,
.bootstrap = bootstrap,
.host = uri.host_name,
.path_and_query = uri.path_and_query,
.tls_ctx = aws_byte_cursor_eq_c_str_ignore_case(&(uri.scheme), "HTTPS") ? tls_ctx : NULL,
.auth_token = (ecs_token && ecs_token->len) ? aws_byte_cursor_from_string(ecs_token) : nullify_cursor,
.auth_token = auth_token_cursor,
};

ecs_or_imds_provider = aws_credentials_provider_new_ecs(allocator, &ecs_options);
aws_string_destroy(ecs_token);

aws_uri_clean_up(&uri);
} else if (ec2_imds_disable == NULL || aws_string_eq_c_str_ignore_case(ec2_imds_disable, "false")) {
struct aws_credentials_provider_imds_options imds_options = {
.shutdown_options = *shutdown_options,
Expand All @@ -107,6 +110,8 @@ static struct aws_credentials_provider *s_aws_credentials_provider_new_ecs_or_im
aws_string_destroy(ecs_relative_uri);
aws_string_destroy(ecs_full_uri);
aws_string_destroy(ec2_imds_disable);
aws_string_destroy(ecs_token);

return ecs_or_imds_provider;
}

Expand Down
46 changes: 31 additions & 15 deletions source/credentials_provider_ecs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ static void s_on_connection_manager_shutdown(void *user_data);
struct aws_credentials_provider_ecs_impl {
struct aws_http_connection_manager *connection_manager;
struct aws_auth_http_system_vtable *function_table;
struct aws_string *host;
struct aws_string *path_and_query;
struct aws_string *auth_token;
};
Expand Down Expand Up @@ -300,12 +301,13 @@ static void s_ecs_on_stream_complete_fn(struct aws_http_stream *stream, int erro
}

AWS_STATIC_STRING_FROM_LITERAL(s_ecs_accept_header, "Accept");
AWS_STATIC_STRING_FROM_LITERAL(s_ecs_accept_header_value, "*/*");
AWS_STATIC_STRING_FROM_LITERAL(s_ecs_accept_header_value, "application/json");
AWS_STATIC_STRING_FROM_LITERAL(s_ecs_user_agent_header, "User-Agent");
AWS_STATIC_STRING_FROM_LITERAL(s_ecs_user_agent_header_value, "aws-sdk-crt/ecs-credentials-provider");
AWS_STATIC_STRING_FROM_LITERAL(s_ecs_h1_0_keep_alive_header, "Connection");
AWS_STATIC_STRING_FROM_LITERAL(s_ecs_h1_0_keep_alive_header_value, "keep-alive");
AWS_STATIC_STRING_FROM_LITERAL(s_ecs_authorization_header, "authorization");
AWS_STATIC_STRING_FROM_LITERAL(s_ecs_authorization_header, "Authorization");
AWS_STATIC_STRING_FROM_LITERAL(s_ecs_accept_encoding_header, "Accept-Encoding");
AWS_STATIC_STRING_FROM_LITERAL(s_ecs_accept_encoding_header_value, "identity");
AWS_STATIC_STRING_FROM_LITERAL(s_ecs_host_header, "Host");

static int s_make_ecs_http_query(
struct aws_credentials_provider_ecs_user_data *ecs_user_data,
Expand All @@ -320,6 +322,14 @@ static int s_make_ecs_http_query(

struct aws_credentials_provider_ecs_impl *impl = ecs_user_data->ecs_provider->impl;

struct aws_http_header host_header = {
.name = aws_byte_cursor_from_string(s_ecs_host_header),
.value = aws_byte_cursor_from_string(impl->host),
};
if (aws_http_message_add_header(request, host_header)) {
goto on_error;
}

if (impl->auth_token != NULL) {
struct aws_http_header auth_header = {
.name = aws_byte_cursor_from_string(s_ecs_authorization_header),
Expand All @@ -338,19 +348,19 @@ static int s_make_ecs_http_query(
goto on_error;
}

struct aws_http_header user_agent_header = {
.name = aws_byte_cursor_from_string(s_ecs_user_agent_header),
.value = aws_byte_cursor_from_string(s_ecs_user_agent_header_value),
struct aws_http_header accept_encoding_header = {
.name = aws_byte_cursor_from_string(s_ecs_accept_encoding_header),
.value = aws_byte_cursor_from_string(s_ecs_accept_encoding_header_value),
};
if (aws_http_message_add_header(request, user_agent_header)) {
if (aws_http_message_add_header(request, accept_encoding_header)) {
goto on_error;
}

struct aws_http_header keep_alive_header = {
.name = aws_byte_cursor_from_string(s_ecs_h1_0_keep_alive_header),
.value = aws_byte_cursor_from_string(s_ecs_h1_0_keep_alive_header_value),
struct aws_http_header user_agent_header = {
.name = aws_byte_cursor_from_string(s_ecs_user_agent_header),
.value = aws_byte_cursor_from_string(s_ecs_user_agent_header_value),
};
if (aws_http_message_add_header(request, keep_alive_header)) {
if (aws_http_message_add_header(request, user_agent_header)) {
goto on_error;
}

Expand Down Expand Up @@ -461,6 +471,7 @@ static void s_credentials_provider_ecs_destroy(struct aws_credentials_provider *

aws_string_destroy(impl->path_and_query);
aws_string_destroy(impl->auth_token);
aws_string_destroy(impl->host);

/* aws_http_connection_manager_release will eventually leads to call of s_on_connection_manager_shutdown,
* which will do memory release for provider and impl. So We should be freeing impl
Expand Down Expand Up @@ -557,16 +568,21 @@ struct aws_credentials_provider *aws_credentials_provider_new_ecs(
goto on_error;
}
if (options->auth_token.len != 0) {
impl->auth_token = aws_string_new_from_array(allocator, options->auth_token.ptr, options->auth_token.len);
impl->auth_token = aws_string_new_from_cursor(allocator, &options->auth_token);
if (impl->auth_token == NULL) {
goto on_error;
}
}
impl->path_and_query =
aws_string_new_from_array(allocator, options->path_and_query.ptr, options->path_and_query.len);
impl->path_and_query = aws_string_new_from_cursor(allocator, &options->path_and_query);
if (impl->path_and_query == NULL) {
goto on_error;
}

impl->host = aws_string_new_from_cursor(allocator, &options->host);
if (impl->host == NULL) {
goto on_error;
}

provider->shutdown_options = options->shutdown_options;

aws_tls_connection_options_clean_up(&tls_connection_options);
Expand Down

0 comments on commit c74534c

Please sign in to comment.