Assume role with web identity provider #587

alindeman · 2020-05-12T20:10:48Z

AWS allows users and systems to assume roles using OpenID Connect tokens via the AssumeRoleWithWebIdentity API call. This API allows role assumption via "Amazon Cognito, Login with Amazon, Facebook, Google, or any OpenID Connect-compatible identity provider."

This change proposes that aws-vault support AssumeRoleWithWebIdentity, either by fetching a token from a static file (via the standard web_identity_token_file config option) or by executing a process (via the web_identity_token_process config option, similar to credential_process but adapted for web identity). The latter would be a custom option for aws-vault, and would allow for interactive logins through OpenID Connect identity providers that can redirect back to localhost or urn:ietf:wg:oauth:2.0:oob.

A static example:

[profile foo]
role_arn = arn:aws:iam::123457890:role/foo
web_identity_token_file = /path/to/file

A dynamic example, using a tool like oidccli:

[profile foo]
role_arn = arn:aws:iam::123457890:role/foo
web_identity_token_process = oidccli -issuer=https://example.com -client-id=aws -client-secret=localonly raw

AWS allows users and systems to assume roles using OpenID Connect tokens via the [AssumeRoleWithWebIdentity API call]. This API allows role assumption via "Amazon Cognito, Login with Amazon, Facebook, Google, or any OpenID Connect-compatible identity provider." This change proposes that aws-vault support AssumeRoleWithWebIdentity, either by fetching a token from a static file (via the standard [web_identity_token_file] config option) or by executing a process (via the web_identity_token_process config option, similar to [credential_process] but adapted for web identity). The latter would be a custom option for aws-vault, and would allow for interactive logins through OpenID Connect identity providers that can redirect back to localhost or [urn:ietf:wg:oauth:2.0:oob]. A static example: ``` [profile foo] role_arn = arn:aws:iam::123457890:role/foo web_identity_token_file = /path/to/file ``` A dynamic example, using a tool like [oidccli]: ``` [profile foo] role_arn = arn:aws:iam::123457890:role/foo web_identity_token_process = oidccli -issuer=https://example.com -client-id=aws -client-secret=localonly raw ``` [AssumeRoleWithWebIdentity API call]: https://docs.aws.amazon.com/STS/latest/APIReference/API_AssumeRoleWithWebIdentity.html [web_identity_token_file]: https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-role.html#cli-configure-role-oidc [credential_process]: https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-sourcing-external.html [urn:ietf:wg:oauth:2.0:oob]: https://cloud.google.com/iap/docs/authentication-howto#signing_in_to_the_application [oidccli]: https://github.com/pardot/oidc/blob/master/cmd/oidccli

mtibben · 2020-05-14T23:50:16Z

vault/assumerolewithwebidentityprovider.go

+		cmdArgs = []string{"cmd.exe", "/C", p.WebIdentityTokenProcess}
+	} else {
+		cmdArgs = []string{"/bin/sh", "-c", p.WebIdentityTokenProcess}
+	}


Would using https://github.com/mattn/go-shellwords (instead of invoking the shell) be a better approach? Or are there specific shell features you're expecting to be used?

This function is admittedly modeled after the aws-sdk-go's processcreds provider, since I modeled web_identity_token_process after credential_process. That function invokes a shell, so I was inspired to do it here as well.

If you think it's better to avoid executing a shell, I'd be happy to implement it that way though.

mtibben · 2020-05-14T23:56:21Z

vault/vault.go

