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

sts v1.25.2 or higher - AssumeRoleWithSAML fails trying to access the Metadata Server #2392

Closed
bitte-ein-bit opened this issue Nov 28, 2023 · 7 comments
Labels
bug This issue is a bug.

Comments

@bitte-ein-bit
Copy link

Describe the bug

This code is working up to sts v.15.1. Later versions fail trying to talk to the metadata server.

Expected Behavior

$ go run main.go
panic: operation error STS: AssumeRoleWithSAML, https response error StatusCode: 400, RequestID: c1bf01b3-76c5-493c-a444-20ce1d52db46, api error ValidationError: 2 validation errors detected: Value 'test' at 'roleArn' failed to satisfy constraint: Member must have length greater than or equal to 20; Value 'test' at 'principalArn' failed to satisfy constraint: Member must have length greater than or equal to 20

goroutine 1 [running]:
main.main()
	/Users/user/bug/main.go:30 +0x220
exit status 2

Current Behavior

$ go run main.go
panic: operation error STS: AssumeRoleWithSAML, get identity: get credentials: failed to refresh cached credentials, no EC2 IMDS role found, operation error ec2imds: GetMetadata, exceeded maximum number of attempts, 3, request send failed, Get "http://169.254.169.254/latest/meta-data/iam/security-credentials/": dial tcp 169.254.169.254:80: connect: host is down

goroutine 1 [running]:
main.main()
	/Users/user/bug/main.go:30 +0x220
exit status 2

Reproduction Steps

The SSCCE is broken due to the requirement for a dynamic SAML payload. It's good enough to show the problem.

package main

