From 0925f5e864bc3510fd869f8812e2930a406d97e8 Mon Sep 17 00:00:00 2001 From: Patrick Jones Date: Fri, 6 Aug 2021 12:56:36 -0700 Subject: [PATCH] google/externalaccount: validate tokenURL and ServiceAccountImpersonationURL --- google/internal/externalaccount/aws_test.go | 4 +- .../externalaccount/basecredentials.go | 93 +++++++++++++++---- .../externalaccount/basecredentials_test.go | 40 ++++++++ google/internal/externalaccount/clientauth.go | 3 +- .../externalaccount/clientauth_test.go | 3 +- .../internal/externalaccount/impersonate.go | 3 +- .../externalaccount/impersonate_test.go | 7 +- .../externalaccount/sts_exchange_test.go | 3 +- .../internal/externalaccount/urlcredsource.go | 3 +- 9 files changed, 135 insertions(+), 24 deletions(-) diff --git a/google/internal/externalaccount/aws_test.go b/google/internal/externalaccount/aws_test.go index 669ba1e..d5de415 100644 --- a/google/internal/externalaccount/aws_test.go +++ b/google/internal/externalaccount/aws_test.go @@ -650,7 +650,7 @@ func TestAwsCredential_BasicRequestWithDefaultEnv(t *testing.T) { getenv = setEnvironment(map[string]string{ "AWS_ACCESS_KEY_ID": "AKIDEXAMPLE", "AWS_SECRET_ACCESS_KEY": "wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY", - "AWS_DEFAULT_REGION": "us-west-1", + "AWS_DEFAULT_REGION": "us-west-1", }) base, err := tfc.parse(context.Background()) @@ -688,7 +688,7 @@ func TestAwsCredential_BasicRequestWithTwoRegions(t *testing.T) { "AWS_ACCESS_KEY_ID": "AKIDEXAMPLE", "AWS_SECRET_ACCESS_KEY": "wJalrXUtnFEMI/K7MDENG+bPxRfiCYEXAMPLEKEY", "AWS_REGION": "us-west-1", - "AWS_DEFAULT_REGION": "us-east-1", + "AWS_DEFAULT_REGION": "us-east-1", }) base, err := tfc.parse(context.Background()) diff --git a/google/internal/externalaccount/basecredentials.go b/google/internal/externalaccount/basecredentials.go index a4d45d9..6fedabc 100644 --- a/google/internal/externalaccount/basecredentials.go +++ b/google/internal/externalaccount/basecredentials.go @@ -7,10 +7,12 @@ package externalaccount import ( "context" "fmt" - "golang.org/x/oauth2" "net/http" + "regexp" "strconv" "time" + + "golang.org/x/oauth2" ) // now aliases time.Now for testing @@ -22,43 +24,102 @@ var now = func() time.Time { type Config struct { // Audience is the Secure Token Service (STS) audience which contains the resource name for the workload // identity pool or the workforce pool and the provider identifier in that pool. - Audience string + Audience string // SubjectTokenType is the STS token type based on the Oauth2.0 token exchange spec // e.g. `urn:ietf:params:oauth:token-type:jwt`. - SubjectTokenType string + SubjectTokenType string // TokenURL is the STS token exchange endpoint. - TokenURL string + TokenURL string // TokenInfoURL is the token_info endpoint used to retrieve the account related information ( // user attributes like account identifier, eg. email, username, uid, etc). This is // needed for gCloud session account identification. - TokenInfoURL string + TokenInfoURL string // ServiceAccountImpersonationURL is the URL for the service account impersonation request. This is only // required for workload identity pools when APIs to be accessed have not integrated with UberMint. ServiceAccountImpersonationURL string // ClientSecret is currently only required if token_info endpoint also // needs to be called with the generated GCP access token. When provided, STS will be // called with additional basic authentication using client_id as username and client_secret as password. - ClientSecret string + ClientSecret string // ClientID is only required in conjunction with ClientSecret, as described above. - ClientID string + ClientID string // CredentialSource contains the necessary information to retrieve the token itself, as well // as some environmental information. - CredentialSource CredentialSource + CredentialSource CredentialSource // QuotaProjectID is injected by gCloud. If the value is non-empty, the Auth libraries // will set the x-goog-user-project which overrides the project associated with the credentials. - QuotaProjectID string + QuotaProjectID string // Scopes contains the desired scopes for the returned access token. - Scopes []string + Scopes []string +} + +// Each element consists of a list of patterns. validateURLs checks for matches +// that include all elements in a given list, in that order. +var ( + validTokenURLPatterns = []string{ + "https://[^\\.]+\\.sts\\.googleapis\\.com", + "https://sts\\.googleapis\\.com", + "https://sts\\.[^\\.]+\\.googleapis\\.com", + "https://[^\\.]+-sts\\.googleapis\\.com", + } + validImpersonateURLPatterns = []string{ + "https://[^\\.]+\\.iamcredentials\\.googleapis\\.com", + "https://iamcredentials\\.googleapis\\.com", + "https://iamcredentials\\.[^\\.]+\\.googleapis\\.com", + "https://[^\\.]+-iamcredentials\\.googleapis\\.com", + } +) + +func validateURL(input string, patterns []string) (bool, error) { + for _, pattern := range patterns { + valid, err := regexp.MatchString(pattern, input) + if err != nil { + return false, err + } + if valid { + return true, nil + } + } + return false, nil } // TokenSource Returns an external account TokenSource struct. This is to be called by package google to construct a google.Credentials. -func (c *Config) TokenSource(ctx context.Context) oauth2.TokenSource { +func (c *Config) TokenSource(ctx context.Context) (oauth2.TokenSource, error) { + return c.tokenSource(ctx, false) +} + +// tokenSource is a private function that's directly called by some of the tests, +// because the unit test URLs are mocked, and would otherwise fail the +// validity check. +func (c *Config) tokenSource(ctx context.Context, testing bool) (oauth2.TokenSource, error) { + if !testing { + // Check the validity of TokenURL. + valid, err := validateURL(c.TokenURL, validTokenURLPatterns) + if err != nil { + return nil, err + } + if !valid { + return nil, fmt.Errorf("oauth2/google: invalid TokenURL provided while constructing tokenSource") + } + + // If ServiceAccountImpersonationURL is present, check its validity. + if c.ServiceAccountImpersonationURL != "" { + valid, err := validateURL(c.ServiceAccountImpersonationURL, validImpersonateURLPatterns) + if err != nil { + return nil, err + } + if !valid { + return nil, fmt.Errorf("oauth2/google: invalid ServiceAccountImpersonationURL provided while constructing tokenSource") + } + } + } + ts := tokenSource{ ctx: ctx, conf: c, } if c.ServiceAccountImpersonationURL == "" { - return oauth2.ReuseTokenSource(nil, ts) + return oauth2.ReuseTokenSource(nil, ts), nil } scopes := c.Scopes ts.conf.Scopes = []string{"https://www.googleapis.com/auth/cloud-platform"} @@ -68,7 +129,7 @@ func (c *Config) TokenSource(ctx context.Context) oauth2.TokenSource { scopes: scopes, ts: oauth2.ReuseTokenSource(nil, ts), } - return oauth2.ReuseTokenSource(nil, imp) + return oauth2.ReuseTokenSource(nil, imp), nil } // Subject token file types. @@ -78,9 +139,9 @@ const ( ) type format struct { - // Type is either "text" or "json". When not provided "text" type is assumed. + // Type is either "text" or "json". When not provided "text" type is assumed. Type string `json:"type"` - // SubjectTokenFieldName is only required for JSON format. This would be "access_token" for azure. + // SubjectTokenFieldName is only required for JSON format. This would be "access_token" for azure. SubjectTokenFieldName string `json:"subject_token_field_name"` } @@ -128,7 +189,7 @@ type baseCredentialSource interface { subjectToken() (string, error) } -// tokenSource is the source that handles external credentials. It is used to retrieve Tokens. +// tokenSource is the source that handles external credentials. It is used to retrieve Tokens. type tokenSource struct { ctx context.Context conf *Config diff --git a/google/internal/externalaccount/basecredentials_test.go b/google/internal/externalaccount/basecredentials_test.go index 1ebb227..20923a7 100644 --- a/google/internal/externalaccount/basecredentials_test.go +++ b/google/internal/externalaccount/basecredentials_test.go @@ -95,3 +95,43 @@ func TestToken(t *testing.T) { } } + +func TestValidateURL(t *testing.T) { + var urlValidityTests = []struct { + input string + pattern []string + result bool + }{ + {"https://sts.googleapis.com", validTokenURLPatterns, true}, + {"https://.sts.google.com", validTokenURLPatterns, false}, + {"https://badsts.googleapis.com", validTokenURLPatterns, false}, + {"https://sts.asfeasfesef.googleapis.com", validTokenURLPatterns, true}, + {"https://sts.asfe.asfesef.googleapis.com", validTokenURLPatterns, false}, + {"https://sts..googleapis.com", validTokenURLPatterns, false}, + {"https://-sts.googleapis.com", validTokenURLPatterns, false}, + {"https://us-east-1-sts.googleapis.com", validTokenURLPatterns, true}, + {"https://us-ea.st-1-sts.googleapis.com", validTokenURLPatterns, false}, + // Repeat for iamcredentials as well + {"https://iamcredentials.googleapis.com", validImpersonateURLPatterns, true}, + {"https://.iamcredentials.googleapis.com", validImpersonateURLPatterns, false}, + {"https://badiamcredentials.googleapis.com", validImpersonateURLPatterns, false}, + {"https://iamcredentials.asfeasfesef.googleapis.com", validImpersonateURLPatterns, true}, + {"https://iamcredentials.asfe.asfesef.googleapis.com", validImpersonateURLPatterns, false}, + {"https://iamcredentials..googleapis.com", validImpersonateURLPatterns, false}, + {"https://-iamcredentials.googleapis.com", validImpersonateURLPatterns, false}, + {"https://us-east-1-iamcredentials.googleapis.com", validImpersonateURLPatterns, true}, + {"https://us-ea.st-1-iamcredentials.googleapis.com", validImpersonateURLPatterns, false}, + } + for _, tt := range urlValidityTests { + t.Run(" "+tt.input, func(t *testing.T) { // We prepend a space ahead of the test input when outputting for sake of readability. + valid, err := validateURL(tt.input, tt.pattern) + if err != nil { + t.Errorf("validateURL returned an error: %v", err) + return + } + if valid != tt.result { + t.Errorf("got %v, want %v", valid, tt.result) + } + }) + } +} diff --git a/google/internal/externalaccount/clientauth.go b/google/internal/externalaccount/clientauth.go index 62c2e36..99987ce 100644 --- a/google/internal/externalaccount/clientauth.go +++ b/google/internal/externalaccount/clientauth.go @@ -6,9 +6,10 @@ package externalaccount import ( "encoding/base64" - "golang.org/x/oauth2" "net/http" "net/url" + + "golang.org/x/oauth2" ) // clientAuthentication represents an OAuth client ID and secret and the mechanism for passing these credentials as stated in rfc6749#2.3.1. diff --git a/google/internal/externalaccount/clientauth_test.go b/google/internal/externalaccount/clientauth_test.go index 38633e3..bfb339d 100644 --- a/google/internal/externalaccount/clientauth_test.go +++ b/google/internal/externalaccount/clientauth_test.go @@ -5,11 +5,12 @@ package externalaccount import ( - "golang.org/x/oauth2" "net/http" "net/url" "reflect" "testing" + + "golang.org/x/oauth2" ) var clientID = "rbrgnognrhongo3bi4gb9ghg9g" diff --git a/google/internal/externalaccount/impersonate.go b/google/internal/externalaccount/impersonate.go index 1f6009b..64edb56 100644 --- a/google/internal/externalaccount/impersonate.go +++ b/google/internal/externalaccount/impersonate.go @@ -9,11 +9,12 @@ import ( "context" "encoding/json" "fmt" - "golang.org/x/oauth2" "io" "io/ioutil" "net/http" "time" + + "golang.org/x/oauth2" ) // generateAccesstokenReq is used for service account impersonation diff --git a/google/internal/externalaccount/impersonate_test.go b/google/internal/externalaccount/impersonate_test.go index 197fe3c..acf08d2 100644 --- a/google/internal/externalaccount/impersonate_test.go +++ b/google/internal/externalaccount/impersonate_test.go @@ -6,6 +6,7 @@ package externalaccount import ( "context" + "fmt" "io/ioutil" "net/http" "net/http/httptest" @@ -76,7 +77,11 @@ func TestImpersonation(t *testing.T) { defer targetServer.Close() testImpersonateConfig.TokenURL = targetServer.URL - ourTS := testImpersonateConfig.TokenSource(context.Background()) + ourTS, err := testImpersonateConfig.tokenSource(context.Background(), true) + if err != nil { + fmt.Println(testImpersonateConfig.TokenURL) + t.Fatalf("Failed to create TokenSource: %v", err) + } oldNow := now defer func() { now = oldNow }() diff --git a/google/internal/externalaccount/sts_exchange_test.go b/google/internal/externalaccount/sts_exchange_test.go index 3d498c6..d7507a4 100644 --- a/google/internal/externalaccount/sts_exchange_test.go +++ b/google/internal/externalaccount/sts_exchange_test.go @@ -7,12 +7,13 @@ package externalaccount import ( "context" "encoding/json" - "golang.org/x/oauth2" "io/ioutil" "net/http" "net/http/httptest" "net/url" "testing" + + "golang.org/x/oauth2" ) var auth = clientAuthentication{ diff --git a/google/internal/externalaccount/urlcredsource.go b/google/internal/externalaccount/urlcredsource.go index 91b8f20..16dca65 100644 --- a/google/internal/externalaccount/urlcredsource.go +++ b/google/internal/externalaccount/urlcredsource.go @@ -9,10 +9,11 @@ import ( "encoding/json" "errors" "fmt" - "golang.org/x/oauth2" "io" "io/ioutil" "net/http" + + "golang.org/x/oauth2" ) type urlCredentialSource struct {