From 5eea62b47a9e3ae499cc7bd3364556a70831592d Mon Sep 17 00:00:00 2001 From: Paul Glass Date: Wed, 13 Apr 2022 14:31:37 -0500 Subject: [PATCH] acl: Adjust region handling in AWS IAM auth method (#12774) * acl: Adjust region handling in AWS IAM auth method --- .changelog/12774.txt | 3 + agent/consul/authmethod/awsauth/aws.go | 4 - agent/consul/authmethod/awsauth/aws_test.go | 13 +-- internal/iamauth/config.go | 13 ++- internal/iamauth/token.go | 78 +++++++++++-- internal/iamauth/token_test.go | 119 ++++++++++++++++++++ internal/iamauth/util.go | 23 +--- 7 files changed, 212 insertions(+), 41 deletions(-) create mode 100644 .changelog/12774.txt diff --git a/.changelog/12774.txt b/.changelog/12774.txt new file mode 100644 index 000000000..d5aca735d --- /dev/null +++ b/.changelog/12774.txt @@ -0,0 +1,3 @@ +```release-note:improvement +acl: Improve handling of region-specific endpoints in the AWS IAM auth method. As part of this, the `STSRegion` field was removed from the auth method config. +``` diff --git a/agent/consul/authmethod/awsauth/aws.go b/agent/consul/authmethod/awsauth/aws.go index 32320e3f7..f3995cdc5 100644 --- a/agent/consul/authmethod/awsauth/aws.go +++ b/agent/consul/authmethod/awsauth/aws.go @@ -57,9 +57,6 @@ type Config struct { // STSEndpoint is the AWS STS endpoint where sts:GetCallerIdentity requests will be sent. // Note that the Host header in a signed request cannot be changed. STSEndpoint string `json:",omitempty"` - // STSRegion is the region for the AWS STS service. This should only be set if STSEndpoint - // is set, and must match the region of the STSEndpoint. - STSRegion string `json:",omitempty"` // AllowedSTSHeaderValues is a list of additional allowed headers on the sts:GetCallerIdentity // request in the bearer token. A default list of necessary headers is allowed in any case. @@ -75,7 +72,6 @@ func (c *Config) convertForLibrary() *iamauth.Config { MaxRetries: c.MaxRetries, IAMEndpoint: c.IAMEndpoint, STSEndpoint: c.STSEndpoint, - STSRegion: c.STSRegion, AllowedSTSHeaderValues: c.AllowedSTSHeaderValues, ServerIDHeaderName: IAMServerIDHeaderName, diff --git a/agent/consul/authmethod/awsauth/aws_test.go b/agent/consul/authmethod/awsauth/aws_test.go index 8ee507692..3025275cf 100644 --- a/agent/consul/authmethod/awsauth/aws_test.go +++ b/agent/consul/authmethod/awsauth/aws_test.go @@ -24,9 +24,8 @@ func TestNewValidator(t *testing.T) { IAMEntityTags: []string{"tag-1"}, ServerIDHeaderValue: "x-some-header", MaxRetries: 3, - IAMEndpoint: "iam-endpoint", - STSEndpoint: "sts-endpoint", - STSRegion: "sts-region", + IAMEndpoint: "http://iam-endpoint", + STSEndpoint: "http://sts-endpoint", AllowedSTSHeaderValues: []string{"header-value"}, ServerIDHeaderName: "X-Consul-IAM-ServerID", GetEntityMethodHeader: "X-Consul-IAM-GetEntity-Method", @@ -44,9 +43,8 @@ func TestNewValidator(t *testing.T) { "IAMEntityTags": []string{"tag-1"}, "ServerIDHeaderValue": "x-some-header", "MaxRetries": 3, - "IAMEndpoint": "iam-endpoint", - "STSEndpoint": "sts-endpoint", - "STSRegion": "sts-region", + "IAMEndpoint": "http://iam-endpoint", + "STSEndpoint": "http://sts-endpoint", "AllowedSTSHeaderValues": []string{"header-value"}, } @@ -224,7 +222,6 @@ func setup(t *testing.T, config map[string]interface{}, server *iamauthtest.Serv fakeAws := iamauthtest.NewTestServer(t, server) config["STSEndpoint"] = fakeAws.URL + "/sts" - config["STSRegion"] = "fake-region" config["IAMEndpoint"] = fakeAws.URL + "/iam" method := &structs.ACLAuthMethod{ @@ -241,7 +238,7 @@ func setup(t *testing.T, config map[string]interface{}, server *iamauthtest.Serv Creds: credentials.NewStaticCredentials("fake", "fake", ""), IncludeIAMEntity: v.config.EnableIAMEntityDetails, STSEndpoint: v.config.STSEndpoint, - STSRegion: v.config.STSRegion, + STSRegion: "fake-region", Logger: nullLogger, ServerIDHeaderValue: v.config.ServerIDHeaderValue, ServerIDHeaderName: v.config.ServerIDHeaderName, diff --git a/internal/iamauth/config.go b/internal/iamauth/config.go index a8a6b61d5..d3c722c55 100644 --- a/internal/iamauth/config.go +++ b/internal/iamauth/config.go @@ -15,7 +15,6 @@ type Config struct { MaxRetries int IAMEndpoint string STSEndpoint string - STSRegion string AllowedSTSHeaderValues []string // Customizable header names @@ -65,5 +64,17 @@ func (c *Config) Validate() error { "GetEntityHeadersHeader, and GetEntityBodyHeader when EnableIAMEntityDetails=true") } + if c.STSEndpoint != "" { + if _, err := parseUrl(c.STSEndpoint); err != nil { + return fmt.Errorf("STSEndpoint is invalid: %s", err) + } + } + + if c.IAMEndpoint != "" { + if _, err := parseUrl(c.IAMEndpoint); err != nil { + return fmt.Errorf("IAMEndpoint is invalid: %s", err) + } + } + return nil } diff --git a/internal/iamauth/token.go b/internal/iamauth/token.go index 91994b510..10422ca6c 100644 --- a/internal/iamauth/token.go +++ b/internal/iamauth/token.go @@ -13,9 +13,7 @@ import ( ) const ( - amzHeaderPrefix = "X-Amz-" - defaultIAMEndpoint = "https://iam.amazonaws.com" - defaultSTSEndpoint = "https://sts.amazonaws.com" + amzHeaderPrefix = "X-Amz-" ) var defaultAllowedSTSRequestHeaders = []string{ @@ -98,6 +96,10 @@ func NewBearerToken(loginToken string, config *Config) (*BearerToken, error) { token.getIAMEntityHeader = header token.parsedIAMEntityURL = parsedUrl + if err := token.validateIAMHostname(); err != nil { + return nil, err + } + reqType, err := token.validateIAMEntityBody() if err != nil { return nil, err @@ -112,6 +114,9 @@ func (t *BearerToken) validate() error { if t.getCallerIdentityMethod != "POST" { return fmt.Errorf("iam_http_request_method must be POST") } + if err := t.validateSTSHostname(); err != nil { + return err + } if err := t.validateGetCallerIdentityBody(); err != nil { return err } @@ -121,6 +126,62 @@ func (t *BearerToken) validate() error { return nil } +// validateSTSHostname checks the CallerIdentityURL in the BearerToken +// either matches the admin configured STSEndpoint or, if STSEndpoint is not set, +// that the URL matches a known Amazon AWS hostname for the STS service, one of: +// +// sts.amazonaws.com +// sts.*.amazonaws.com +// sts-fips.amazonaws.com +// sts-fips.*.amazonaws.com +// +// See https://docs.aws.amazon.com/general/latest/gr/sts.html +func (t *BearerToken) validateSTSHostname() error { + if t.config.STSEndpoint != "" { + // If an STS endpoint is configured, we (elsewhere) send the request to that endpoint. + return nil + } + if t.parsedCallerIdentityURL == nil { + return fmt.Errorf("invalid GetCallerIdentity URL: %v", t.getCallerIdentityURL) + } + + // Otherwise, validate the hostname looks like a known STS endpoint. + host := t.parsedCallerIdentityURL.Hostname() + if strings.HasSuffix(host, ".amazonaws.com") && + (strings.HasPrefix(host, "sts.") || strings.HasPrefix(host, "sts-fips.")) { + return nil + } + return fmt.Errorf("invalid STS hostname: %q", host) +} + +// validateIAMHostname checks the IAMEntityURL in the BearerToken +// either matches the admin configured IAMEndpoint or, if IAMEndpoint is not set, +// that the URL matches a known Amazon AWS hostname for the IAM service, one of: +// +// iam.amazonaws.com +// iam.*.amazonaws.com +// iam-fips.amazonaws.com +// iam-fips.*.amazonaws.com +// +// See https://docs.aws.amazon.com/general/latest/gr/iam-service.html +func (t *BearerToken) validateIAMHostname() error { + if t.config.IAMEndpoint != "" { + // If an IAM endpoint is configured, we (elsewhere) send the request to that endpoint. + return nil + } + if t.parsedIAMEntityURL == nil { + return fmt.Errorf("invalid IAM URL: %v", t.getIAMEntityURL) + } + + // Otherwise, validate the hostname looks like a known IAM endpoint. + host := t.parsedIAMEntityURL.Hostname() + if strings.HasSuffix(host, ".amazonaws.com") && + (strings.HasPrefix(host, "iam.") || strings.HasPrefix(host, "iam-fips.")) { + return nil + } + return fmt.Errorf("invalid IAM hostname: %q", host) +} + // https://github.com/hashicorp/vault/blob/b17e3256dde937a6248c9a2fa56206aac93d07de/builtin/credential/aws/path_login.go#L1439 func (t *BearerToken) validateGetCallerIdentityBody() error { allowedValues := url.Values{ @@ -265,7 +326,7 @@ func parseUrl(s string) (*url.URL, error) { return nil, err } // url.Parse doesn't error on empty string - if u == nil || u.Scheme == "" || u.Host == "" || u.Path == "" { + if u == nil || u.Scheme == "" || u.Host == "" { return nil, fmt.Errorf("url is invalid: %q", s) } return u, nil @@ -275,10 +336,9 @@ func parseUrl(s string) (*url.URL, error) { // from the bearer token. func (t *BearerToken) GetCallerIdentityRequest() (*http.Request, error) { // NOTE: We need to ensure we're calling STS, instead of acting as an unintended network proxy - // The protection against this is that this method will only call the endpoint specified in the - // client config (defaulting to sts.amazonaws.com), so it would require an admin to override - // the endpoint to talk to alternate web addresses - endpoint := defaultSTSEndpoint + // We validate up-front that t.getCallerIdentityURL is a known AWS STS hostname. + // Otherwise, we send to the admin-configured STSEndpoint. + endpoint := t.getCallerIdentityURL if t.config.STSEndpoint != "" { endpoint = t.config.STSEndpoint } @@ -295,7 +355,7 @@ func (t *BearerToken) GetCallerIdentityRequest() (*http.Request, error) { // GetEntityRequest returns the iam:GetUser or iam:GetRole request from the request details, // if present, embedded in the headers of the sts:GetCallerIdentity request. func (t *BearerToken) GetEntityRequest() (*http.Request, error) { - endpoint := defaultIAMEndpoint + endpoint := t.getIAMEntityURL if t.config.IAMEndpoint != "" { endpoint = t.config.IAMEndpoint } diff --git a/internal/iamauth/token_test.go b/internal/iamauth/token_test.go index 4de7ba715..42f81151d 100644 --- a/internal/iamauth/token_test.go +++ b/internal/iamauth/token_test.go @@ -27,6 +27,7 @@ func TestNewBearerToken(t *testing.T) { GetEntityURLHeader: "X-Consul-IAM-GetEntity-URL", GetEntityHeadersHeader: "X-Consul-IAM-GetEntity-Headers", GetEntityBodyHeader: "X-Consul-IAM-GetEntity-Body", + STSEndpoint: validBearerTokenParsed.getCallerIdentityURL, }, expToken: validBearerTokenWithRoleParsed, }, @@ -268,6 +269,124 @@ func TestValidateIAMEntityBody(t *testing.T) { } } +func TestValidateSTSHostname(t *testing.T) { + cases := []struct { + url string + ok bool + }{ + // https://docs.aws.amazon.com/general/latest/gr/sts.html + {"sts.us-east-2.amazonaws.com", true}, + {"sts-fips.us-east-2.amazonaws.com", true}, + {"sts.us-east-1.amazonaws.com", true}, + {"sts-fips.us-east-1.amazonaws.com", true}, + {"sts.us-west-1.amazonaws.com", true}, + {"sts-fips.us-west-1.amazonaws.com", true}, + {"sts.us-west-2.amazonaws.com", true}, + {"sts-fips.us-west-2.amazonaws.com", true}, + {"sts.af-south-1.amazonaws.com", true}, + {"sts.ap-east-1.amazonaws.com", true}, + {"sts.ap-southeast-3.amazonaws.com", true}, + {"sts.ap-south-1.amazonaws.com", true}, + {"sts.ap-northeast-3.amazonaws.com", true}, + {"sts.ap-northeast-2.amazonaws.com", true}, + {"sts.ap-southeast-1.amazonaws.com", true}, + {"sts.ap-southeast-2.amazonaws.com", true}, + {"sts.ap-northeast-1.amazonaws.com", true}, + {"sts.ca-central-1.amazonaws.com", true}, + {"sts.eu-central-1.amazonaws.com", true}, + {"sts.eu-west-1.amazonaws.com", true}, + {"sts.eu-west-2.amazonaws.com", true}, + {"sts.eu-south-1.amazonaws.com", true}, + {"sts.eu-west-3.amazonaws.com", true}, + {"sts.eu-north-1.amazonaws.com", true}, + {"sts.me-south-1.amazonaws.com", true}, + {"sts.sa-east-1.amazonaws.com", true}, + {"sts.us-gov-east-1.amazonaws.com", true}, + {"sts.us-gov-west-1.amazonaws.com", true}, + + // prefix must be either 'sts.' or 'sts-fips.' + {".amazonaws.com", false}, + {"iam.amazonaws.com", false}, + {"other.amazonaws.com", false}, + // suffix must be '.amazonaws.com' and not some other domain + {"stsamazonaws.com", false}, + {"sts-fipsamazonaws.com", false}, + {"sts.stsamazonaws.com", false}, + {"sts.notamazonaws.com", false}, + {"sts-fips.stsamazonaws.com", false}, + {"sts-fips.notamazonaws.com", false}, + {"sts.amazonaws.com.spoof", false}, + {"sts.amazonaws.spoof.com", false}, + {"xyz.sts.amazonaws.com", false}, + } + for _, c := range cases { + t.Run(c.url, func(t *testing.T) { + url := "https://" + c.url + parsedUrl, err := parseUrl(url) + require.NoError(t, err) + + token := &BearerToken{ + config: &Config{}, + getCallerIdentityURL: url, + parsedCallerIdentityURL: parsedUrl, + } + err = token.validateSTSHostname() + if c.ok { + require.NoError(t, err) + } else { + require.Error(t, err) + } + }) + } +} + +func TestValidateIAMHostname(t *testing.T) { + cases := []struct { + url string + ok bool + }{ + // https://docs.aws.amazon.com/general/latest/gr/iam-service.html + {"iam.amazonaws.com", true}, + {"iam-fips.amazonaws.com", true}, + {"iam.us-gov.amazonaws.com", true}, + {"iam-fips.us-gov.amazonaws.com", true}, + + // prefix must be either 'iam.' or 'aim-fips.' + {".amazonaws.com", false}, + {"sts.amazonaws.com", false}, + {"other.amazonaws.com", false}, + // suffix must be '.amazonaws.com' and not some other domain + {"iamamazonaws.com", false}, + {"iam-fipsamazonaws.com", false}, + {"iam.iamamazonaws.com", false}, + {"iam.notamazonaws.com", false}, + {"iam-fips.iamamazonaws.com", false}, + {"iam-fips.notamazonaws.com", false}, + {"iam.amazonaws.com.spoof", false}, + {"iam.amazonaws.spoof.com", false}, + {"xyz.iam.amazonaws.com", false}, + } + for _, c := range cases { + t.Run(c.url, func(t *testing.T) { + url := "https://" + c.url + parsedUrl, err := parseUrl(url) + require.NoError(t, err) + + token := &BearerToken{ + config: &Config{}, + getCallerIdentityURL: url, + parsedIAMEntityURL: parsedUrl, + } + err = token.validateIAMHostname() + if c.ok { + require.NoError(t, err) + } else { + require.Error(t, err) + } + }) + } +} + var ( validBearerTokenJson = `{ "iam_http_request_method":"POST", diff --git a/internal/iamauth/util.go b/internal/iamauth/util.go index bfd5f22d7..b92270cfd 100644 --- a/internal/iamauth/util.go +++ b/internal/iamauth/util.go @@ -39,12 +39,10 @@ type LoginInput struct { func GenerateLoginData(in *LoginInput) (map[string]interface{}, error) { cfg := aws.Config{ Credentials: in.Creds, - Region: aws.String(in.STSRegion), - } - if in.STSEndpoint != "" { - cfg.Endpoint = aws.String(in.STSEndpoint) - } else { - cfg.EndpointResolver = endpoints.ResolverFunc(stsSigningResolver) + // These are empty strings by default (i.e. not enabled) + Region: aws.String(in.STSRegion), + Endpoint: aws.String(in.STSEndpoint), + STSRegionalEndpoint: endpoints.RegionalSTSEndpoint, } stsSession, err := session.NewSessionWithOptions(session.Options{Config: cfg}) @@ -102,19 +100,6 @@ func GenerateLoginData(in *LoginInput) (map[string]interface{}, error) { }, nil } -// STS is a really weird service that used to only have global endpoints but now has regional endpoints as well. -// For backwards compatibility, even if you request a region other than us-east-1, it'll still sign for us-east-1. -// See, e.g., https://docs.aws.amazon.com/IAM/latest/UserGuide/id_credentials_temp_enable-regions.html#id_credentials_temp_enable-regions_writing_code -// So we have to shim in this EndpointResolver to force it to sign for the right region -func stsSigningResolver(service, region string, optFns ...func(*endpoints.Options)) (endpoints.ResolvedEndpoint, error) { - defaultEndpoint, err := endpoints.DefaultResolver().EndpointFor(service, region, optFns...) - if err != nil { - return defaultEndpoint, err - } - defaultEndpoint.SigningRegion = region - return defaultEndpoint, nil -} - func formatSignedEntityRequest(svc *sts.STS, in *LoginInput) (*request.Request, error) { // We need to retrieve the IAM user or role for the iam:GetRole or iam:GetUser request. // GetCallerIdentity returns this and requires no permissions.