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

Add LoadOptions hook to configure STS/SSO credential clients #2686

Open
2 tasks done
gdavison opened this issue Jun 17, 2024 · 10 comments
Open
2 tasks done

Add LoadOptions hook to configure STS/SSO credential clients #2686

gdavison opened this issue Jun 17, 2024 · 10 comments
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue queued This issues is on the AWS team's backlog

Comments

@gdavison
Copy link
Contributor

Acknowledgements

Describe the bug

When

  • using a named profile in the shared config file which assumes a role (i.e. calls STS during credential resolution in config.LoadDefaultConfig),
  • setting the global UseFIPSEndpoint to true, and
  • using a region where STS does not have a FIPS endpoint (e.g. ca-central-1)

resolving credentials fails with the error

Retrieving credentials: failed to refresh cached credentials, operation error STS: AssumeRole, https response error StatusCode: 0, RequestID: , request send failed, Post "https://sts-fips.ca-central-1.amazonaws.com/": dial tcp: lookup sts-fips.ca-central-1.amazonaws.com: no such host

When overriding the endpoint, as suggested in the AWS CLI documentation, it fails with the error

Retrieving credentials: failed to refresh cached credentials, operation error STS: AssumeRole, failed to resolve service endpoint, endpoint rule error, Invalid Configuration: FIPS and custom endpoint are not supported

Expected Behavior

