diff --git a/pkg/authorization/authorizer/attributes_builder.go b/pkg/authorization/authorizer/attributes_builder.go index 25d70817fa6e..35e4181139ca 100644 --- a/pkg/authorization/authorizer/attributes_builder.go +++ b/pkg/authorization/authorizer/attributes_builder.go @@ -5,15 +5,14 @@ import ( "strings" kapi "k8s.io/kubernetes/pkg/api" - kapiserver "k8s.io/kubernetes/pkg/apiserver" ) type openshiftAuthorizationAttributeBuilder struct { contextMapper kapi.RequestContextMapper - infoResolver *kapiserver.RequestInfoResolver + infoResolver RequestInfoResolver } -func NewAuthorizationAttributeBuilder(contextMapper kapi.RequestContextMapper, infoResolver *kapiserver.RequestInfoResolver) AuthorizationAttributeBuilder { +func NewAuthorizationAttributeBuilder(contextMapper kapi.RequestContextMapper, infoResolver RequestInfoResolver) AuthorizationAttributeBuilder { return &openshiftAuthorizationAttributeBuilder{contextMapper, infoResolver} } diff --git a/pkg/authorization/authorizer/browser_safe_request_info_resolver.go b/pkg/authorization/authorizer/browser_safe_request_info_resolver.go new file mode 100644 index 000000000000..07487f68969b --- /dev/null +++ b/pkg/authorization/authorizer/browser_safe_request_info_resolver.go @@ -0,0 +1,75 @@ +package authorizer + +import ( + "net/http" + + kapi "k8s.io/kubernetes/pkg/api" + kapiserver "k8s.io/kubernetes/pkg/apiserver" + "k8s.io/kubernetes/pkg/util/sets" +) + +type browserSafeRequestInfoResolver struct { + // infoResolver is used to determine info for the request + infoResolver RequestInfoResolver + + // contextMapper is used to look up the context corresponding to a request + // to obtain the user associated with the request + contextMapper kapi.RequestContextMapper + + // list of groups, any of which indicate the request is authenticated + authenticatedGroups sets.String +} + +func NewBrowserSafeRequestInfoResolver(contextMapper kapi.RequestContextMapper, authenticatedGroups sets.String, infoResolver RequestInfoResolver) RequestInfoResolver { + return &browserSafeRequestInfoResolver{ + contextMapper: contextMapper, + authenticatedGroups: authenticatedGroups, + infoResolver: infoResolver, + } +} + +func (a *browserSafeRequestInfoResolver) GetRequestInfo(req *http.Request) (kapiserver.RequestInfo, error) { + requestInfo, err := a.infoResolver.GetRequestInfo(req) + if err != nil { + return requestInfo, err + } + + if !requestInfo.IsResourceRequest { + return requestInfo, nil + } + + isProxyVerb := requestInfo.Verb == "proxy" + isProxySubresource := requestInfo.Subresource == "proxy" + + if !isProxyVerb && !isProxySubresource { + // Requests to non-proxy resources don't expose HTML or HTTP-handling user content to browsers + return requestInfo, nil + } + + if len(req.Header.Get("X-CSRF-Token")) > 0 { + // Browsers cannot set custom headers on direct requests + return requestInfo, nil + } + + if ctx, hasContext := a.contextMapper.Get(req); hasContext { + user, hasUser := kapi.UserFrom(ctx) + if hasUser && a.authenticatedGroups.HasAny(user.GetGroups()...) { + // An authenticated request indicates this isn't a browser page load. + // Browsers cannot make direct authenticated requests. + // This depends on the API not enabling basic or cookie-based auth. + return requestInfo, nil + } + + } + + // TODO: compare request.Host to a list of hosts allowed for the requestInfo.Namespace (e.g. .proxy.example.com) + + if isProxyVerb { + requestInfo.Verb = "unsafeproxy" + } + if isProxySubresource { + requestInfo.Subresource = "unsafeproxy" + } + + return requestInfo, nil +} diff --git a/pkg/authorization/authorizer/browser_safe_request_info_resolver_test.go b/pkg/authorization/authorizer/browser_safe_request_info_resolver_test.go new file mode 100644 index 000000000000..0cbce736fb0d --- /dev/null +++ b/pkg/authorization/authorizer/browser_safe_request_info_resolver_test.go @@ -0,0 +1,147 @@ +package authorizer + +import ( + "net/http" + "testing" + + kapi "k8s.io/kubernetes/pkg/api" + kapiserver "k8s.io/kubernetes/pkg/apiserver" + "k8s.io/kubernetes/pkg/auth/user" + "k8s.io/kubernetes/pkg/util/sets" +) + +func TestUpstreamInfoResolver(t *testing.T) { + subresourceRequest, _ := http.NewRequest("GET", "/api/v1/namespaces/myns/pods/mypod/proxy", nil) + proxyRequest, _ := http.NewRequest("GET", "/api/v1/proxy/nodes/mynode", nil) + + testcases := map[string]struct { + Request *http.Request + ExpectedVerb string + ExpectedSubresource string + }{ + "unsafe proxy subresource": { + Request: subresourceRequest, + ExpectedVerb: "get", + ExpectedSubresource: "proxy", // should be "unsafeproxy" or similar once check moves upstream + }, + "unsafe proxy verb": { + Request: proxyRequest, + ExpectedVerb: "proxy", // should be "unsafeproxy" or similar once check moves upstream + }, + } + + for k, tc := range testcases { + resolver := &kapiserver.RequestInfoResolver{ + APIPrefixes: sets.NewString("api", "osapi", "oapi", "apis"), + GrouplessAPIPrefixes: sets.NewString("api", "osapi", "oapi"), + } + + info, err := resolver.GetRequestInfo(tc.Request) + if err != nil { + t.Errorf("%s: unexpected error: %v", k, err) + continue + } + + if info.Verb != tc.ExpectedVerb { + t.Errorf("%s: expected verb %s, got %s. If kapiserver.RequestInfoResolver now adjusts attributes for proxy safety, investigate removing the NewBrowserSafeRequestInfoResolver wrapper.", k, tc.ExpectedVerb, info.Verb) + } + if info.Subresource != tc.ExpectedSubresource { + t.Errorf("%s: expected verb %s, got %s. If kapiserver.RequestInfoResolver now adjusts attributes for proxy safety, investigate removing the NewBrowserSafeRequestInfoResolver wrapper.", k, tc.ExpectedSubresource, info.Subresource) + } + } +} + +func TestBrowserSafeRequestInfoResolver(t *testing.T) { + testcases := map[string]struct { + RequestInfo kapiserver.RequestInfo + Context kapi.Context + Host string + Headers http.Header + + ExpectedVerb string + ExpectedSubresource string + }{ + "non-resource": { + RequestInfo: kapiserver.RequestInfo{IsResourceRequest: false, Verb: "GET"}, + ExpectedVerb: "GET", + }, + + "non-proxy": { + RequestInfo: kapiserver.RequestInfo{IsResourceRequest: true, Verb: "get", Resource: "pods", Subresource: "logs"}, + ExpectedVerb: "get", + ExpectedSubresource: "logs", + }, + + "unsafe proxy subresource": { + RequestInfo: kapiserver.RequestInfo{IsResourceRequest: true, Verb: "get", Resource: "pods", Subresource: "proxy"}, + ExpectedVerb: "get", + ExpectedSubresource: "unsafeproxy", + }, + "unsafe proxy verb": { + RequestInfo: kapiserver.RequestInfo{IsResourceRequest: true, Verb: "proxy", Resource: "nodes"}, + ExpectedVerb: "unsafeproxy", + }, + "unsafe proxy verb anonymous": { + Context: kapi.WithUser(kapi.NewContext(), &user.DefaultInfo{Name: "system:anonymous", Groups: []string{"system:unauthenticated"}}), + RequestInfo: kapiserver.RequestInfo{IsResourceRequest: true, Verb: "proxy", Resource: "nodes"}, + ExpectedVerb: "unsafeproxy", + }, + + "proxy subresource authenticated": { + Context: kapi.WithUser(kapi.NewContext(), &user.DefaultInfo{Name: "bob", Groups: []string{"system:authenticated"}}), + RequestInfo: kapiserver.RequestInfo{IsResourceRequest: true, Verb: "get", Resource: "pods", Subresource: "proxy"}, + ExpectedVerb: "get", + ExpectedSubresource: "proxy", + }, + "proxy subresource custom header": { + RequestInfo: kapiserver.RequestInfo{IsResourceRequest: true, Verb: "get", Resource: "pods", Subresource: "proxy"}, + Headers: http.Header{"X-Csrf-Token": []string{"1"}}, + ExpectedVerb: "get", + ExpectedSubresource: "proxy", + }, + } + + for k, tc := range testcases { + resolver := NewBrowserSafeRequestInfoResolver( + &testContextMapper{tc.Context}, + sets.NewString("system:authenticated"), + &testInfoResolver{tc.RequestInfo}, + ) + + req, _ := http.NewRequest("GET", "/", nil) + req.Host = tc.Host + req.Header = tc.Headers + + info, err := resolver.GetRequestInfo(req) + if err != nil { + t.Errorf("%s: unexpected error: %v", k, err) + continue + } + + if info.Verb != tc.ExpectedVerb { + t.Errorf("%s: expected verb %s, got %s", k, tc.ExpectedVerb, info.Verb) + } + if info.Subresource != tc.ExpectedSubresource { + t.Errorf("%s: expected verb %s, got %s", k, tc.ExpectedSubresource, info.Subresource) + } + } +} + +type testContextMapper struct { + context kapi.Context +} + +func (t *testContextMapper) Get(req *http.Request) (kapi.Context, bool) { + return t.context, t.context != nil +} +func (t *testContextMapper) Update(req *http.Request, ctx kapi.Context) error { + return nil +} + +type testInfoResolver struct { + info kapiserver.RequestInfo +} + +func (t *testInfoResolver) GetRequestInfo(req *http.Request) (kapiserver.RequestInfo, error) { + return t.info, nil +} diff --git a/pkg/authorization/authorizer/interfaces.go b/pkg/authorization/authorizer/interfaces.go index 0c430749eb5d..81b64eb6197f 100644 --- a/pkg/authorization/authorizer/interfaces.go +++ b/pkg/authorization/authorizer/interfaces.go @@ -4,6 +4,7 @@ import ( "net/http" kapi "k8s.io/kubernetes/pkg/api" + kapiserver "k8s.io/kubernetes/pkg/apiserver" "k8s.io/kubernetes/pkg/auth/user" "k8s.io/kubernetes/pkg/util/sets" ) @@ -17,6 +18,10 @@ type AuthorizationAttributeBuilder interface { GetAttributes(request *http.Request) (AuthorizationAttributes, error) } +type RequestInfoResolver interface { + GetRequestInfo(req *http.Request) (kapiserver.RequestInfo, error) +} + type AuthorizationAttributes interface { GetVerb() string GetAPIVersion() string diff --git a/pkg/cmd/server/origin/master_config.go b/pkg/cmd/server/origin/master_config.go index 1e0b4fb3e0ca..61dbb1048f89 100644 --- a/pkg/cmd/server/origin/master_config.go +++ b/pkg/cmd/server/origin/master_config.go @@ -375,7 +375,16 @@ func newAuthorizer(policyClient policyclient.ReadOnlyPolicyClient, projectReques } func newAuthorizationAttributeBuilder(requestContextMapper kapi.RequestContextMapper) authorizer.AuthorizationAttributeBuilder { - authorizationAttributeBuilder := authorizer.NewAuthorizationAttributeBuilder(requestContextMapper, &apiserver.RequestInfoResolver{APIPrefixes: sets.NewString("api", "osapi", "oapi", "apis"), GrouplessAPIPrefixes: sets.NewString("api", "osapi", "oapi")}) + // Default API request resolver + requestInfoResolver := &apiserver.RequestInfoResolver{APIPrefixes: sets.NewString("api", "osapi", "oapi", "apis"), GrouplessAPIPrefixes: sets.NewString("api", "osapi", "oapi")} + // Wrap with a resolver that detects unsafe requests and modifies verbs/resources appropriately so policy can address them separately + browserSafeRequestInfoResolver := authorizer.NewBrowserSafeRequestInfoResolver( + requestContextMapper, + sets.NewString(bootstrappolicy.AuthenticatedGroup), + requestInfoResolver, + ) + + authorizationAttributeBuilder := authorizer.NewAuthorizationAttributeBuilder(requestContextMapper, browserSafeRequestInfoResolver) return authorizationAttributeBuilder }