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

Use iam credentials for login, allowing session expiry of 12h #92

Merged
merged 3 commits into from
Sep 23, 2016

Conversation

lox
Copy link
Collaborator

@lox lox commented Sep 22, 2016

Based on feedback in this aws forum thread, it became obvious that the new longer federated session expiry could be used if IAM credentials were used to assume the role, vs our standard iam -> session token -> assume role -> federation tokens.

I'm not sure why I never realized that the session token step was unneeded. I suspect it was because it adds complexity to the code, as you need to conditionally apply MFA to the AssumeRole call if there isn't a session with MFA already applied to it.

Regardless, this works, although I've been drinking a lot of wine, so I might be imagining it all.

@lox
Copy link
Collaborator Author

lox commented Sep 22, 2016

Fixes #77.

@mtibben
Copy link
Member

mtibben commented Sep 23, 2016

👍 lgtm

@lox
Copy link
Collaborator Author

lox commented Sep 23, 2016

I don't have MFA for my test accounts, so would appreciate some testing with that.

@lox
Copy link
Collaborator Author

lox commented Sep 23, 2016

For posterity, this was my monologue last night:


Y’know, when it comes down to it, why do we even create Sessions if there isn’t an MFA on it?

[7:09]  
Basically the aim of aws-vault is to just not expose IAM credentials to the console or downstream apps.

[7:09]  
The mechanism it does that by is creating temporary credentials with STS. (edited)

[7:10]  
So the options are either GetSessionToken, or AssumeRole.

[7:10]  
If there is a role to be assumed, it should just use AssumeRole, otherwise GetSessionToken.

[7:11]  
I see no good reason to go IAM -> GetSessionToken -> AssumeRole

[7:11]  
I guess the only reason might be that you might want to use MFA to create a long-ish session, which you can then assume roles from, because AssumeRole is capped at 1 hour.

[7:13]  
Currently aws-vault caches session tokens in the keychain, but not assumed role sessions.

[7:13]  
So it’s probably worth keeping that session creating behaviour, except for in login.

[7:13]  
Good talk.

DurationVar(&input.FederationTokenDuration)

cmd.Flag("assume-role-ttl", "Expiration time for aws assumed role").
Default("15m").
Copy link
Member

Choose a reason for hiding this comment

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

This may have been discussed previously - the aws cli defaults to 1 hour, should we match it?

The temporary security credentials are valid for the duration that you specified when calling assume-role , which can be from 900 seconds (15 minutes) to a maximum of 3600 seconds (1 hour). The default is 1 hour.

- http://docs.aws.amazon.com/cli/latest/reference/sts/assume-role.html

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I think that probably makes sense.

@mtibben
Copy link
Member

mtibben commented Sep 23, 2016

I've tested with MFA and also --server and haven't had any issues

@lox
Copy link
Collaborator Author

lox commented Sep 23, 2016

Nice.

@pda
Copy link
Collaborator

pda commented Sep 23, 2016

Seems to work for me.

❯ ./aws-vault login redacted-admin
2016/09/23 14:23:39 Parsing config file /Users/pda/.aws/config
2016/09/23 14:23:39 Looking up keyring for 99designs
2016/09/23 14:23:39 Opening keychain /Users/pda/Library/Keychains/aws-vault.keychain
Enter token for arn:aws:iam::redacted:mfa/redacted: 
2016/09/23 14:23:46 Assuming role arn:aws:iam::redacted:role/redacted with iam credentials
2016/09/23 14:23:48 Using role ****************redacted, expires in 14m59.534745467s
2016/09/23 14:23:48 Creating federation login token, expires in 12h0m0s

@lox
Copy link
Collaborator Author

lox commented Sep 23, 2016

Ship it at your leisure then.

@pda pda merged commit 06f177d into master Sep 23, 2016
@pda pda deleted the use-iam-credentials-for-login branch September 23, 2016 04:33
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.

None yet

3 participants