import (
	"context"

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

func main() {

input := sts.AssumeRoleWithSAMLInput{
	PrincipalArn:    aws.String("test"),
	RoleArn:         aws.String("test"),
	SAMLAssertion:   aws.String("test"),
}

ctx := context.Background()

config, err := config.LoadDefaultConfig(ctx, config.WithRegion("us-east-1"))
if err != nil {
	panic(err)
}

svc := sts.NewFromConfig(config)

_, err = svc.AssumeRoleWithSAML(ctx, &input)
if err != nil {
	panic(err)
}

}

Possible Solution

No response

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

example.com/broken github.com/aws/[email protected]
example.com/broken github.com/aws/[email protected]
example.com/broken github.com/aws/aws-sdk-go-v2/[email protected]
example.com/broken github.com/aws/aws-sdk-go-v2/[email protected]
example.com/broken github.com/aws/aws-sdk-go-v2/feature/ec2/[email protected]
example.com/broken github.com/aws/aws-sdk-go-v2/internal/[email protected]
example.com/broken github.com/aws/aws-sdk-go-v2/internal/endpoints/[email protected]
example.com/broken github.com/aws/aws-sdk-go-v2/internal/[email protected]
example.com/broken github.com/aws/aws-sdk-go-v2/service/internal/[email protected]
example.com/broken github.com/aws/aws-sdk-go-v2/service/internal/[email protected]
example.com/broken github.com/aws/aws-sdk-go-v2/service/[email protected]
example.com/broken github.com/aws/aws-sdk-go-v2/service/[email protected]
example.com/broken github.com/aws/aws-sdk-go-v2/service/[email protected]
example.com/broken github.com/aws/[email protected]
github.com/aws/[email protected] github.com/jmespath/[email protected]
github.com/aws/[email protected] golang.org/x/[email protected]
github.com/aws/[email protected] golang.org/x/[email protected]
github.com/aws/[email protected] github.com/aws/[email protected]
github.com/aws/[email protected] github.com/google/[email protected]
github.com/aws/[email protected] github.com/jmespath/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/feature/ec2/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/internal/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/service/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/service/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/service/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/google/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/internal/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/internal/endpoints/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/service/internal/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/service/internal/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/feature/ec2/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/service/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/service/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/service/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/google/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/internal/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/internal/endpoints/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/service/internal/[email protected]
github.com/aws/aws-sdk-go-v2/[email protected] github.com/aws/aws-sdk-go-v2/service/internal/[email protected]
github.com/aws/aws-sdk-go-v2/feature/ec2/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/feature/ec2/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/feature/ec2/[email protected] github.com/google/[email protected]
github.com/aws/aws-sdk-go-v2/internal/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/internal/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/internal/endpoints/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/internal/endpoints/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/internal/endpoints/[email protected] github.com/google/[email protected]
github.com/aws/aws-sdk-go-v2/service/internal/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/service/internal/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/service/internal/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/service/internal/[email protected] github.com/google/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/aws/aws-sdk-go-v2/internal/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/aws/aws-sdk-go-v2/internal/endpoints/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/google/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/aws/aws-sdk-go-v2/internal/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/aws/aws-sdk-go-v2/internal/endpoints/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/google/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/aws/aws-sdk-go-v2/internal/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/aws/aws-sdk-go-v2/internal/endpoints/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/aws/aws-sdk-go-v2/service/internal/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/aws/aws-sdk-go-v2/service/internal/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/aws/[email protected]
github.com/aws/aws-sdk-go-v2/service/[email protected] github.com/google/[email protected]
github.com/aws/[email protected] github.com/google/[email protected]

Compiler and Version used

go version go1.21.3 darwin/arm64

Operating System and version

MacOS 14.1.1

@bitte-ein-bit bitte-ein-bit added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 28, 2023
bitte-ein-bit added a commit to allcloud-io/clisso that referenced this issue Nov 28, 2023
github.com/aws/aws-sdk-go-v2/service/sts v1.25.2 or later is broken; see aws/aws-sdk-go-v2#2392.
@lucix-aws
Copy link
Contributor

I think there may be a separate issue going on in the "current behavior" snippet - the metadata server (IMDS) that it's trying to contact doesn't have anything to do with STS itself - IMDS is the default credential provider we resolve to if we don't pick up on anything else from the environment (static creds, another configured provider, etc.). So, basically what this error means is that we didn't find any more explicit credentials in your environment, we fell back to using the IMDS credentials provider, and we tried to get credentials from it but failed, which as a process is all separate from the STS service client.

Concerning the "expected behavior" - I'm confused by the validation errors. I could be missing something but as far as I can tell, sts 1.25.1, 1.25.2, and the latest all have the same validators:

func validateOpAssumeRoleWithSAMLInput(v *AssumeRoleWithSAMLInput) error {
    if v == nil {
        return nil
    }
    invalidParams := smithy.InvalidParamsError{Context: "AssumeRoleWithSAMLInput"}
    if v.RoleArn == nil {
        invalidParams.Add(smithy.NewErrParamRequired("RoleArn"))
    }
    if v.PrincipalArn == nil {
        invalidParams.Add(smithy.NewErrParamRequired("PrincipalArn"))
    }
    if v.SAMLAssertion == nil {
        invalidParams.Add(smithy.NewErrParamRequired("SAMLAssertion"))
    }
    if invalidParams.Len() > 0 {
        return invalidParams
    } else {
        return nil
    }
}       

more specifically it's just asserting that these parameters exist. In all of the versions that I would think to check here (the broken one, one before broken, and current) I'm not seeing a length constraint imposed on roleArn or principalArn.

I'm also confused by the expected behavior being a validation failure, though - can you elaborate why that's the case?

@lucix-aws lucix-aws added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Nov 30, 2023
@RanVaknin RanVaknin removed the needs-triage This issue or PR still needs to be triaged. label Nov 30, 2023
@bitte-ein-bit
Copy link
Author

Hey @lucix-aws,

I'm sorry for leading you astray. I'll make sure to try to elaborate.

The original code takes a SAMLAssertion created by OneLogin as an IDP and "transforms" this payload into valid AWS credentials using the call to assumeRoleWithSAML. By the nature of the process, no valid AWS credentials are available when calling the AWS API.

The SAMLAssertion is short-lived and signed. For a fully working SSCCE, a valid assertion is needed, though. I've taken your feedback and overhauled the code to highlight the problem better.

package main

import (
	"context"
	"errors"
	"fmt"

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

func main() {

	input := sts.AssumeRoleWithSAMLInput{
		PrincipalArn:    aws.String("arn:aws:iam::123456789012:saml-provider/onelogin"),
		RoleArn:         aws.String("arn:aws:iam::123456789012:role/admins"),
		SAMLAssertion:   aws.String("invalid"),
	}

	ctx := context.Background()

	config, err := config.LoadDefaultConfig(ctx, config.WithRegion("us-east-1"))
	if err != nil {
		panic(err)
	}

	svc := sts.NewFromConfig(config)

	_, err = svc.AssumeRoleWithSAML(ctx, &input)

	// Since we have no accurate SAML assertion, we expect an error
	// Let's panic if we get another error than "InvalidIdentityToken"
	var ae smithy.APIError
	if errors.As(err, &ae) {
		if ae.ErrorCode() != "InvalidIdentityToken" {
			fmt.Println("Unexpected error: ", ae.ErrorCode())
			panic(err)
		}
	} else {
		// failed to convert error to smithy.APIError
		panic(err)
	}
	fmt.Println("Expected error: ", ae.ErrorCode())

}

Expected Behavior

The STS call AssumeRoleWithSAML does not require valid AWS credentials.

$ go get github.com/aws/aws-sdk-go-v2/service/[email protected]
go: downgraded github.com/aws/aws-sdk-go-v2/service/sts v1.25.2 => v1.25.1
$ go run main.go
Expected error:  InvalidIdentityToken

Actual Behavior

The STS call AssumeRoleWithSAML requires valid AWS credentials.

$ go get github.com/aws/aws-sdk-go-v2/service/[email protected]
go: upgraded github.com/aws/aws-sdk-go-v2/service/sts v1.25.1 => v1.25.2
$ go run main.go
panic: operation error STS: AssumeRoleWithSAML, get identity: get credentials: failed to refresh cached credentials, no EC2 IMDS role found, operation error ec2imds: GetMetadata, request canceled, context deadline exceeded

goroutine 1 [running]:
main.main()
	/Users/user/bug/main.go:44 +0x37c
exit status 2

go.mod / go.sum

go.sum
go.mod

@lucix-aws
Copy link
Contributor

That clears things up - thanks for clarifying.

So what you're doing is performing an operation anonymously, which is a valid path for AssumeRoleWithSAML, and with that knowledge the underlying issue becomes clear -- I should have checked for this, but the major difference between 1.25.1 and 1.25.2 was that the latter introduced the Smithy reference authentication refactor. That change fixed one aspect of this operation's behavior but broke another in the way you're experiencing.

To restore the expected behavior (technical explanation will follow): explicitly nil your credentials provider in the client like so:

	svc := sts.NewFromConfig(config, func(o *sts.Options) {
            o.Credentials = nil
	})

Can you try that and let me know if that works?

@bitte-ein-bit
Copy link
Author

@lucix-aws

Yes, explicitly nil the credentials provider restored the behavior.

If I understood correctly, it's a workaround and doesn't need to stay forever. Correct?

bitte-ein-bit added a commit to allcloud-io/clisso that referenced this issue Nov 30, 2023
@lucix-aws
Copy link
Contributor

Not necessarily, in order to understand why I have to provide the technical background here:

AWS operations are generally authenticated through sigv4 by default, which requires a set of credentials. Operations can however be specified as having "optional" auth - AssumeRoleWithSAML is an example of this. In practice "optional" auth means that a request can made without any signing/authentication/authorization header/etc.

How a client is supposed to choose which authentication method to use is to figure out the list of possible authentication methods for every operation called. Once it has that list, the SDK will iterate over each option (it's sorted by most preferred method first) and check to see whether the client is configured with the necessary credentials provider to make the request. It will pick the first scheme that it can find for which a credentials provider exists, with the exception of the anonymous option, which it will just pick immediately since there are no credentials necessary.

In practice this translates to the following for an operation like AssumeRoleWithSAML:

  1. you call the operation
  2. we "resolve" the list of options, which will be [sigv4, anonymous]
  3. first we see sigv4 - we check to see if the client has options.Credentials provider set. if it does, we choose to use sigv4 and go on to call Retrieve on your credentials provider and use that to sign the request. otherwise, move to the next step
  4. next we see anonymous, which we select instantly.

In practice all "optional"-auth operations are modeled this way - you CAN use credentials with the request, and we'll prefer that if a provider for them is set, otherwise we'll use anonymous auth.

There was previously a bug in the way we handled this though - if an operation was marked as having "anonymous" as an option, we selected that blindly. This means you couldn't sign through sigv4 if you had credentials and wanted to. A side effect of that bug was that you could implicitly use anonymous auth since it didn't matter if you had a broken credentials provider configured, we'd never try to use it anyway.

The upgrade to 1.25.2 included a huge authentication overhaul that implicitly fixed this bug.

There's a side effect though, which you've encountered: we will effectively always set a credentials provider through LoadDefaultConfig - because we will always default to IMDS at the end of the provider chain, since it's basically the fallback when we don't see anything else configured, unless you explicitly turn it off via AWS_EC2_METADATA_DISABLED=true. Since there's always been a credentials provider loaded by default this way, we'll get stuck on trying to use sigv4 (i.e. we'll always select it in step 3 above) and fail since the credentials provider call itself will fail. Setting credentials to nil explicitly tells the client (or on the operation itself with functional options) that you want to perform this operation anonymously (basically to fail "loading" sigv4 in step 3 and move on to 4).

So given that the behavior is now correct on spec I'm inclined to consider this a breaking fix. I've raised this nuance internally with some other SDK owners and we've agreed that the authentication method resolution here is what we've aligned on generally.

I will say that this issue has highlighted the need for better developer guide coverage in the authentication area. Our users shouldn't need to understand the internals of auth scheme resolution here to operate, we need to make clear the way in which you tap into anonymous auth where supported.

@lucix-aws lucix-aws removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Nov 30, 2023
bitte-ein-bit added a commit to allcloud-io/clisso that referenced this issue Dec 1, 2023
bitte-ein-bit added a commit to allcloud-io/clisso that referenced this issue Dec 1, 2023
bitte-ein-bit added a commit to allcloud-io/clisso that referenced this issue Dec 1, 2023
Copy link

github-actions bot commented Dec 1, 2023

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@lucix-aws
Copy link
Contributor

@bitte-ein-bit -- sorry for the churn on this -- we did some more investigation internally and it turns out that switching to attempt to use sigv4 on these two STS methods AssumeRoleWithSAML and AssumeRoleWithWebIdentity was not correct. The only authentication method supported by these operations is the anonymous/no-auth one (i.e. the option list like in step 2 of my description above is just [anonymous].

I've just merged #2410 to restore the correct behavior.

It's still fine to explicitly set options.Credentials to nil in your client constructor, but it's no longer strictly necessary.

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

No branches or pull requests

3 participants