uswitch / kiam

Integrate AWS IAM with Kubernetes
Apache License 2.0
1.15k stars 238 forks source link

kiam fails to resolve regional sts endpoint in us-iso-east-1 and us-isob-east-1 #410

Closed GraceAtwood closed 3 years ago

GraceAtwood commented 4 years ago

Hello!

We're deploying kiam in two of AWS's air gapped regions and we're finding that the sts endpoint fails to resolve.

In us-iso-east-1, we're falling to this statement: https://github.com/uswitch/kiam/blob/master/pkg/aws/sts/gateway.go#L73 And getting the error

Regional STS endpoint does not exist: sts.us-iso-east-1.amazonaws.com

After peeking around the code, I think we need special handling for these regions the same way china has special handling like here https://github.com/uswitch/kiam/blob/master/pkg/aws/sts/gateway.go

    if strings.HasPrefix(region, "cn-") {
        host = fmt.Sprintf("sts.%s.amazonaws.com.cn", region)
    } else if strings.HasPrefix(region, "us-isob") {
                host = fmt.Sprintf("sts.%s.sc2s.sgov.gov", region)
    } else if strings.HasPrefix(region, "us-iso") {
                host = fmt.Sprintf("sts.%s.c2s.ic.gov", region)
        } else {
        host = fmt.Sprintf("sts.%s.amazonaws.com", region)
    }

The Go AWS SDK handles these regions itself (endpoints.json) though so I wonder if there isn't some other issue? I'm not familiar enough with the code to know for sure so sorry if this is some larger issue. Thank you for any help you can provide!

GraceAtwood commented 4 years ago

If I'm on the right track with the changes above and there's not a larger change required, I'd love to submit the PR :D

pingles commented 4 years ago

I wonder if this is related to #368 and #350. If so, I'd love a PR to simplify the endpoints code so it addresses all the issues.

If I can get more time I'll try and take a look in an evening or at the weekend but in case someone else wants to pick up earlier, it'd be nice to solve all these issues together ahead of a v4.

pingles commented 4 years ago

@Xanneth it seems like not all endpoints are included in the endpoints.json you linked. Looking at the AWS Docs it lists a bunch of endpoints here: https://docs.aws.amazon.com/general/latest/gr/sts.html

Of note though, there's no sts.us-iso-east-1.amazonaws.com I can resolve and the error you see is because Kiam uses a DNS check to see if it's valid.

pingles commented 4 years ago

We could change it to be more flexible but not sure how it would work in practice if the name isn't resolvable? Is this a region that's only accessible to some accounts?

GraceAtwood commented 4 years ago

@pingles Thanks for the reply!

You won't be able to resolve sts.us-iso-east-1.amazonaws.com because that certainly doesn't exist. sts.us-iso-east-1.c2s.ic.gov, however, most certainly exists but you also won't be able to reach it on the open internet because it's in a classified region, airgapped from the rest of AWS. You can read more about it here if you want: https://aws.amazon.com/federal/us-intelligence-community/

Further, the endpoint won't be directly listed in endpoints.json (the Go SDK team moved their endpoints.json gen recently, but the .net team didn't so you can still refer to it directly from the .net sdk: endpoints.json. It's the same endpoints.json.) Instead of directly listing it, if no endpoint is declared for a service in a partition, then it'll fall back to the defaults for that partition:

Here you can see that there's no sts endpoint for us-iso-east-1: https://github.com/aws/aws-sdk-net/blob/master/sdk/src/Core/endpoints.json#L9377

Therefore, it'll fall back to the defaults provided for the partition: https://github.com/aws/aws-sdk-net/blob/master/sdk/src/Core/endpoints.json#L9053-L9070

So you can see the default pattern is "{service}.{region}.{dnsSuffix}" and just a few lines down you can see what those values will be for this partition. Therefore, that produces sts.us-iso-east-1.c2s.ic.gov.

That's why I recommended the inclusion of a line like this:

        else if strings.HasPrefix(region, "us-iso") {
                host = fmt.Sprintf("sts.%s.c2s.ic.gov", region)
        }
pingles commented 4 years ago

Perfect, thanks for that, I'll add that in and add a test.

ryphon commented 2 years ago

@pingles Hey, I hate to re-comment on this, but it appears this change actually broke us-isob. The SDK endpoints.json seems to include isob, but this change, I think, is forcing the isob installations to point at iso.

The endpoint should probably be sts.%s.sc2s.sgov.gov, but doing HasPrefix I think, is going to fail, since the isob region also has the prefix of us-iso.