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

MFA support proposal #842

Closed
pwaller opened this issue Sep 17, 2016 · 13 comments
Closed

MFA support proposal #842

pwaller opened this issue Sep 17, 2016 · 13 comments
Assignees
Labels
feature-request A feature should be added or improved.

Comments

@pwaller
Copy link
Contributor

pwaller commented Sep 17, 2016

The code currently says // MFA not supported.

In #841 I made some changes to enable any [MFA] + [assume-role] workflow -even a cumbersome one- so I could get started using MFA and have a good process to follow. I don't think this is a good implementation.

It is OK, but it might be possible to do much better. In particular, it doesn't handle credential refreshing. Credentials obtained with assume-role are only valid for a maximum of 1 hour, so they cease being usable within an hour and require another MFA token if the assume-role policy demands that there is an MFA token attached to the request. On the other hand, get-session-token can be used to get a token which lasts much longer and this token can also be used to assume-role without resupplying the MFA token.

Here is the workflow I would like to support:

~/.aws/config:

[default]
aws_access_key_id=FOO
aws_secret_access_key=BAR
mfa_serial = GAHP01234567

[profile other-role]
role_arn = arn:aws:iam::000000000001:role/other-role
source_profile = default
external_id = 123456789012

I want to be able to:

  1. Run things under the default profile, prompting for an MFA serial as required and caching the credentials for a configurable length of time.
  2. Run things with AWS_DEFAULT_PROFILE=other-role, and automatically assume-role using the temporary credentials ala (1) (or request new ones prompting for an MFA token again).

Is my request reasonable? Is it straightforward to achieve, or are there hidden pitfalls I haven't spotted? Is my interpretation of how get-session-token and assume-role will work together correct?

