diff --git a/staging/src/k8s.io/apiserver/pkg/server/filters/cors_test.go b/staging/src/k8s.io/apiserver/pkg/server/filters/cors_test.go index afab083a883..cf47aad246e 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/filters/cors_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/filters/cors_test.go @@ -68,6 +68,25 @@ func TestCORSAllowedOrigins(t *testing.T) { origins: []string{"http://example.com", "example.com"}, allowed: true, }, + { + // CVE-2022-1996: regular expression 'example.com' matches + // 'example.com.hacker.org' because it does not pin to the start + // and end of the host. + // The CVE should not occur in a real kubernetes cluster since we + // validate the regular expression specified in --cors-allowed-origins + // to ensure that it pins to the start and end of the host name. + name: "regex does not pin, CVE-2022-1996 is not prevented", + allowedOrigins: []string{"example.com"}, + origins: []string{"http://example.com.hacker.org", "http://example.com.hacker.org:8080"}, + allowed: true, + }, + { + // with a proper regular expression we can prevent CVE-2022-1996 + name: "regex pins to start/end of the host name, CVE-2022-1996 is prevented", + allowedOrigins: []string{`//example.com(:|$)`}, + origins: []string{"http://example.com.hacker.org", "http://example.com.hacker.org:8080"}, + allowed: false, + }, } for _, test := range tests { diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/server_run_options.go b/staging/src/k8s.io/apiserver/pkg/server/options/server_run_options.go index 44a67345c2a..bd67ae5dba7 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/server_run_options.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/server_run_options.go @@ -19,6 +19,7 @@ package options import ( "fmt" "net" + "regexp" "strings" "time" @@ -30,6 +31,16 @@ import ( "github.com/spf13/pflag" ) +const ( + corsAllowedOriginsHelpText = "List of allowed origins for CORS, comma separated. " + + "An allowed origin can be a regular expression to support subdomain matching. " + + "If this list is empty CORS will not be enabled. " + + "Please ensure each expression matches the entire hostname by anchoring " + + "to the start with '^' or including the '//' prefix, and by anchoring to the " + + "end with '$' or including the ':' port separator suffix. " + + "Examples of valid expressions are '//example\\.com(:|$)' and '^https://example\\.com(:|$)'" +) + // ServerRunOptions contains the options while running a generic api server. type ServerRunOptions struct { AdvertiseAddress net.IP @@ -160,6 +171,10 @@ func (s *ServerRunOptions) Validate() []error { if err := validateHSTSDirectives(s.HSTSDirectives); err != nil { errors = append(errors, err) } + + if err := validateCorsAllowedOriginList(s.CorsAllowedOriginList); err != nil { + errors = append(errors, err) + } return errors } @@ -182,6 +197,57 @@ func validateHSTSDirectives(hstsDirectives []string) error { return errors.NewAggregate(allErrors) } +func validateCorsAllowedOriginList(corsAllowedOriginList []string) error { + allErrors := []error{} + validateRegexFn := func(regexpStr string) error { + if _, err := regexp.Compile(regexpStr); err != nil { + return err + } + + // the regular expression should pin to the start and end of the host + // in the origin header, this will prevent CVE-2022-1996. + // possible ways it can pin to the start of host in the origin header: + // - match the start of the origin with '^' + // - match what separates the scheme and host with '//' or '://', + // this pins to the start of host in the origin header. + // possible ways it can match the end of the host in the origin header: + // - match the end of the origin with '$' + // - with a capture group that matches the host and port separator '(:|$)' + // We will relax the validation to check if these regex markers + // are present in the user specified expression. + var pinStart, pinEnd bool + for _, prefix := range []string{"^", "//"} { + if strings.Contains(regexpStr, prefix) { + pinStart = true + break + } + } + for _, suffix := range []string{"$", ":"} { + if strings.Contains(regexpStr, suffix) { + pinEnd = true + break + } + } + if !pinStart || !pinEnd { + return fmt.Errorf("regular expression does not pin to start/end of host in the origin header") + } + return nil + } + + for _, regexp := range corsAllowedOriginList { + if len(regexp) == 0 { + allErrors = append(allErrors, fmt.Errorf("empty value in --cors-allowed-origins, help: %s", corsAllowedOriginsHelpText)) + continue + } + + if err := validateRegexFn(regexp); err != nil { + err = fmt.Errorf("--cors-allowed-origins has an invalid regular expression: %v, help: %s", err, corsAllowedOriginsHelpText) + allErrors = append(allErrors, err) + } + } + return errors.NewAggregate(allErrors) +} + // AddUniversalFlags adds flags for a specific APIServer to the specified FlagSet func (s *ServerRunOptions) AddUniversalFlags(fs *pflag.FlagSet) { // Note: the weird ""+ in below lines seems to be the only way to get gofmt to @@ -193,9 +259,7 @@ func (s *ServerRunOptions) AddUniversalFlags(fs *pflag.FlagSet) { "will be used. If --bind-address is unspecified, the host's default interface will "+ "be used.") - fs.StringSliceVar(&s.CorsAllowedOriginList, "cors-allowed-origins", s.CorsAllowedOriginList, ""+ - "List of allowed origins for CORS, comma separated. An allowed origin can be a regular "+ - "expression to support subdomain matching. If this list is empty CORS will not be enabled.") + fs.StringSliceVar(&s.CorsAllowedOriginList, "cors-allowed-origins", s.CorsAllowedOriginList, corsAllowedOriginsHelpText) fs.StringSliceVar(&s.HSTSDirectives, "strict-transport-security-directives", s.HSTSDirectives, ""+ "List of directives for HSTS, comma separated. If this list is empty, then HSTS directives will not "+ diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/server_run_options_test.go b/staging/src/k8s.io/apiserver/pkg/server/options/server_run_options_test.go index 1dd103a15e1..3dbd1c3fb6f 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/server_run_options_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/server_run_options_test.go @@ -17,6 +17,7 @@ limitations under the License. package options import ( + "fmt" "strings" "testing" "time" @@ -164,7 +165,7 @@ func TestServerRunOptionsValidate(t *testing.T) { name: "Test when ServerRunOptions is valid", testOptions: &ServerRunOptions{ AdvertiseAddress: netutils.ParseIPSloppy("192.168.10.10"), - CorsAllowedOriginList: []string{"10.10.10.100", "10.10.10.200"}, + CorsAllowedOriginList: []string{"^10.10.10.100$", "^10.10.10.200$"}, HSTSDirectives: []string{"max-age=31536000", "includeSubDomains", "preload"}, MaxRequestsInFlight: 400, MaxMutatingRequestsInFlight: 200, @@ -189,3 +190,74 @@ func TestServerRunOptionsValidate(t *testing.T) { }) } } + +func TestValidateCorsAllowedOriginList(t *testing.T) { + tests := []struct { + regexp [][]string + errShouldContain string + }{ + { + regexp: [][]string{ + {}, // empty list, the cluster operator wants to disable CORS + {`^http://foo.com$`}, + {`^http://foo.com`}, // valid, because we relaxed the validation + {`://foo.com$`}, + {`//foo.com$`}, + {`^http://foo.com(:|$)`}, + {`://foo.com(:|$)`}, + {`//foo.com(:|$)`}, + {`(^foo.com$)`}, + {`^http://foo.com$`, `//bar.com(:|$)`}, + }, + errShouldContain: "", + }, + { + // empty string, indicates that the cluster operator + // specified --cors-allowed-origins="" + regexp: [][]string{ + {`^http://foo.com$`, ``}, + }, + errShouldContain: "empty value in --cors-allowed-origins", + }, + { + regexp: [][]string{ + {`^foo.com`}, + {`//foo.com`}, + {`foo.com$`}, + {`foo.com(:|$)`}, + }, + errShouldContain: "regular expression does not pin to start/end of host in the origin header", + }, + { + regexp: [][]string{ + {`^http://foo.com$`, `^foo.com`}, // one good followed by a bad one + }, + errShouldContain: "regular expression does not pin to start/end of host in the origin header", + }, + } + + for _, test := range tests { + for _, regexp := range test.regexp { + t.Run(fmt.Sprintf("regexp/%s", regexp), func(t *testing.T) { + options := NewServerRunOptions() + if errs := options.Validate(); len(errs) != 0 { + t.Fatalf("wrong test setup: %#v", errs) + } + + options.CorsAllowedOriginList = regexp + errsGot := options.Validate() + switch { + case len(test.errShouldContain) == 0: + if len(errsGot) != 0 { + t.Errorf("expected no error, but got: %v", errsGot) + } + default: + if len(errsGot) == 0 || + !strings.Contains(utilerrors.NewAggregate(errsGot).Error(), test.errShouldContain) { + t.Errorf("expected error to contain: %s, but got: %v", test.errShouldContain, errsGot) + } + } + }) + } + } +}