Skip to content

Conversation

kpcraig
Copy link
Contributor

@kpcraig kpcraig commented Nov 28, 2024

Description

Adds two fields, sts_fallback_endpoints and sts_fallback_regions to allow credential requests to try other sts regions if the first one fails.

To test this one, set up a secrets/aws mount in the usual way:

vault auth enable aws
vault write aws/config/root \
  access_key=$AWS_ACCESS_KEY_ID secret_key=$AWS_SECRET_ACCESS_KEY \
  sts_endpoint=https://sts.us-west-1.amazonaws.com
  sts_region=us-west-1
  sts_fallback_endpoints=https://sts.us-east-2.amazonaws.com,https://sts.us-east-1.amazonaws.com
  sts_fallback_regions=us-east-2,us-east-1
vault write aws/roles/dev-role-iam credential_type=assumed_role role_arns=arn:aws:iam::XXXXXXXXXX:role/foobar
vault read aws/sts/dev-role-iam

> Key                Value
---                -----
lease_id           aws/creds/dev-role-iam/.....

Now, 'break' us-west-1 somehow, (I suggest messing with /etc/hosts), and you should get a failure:

vault read aws/sts/dev-role-iam

Error reading aws/creds/dev-role-iam: Error making API request.

URL: GET http://localhost:8200/v1/aws/creds/dev-role-iam
Code: 400. Errors:

* Error assuming role: RequestError: send request failed
caused by: Post "https://sts.us-east-2.amazonaws.com/": dial tcp 127.0.0.1:443: connect: connection refused

Experiment with breaking and unbreaking the route to AWS servers - the credential should succeed as long as one sts domain works.

TODO only if you're a HashiCorp employee

  • Jira: If this change has an associated Jira, it's referenced either
    in the PR description, commit message, or branch name.

@kpcraig kpcraig requested a review from a team as a code owner November 28, 2024 02:15
@github-actions github-actions bot added the hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed label Nov 28, 2024
@kpcraig kpcraig requested a review from a team as a code owner November 28, 2024 02:17
@kpcraig kpcraig requested review from kartiklunkad26 and removed request for a team November 28, 2024 02:17
Copy link

github-actions bot commented Nov 28, 2024

CI Results:
All Go tests succeeded! ✅

Copy link

github-actions bot commented Nov 28, 2024

Build Results:
All builds succeeded! ✅

@kpcraig kpcraig changed the title Add STS Fallback parameters to secrets-aws engine VAULT-32804: Add STS Fallback parameters to secrets-aws engine Nov 28, 2024
Co-authored-by: Robert <17119716+robmonte@users.noreply.github.com>
robmonte
robmonte previously approved these changes Dec 4, 2024
fairclothjm
fairclothjm previously approved these changes Dec 4, 2024
Copy link
Contributor

@fairclothjm fairclothjm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, LGTM! I added a few suggestions but they should not hold this up.

return nil, fmt.Errorf("error reading root configuration: %w", err)
var configs []*aws.Config

// I'm not sure this is a valid scenario, but the previous code had it as a case.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is possible since none of the aws/config/root fields are required by the API as they can be either set to defaults or the environment. i.e., I don't believe it is required to explicitly call aws/config/root and in that case entry will be nil.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the comment to reflect this

fairclothjm
fairclothjm previously approved these changes Dec 5, 2024
schavis
schavis previously approved these changes Dec 5, 2024
Copy link
Contributor

@schavis schavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please apply the formatting fix before merging. Otherwise lgtm

Co-authored-by: Sarah Chavis <62406755+schavis@users.noreply.github.com>
@kpcraig kpcraig dismissed stale reviews from schavis and fairclothjm via 2e8336d December 5, 2024 21:03
@kpcraig kpcraig merged commit d8482b0 into main Dec 5, 2024
93 checks passed
@kpcraig kpcraig deleted the VAULT-32804/sts-fallback-endpoints branch December 5, 2024 21:22
Monkeychip pushed a commit that referenced this pull request Dec 18, 2024
Co-authored-by: Sarah Chavis <62406755+schavis@users.noreply.github.com>

---------

Co-authored-by: Robert <17119716+robmonte@users.noreply.github.com>
Co-authored-by: Sarah Chavis <62406755+schavis@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hashicorp-contributed-pr If the PR is HashiCorp (i.e. not-community) contributed pr/no-milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants