Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

retrive session credentials recurisvely #369

Merged
merged 5 commits into from
May 20, 2019

Conversation

jstewmon
Copy link
Collaborator

  • mfa token only needs to be provided once per source rather than once
    per profile by adding mfa_serial = <arn> to the root source profile
  • allows assume role chains (e.g. roleB <- roleA <- master)

resolves #329
resolved #310

@rossmckelvie
Copy link
Contributor

YES! I've been meaning to make an attempt at patching this. This is the only gripe our developers have -- we require MFA for every assume role and have architected our AWS organization using the latest multi-account reference architecture so we have quite a bit of accounts & roles.

I'm got your branch built locally and will report back if I come across any issues.

Copy link
Contributor

@rossmckelvie rossmckelvie left a comment

Choose a reason for hiding this comment

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

Helped with some of the documentation. Might also want to consider updating the example in USAGE.md

README.md Outdated
@@ -94,6 +94,36 @@ Then when you use the `admin` profile, `aws-vault` will look in the `read-only`

**Note:** If you have an MFA device attached to your account, the STS service will generate session tokens that are *invalid* unless you provide an MFA code. To enable MFA for a profile, specify the `mfa_serial` in `~/.aws/config`. You can retrieve the MFA's serial (ARN) in the web console, or you can usually derive it pretty easily using the format `arn:aws:iam::[account-id]:mfa/[your-iam-username]`. If you have an account with an MFA associated, but you don't provide the IAM, you are unable to call IAM services, even if you have the correct permissions to do so. `mfa_serial` will not be inherited from the profile designated in `source_profile` - you must include a reference to `mfa_serial` in every profile you wish to use it with.

Alternatively, you can also specify your `mfa_serial` on your source profile. This can be very convenient if you routinely assume multiple roles from the same source because you will only need to provide an MFA token once per source profile session:
Copy link
Contributor

Choose a reason for hiding this comment

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

The above line has a sentence which these changes contradict: mfa_serial will not be inherited from the profile designated in source_profile. From reviewing the changes you made, I think we can clean up the docs a bit. Removing the last sentence from L95, I would replace your docs with the following to clear up how the behavior has changed. Also fixed some of the source_profile examples to be working and the double mfa_serial=mfa_serial= in the first example.


mfa_serial will be inherited from the profile designated in source_profile, this can be very convenient if you routinely assume multiple roles from the same source as a MFA token is only provided once per source_profile session. You can always omit the mfa_serial on the source_profile to make MFA role-specific.

Here is an example where two roles defined in profile's admin-a & admin-b do not have the mfa_serial attribute, inheriting from the master profile instead.

[profile master]
mfa_serial = arn:aws:iam::123456789012:mfa/jonsmith

[profile admin-a]
source_profile = master
role_arn = arn:aws:iam::123456789012:role/admin-access

[profile admin-b]
source_profile = master
role_arn = arn:aws:iam::987654321987:role/admin-access

The developer only needs to configure their master profile; the list of profile config can be copied from a wiki (this is very useful when you have 25+ aws accounts with multiple roles in each).

You can also define a chain of roles to assume:

[profile master]
mfa_serial=mfa_serial = arn:aws:iam::123456789012:mfa/jonsmith

[profile intermediary]
source_profile = master
role_arn = arn:aws:iam::123456789012:role/intermediary

[profile target]
source_profile = intermediary
role_arn = arn:aws:iam::123456789012:role/target

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, good suggestions. I incorporated your feedback in a new commit, but I didn't bother changing the profile name from read-only to master b/c I thought it was preferable to use names consistent with the other examples.

@jstewmon
Copy link
Collaborator Author

jstewmon commented May 16, 2019

Just leaving a note here that I'm still looking at the rotate command to make it respect the mfa_serial option during both phases of the operation.

Edit: rotate now respects mfa_serial on the profile

@mtibben mtibben requested a review from lox May 17, 2019 06:41
@jstewmon
Copy link
Collaborator Author

@lox I think this reasonably complete now if you would be so kind as to review and provide feedback.

- mfa token only needs to be provided once per source rather than once
  per profile by adding `mfa_serial = <arn>` to the root source profile
- allows assume role chains (e.g. roleB <- roleA <- master)
The flag defaults to false but will be switched to true if the given
profile does not have a SourceProfile, since a session token cannot be
used to create a sign in url.

SessionDuration must be omitted from the request to get the sign in
token if the credentials were from an assumed role that used a session.
if err != nil {
return credentials.Value{}, err
}
if profile, exists := p.config.Profile(p.profile); exists && profile.SourceProfile != "" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Reading this code without the PR context takes a while to parse intent. Perhaps update the comment to make it clear what is happening and why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated the comment to explain the control flow.

@lox
Copy link
Collaborator

lox commented May 19, 2019

This seems like a good idea to me and the implementation looks sound.

I wonder how the aws-cli handles mfa_serial on a profile via source_profile? 🤔 Does this break compatibility between the two?

@jstewmon
Copy link
Collaborator Author

jstewmon commented May 20, 2019

@lox thanks for the review!

botocore and the aws-cli only recognizes the mfa_serial option when there is a role_arn option, so users leveraging this config may notice a lack of compatibility between aws-vault and aws-cli (and aws SDKs in general).

I didn't anticipate this being a concern b/c an aws-vault managed config is already incompatible with the AWS toolchain, since the latter always expects the root profile to contain the credentials.

