From cb73fb69cd7947d0c2c72d2c99b2bcda7784c91e Mon Sep 17 00:00:00 2001 From: hc-github-team-secure-vault-core <82990506+hc-github-team-secure-vault-core@users.noreply.github.com> Date: Fri, 29 Sep 2023 22:52:53 -0400 Subject: [PATCH] backport of commit 80e1912adb9e67aa5ec43706f769740935c7bf7a (#23425) 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/23366.txt | 3 ++ website/content/docs/release-notes/1.15.0.mdx | 5 +- .../docs/upgrading/upgrade-to-1.15.x.mdx | 2 + .../partials/known-issues/aws-auth-panics.mdx | 18 +++++++ 7 files changed, 93 insertions(+), 19 deletions(-) create mode 100644 changelog/23366.txt create mode 100644 website/content/partials/known-issues/aws-auth-panics.mdx 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/23366.txt b/changelog/23366.txt new file mode 100644 index 0000000000..32405057f5 --- /dev/null +++ b/changelog/23366.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 diff --git a/website/content/docs/release-notes/1.15.0.mdx b/website/content/docs/release-notes/1.15.0.mdx index 1d5eecef36..077af022f0 100644 --- a/website/content/docs/release-notes/1.15.0.mdx +++ b/website/content/docs/release-notes/1.15.0.mdx @@ -16,6 +16,7 @@ description: |- Version | Issue ------- | ----- 1.15.0+ | [Vault no longer reports rollback metrics by mountpoint](/vault/docs/upgrading/upgrade-to-1.15.x#rollback-metrics) +1.15.0 | [Panic in AWS auth method during IAM-based login](/vault/docs/upgrading/upgrade-to-1.15.x#panic-in-aws-auth-method-during-iam-based-login) ## Vault companion updates @@ -59,7 +60,7 @@ Follow the learn more links for more information, or browse the list of - + Vault Agent @@ -76,7 +77,7 @@ Follow the learn more links for more information, or browse the list of GA - Fetch secrets directly into your application as environment variables. + Fetch secrets directly into your application as environment variables.

Learn more: Process Supervisor Mode diff --git a/website/content/docs/upgrading/upgrade-to-1.15.x.mdx b/website/content/docs/upgrading/upgrade-to-1.15.x.mdx index 9b2b61881a..b1de7d0e3c 100644 --- a/website/content/docs/upgrading/upgrade-to-1.15.x.mdx +++ b/website/content/docs/upgrading/upgrade-to-1.15.x.mdx @@ -50,3 +50,5 @@ option. @include 'known-issues/transit-managed-keys-panics.mdx' @include 'known-issues/transit-managed-keys-sign-fails.mdx' + +@include 'known-issues/aws-auth-panics.mdx' diff --git a/website/content/partials/known-issues/aws-auth-panics.mdx b/website/content/partials/known-issues/aws-auth-panics.mdx new file mode 100644 index 0000000000..d758253a15 --- /dev/null +++ b/website/content/partials/known-issues/aws-auth-panics.mdx @@ -0,0 +1,18 @@ +### Panic in AWS auth method during IAM-based login + +#### Affected versions + +- 1.15.0 + +#### Issue + +A panic can occur in the AWS auth method during [IAM-based](/vault/docs/auth/aws#iam-auth-method) +login when a [client config](/vault/api-docs/auth/aws#configure-client) does not exist. + +#### Workaround + +The panic can be avoided by writing an empty [client config](/vault/api-docs/auth/aws#configure-client): + +```shell-session +vault write -f auth/aws/config/client +```