From 8202c7db4b3bc4b01bb1c5b7a40c7e03918b221e Mon Sep 17 00:00:00 2001 From: Robert <17119716+robmonte@users.noreply.github.com> Date: Fri, 6 Oct 2023 14:29:28 -0500 Subject: [PATCH] auth/aws: fix panic in IAM-based login when client config doesn't exist (#23555) Co-authored-by: Austin Gebauer <34121980+austingebauer@users.noreply.github.com> --- builtin/credential/aws/backend_test.go | 2 +- builtin/credential/aws/path_login.go | 32 +++++++-------- builtin/credential/aws/path_login_test.go | 50 +++++++++++++++++++++++ changelog/23555.txt | 3 ++ 4 files changed, 70 insertions(+), 17 deletions(-) create mode 100644 changelog/23555.txt diff --git a/builtin/credential/aws/backend_test.go b/builtin/credential/aws/backend_test.go index d737492391..d56478266d 100644 --- a/builtin/credential/aws/backend_test.go +++ b/builtin/credential/aws/backend_test.go @@ -1504,7 +1504,7 @@ func buildCallerIdentityLoginData(request *http.Request, roleName string) (map[s "iam_request_url": base64.StdEncoding.EncodeToString([]byte(request.URL.String())), "iam_request_headers": base64.StdEncoding.EncodeToString(headersJson), "iam_request_body": base64.StdEncoding.EncodeToString(requestBody), - "request_role": roleName, + "role": roleName, }, nil } diff --git a/builtin/credential/aws/path_login.go b/builtin/credential/aws/path_login.go index 68a1708ee9..ca74344172 100644 --- a/builtin/credential/aws/path_login.go +++ b/builtin/credential/aws/path_login.go @@ -292,7 +292,7 @@ func (b *backend) pathLoginIamGetRoleNameCallerIdAndEntity(ctx context.Context, config, err := b.lockedClientConfigEntry(ctx, req.Storage) if err != nil { - return "", nil, nil, logical.ErrorResponse("error getting configuration"), nil + return "", nil, nil, nil, fmt.Errorf("error getting configuration: %w", err) } endpoint := "https://sts.amazonaws.com" @@ -319,23 +319,23 @@ func (b *backend) pathLoginIamGetRoleNameCallerIdAndEntity(ctx context.Context, if config.MaxRetries >= 0 { maxRetries = config.MaxRetries } - } - // Extract and use a regional STS endpoint - // based on the region set in the Authorization header. - if config.UseSTSRegionFromClient { - clientSpecifiedRegion, err := awsRegionFromHeader(headers.Get("Authorization")) - if err != nil { - return "", nil, nil, logical.ErrorResponse("region missing from Authorization header"), nil + // Extract and use a regional STS endpoint + // based on the region set in the Authorization header. + if config.UseSTSRegionFromClient { + clientSpecifiedRegion, err := awsRegionFromHeader(headers.Get("Authorization")) + if err != nil { + return "", nil, nil, logical.ErrorResponse("region missing from Authorization header"), nil + } + + url, err := stsRegionalEndpoint(clientSpecifiedRegion) + if err != nil { + return "", nil, nil, logical.ErrorResponse(err.Error()), nil + } + + b.Logger().Debug("use_sts_region_from_client set; using region specified from header", "region", clientSpecifiedRegion) + endpoint = url } - - url, err := stsRegionalEndpoint(clientSpecifiedRegion) - if err != nil { - return "", nil, nil, logical.ErrorResponse(err.Error()), nil - } - - b.Logger().Debug("use_sts_region_from_client set; using region specified from header", "region", clientSpecifiedRegion) - endpoint = url } b.Logger().Debug("submitting caller identity request", "endpoint", endpoint) diff --git a/builtin/credential/aws/path_login_test.go b/builtin/credential/aws/path_login_test.go index 26d9b194ad..b3a9c63913 100644 --- a/builtin/credential/aws/path_login_test.go +++ b/builtin/credential/aws/path_login_test.go @@ -308,6 +308,56 @@ func TestBackend_validateVaultPostRequestValues(t *testing.T) { } } +// TestBackend_pathLogin_NoClientConfig covers logging in via IAM auth when the +// client config does not exist. This is a regression test to cover potential +// panics when referencing the potentially-nil config in the login handler. For +// details see https://github.com/hashicorp/vault/issues/23361. +func TestBackend_pathLogin_NoClientConfig(t *testing.T) { + storage := new(logical.InmemStorage) + config := logical.TestBackendConfig() + config.StorageView = storage + b, err := Backend(config) + if err != nil { + t.Fatal(err) + } + + err = b.Setup(context.Background(), config) + if err != nil { + t.Fatal(err) + } + + // Intentionally left out the client configuration + + roleEntry := &awsRoleEntry{ + RoleID: "foo", + Version: currentRoleStorageVersion, + AuthType: iamAuthType, + } + err = b.setRole(context.Background(), storage, testValidRoleName, roleEntry) + if err != nil { + t.Fatal(err) + } + + loginData, err := defaultLoginData() + if err != nil { + t.Fatal(err) + } + loginRequest := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "login", + Storage: storage, + Data: loginData, + Connection: &logical.Connection{}, + } + resp, err := b.HandleRequest(context.Background(), loginRequest) + if err != nil { + t.Fatalf("expected nil error, got: %v", err) + } + if !resp.IsError() { + t.Fatalf("expected error response, got: %+v", resp) + } +} + // TestBackend_pathLogin_IAMHeaders tests login with iam_request_headers, // supporting both base64 encoded string and JSON headers func TestBackend_pathLogin_IAMHeaders(t *testing.T) { diff --git a/changelog/23555.txt b/changelog/23555.txt new file mode 100644 index 0000000000..32405057f5 --- /dev/null +++ b/changelog/23555.txt @@ -0,0 +1,3 @@ +```release-note:bug +auth/aws: Fixes a panic that can occur in IAM-based login when a [client config](https://developer.hashicorp.com/vault/api-docs/auth/aws#configure-client) does not exist. +``` \ No newline at end of file