From 900791d3ac2c7a6da58ad2fb2e58fac46b5009dc Mon Sep 17 00:00:00 2001 From: jennybuckley Date: Tue, 3 Jul 2018 11:20:16 -0700 Subject: [PATCH] Add additional authorization check for create-on-update Kubernetes-commit: cc5c17e554a4d8f802043b337ca0787ec0ce7475 --- pkg/endpoints/groupversion.go | 6 ++++ pkg/endpoints/handlers/rest.go | 2 ++ pkg/endpoints/handlers/update.go | 49 +++++++++++++++++++++++++++++++- pkg/endpoints/installer.go | 1 + pkg/server/config.go | 1 + pkg/server/genericapiserver.go | 7 +++++ 6 files changed, 65 insertions(+), 1 deletion(-) diff --git a/pkg/endpoints/groupversion.go b/pkg/endpoints/groupversion.go index 23d13adc3..695c62b59 100644 --- a/pkg/endpoints/groupversion.go +++ b/pkg/endpoints/groupversion.go @@ -28,6 +28,7 @@ import ( utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/endpoints/discovery" "k8s.io/apiserver/pkg/registry/rest" openapicommon "k8s.io/kube-openapi/pkg/common" @@ -71,6 +72,11 @@ type APIGroupVersion struct { Linker runtime.SelfLinker UnsafeConvertor runtime.ObjectConvertor + // Authorizer determines whether a user is allowed to make a certain request. The Handler does a preliminary + // authorization check using the request URI but it may be necessary to make additional checks, such as in + // the create-on-update case + Authorizer authorizer.Authorizer + Admit admission.Interface MinRequestTimeout time.Duration diff --git a/pkg/endpoints/handlers/rest.go b/pkg/endpoints/handlers/rest.go index d42c0194d..a941f6fba 100644 --- a/pkg/endpoints/handlers/rest.go +++ b/pkg/endpoints/handlers/rest.go @@ -35,6 +35,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/endpoints/handlers/responsewriters" "k8s.io/apiserver/pkg/endpoints/metrics" "k8s.io/apiserver/pkg/endpoints/request" @@ -54,6 +55,7 @@ type RequestScope struct { Defaulter runtime.ObjectDefaulter Typer runtime.ObjectTyper UnsafeConvertor runtime.ObjectConvertor + Authorizer authorizer.Authorizer TableConvertor rest.TableConvertor OpenAPISchema openapiproto.Schema diff --git a/pkg/endpoints/handlers/update.go b/pkg/endpoints/handlers/update.go index 0f9943d79..ac318f276 100644 --- a/pkg/endpoints/handlers/update.go +++ b/pkg/endpoints/handlers/update.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "net/http" + "sync" "time" "k8s.io/apimachinery/pkg/api/errors" @@ -27,6 +28,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/audit" + "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" @@ -102,6 +104,19 @@ func UpdateResource(r rest.Updater, scope RequestScope, admit admission.Interfac }) } + createAuthorizerAttributes := authorizer.AttributesRecord{ + User: userInfo, + ResourceRequest: true, + Path: req.URL.Path, + Verb: "create", + APIGroup: scope.Resource.Group, + APIVersion: scope.Resource.Version, + Resource: scope.Resource.Resource, + Subresource: scope.Subresource, + Namespace: namespace, + Name: name, + } + trace.Step("About to store object in database") wasCreated := false result, err := finishRequest(timeout, func() (runtime.Object, error) { @@ -109,7 +124,7 @@ func UpdateResource(r rest.Updater, scope RequestScope, admit admission.Interfac ctx, name, rest.DefaultUpdatedObjectInfo(obj, transformers...), - rest.AdmissionToValidateObjectFunc(admit, staticAdmissionAttributes), + withAuthorization(rest.AdmissionToValidateObjectFunc(admit, staticAdmissionAttributes), scope.Authorizer, createAuthorizerAttributes), rest.AdmissionToValidateObjectUpdateFunc(admit, staticAdmissionAttributes), false, ) @@ -141,3 +156,35 @@ func UpdateResource(r rest.Updater, scope RequestScope, admit admission.Interfac transformResponseObject(ctx, scope, req, w, status, result) } } + +func withAuthorization(validate rest.ValidateObjectFunc, a authorizer.Authorizer, attributes authorizer.Attributes) rest.ValidateObjectFunc { + var once sync.Once + var authorizerDecision authorizer.Decision + var authorizerReason string + var authorizerErr error + return func(obj runtime.Object) error { + if a == nil { + return errors.NewInternalError(fmt.Errorf("no authorizer provided, unable to authorize a create on update")) + } + once.Do(func() { + authorizerDecision, authorizerReason, authorizerErr = a.Authorize(attributes) + }) + // an authorizer like RBAC could encounter evaluation errors and still allow the request, so authorizer decision is checked before error here. + if authorizerDecision == authorizer.DecisionAllow { + // Continue to validating admission + return validate(obj) + } + if authorizerErr != nil { + return errors.NewInternalError(authorizerErr) + } + + // The user is not authorized to perform this action, so we need to build the error response + gr := schema.GroupResource{ + Group: attributes.GetAPIGroup(), + Resource: attributes.GetResource(), + } + name := attributes.GetName() + err := fmt.Errorf("%v", authorizerReason) + return errors.NewForbidden(gr, name, err) + } +} diff --git a/pkg/endpoints/installer.go b/pkg/endpoints/installer.go index 155e39d7f..27ac14e3e 100644 --- a/pkg/endpoints/installer.go +++ b/pkg/endpoints/installer.go @@ -483,6 +483,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag Defaulter: a.group.Defaulter, Typer: a.group.Typer, UnsafeConvertor: a.group.UnsafeConvertor, + Authorizer: a.group.Authorizer, // TODO: Check for the interface on storage TableConvertor: tableProvider, diff --git a/pkg/server/config.go b/pkg/server/config.go index be3efc999..00dac9e43 100644 --- a/pkg/server/config.go +++ b/pkg/server/config.go @@ -464,6 +464,7 @@ func (c completedConfig) New(name string, delegationTarget DelegationTarget) (*G admissionControl: c.AdmissionControl, Serializer: c.Serializer, AuditBackend: c.AuditBackend, + Authorizer: c.Authorization.Authorizer, delegationTarget: delegationTarget, HandlerChainWaitGroup: c.HandlerChainWaitGroup, diff --git a/pkg/server/genericapiserver.go b/pkg/server/genericapiserver.go index 8c6949134..b6e500c61 100644 --- a/pkg/server/genericapiserver.go +++ b/pkg/server/genericapiserver.go @@ -36,6 +36,7 @@ import ( utilwaitgroup "k8s.io/apimachinery/pkg/util/waitgroup" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/audit" + "k8s.io/apiserver/pkg/authorization/authorizer" genericapi "k8s.io/apiserver/pkg/endpoints" "k8s.io/apiserver/pkg/endpoints/discovery" "k8s.io/apiserver/pkg/registry/rest" @@ -138,6 +139,11 @@ type GenericAPIServer struct { // auditing. The backend is started after the server starts listening. AuditBackend audit.Backend + // Authorizer determines whether a user is allowed to make a certain request. The Handler does a preliminary + // authorization check using the request URI but it may be necessary to make additional checks, such as in + // the create-on-update case + Authorizer authorizer.Authorizer + // enableAPIResponseCompression indicates whether API Responses should support compression // if the client requests it via Accept-Encoding enableAPIResponseCompression bool @@ -422,6 +428,7 @@ func (s *GenericAPIServer) newAPIGroupVersion(apiGroupInfo *APIGroupInfo, groupV MinRequestTimeout: s.minRequestTimeout, EnableAPIResponseCompression: s.enableAPIResponseCompression, OpenAPIConfig: s.openAPIConfig, + Authorizer: s.Authorizer, } }