The net effect I want is that the credentials are completely neutered unless MFA has been recently presented by way of a {"Condition": {"Bool": {"aws:MultiFactorAuthPresent": "true"}} applied to the policies which enable the credentials to do anything. Is what I want reasonable?

@pwaller
Copy link
Contributor Author

pwaller commented Sep 17, 2016

To preempt an obvious response: I am aware I can achieve this effect by manipulating the environment myself. What I would like is a standard way of securely using MFA and roles implemented by the library so that it's not necessary to do this. Software implemented using aws-sdk-go should be able to work with MFA and assume-role "for-free", or easily opt-in by way of configuration applied to .aws/config. What I'm after here is not to just solve the problem for myself, I want to solve it for colleagues and users of the software I make using aws-sdk-go.

@jasdel jasdel self-assigned this Sep 19, 2016
@jasdel
Copy link
Contributor

jasdel commented Sep 19, 2016

Thanks for opening the proposal @pwaller. We'd like to add MFA support to the AWS SDK for Go. This process might help us drive that design, and implementation.

I think we can break this issue down into two parts, default credentials, and STS credential provider. Adding MFA to default credentials I think will depend on a modification to the STS credential provider.

Currently the only way to get a MFA token into the STS credential provider is to set the TokenCode field when creating the provider. This works fine for the first time the credentials are retrieved but most likely will not work for any credentials refresh calls. I think we should investigate a way to modify the STS credential provider to take some kind of additional input for the AssumeRoleProvider type that will allow users to provide the updated token's asynchronously.

Once we have the STS credential provider updated, I think we can start taking a look at how this can be expanded to the default credential chain, and shared config. Ideally it would be some concept/config we can replicate across the other AWS SDKs.

@jasdel
Copy link
Contributor

jasdel commented Sep 19, 2016

For the STS credential provider's MFA support I'm thinking that we could add an additional field to AssumeRoleProvider, such as TokenProvider. This field would be a type or interface, that the user would need to set when constructing the AssumeRoleProvider. The AssumeRoleProvider would use this field each time its Retrieve method is called. The value returned from the field would be added to the STS AssumeRole request.

Maybe something like:

type TokenProvider interface {
    Token() (string, error)
}

I'm not sure if this type's method would need to take any input values or not.

@pwaller
Copy link
Contributor Author

pwaller commented Sep 19, 2016

Mostly sounds reasonable. I think what needs to happen is first this needs to be made to work:

[default]
aws_access_key_id=FOO
aws_secret_access_key=BAR
mfa_serial = GAHP01234567
mfa_cache_lifetime = 12h

The thing is, the STS credential provider you linked to demands a role to assume. But I think the workflow that is really desired to make the above work is to call get-session-token, and cache the result of this.

This session token can then later be used to assume roles that require MFA. This is useful because an assumed role's credentials can only last up to 1 hour, whereas a session token can last considerably longer. So I would want to see this cache the get-session-token credentials and then, if a role is specified to be assumed, assume a role as often as is needed.

@jasdel jasdel added the feature-request A feature should be added or improved. label Sep 19, 2016
@pwaller
Copy link
Contributor Author

pwaller commented Sep 19, 2016

I have put together https://github.com/pwaller/aws-creds which is a program you can run which does the credential caching I described in the post above, if you want to see what that feels like.

@jasdel
Copy link
Contributor

jasdel commented Sep 19, 2016

Thanks for the clarification @pwaller. I agree the existing AssumeRole in not sufficient in this case. I think a new credential provider for the SDK(s) to use will need to be created thats independent, or expands on STS credential provider. With that said, the basic idea of a TokenProvider still applies but would be added to this new session token credential provider instead of, or in addition to the AssumeRoleProvider.

@pwaller
Copy link
Contributor Author

pwaller commented Oct 28, 2016

Hi @jasdel, what is the best way to progress this from here? Is it something you are looking to implement before the end of the year, or is that unlikely to happen?

@jasdel
Copy link
Contributor

jasdel commented Oct 28, 2016

@pwaller thanks for getting back with us. This work is on our backlog. We've not started work on this feature yet. Though we are always glad to help guide any PRs people would like to contribute.

For this feature I think the discussion here is on the right track. The implementation could start with a session token credential provider from there additional functionality could be added. Additionally work to have a MFATokenProvider that is used by both the new session token credential provider and the existing AssumeRoleProvider.

Once we have this working in the SDK we can take a look next how to integrate the shared config file ~/.aws/config.

@zmalik
Copy link

zmalik commented Dec 9, 2016

Hi @jasdel and @pwaller
I was trying to have a workaround for this, as I also need to use MFA.
And what I'm doing is using the STS credential provider to assume the role. Not sure if it is the right approach. But atleast I avoid running aws-cli

fmt.Print("Enter MFA code: \n")
var token string
fmt.Scanln(&token)
creds := stscreds.NewCredentials(session, roleARN, func(o *stscreds.AssumeRoleProvider) {
			o.Duration = time.Hour
			o.ExpiryWindow = 5 * time.Minute
			o.RoleSessionName = iamSession
			o.TokenCode = aws.String(token)
			o.SerialNumber = aws.String(serial)
		})

And later using this credentials for the session. Is this a right workaround for MFA?
And its working as for now but not as elegant as I would like it to be.
So if I get it right this token could be provided by some TokenProvider like @jasdel mentioned above, so people can implement is as they require, stdin or other integrations

type TokenProvider interface {
    Token() (string, error)
}

@oli-g
Copy link

oli-g commented Jan 27, 2017

Hi @jasdel! Do you have any news to share on the topic here? This seems to be a much needed feature, that will ease configuration and usage of tools that builds on this SDK (check this Terraform pull request for example).

@jasdel
Copy link
Contributor

jasdel commented Jan 28, 2017

Thanks for the request @oli-g I don't have any additional information to share at the moment. This feature is still on our backlog, but requests like this help us prioritize the tasks we address next.

@jasdel
Copy link
Contributor

jasdel commented Feb 18, 2017

Hi @pwaller, @oli-g, and @zmalik I've created PR #1088 that adds support for MFA tokens with assume role via the shared config via the Session and via stscreds.AssumeRoleProvider directly.

For this change I went with a function func() (string, error) instead of interface out of simplicity.

I still need to make another pass over the docs to clarify expectations and usage of the MFA support.

It would be great if you could take a look at the PR. Any feedback would be very helpful.

jasdel added a commit that referenced this issue Feb 22, 2017
Adds support for assuming IAM roles with MFA enabled. A TokenProvider
func was added to`stscreds.AssumeRoleProvider` that will be called each
time the role's credentials need to be refreshed. A basic token provider
that sources the MFA token from stdin as `stscreds.StdinTokenProvider`.

This change also adds a new session option, `AssumeRoleTokenProvider`.
The value of this field will be passed to the `stscreds.AssumeRoleProvider`
if the shared configuration is enabled and the config (`~/.aws/config`) or 
credentials files (`~/.aws/credentials`) specify a role to assume
with MFA.

In order for the SDK to assume a role with MFA the `SharedConfigState`
session option must be set to `SharedConfigEnable`, or `AWS_SDK_LOAD_CONFIG`
environment variable set.

Creating an AssumeRoleProvider with MFA:
===

```go
// Initial credentials loaded from SDK's default credential chain. Such as
// the environment, shared credentials (~/.aws/credentials), or EC2 Instance
// Role. These credentials will be used to to make the STS Assume Role API.
sess := session.Must(session.NewSession())

// Create the credentials from AssumeRoleProvider to assume the role
// referenced by the "myRoleARN" ARN. Prompting for MFA token from stdin.
creds := stscreds.NewCredentials(sess, "myRoleARN", func(p *stscreds.AssumeRoleProvider) {
    p.SerialNumber = aws.String("myTokenSerialNumberOrARN")
    p.TokenProvider = stscreds.StdinTokenProvider
})

// Create service client value configured for credentials
// from assumed role.
svc := s3.New(sess, &aws.Config{Credentials: creds})
```

Creating a Session with shared config enabled to assume a role with MFA:
===

```go
sess := session.Must(session.NewSessionWithOptions(session.Options{
    AssumeRoleTokenProvider: stscreds.StdinTokenProvider,
    SharedConfigState:       session.SharedConfigEnable,
}))

// Create service client value configured for credentials
// from assumed role.
svc := s3.New(sess)
```

Fix #842
Related To hashicorp/terraform#9349
@jasdel
Copy link
Contributor

jasdel commented Feb 22, 2017

HI All I merged in #1088 which adds MFA support to the SDK. Let us know if you have any issues or feedback. thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

4 participants