In my experience, the maintainers of the SDKs (and aws-cli) are interested in improving developer ergonomics with capabilities like this, but the credentials providers in botocore can be difficult to update for historical reasons. For example: boto/botocore#1329

I think that a plan that reasonably converges compatibility between aws-vault and the cli would be to add another feature to aws-vault to allow it to act as a credential_process, so that you'd have something like this:

# ~/.aws/credentials
[default]
credential_process = aws-vault echo-json default

@lox
Copy link
Collaborator

lox commented May 20, 2019

I swear we implemented credential_process recently 🤔

I think that context makes sense, we should put it in the README to describe how we approach compatibility with aws-cli.

@lox lox merged commit 5a0643d into 99designs:master May 20, 2019
@jstewmon
Copy link
Collaborator Author

@lox , you're right about credential_process support... it just isn't where I thought it would be aws-vault exec <profile> --json will print the credentials as json.

It looks like our new behavior is buggy with that flag because the Expiration field is populated by adding duration to the current time instead of using the Expiration of the credentials. I suppose the old behavior was a quick fix b/c the credentials Value doesn't have Expiration.

@frezbo
Copy link
Contributor

frezbo commented May 21, 2019

@jstewmon Thanks for that catch, when I implemented --json, I never thought we would be using existing sessions, Good work 👍

@jstewmon jstewmon deleted the recursive-retrieve branch May 21, 2019 13:27
@vs-jawad
Copy link

@mtibben @lox Is there any idea when this will be released? It's in a confusing state right now with the master readme reflecting changes that haven't been released yet and master being quite a few commits behind the latest release

@FernandoMiguel
Copy link
Collaborator

I do love how this feature simplifies the config file.
Sadly, it breaks aws-vault login.
Even passing ttl parameters, the session only lasts 15 minutes, not the 12h it did before.
Any ideas how to fix that?

@rossmckelvie
Copy link
Contributor

@FernandoMiguel what flag are you passing? Login doesn’t respect the same environment variables that exec does, but you can pass the flag. I’ve tested this up to 1hr which is my max configured time to have a session or role assumed, IMO not sure why you’d want anything longer if you’re already using a tool like aws-vault.

from exec:

	cmd.Flag("assume-role-ttl", "Expiration time for aws assumed role").
		Default("15m").
		OverrideDefaultFromEnvar("AWS_ASSUME_ROLE_TTL").
		DurationVar(&input.RoleDuration)

from login:

	cmd.Flag("assume-role-ttl", "Expiration time for aws assumed role").
		Default("15m").
		DurationVar(&input.AssumeRoleDuration)

@FernandoMiguel
Copy link
Collaborator

FernandoMiguel commented Jun 19, 2019

from

$ aws-vault login --help
usage: aws-vault login [<flags>] <profile>

Generate a login link for the AWS Console

Flags:
      --help                     Show context-sensitive help (also try --help-long and --help-man).
      --version                  Show application version.
      --debug                    Show debugging output
      --backend=BACKEND          Secret backend to use [keychain pass file]
      --prompt=terminal          Prompt driver to use [terminal osascript zenity]
      --keychain="aws-vault"     Name of macOS keychain to use, if it doesn't exist it will be created
      --pass-dir=PASS-DIR        Pass password store directory
      --pass-cmd=PASS-CMD        Name of the pass executable
      --pass-prefix=PASS-PREFIX  Prefix to prepend to the item path stored in pass
  -n, --no-session               Use root credentials, no session created
  -t, --mfa-token=MFA-TOKEN      The mfa token to use
      --path=PATH                The AWS service you would like access
  -f, --federation-token-ttl=12h
                                 Expiration time for aws console session
      --assume-role-ttl=15m      Expiration time for aws assumed role
  -s, --stdout                   Print login URL to stdout instead of opening in default browser

Args:
  <profile>  Name of the profile

so i pass --federation-token-ttl=8h
--assume-role-ttl=1h

@rossmckelvie
Copy link
Contributor

@FernandoMiguel can you try the assume role ttl flag? We’ll look into the federated token arg but I do know that I was able to get extended sessions using assume-role-ttl

@FernandoMiguel
Copy link
Collaborator

@rossmckelvie i use assume role too, as posted above

@jstewmon
Copy link
Collaborator Author

@FernandoMiguel I ran aws-vault login --assume-role-ttl=1h <profile> this morning and my session lasted 1h.

@FernandoMiguel
Copy link
Collaborator

@jstewmon mine seem to be expiring much sooner than that.
and before we had the MFA only on the source_profile, it would last 8h (a working day)

@jstewmon
Copy link
Collaborator Author

@FernandoMiguel the issue is that the maximum duration for AssumeRole is 1h when the request is made with a Session Token.

With #383 you can put the mfa_serial back on the role profile and use aws-vault login --assume-role-ttl=12h --no-session <profile> and get prompted for your MFA to AssuleRole, allowing for a login session up to the max duration.

It would be nice if you could use --no-session and get the mfa prompt without having to define mfa_serial on any role profiles. I'll try to look into that as time allows.

@FernandoMiguel
Copy link
Collaborator

@jstewmon so the issue is having the session login to the source profile and then assume a dependant role, making the session limited by the mfa token, instead of a federation link of the destination role?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assuming multiple accounts requires a fresh TOTP code per account Using nested profiles / role assumptions
6 participants