@@ -192,6 +225,8 @@ func (t *tempCredsCreator) provider(config *Config) (credentials.Provider, error
 		}
 	} else if config.HasSSOStartURL() {
 		return NewSSORoleCredentialsProvider(t.keyring.Keyring, config)
+	} else if config.HasRole() && (config.HasWebIdentityTokenFile() || config.HasWebIdentityTokenProcess()) {
+		return NewAssumeRoleWithWebIdentityProvider(credentials.NewCredentials(sourceCredProvider), t.keyring.Keyring, config)


Referencing sourceCredProvider inside this if-else block doesn't seem right, as the sourceCredProvider is defined in the if-else. Does this condition need to move after the final else?

Good catch. Since we're using a web identity to assume a role, we don't actually need source credentials at all. This is a copy & paste error.

I've updated it in 6f25cc6 and c1e355e to use the same method for constructing a session as NewSSORoleCredentialsProvider which also does not require a sourceCredProvider.

mtibben · 2020-05-15T00:45:03Z

USAGE.md

+
+AWS supports assuming roles using [web identity federation and OpenID Connect](https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-role.html#cli-configure-role-oidc), including login using Amazon, Google, Facebook or any other OpenID Connect server. The configuration options are as follows:
+* `web_identity_token_file` A file that contains an OpenID Connect identity token. The token is loaded and passed as the `WebIdentityToken` argument of the `AssumeRoleWithWebIdentity` operation.
+* `web_identity_token_process` A command that executes to generate an OpenID Connect identity token. The token written to the command's standard out is passed as the `WebIdentityToken` argument of the `AssumeRoleWithWebIdentity` operation.


I can't find any reference to web_identity_token_process in the AWS CLI docs. Is this a new config parameter?

Ah yes I see from the PR description that it is.

Although we have endeavoured to retain compatibility with the aws cli config, this seems pretty reasonable.

We should document this further perhaps in the config section of this doc, pointing out it is not standard

Although we have endeavoured to retain compatibility with the aws cli config, this seems pretty reasonable.

Awesome. It's designed to be really similar to the officially-supported credential_process option, but for retrieving a web identity token instead of an AWS access key/secret key/session key directly.

We should document this further perhaps in the config section of this doc, pointing out it is not standard

👍 What do you think of 4d712c1?

mtibben · 2020-05-15T02:04:11Z

vault/assumerolewithwebidentityprovider.go

+	r, w, err := os.Pipe()
+	if err != nil {
+		return "", err
+	}
+	defer r.Close()
+	defer w.Close()
+
+	cmd := exec.Command(cmdArgs[0], cmdArgs[1:]...)
+	cmd.Env = os.Environ()
+	cmd.Stdin = os.Stdin
+	cmd.Stdout = w
+	cmd.Stderr = os.Stderr
+
+	if err := cmd.Start(); err != nil {
+		return "", fmt.Errorf("failed to start command %q: %v", p.WebIdentityTokenProcess, err)
+	}
+	defer func() { _ = cmd.Process.Kill() }()
+
+	waitCh := make(chan error, 1)
+	go func() { waitCh <- cmd.Wait() }()
+
+	b := bytes.NewBuffer(make([]byte, 0, defaultMaxBufSize))
+	readCh := make(chan error, 1)
+	go func() {
+		w.Close() // close our write end of the pipe
+		defer r.Close()
+
+		_, err := io.CopyN(b, r, int64(defaultMaxBufSize))
+		readCh <- err
+	}()
+
+	timer := time.NewTimer(defaultTimeout)
+	defer timer.Stop()
+
+	// Wait for process to exit (or timeout)
+	select {
+	case err := <-waitCh: // process exited
+		if err != nil {
+			return "", fmt.Errorf("command %q failed: %v", p.WebIdentityTokenProcess, err)
+		}
+	case <-timer.C: // timeout
+		return "", fmt.Errorf("command %q timed out after %s", p.WebIdentityTokenProcess, defaultTimeout)
+	}
+
+	// Wait for read to finish (or timeout)
+	select {
+	case err := <-readCh: // process exited
+		if err != nil && err != io.EOF {
+			return "", fmt.Errorf("read output from %q failed: %v", p.WebIdentityTokenProcess, err)
+		}
+	case <-timer.C: // timeout
+		return "", fmt.Errorf("command %q timed out after %s", p.WebIdentityTokenProcess, defaultTimeout)
+	}


There is a lot of code here with channels, defers, etc, and I'm wondering if this is more complex than it needs to be.

Would a out, err := exec.Command(cmdArgs).Output() suffice here?

It would suffice for the simple case, yes. This bit of code is again inspired by aws-sdk-go's implementation of processcreds.

This code is more defensive than exec.Command(cmdArgs).Output() would be in some cases. Specifically:

If the process writes more than defaultMaxBufSize bytes to stdout, we will stop reading and close our end of the pipe. The process will die with EPIPE/SIGPIPE. The amount of memory we use as a buffer is fixed. On the other hand, Output is implemented with a bytes.Buffer which will expand 'infinitely' (until it runs out of memory).

It will timeout and kill the process after defaultTimeout, but this could be also achieved using exec.CommandContext().

Since aws-vault is configured by the user for their own use on their own machine, we might not need to be as defensive to processes that run away and write lots to stdout. I think in the worse case, aws-vault would panic with ErrTooLarge and die? If you think that's the case, I'm happy to simplify this to use exec.Command().Output or exec.ContextCommand().Output(). Let me know :)

Yes I don't think we need to be as defensive as the SDK, just go with the simple code I reckon

Sounds good. I pushed that up as 78eb664.

We do not need to be as defensive as the SDK in this case: 99designs#587 (comment)

mtibben · 2020-05-18T23:54:52Z

Thank you @alindeman !

alindeman · 2020-05-19T14:07:46Z

Thanks for pulling this in @mtibben. I'm fairly motived to see v6 ship soon, so if there are any other issues I can help with around that, definitely let me know 😄

alindeman added 2 commits May 12, 2020 16:17

Add USAGE documentation

8b948f2

alindeman force-pushed the assume-role-with-web-identity-provider branch from 3e75580 to 8b948f2 Compare May 12, 2020 20:17

mtibben reviewed May 14, 2020

View reviewed changes

mtibben reviewed May 15, 2020

View reviewed changes

mtibben added the awaiting response label May 15, 2020

alindeman added 3 commits May 15, 2020 14:24

No sourceCredProvider is needed when assuming via web identity

6f25cc6

Clarify that web_identity_token_process is a non-standard option

4d712c1

Use Region, not SSORegion

c1e355e

alindeman force-pushed the assume-role-with-web-identity-provider branch from 331c84e to c1e355e Compare May 15, 2020 19:47

alindeman added 2 commits May 18, 2020 15:26

Simplify exec code

78eb664

We do not need to be as defensive as the SDK in this case: 99designs#587 (comment)

Remove unused constants

eef2d28

mtibben removed the awaiting response label May 18, 2020

mtibben merged commit 85ab787 into 99designs:master May 18, 2020

mtibben added this to the v6 milestone May 18, 2020

lstoll mentioned this pull request Mar 6, 2023

Web identity + role fails to validate in the 7.0.0 release #1177

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Assume role with web identity provider #587

Assume role with web identity provider #587

alindeman commented May 12, 2020

mtibben May 14, 2020

alindeman May 15, 2020

mtibben May 14, 2020

alindeman May 15, 2020 •

edited

Loading

mtibben May 15, 2020

mtibben May 15, 2020

alindeman May 15, 2020

mtibben May 15, 2020

alindeman May 15, 2020

mtibben May 16, 2020

alindeman May 18, 2020

mtibben commented May 18, 2020

alindeman commented May 19, 2020

Assume role with web identity provider #587

Assume role with web identity provider #587

Conversation

alindeman commented May 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alindeman May 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtibben commented May 18, 2020

alindeman commented May 19, 2020

alindeman May 15, 2020 •

edited

Loading