diff --git a/internal/api/urls.go b/internal/api/urls.go index a239634a8..f6009bcf3 100644 --- a/internal/api/urls.go +++ b/internal/api/urls.go @@ -21,15 +21,7 @@ var ( ) func isImmutableHost(host string) bool { - knownHostNames := map[string]bool{ - "localhost": true, - "stella": true, - } - - // get rid of port - portlessHost := strings.Split(host, ":")[0] - - if knownHostNames[portlessHost] { + if IsKnownHostName(host) { return true } @@ -37,11 +29,33 @@ func isImmutableHost(host string) bool { if strings.HasPrefix(host, "[") { return true } + portlessHost := strings.Split(host, ":")[0] _, _, err := net.ParseCIDR(portlessHost + "/24") return err == nil } +func IsKnownHostName(host string) bool { + if strings.HasPrefix(host, "http") { + parsedUrl, err := url.Parse(host) + if err != nil { + return false + } + host = parsedUrl.Host + } + + knownHostNames := map[string]bool{ + "localhost": true, + "127.0.0.1": true, + "stella": true, + } + + // get rid of port + portlessHost := strings.Split(host, ":")[0] + + return knownHostNames[portlessHost] +} + func GetCanonicalApiUrlFromString(userDefinedUrl string) (string, error) { result := "" url, err := url.Parse(userDefinedUrl) diff --git a/internal/api/urls_test.go b/internal/api/urls_test.go index 5eafbcfb7..00d96ca07 100644 --- a/internal/api/urls_test.go +++ b/internal/api/urls_test.go @@ -100,3 +100,29 @@ func Test_isImmutableHost(t *testing.T) { assert.False(t, isImmutableHost(host), host) } } + +func Test_IsKnownHostName(t *testing.T) { + testCases := []struct { + host string + expected bool + }{ + {"localhost", true}, + {"localhost:8080", true}, + {"127.0.0.1", true}, + {"127.0.0.1:9000", true}, + {"stella", true}, + {"stella:8000", true}, + {"http://localhost", true}, + {"http://localhost:8080", true}, + {"https://127.0.0.1:9000", true}, + {"http://stella:8000", true}, + {"192.168.1.1", false}, + {"example.com", false}, + {"http://example.com", false}, + } + + for _, tc := range testCases { + actual := IsKnownHostName(tc.host) + assert.Equal(t, tc.expected, actual, "IsKnownHostName(%q) = %v, want %v", tc.host, actual, tc.expected) + } +} diff --git a/pkg/app/app.go b/pkg/app/app.go index f0986a700..106a065f7 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -2,6 +2,8 @@ package app import ( "crypto/fips140" + "errors" + "fmt" "io" "log" "net/http" @@ -137,6 +139,12 @@ func defaultFuncApiUrl(globalConfig configuration.Configuration, logger *zerolog if err != nil { logger.Warn().Err(err).Str(configuration.API_URL, urlString).Msg("failed to get api url") } + + if isValid, validationErr := auth.IsValidAuthHost(apiString, config.GetString(auth.CONFIG_KEY_ALLOWED_HOST_REGEXP)); !isValid || validationErr != nil { + hostNameErr := fmt.Errorf("host name is not snyk.io or snykgov.io") + logger.Err(hostNameErr).Msg("host name is not snyk.io or snykgov.io") + return nil, errors.Join(validationErr, hostNameErr) + } return apiString, nil } return callback diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 4749014ed..ad5ee67fa 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -115,7 +115,7 @@ func Test_CreateAppEngine_config_replaceV1inApi(t *testing.T) { config := engine.GetConfiguration() - expectApiUrl := "https://api.somehost:2134" + expectApiUrl := "https://api.snyk.io" config.Set(configuration.API_URL, expectApiUrl+"/v1") actualApiUrl := config.GetString(configuration.API_URL) @@ -148,22 +148,22 @@ func Test_EnsureAuthConfigurationPrecedence(t *testing.T) { name: "only user-defined API URL is defined, use that", patPayload: "", oauthJWTPayload: "", - userDefinedApiUrl: "https://api.user", - expectedURL: "https://api.user", + userDefinedApiUrl: "https://api.snyk.io", + expectedURL: "https://api.snyk.io", }, { name: "with a broken PAT configured and a user-defined API URL, user-defined API URL should take precedence", patPayload: `{broken`, oauthJWTPayload: "", - userDefinedApiUrl: "https://api.user", - expectedURL: "https://api.user", + expectedURL: "https://api.snyk.io", + userDefinedApiUrl: "https://api.snyk.io", }, { name: "with an empty PAT configured and a user-defined API URL, user-defined API URL should take precedence", patPayload: `{}`, oauthJWTPayload: "", - userDefinedApiUrl: "https://api.user", - expectedURL: "https://api.user", + expectedURL: "https://api.snyk.io", + userDefinedApiUrl: "https://api.snyk.io", }, { name: "with a PAT configured and a user-defined API URL, PAT host should take precedence", @@ -176,22 +176,22 @@ func Test_EnsureAuthConfigurationPrecedence(t *testing.T) { name: "with a broken OAuth with no host configured and a user-defined API URL, user-defined API URL should take precedence", patPayload: "", oauthJWTPayload: `{broken`, - userDefinedApiUrl: "https://api.user", - expectedURL: "https://api.user", + expectedURL: "https://api.snyk.io", + userDefinedApiUrl: "https://api.snyk.io", }, { name: "with OAuth with no host configured and a user-defined API URL, user-defined API URL should take precedence", patPayload: "", oauthJWTPayload: `{"sub":"1234567890","name":"John Doe","iat":1516239022,"aud":[]}`, - userDefinedApiUrl: "https://api.user", - expectedURL: "https://api.user", + expectedURL: "https://api.snyk.io", + userDefinedApiUrl: "https://api.snyk.io", }, { name: "with OAuth configured and a user-defined API URL, OAuth audience should take precedence", patPayload: "", - oauthJWTPayload: `{"sub":"1234567890","name":"John Doe","iat":1516239022,"aud":["https://api.oauth"]}`, - userDefinedApiUrl: "https://api.user", - expectedURL: "https://api.oauth", + oauthJWTPayload: `{"sub":"1234567890","name":"John Doe","iat":1516239022,"aud":["https://api.eu.snyk.io"]}`, + userDefinedApiUrl: "https://api.snyk.io", + expectedURL: "https://api.eu.snyk.io", }, { name: "with only PAT configured, use PAT host", @@ -203,9 +203,9 @@ func Test_EnsureAuthConfigurationPrecedence(t *testing.T) { { name: "with only OAuth configured, use OAuth audience", patPayload: "", - oauthJWTPayload: `{"sub":"1234567890","name":"John Doe","iat":1516239022,"aud":["https://api.oauth"]}`, + oauthJWTPayload: `{"sub":"1234567890","name":"John Doe","iat":1516239022,"aud":["https://api.snyk.io"]}`, userDefinedApiUrl: "", - expectedURL: "https://api.oauth", + expectedURL: "https://api.snyk.io", }, // This is not a likely scenario, as you cannot define both at the same time. However, it will potentially // catch regressions if this test starts to fail. @@ -307,13 +307,13 @@ func Test_CreateAppEngine_config_OauthAudHasPrecedence(t *testing.T) { config := configuration.New() config.Set(auth.CONFIG_KEY_OAUTH_TOKEN, // JWT generated at https://jwt.io with claim: - // "aud": ["https://api.example.com"] - `{"access_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJhdWQiOlsiaHR0cHM6Ly9hcGkuZXhhbXBsZS5jb20iXX0.hWq0fKukObQSkphAdyEC7-m4jXIb4VdWyQySmmgy0GU"}`, + // "aud": ["https://api.snyk.io"] + `{"access_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJhdWQiOlsiaHR0cHM6Ly9hcGkuc255ay5pbyJdfQ.vww25T4UtkxEzQzTysDI5zSi9XOYmXC5CXgxfp6mWtA"}`, ) logger := log.New(os.Stderr, "", 0) t.Run("Audience claim takes precedence of configured value", func(t *testing.T) { - expectedApiUrl := "https://api.example.com" + expectedApiUrl := "https://api.snyk.io" localConfig := config.Clone() localConfig.Set(configuration.API_URL, "https://api.dev.snyk.io") @@ -325,7 +325,7 @@ func Test_CreateAppEngine_config_OauthAudHasPrecedence(t *testing.T) { }) t.Run("nothing configured", func(t *testing.T) { - expectedApiUrl := "https://api.example.com" + expectedApiUrl := "https://api.snyk.io" localConfig := config.Clone() engine := CreateAppEngineWithOptions(WithConfiguration(localConfig), WithLogger(logger)) diff --git a/pkg/auth/authHost.go b/pkg/auth/authHost.go index 13fe5cd96..80a3dc43e 100644 --- a/pkg/auth/authHost.go +++ b/pkg/auth/authHost.go @@ -27,8 +27,12 @@ func redirectAuthHost(instance string) (string, error) { return canonicalizedInstanceUrl.Host, nil } -func IsValidAuthHost(instance string, redirectAuthHostRE string) (bool, error) { - isValidHost, err := utils.MatchesRegex(instance, redirectAuthHostRE) +func IsValidAuthHost(instance string, authHostRegex string) (bool, error) { + if api.IsKnownHostName(instance) { + return true, nil + } + + isValidHost, err := utils.MatchesRegex(instance, authHostRegex) if err != nil { return false, err } diff --git a/pkg/local_workflows/code_workflow_test.go b/pkg/local_workflows/code_workflow_test.go index 524e6d2c6..5e120fb4b 100644 --- a/pkg/local_workflows/code_workflow_test.go +++ b/pkg/local_workflows/code_workflow_test.go @@ -17,6 +17,8 @@ import ( "github.com/snyk/code-client-go/sarif" "github.com/snyk/code-client-go/scan" "github.com/snyk/error-catalog-golang-public/code" + "github.com/snyk/go-application-framework/internal/constants" + "github.com/snyk/go-application-framework/pkg/auth" "github.com/spf13/pflag" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -77,6 +79,7 @@ func Test_Code_entrypoint(t *testing.T) { config := configuration.NewWithOpts() config.Set(configuration.API_URL, server.URL) config.Set(configuration.ORGANIZATION, org) + config.AddDefaultValue(auth.CONFIG_KEY_ALLOWED_HOST_REGEXP, configuration.StandardDefaultValueFunction(constants.SNYK_DEFAULT_ALLOWED_HOST_REGEXP)) engine := workflow.NewWorkFlowEngine(config) diff --git a/pkg/local_workflows/network_utils/snyk_request_id_test.go b/pkg/local_workflows/network_utils/snyk_request_id_test.go index 06f814cee..efe54aed7 100644 --- a/pkg/local_workflows/network_utils/snyk_request_id_test.go +++ b/pkg/local_workflows/network_utils/snyk_request_id_test.go @@ -4,6 +4,7 @@ import ( "net/http" "testing" + "github.com/snyk/go-application-framework/internal/constants" "github.com/stretchr/testify/assert" "github.com/snyk/go-application-framework/pkg/configuration" @@ -14,6 +15,7 @@ func Test_AddRequestId(t *testing.T) { t.Run("Add missing snyk-request-id", func(t *testing.T) { config := configuration.NewInMemory() net := networking.NewNetworkAccess(config) + config.Set(configuration.API_URL, constants.SNYK_DEFAULT_API_URL) // use method under test AddSnykRequestId(net) @@ -31,6 +33,7 @@ func Test_AddRequestId(t *testing.T) { t.Run("Do not override snyk-request-id", func(t *testing.T) { config := configuration.NewInMemory() net := networking.NewNetworkAccess(config) + config.Set(configuration.API_URL, "https://api.snyk.io") expectedValue := "pre-existing-id" // use method under test diff --git a/pkg/networking/logging_test.go b/pkg/networking/logging_test.go index 7bbd4e96b..351a1c994 100644 --- a/pkg/networking/logging_test.go +++ b/pkg/networking/logging_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/rs/zerolog" + "github.com/snyk/go-application-framework/internal/constants" "github.com/stretchr/testify/assert" "github.com/snyk/go-application-framework/pkg/configuration" @@ -299,6 +300,7 @@ func Test_LogResponse_skipsBinaryContent(t *testing.T) { func Test_logRoundTrip(t *testing.T) { config := configuration.NewWithOpts() + config.Set(configuration.API_URL, constants.SNYK_DEFAULT_API_URL) expectedResponseBody := "hello client" expectedResponseBodyError := "who are you?" expectedRequestBody := "hello server" diff --git a/pkg/networking/middleware/auth_header.go b/pkg/networking/middleware/auth_header.go index 25c39cc4d..f84c95f81 100644 --- a/pkg/networking/middleware/auth_header.go +++ b/pkg/networking/middleware/auth_header.go @@ -45,6 +45,11 @@ func (n *AuthHeaderMiddleware) RoundTrip(request *http.Request) (*http.Response, return n.next.RoundTrip(newRequest) } +// ShouldRequireAuthentication checks if a request requires authentication. +// apiUrl is the configured API URL. +// url is the URL of the request. +// additionalSubdomains is a list of additional subdomains to check. +// additionalUrls is a list of additional URLs to check. func ShouldRequireAuthentication( apiUrl string, url *url.URL, @@ -98,11 +103,14 @@ func AddAuthenticationHeader( config configuration.Configuration, request *http.Request, ) error { - apiUrl := config.GetString(configuration.API_URL) + apiUrl, err := config.GetStringWithError(configuration.API_URL) + if err != nil { + return errors.Join(err, ErrAuthenticationFailed) + } additionalSubdomains := config.GetStringSlice(configuration.AUTHENTICATION_SUBDOMAINS) additionalUrls := config.GetStringSlice(configuration.AUTHENTICATION_ADDITIONAL_URLS) - isSnykApi, err := ShouldRequireAuthentication(apiUrl, request.URL, additionalSubdomains, additionalUrls) + isSnykApi, err := ShouldRequireAuthentication(apiUrl, request.URL, additionalSubdomains, additionalUrls) // requests to the api automatically get an authentication token attached if !isSnykApi { return err diff --git a/pkg/networking/middleware/auth_header_test.go b/pkg/networking/middleware/auth_header_test.go index 3c85b92d7..e8401f428 100644 --- a/pkg/networking/middleware/auth_header_test.go +++ b/pkg/networking/middleware/auth_header_test.go @@ -106,6 +106,43 @@ func Test_AddAuthenticationHeader(t *testing.T) { assert.NoError(t, err) } +func Test_isSnykHostname(t *testing.T) { + cases := []struct { + url string + isValid bool + }{ + // Valid hostnames + {"https://foobar.my.snyk.io", true}, + {"https://api.snyk.io", true}, + {"https://snyk.io", true}, + {"https://snykgov.io", true}, + {"https://api.snykgov.io", true}, + {"https://app.au.snyk.io", true}, + {"https://deeproxy.eu.snyk.io", true}, + {"https://deeproxy.snykgov.io", true}, + + // Invalid hostnames + {"https://api-snyk.io", false}, + {"https://api.staging-snyk.io", false}, + {"https://api.eu-snyk.io", false}, + {"https://example.com", false}, + {"https://snyk.io.evil.com", false}, + {"https://fakesnyk.io", false}, + {"https://notsnykgov.io", false}, + {"https://snykgov.io.attacker.com", false}, + } + + apiUrl := "https://api.snyk.io" + for _, tc := range cases { + t.Run(tc.url, func(t *testing.T) { + requestUrl, err := url.Parse(tc.url) + assert.NoError(t, err) + _, err = middleware.ShouldRequireAuthentication(apiUrl, requestUrl, []string{}, []string{}) + assert.NoError(t, err) + }) + } +} + func TestAuthenticationError_Is(t *testing.T) { ctrl := gomock.NewController(t) config := configuration.New() diff --git a/pkg/networking/networking_test.go b/pkg/networking/networking_test.go index 1970f3e7c..53ca7e0ba 100644 --- a/pkg/networking/networking_test.go +++ b/pkg/networking/networking_test.go @@ -450,14 +450,15 @@ func Test_UserAgentInfo_Complete(t *testing.T) { func TestNetworkImpl_Clone(t *testing.T) { config := configuration.NewWithOpts(configuration.WithAutomaticEnv()) + config.Set(configuration.API_URL, constants.SNYK_DEFAULT_API_URL) network := NewNetworkAccess(config) - config2 := configuration.NewWithOpts(configuration.WithAutomaticEnv()) + config2.Set(configuration.API_URL, constants.SNYK_DEFAULT_API_URL) config2.Set(configuration.AUTHENTICATION_TOKEN, "test") clonedNetwork := network.Clone() clonedNetwork.SetConfiguration(config2) - url1, err := url.Parse("") + url1, err := url.Parse("https://api.snyk.io") assert.NoError(t, err) req1 := &http.Request{ Header: http.Header{}, @@ -480,6 +481,7 @@ func TestNetworkImpl_Clone(t *testing.T) { func TestNetworkImpl_ErrorHandler(t *testing.T) { expectedErr := snyk.NewUnauthorisedError("no auth") config := configuration.NewWithOpts(configuration.WithAutomaticEnv()) + config.Set(configuration.API_URL, constants.SNYK_DEFAULT_API_URL) handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusUnauthorized)