acl: Adjust region handling in AWS IAM auth method (#12774)

* acl: Adjust region handling in AWS IAM auth method

.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.
+```

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,

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",
+		IAMEndpoint:            "http://iam-endpoint",
-		STSEndpoint:            "sts-endpoint",
+		STSEndpoint:            "http://sts-endpoint",
-		STSRegion:              "sts-region",
 		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",
+			"IAMEndpoint":            "http://iam-endpoint",
-			"STSEndpoint":            "sts-endpoint",
+			"STSEndpoint":            "http://sts-endpoint",
-			"STSRegion":              "sts-region",
 			"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,

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
 }

internal/iamauth/token.go

@@ -14,8 +14,6 @@ import (
 
 const (
 	amzHeaderPrefix = "X-Amz-"
-	defaultIAMEndpoint = "https://iam.amazonaws.com"
-	defaultSTSEndpoint = "https://sts.amazonaws.com"
 )
 
 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
+	// We validate up-front that t.getCallerIdentityURL is a known AWS STS hostname.
-	// client config (defaulting to sts.amazonaws.com), so it would require an admin to override
+	// Otherwise, we send to the admin-configured STSEndpoint.
-	// the endpoint to talk to alternate web addresses
+	endpoint := t.getCallerIdentityURL
-	endpoint := defaultSTSEndpoint
 	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
 	}

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",

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,
+		// These are empty strings by default (i.e. not enabled)
 		Region:              aws.String(in.STSRegion),
-	}
+		Endpoint:            aws.String(in.STSEndpoint),
-	if in.STSEndpoint != "" {
+		STSRegionalEndpoint: endpoints.RegionalSTSEndpoint,
-		cfg.Endpoint = aws.String(in.STSEndpoint)
-	} else {
-		cfg.EndpointResolver = endpoints.ResolverFunc(stsSigningResolver)
 	}
 
 	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.