Based on previous discussion (#2336 (comment)), the first failure is expected.

When providing a custom endpoint, the UseFIPSEndpoint (and UseDualStackEndpoint) flags should be cleared so that the endpoint can be used.

Current Behavior

Because config.LoadDefaultConfig creates its own STS client internally, there is no way to clear the UseFIPSEndpoint setting when also using a custom endpoint.

Using a global aws.EndpointResolverWithOptions does work to set the endpoint without the Invalid Configuration: FIPS and custom endpoint are not supported error, but aws.EndpointResolverWithOptions is now deprecated. It also doesn't directly support setting endpoints via environment variables or the shared configuration file.

Note that the AWS CLI also fails in this situation:

AWS_PROFILE="<profile-that-assumes-role>" AWS_REGION=ca-central-1 AWS_USE_FIPS_ENDPOINT=true aws s3api list-buckets --endpoint-url="https://s3.ca-central-1.amazonaws.com/"

fails with

Could not connect to the endpoint URL: "https://sts-fips.ca-central-1.amazonaws.com/"

Reproduction Steps

package main

import (
	"context"
	"fmt"
	"os"

	"github.com/aws/aws-sdk-go-v2/config"
)

func main() {
	ctx := context.Background()

	// ca-central-1 does not support FIPS for STS
	os.Setenv("AWS_REGION", "ca-central-1")
	os.Setenv("AWS_USE_FIPS_ENDPOINT", "true")

	os.Setenv("AWS_ENDPOINT_URL_STS", "https://sts.ca-central-1.amazonaws.com/")

	os.Setenv("AWS_PROFILE", "assume-role")

	cfg, err := config.LoadDefaultConfig(ctx)
	if err != nil {
		fmt.Printf("Loading configuration: %s\n", err)
		os.Exit(1)
	}

	_, err = cfg.Credentials.Retrieve(ctx)
	if err != nil {
		fmt.Printf("Retrieving credentials: %s\n", err)
		os.Exit(1)
	}

	fmt.Println("Success.")
}

The config file

[source]
aws_access_key_id     = ****
aws_secret_access_key = ****

[assume-role]
source_profile = source
role_arn       = arn:aws:iam::123456789012:role/OrganizationAccountAccessRole

Possible Solution

No response

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

require github.com/aws/aws-sdk-go-v2/config v1.27.19

require (
github.com/aws/aws-sdk-go-v2 v1.28.0 // indirect
github.com/aws/aws-sdk-go-v2/credentials v1.17.19 // indirect
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.6 // indirect
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.10 // indirect
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.10 // indirect
github.com/aws/aws-sdk-go-v2/internal/ini v1.8.0 // indirect
github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.2 // indirect
github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.12 // indirect
github.com/aws/aws-sdk-go-v2/service/sso v1.20.12 // indirect
github.com/aws/aws-sdk-go-v2/service/ssooidc v1.24.6 // indirect
github.com/aws/aws-sdk-go-v2/service/sts v1.28.13 // indirect
github.com/aws/smithy-go v1.20.2 // indirect
)

Compiler and Version used

go version go1.22.1 darwin/arm64

Operating System and version

macOS 13.6.7

@RanVaknin
Copy link
Contributor

Hi @gdavison ,

Thanks for reaching out. You can pass in WithAssumeRoleCredentialOptions to your config, create an new sts client internally and assign it a new config options that disable fips. Something like this:

package main

import (
	"context"
	"fmt"
	"github.com/aws/aws-sdk-go-v2/aws"
	"github.com/aws/aws-sdk-go-v2/config"
	"github.com/aws/aws-sdk-go-v2/credentials/stscreds"
	"github.com/aws/aws-sdk-go-v2/service/sts"
	"os"
)

func main() {
	ctx := context.Background()
	os.Setenv("AWS_USE_FIPS_ENDPOINT", "true")
	os.Setenv("AWS_ENDPOINT_URL_STS", "https://sts.ca-central-1.amazonaws.com/")
	os.Setenv("AWS_REGION", "ca-central-1")
	os.Setenv("AWS_PROFILE", "assume-role")

	cfg, err := config.LoadDefaultConfig(ctx,
		config.WithRegion("ca-central-1"),
		config.WithAssumeRoleCredentialOptions(func(o *stscreds.AssumeRoleOptions) {
			secondCfg, err := config.LoadDefaultConfig(
				context.TODO(),
				config.WithRegion("ca-central-1"),
				config.WithClientLogMode(aws.LogRequestWithBody),
				config.WithUseFIPSEndpoint(aws.FIPSEndpointStateDisabled),
			)
			if err != nil {
				panic(err)
			}
			o.Client = sts.NewFromConfig(secondCfg)
		}),
	)
	if err != nil {
		panic(err)
	}

	_, err = cfg.Credentials.Retrieve(ctx)
	if err != nil {
		panic(err)
	}

	fmt.Println("Success.")
}

let me know if this helps.

Thanks,
Ran~

@RanVaknin RanVaknin added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Jun 20, 2024
Copy link

github-actions bot commented Jul 1, 2024

This issue has not received a response in 1 week. If you want to keep this issue open, please just leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Jul 1, 2024
@tmccombs
Copy link

tmccombs commented Jul 1, 2024

Please keep open

@RanVaknin
Copy link
Contributor

Hi @tmccombs ,

Can you please elaborate on why this needs to be kept open? Did the workaround I provided not work for you?

Thanks,
Ran~

@RanVaknin RanVaknin removed response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. closing-soon This issue will automatically close in 4 days unless further comments are made. labels Jul 1, 2024
@lucix-aws lucix-aws added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jul 1, 2024
@gdavison
Copy link
Contributor Author

gdavison commented Jul 3, 2024

Hi @RanVaknin. I haven't had a chance to check this out.

I assume that @tmccombs has asked to keep this open because this issue relates to an issue that he submitted in https://github.com/hashicorp/terraform-provider-aws

@tmccombs
Copy link

tmccombs commented Jul 3, 2024

Ugh, I wrote a response, and it appears to have been lost somehow.

Anyway, yes, an application could technically work around this, but doing so is rather complex. Consider an application where credentials could come from multiple sources, such as terraform. For this workaround, the application would need to look at the environment variables, and configuration files that the SDK usually looks at, to figure out if it should disable UseFips because an endpoint is specified. And in your example the region is a constant. What if the region comes from the credential source.

Also, in my opinion it would be better for this to be implemented once in the SDK, rather than every application that uses the SDK having to solve it separately, possibly in inconsistent ways.

@gdavison
Copy link
Contributor Author

gdavison commented Jul 3, 2024

Hi @RanVaknin. The sample code doesn't work when retrieving credentials from a shared config file, even without the FIPS setting and custom endpoint. The STS client makes two requests to AssumeRole. The first appears to succeed, and assumes the expected role. However, the second tries to assume the role again, even though it has already been assumed

First request:

SDK 2024/07/03 14:36:08 DEBUG Request
POST / HTTP/1.1
Host: sts.ca-central-1.amazonaws.com
User-Agent: m/E aws-sdk-go-v2/1.30.1 os/macos lang/go#1.22.1 md/GOOS#darwin md/GOARCH#arm64 api/sts#1.30.1
Authorization: AWS4-HMAC-SHA256 Credential=<base role>/**** *****

Action=AssumeRole&DurationSeconds=900&RoleArn=arn%3Aaws%3Aiam%3A%3A123456789012%3Arole%2FOrganizationAccountAccessRole&RoleSessionName=SomeSessionName&Version=2011-06-15
SDK 2024/07/03 14:36:08 DEBUG Response
HTTP/1.1 200 OK
Content-Length: 1540
Content-Type: text/xml
Date: Wed, 03 Jul 2024 21:36:08 GMT

<AssumeRoleResponse xmlns="https://sts.amazonaws.com/doc/2011-06-15/">
  <AssumeRoleResult>
    <AssumedRoleUser>
      <AssumedRoleId>AROAEXAMPLE:SomeSessionName</AssumedRoleId>
      <Arn>arn:aws:sts::123456789012:assumed-role/OrganizationAccountAccessRole/SomeSessionName </Arn>
    </AssumedRoleUser>
    <Credentials>
      <AccessKeyId><</AccessKeyId>
      <SecretAccessKey>EXAMPLEACCESSKEY</SecretAccessKey>
      <SessionToken>*******</SessionToken>
      <Expiration>2024-07-03T21:51:08Z</Expiration>
    </Credentials>
  </AssumeRoleResult>
  <ResponseMetadata>
    <RequestId>****</RequestId>
  </ResponseMetadata>
</AssumeRoleResponse>

The second request, issued immediately after

SDK 2024/07/03 14:36:08 DEBUG Request
POST / HTTP/1.1
Host: sts.ca-central-1.amazonaws.com
User-Agent: m/E aws-sdk-go-v2/1.30.1 os/macos lang/go#1.22.1 md/GOOS#darwin md/GOARCH#arm64 api/sts#1.30.1
Authorization: AWS4-HMAC-SHA256 Credential= EXAMPLEACCESSKEY/**** ****

Action=AssumeRole&DurationSeconds=900&RoleArn=arn%3Aaws%3Aiam%3A%3A123456789012%3Arole%2FOrganizationAccountAccessRole&RoleSessionName=ADifferentSessionName&Version=2011-06-15
SDK 2024/07/03 14:36:08 DEBUG Response
HTTP/1.1 403 Forbidden
Content-Length: 468
Content-Type: text/xml
Date: Wed, 03 Jul 2024 21:36:08 GMT

<ErrorResponse xmlns="https://sts.amazonaws.com/doc/2011-06-15/">
  <Error>
    <Type>Sender</Type>
    <Code>AccessDenied</Code>
    <Message>User: arn:aws:sts::123456789012:assumed-role/OrganizationAccountAccessRole/SomeSessionName is not authorized to perform: sts:AssumeRole on resource: arn:aws:iam:: 123456789012:role/OrganizationAccountAccessRole</Message>
  </Error>
  <RequestId>****</RequestId>
</ErrorResponse>

@gdavison
Copy link
Contributor Author

gdavison commented Jul 3, 2024

Also, to amplify @tmccombs's point above, see hashicorp/terraform-provider-aws#38057 which overrides the EndpointResolverV2 for every service to allow falling back to the non-FIPS endpoint if the FIPS endpoint does not exist. It doesn't handle the Dual Stack case yet.

There would have been more changes, but not all services have an AWS SDK for Go v2 implementation yet 🙂

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jul 4, 2024
@RanVaknin RanVaknin added the needs-review This issue or pull request needs review from a core team member. label Jul 8, 2024
@lucix-aws
Copy link
Contributor

The reason you're getting two AssumeRoles with that snippet is because the inner default config also resolves to using STS credentials. - so your outer config tries to assume role, and then the inner config does the same. You could patch over this with more code but I think we'd start overcomplicating the solution.

It seems like we're basically just missing a simple functional option helper e.g. config.WithSTSClientOptions() etc. that would enable you to set this behavior e.g.

cfg, err := config.LoadDefaultConfig(ctx, confg.WithSTSClientOptions(func(o *sts.Options)) {
    o.EndpointOptions.UseFIPSEndpoint = blah
})

@lucix-aws lucix-aws added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-review This issue or pull request needs review from a core team member. labels Jul 10, 2024
@gdavison
Copy link
Contributor Author

@lucix-aws, yes, I think that would work

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Jul 18, 2024
@lucix-aws lucix-aws added feature-request A feature should be added or improved. queued This issues is on the AWS team's backlog and removed bug This issue is a bug. labels Jul 18, 2024
@lucix-aws lucix-aws changed the title Global UseFIPSEndpoint setting and Assuming IAM Role from shared config file Add LoadOptions hook to configure STS/SSO credential clients Jul 18, 2024
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. p2 This is a standard priority issue queued This issues is on the AWS team's backlog
Projects
None yet
Development

No branches or pull requests

4 participants