Skip to content

Commit

Permalink
Revert "auth/aws: fix panic in IAM-based login when client config doe…
Browse files Browse the repository at this point in the history
…sn't exist (#23366)"

This reverts commit 80e1912.
  • Loading branch information
robmonte committed Oct 3, 2023
1 parent 9774cb0 commit 52a5132
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 92 deletions.
2 changes: 1 addition & 1 deletion builtin/credential/aws/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
"role": roleName,
"request_role": roleName,
}, nil
}

Expand Down
30 changes: 15 additions & 15 deletions builtin/credential/aws/path_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ func (b *backend) pathLoginIamGetRoleNameCallerIdAndEntity(ctx context.Context,

config, err := b.lockedClientConfigEntry(ctx, req.Storage)
if err != nil {
return "", nil, nil, nil, fmt.Errorf("error getting configuration: %w", err)
return "", nil, nil, logical.ErrorResponse("error getting configuration"), nil
}

endpoint := "https://sts.amazonaws.com"
Expand All @@ -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
}

url, err := stsRegionalEndpoint(clientSpecifiedRegion)
if err != nil {
return "", nil, nil, logical.ErrorResponse(err.Error()), 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
}

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)
Expand Down
50 changes: 0 additions & 50 deletions builtin/credential/aws/path_login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,56 +308,6 @@ 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) {
Expand Down
3 changes: 0 additions & 3 deletions changelog/23366.txt

This file was deleted.

5 changes: 2 additions & 3 deletions website/content/docs/release-notes/1.15.0.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ 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

Expand Down Expand Up @@ -60,7 +59,7 @@ Follow the learn more links for more information, or browse the list of
</tr>
</thead>
<tbody>

<tr>
<td rowSpan={2} style={{verticalAlign: 'middle'}}>
Vault Agent
Expand All @@ -77,7 +76,7 @@ Follow the learn more links for more information, or browse the list of
<tr>
<td style={{verticalAlign: 'middle', textAlign: 'center'}}>GA</td>
<td style={{verticalAlign: 'middle'}}>
Fetch secrets directly into your application as environment variables.
Fetch secrets directly into your application as environment variables.
<br /><br />
Learn more: <a href="/vault/docs/agent-and-proxy/agent/process-supervisor">Process Supervisor Mode</a>
</td>
Expand Down
2 changes: 0 additions & 2 deletions website/content/docs/upgrading/upgrade-to-1.15.x.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,3 @@ 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'
18 changes: 0 additions & 18 deletions website/content/partials/known-issues/aws-auth-panics.mdx

This file was deleted.

0 comments on commit 52a5132

Please sign in to comment.