From 9e4ca53c3c3b3ac78681db2fd2c09d471e9535c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Wed, 22 Feb 2023 11:08:41 +0100 Subject: [PATCH] refactor: user/groups exclusions (#6357) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: user/groups exclusions Signed-off-by: Charles-Edouard Brétéché * wildcard Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché --------- Signed-off-by: Charles-Edouard Brétéché Signed-off-by: Charles-Edouard Brétéché --- cmd/cleanup-controller/server.go | 2 +- pkg/config/config.go | 65 ++++++++++++-------- pkg/engine/background.go | 2 +- pkg/engine/exceptions.go | 2 +- pkg/engine/mutation.go | 4 +- pkg/engine/validation.go | 4 +- pkg/userinfo/roleRef.go | 8 +-- pkg/webhooks/handlers/dump.go | 20 +++--- pkg/webhooks/handlers/dump_test.go | 2 +- pkg/webhooks/handlers/filter.go | 22 +++++-- pkg/webhooks/server.go | 10 +-- pkg/webhooks/utils/policy_context_builder.go | 2 +- 12 files changed, 82 insertions(+), 61 deletions(-) diff --git a/cmd/cleanup-controller/server.go b/cmd/cleanup-controller/server.go index 6450e5bf75..42d91fcc81 100644 --- a/cmd/cleanup-controller/server.go +++ b/cmd/cleanup-controller/server.go @@ -71,7 +71,7 @@ func NewServer( "POST", config.CleanupValidatingWebhookServicePath, handlers.FromAdmissionFunc("VALIDATE", validationHandler). - WithDump(debugModeOpts.DumpPayload, nil, nil, nil). + WithDump(debugModeOpts.DumpPayload, nil, nil). WithSubResourceFilter(). WithMetrics(policyLogger, metricsConfig.Config(), metrics.WebhookValidating). WithAdmission(policyLogger.WithName("validate")). diff --git a/pkg/config/config.go b/pkg/config/config.go index 7e16224c15..8d95a1654c 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -95,8 +95,10 @@ var ( kyvernoPodName = osutils.GetEnvWithFallback("KYVERNO_POD_NAME", "kyverno") // kyvernoConfigMapName is the Kyverno configmap name kyvernoConfigMapName = osutils.GetEnvWithFallback("INIT_CONFIG", "kyverno") - // defaultExcludeGroupRole ... - defaultExcludeGroupRole []string = []string{"system:serviceaccounts:kube-system", "system:nodes", "system:kube-scheduler"} + // defaultExcludedUsernames are the usernames excluded by default when matching an incoming admission request + defaultExcludedUsernames []string = []string{"system:kube-scheduler"} + // defaultExcludedGroups are the groups excluded by default when matching an incoming admission request + defaultExcludedGroups []string = []string{"system:serviceaccounts:kube-system", "system:nodes"} // kyvernoDryRunNamespace is the namespace for DryRun option of YAML verification kyvernoDryrunNamespace = osutils.GetEnvWithFallback("KYVERNO_DRYRUN_NAMESPACE", "kyverno-dryrun") ) @@ -137,10 +139,10 @@ type Configuration interface { GetEnableDefaultRegistryMutation() bool // ToFilter checks if the given resource is set to be filtered in the configuration ToFilter(kind, namespace, name string) bool - // GetExcludeGroupRole return exclude roles - GetExcludeGroupRole() []string - // GetExcludeUsername return exclude username - GetExcludeUsername() []string + // GetExcludedGroups return exclude groups + GetExcludedGroups() []string + // GetExcludedUsernames return exclude usernames + GetExcludedUsernames() []string // GetGenerateSuccessEvents return if should generate success events GetGenerateSuccessEvents() bool // FilterNamespaces filters exclude namespace @@ -155,8 +157,8 @@ type Configuration interface { type configuration struct { defaultRegistry string enableDefaultRegistryMutation bool - excludeGroupRole []string - excludeUsername []string + excludedGroups []string + excludedUsernames []string filters []filter generateSuccessEvents bool mux sync.RWMutex @@ -168,7 +170,8 @@ func NewDefaultConfiguration() *configuration { return &configuration{ defaultRegistry: "docker.io", enableDefaultRegistryMutation: true, - excludeGroupRole: defaultExcludeGroupRole, + excludedGroups: defaultExcludedGroups, + excludedUsernames: defaultExcludedUsernames, } } @@ -202,12 +205,6 @@ func (cd *configuration) ToFilter(kind, namespace, name string) bool { return false } -func (cd *configuration) GetExcludeGroupRole() []string { - cd.mux.RLock() - defer cd.mux.RUnlock() - return cd.excludeGroupRole -} - func (cd *configuration) GetDefaultRegistry() string { cd.mux.RLock() defer cd.mux.RUnlock() @@ -220,10 +217,16 @@ func (cd *configuration) GetEnableDefaultRegistryMutation() bool { return cd.enableDefaultRegistryMutation } -func (cd *configuration) GetExcludeUsername() []string { +func (cd *configuration) GetExcludedUsernames() []string { cd.mux.RLock() defer cd.mux.RUnlock() - return cd.excludeUsername + return cd.excludedUsernames +} + +func (cd *configuration) GetExcludedGroups() []string { + cd.mux.RLock() + defer cd.mux.RUnlock() + return cd.excludedGroups } func (cd *configuration) GetGenerateSuccessEvents() bool { @@ -265,10 +268,12 @@ func (cd *configuration) load(cm *corev1.ConfigMap) { defer cd.mux.Unlock() // reset cd.filters = []filter{} - cd.excludeGroupRole = []string{} - cd.excludeUsername = []string{} + cd.excludedUsernames = []string{} + cd.excludedGroups = []string{} cd.generateSuccessEvents = false cd.webhooks = nil + cd.excludedGroups = append(cd.excludedGroups, defaultExcludedGroups...) + cd.excludedUsernames = append(cd.excludedUsernames, defaultExcludedUsernames...) // load filters cd.filters = parseKinds(cm.Data["resourceFilters"]) newDefaultRegistry, ok := cm.Data["defaultRegistry"] @@ -294,10 +299,19 @@ func (cd *configuration) load(cm *corev1.ConfigMap) { cd.enableDefaultRegistryMutation = newEnableDefaultRegistryMutation } // load excludeGroupRole - cd.excludeGroupRole = append(cd.excludeGroupRole, parseRbac(cm.Data["excludeGroupRole"])...) - cd.excludeGroupRole = append(cd.excludeGroupRole, defaultExcludeGroupRole...) + excludedGroups, ok := cm.Data["excludeGroupRole"] + if !ok { + logger.V(6).Info("configuration: No excludeGroupRole defined in ConfigMap") + } else { + cd.excludedGroups = parseRbac(excludedGroups) + } // load excludeUsername - cd.excludeUsername = append(cd.excludeUsername, parseRbac(cm.Data["excludeUsername"])...) + excludedUsernames, ok := cm.Data["excludeUsername"] + if !ok { + logger.V(6).Info("configuration: No excludeUsername defined in ConfigMap") + } else { + cd.excludedUsernames = parseRbac(excludedUsernames) + } // load generateSuccessEvents generateSuccessEvents, ok := cm.Data["generateSuccessEvents"] if ok { @@ -326,9 +340,10 @@ func (cd *configuration) unload() { cd.filters = []filter{} cd.defaultRegistry = "docker.io" cd.enableDefaultRegistryMutation = true - cd.excludeGroupRole = []string{} - cd.excludeUsername = []string{} + cd.excludedUsernames = []string{} + cd.excludedGroups = []string{} cd.generateSuccessEvents = false cd.webhooks = nil - cd.excludeGroupRole = append(cd.excludeGroupRole, defaultExcludeGroupRole...) + cd.excludedGroups = append(cd.excludedGroups, defaultExcludedGroups...) + cd.excludedUsernames = append(cd.excludedUsernames, defaultExcludedUsernames...) } diff --git a/pkg/engine/background.go b/pkg/engine/background.go index ddb86f5237..4c41c405d2 100644 --- a/pkg/engine/background.go +++ b/pkg/engine/background.go @@ -93,7 +93,7 @@ func (e *engine) filterRule( oldResource := policyContext.OldResource() admissionInfo := policyContext.AdmissionInfo() ctx := policyContext.JSONContext() - excludeGroupRole := e.configuration.GetExcludeGroupRole() + excludeGroupRole := e.configuration.GetExcludedGroups() namespaceLabels := policyContext.NamespaceLabels() if err := MatchesResourceDescription(subresourceGVKToAPIResource, newResource, rule, admissionInfo, excludeGroupRole, namespaceLabels, "", policyContext.SubResource()); err != nil { diff --git a/pkg/engine/exceptions.go b/pkg/engine/exceptions.go index 830f59a0a3..7944c058af 100644 --- a/pkg/engine/exceptions.go +++ b/pkg/engine/exceptions.go @@ -60,7 +60,7 @@ func matchesException( subresourceGVKToAPIResource, policyContext.SubResource(), policyContext.AdmissionInfo(), - cfg.GetExcludeGroupRole(), + cfg.GetExcludedGroups(), ) // if there's no error it means a match if err == nil { diff --git a/pkg/engine/mutation.go b/pkg/engine/mutation.go index b4373cc902..470b222790 100644 --- a/pkg/engine/mutation.go +++ b/pkg/engine/mutation.go @@ -57,8 +57,8 @@ func (e *engine) mutate( func(ctx context.Context, span trace.Span) { logger := internal.LoggerWithRule(logger, rule) var excludeResource []string - if len(e.configuration.GetExcludeGroupRole()) > 0 { - excludeResource = e.configuration.GetExcludeGroupRole() + if len(e.configuration.GetExcludedGroups()) > 0 { + excludeResource = e.configuration.GetExcludedGroups() } kindsInPolicy := append(rule.MatchResources.GetKinds(), rule.ExcludeResources.GetKinds()...) diff --git a/pkg/engine/validation.go b/pkg/engine/validation.go index 16113a03d8..21ad85b0a0 100644 --- a/pkg/engine/validation.go +++ b/pkg/engine/validation.go @@ -514,13 +514,13 @@ func matches( subresourceGVKToAPIResource map[string]*metav1.APIResource, cfg config.Configuration, ) bool { - err := MatchesResourceDescription(subresourceGVKToAPIResource, ctx.NewResource(), *rule, ctx.AdmissionInfo(), cfg.GetExcludeGroupRole(), ctx.NamespaceLabels(), "", ctx.SubResource()) + err := MatchesResourceDescription(subresourceGVKToAPIResource, ctx.NewResource(), *rule, ctx.AdmissionInfo(), cfg.GetExcludedGroups(), ctx.NamespaceLabels(), "", ctx.SubResource()) if err == nil { return true } if !reflect.DeepEqual(ctx.OldResource, unstructured.Unstructured{}) { - err := MatchesResourceDescription(subresourceGVKToAPIResource, ctx.OldResource(), *rule, ctx.AdmissionInfo(), cfg.GetExcludeGroupRole(), ctx.NamespaceLabels(), "", ctx.SubResource()) + err := MatchesResourceDescription(subresourceGVKToAPIResource, ctx.OldResource(), *rule, ctx.AdmissionInfo(), cfg.GetExcludedGroups(), ctx.NamespaceLabels(), "", ctx.SubResource()) if err == nil { return true } diff --git a/pkg/userinfo/roleRef.go b/pkg/userinfo/roleRef.go index 097c8a0e24..24a8b61715 100644 --- a/pkg/userinfo/roleRef.go +++ b/pkg/userinfo/roleRef.go @@ -3,8 +3,6 @@ package userinfo import ( "fmt" - "github.com/kyverno/kyverno/pkg/config" - datautils "github.com/kyverno/kyverno/pkg/utils/data" admissionv1 "k8s.io/api/admission/v1" authenticationv1 "k8s.io/api/authentication/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -18,11 +16,7 @@ const ( ) // GetRoleRef gets the list of roles and cluster roles for the incoming api-request -func GetRoleRef(rbLister rbacv1listers.RoleBindingLister, crbLister rbacv1listers.ClusterRoleBindingLister, request *admissionv1.AdmissionRequest, dynamicConfig config.Configuration) ([]string, []string, error) { - keys := append(request.UserInfo.Groups, request.UserInfo.Username) - if datautils.SliceContains(keys, dynamicConfig.GetExcludeGroupRole()...) { - return nil, nil, nil - } +func GetRoleRef(rbLister rbacv1listers.RoleBindingLister, crbLister rbacv1listers.ClusterRoleBindingLister, request *admissionv1.AdmissionRequest) ([]string, []string, error) { // rolebindings roleBindings, err := rbLister.List(labels.Everything()) if err != nil { diff --git a/pkg/webhooks/handlers/dump.go b/pkg/webhooks/handlers/dump.go index b16e06510d..461703fb46 100644 --- a/pkg/webhooks/handlers/dump.go +++ b/pkg/webhooks/handlers/dump.go @@ -6,7 +6,6 @@ import ( "time" "github.com/go-logr/logr" - "github.com/kyverno/kyverno/pkg/config" "github.com/kyverno/kyverno/pkg/userinfo" admissionutils "github.com/kyverno/kyverno/pkg/utils/admission" kubeutils "github.com/kyverno/kyverno/pkg/utils/kube" @@ -22,22 +21,20 @@ func (inner AdmissionHandler) WithDump( enabled bool, rbLister rbacv1listers.RoleBindingLister, crbLister rbacv1listers.ClusterRoleBindingLister, - configuration config.Configuration, ) AdmissionHandler { if !enabled { return inner } - return inner.withDump(rbLister, crbLister, configuration).WithTrace("DUMP") + return inner.withDump(rbLister, crbLister).WithTrace("DUMP") } func (inner AdmissionHandler) withDump( rbLister rbacv1listers.RoleBindingLister, crbLister rbacv1listers.ClusterRoleBindingLister, - configuration config.Configuration, ) AdmissionHandler { return func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse { response := inner(ctx, logger, request, startTime) - dumpPayload(logger, rbLister, crbLister, configuration, request, response) + dumpPayload(logger, rbLister, crbLister, request, response) return response } } @@ -46,11 +43,10 @@ func dumpPayload( logger logr.Logger, rbLister rbacv1listers.RoleBindingLister, crbLister rbacv1listers.ClusterRoleBindingLister, - configuration config.Configuration, request *admissionv1.AdmissionRequest, response *admissionv1.AdmissionResponse, ) { - reqPayload, err := newAdmissionRequestPayload(request, rbLister, crbLister, configuration) + reqPayload, err := newAdmissionRequestPayload(request, rbLister, crbLister) if err != nil { logger.Error(err, "Failed to extract resources") } else { @@ -82,7 +78,11 @@ type admissionRequestPayload struct { Options unstructured.Unstructured `json:"options,omitempty"` } -func newAdmissionRequestPayload(request *admissionv1.AdmissionRequest, rbLister rbacv1listers.RoleBindingLister, crbLister rbacv1listers.ClusterRoleBindingLister, configuration config.Configuration) (*admissionRequestPayload, error) { +func newAdmissionRequestPayload( + request *admissionv1.AdmissionRequest, + rbLister rbacv1listers.RoleBindingLister, + crbLister rbacv1listers.ClusterRoleBindingLister, +) (*admissionRequestPayload, error) { newResource, oldResource, err := admissionutils.ExtractResources(nil, request) if err != nil { return nil, err @@ -95,8 +95,8 @@ func newAdmissionRequestPayload(request *admissionv1.AdmissionRequest, rbLister } } var roles, clusterRoles []string - if rbLister != nil && crbLister != nil && configuration != nil { - if r, cr, err := userinfo.GetRoleRef(rbLister, crbLister, request, configuration); err != nil { + if rbLister != nil && crbLister != nil { + if r, cr, err := userinfo.GetRoleRef(rbLister, crbLister, request); err != nil { return nil, err } else { roles = r diff --git a/pkg/webhooks/handlers/dump_test.go b/pkg/webhooks/handlers/dump_test.go index 1fd5f2843a..bc1d998414 100644 --- a/pkg/webhooks/handlers/dump_test.go +++ b/pkg/webhooks/handlers/dump_test.go @@ -141,7 +141,7 @@ func Test_RedactPayload(t *testing.T) { req := new(admissionv1.AdmissionRequest) err := json.Unmarshal(c.requestPayload, req) assert.NilError(t, err) - payload, err := newAdmissionRequestPayload(req, nil, nil, nil) + payload, err := newAdmissionRequestPayload(req, nil, nil) assert.NilError(t, err) if payload.Object.Object != nil { data, err := datautils.ToMap(payload.Object.Object["data"]) diff --git a/pkg/webhooks/handlers/filter.go b/pkg/webhooks/handlers/filter.go index 07f8dd979e..e69f66b30c 100644 --- a/pkg/webhooks/handlers/filter.go +++ b/pkg/webhooks/handlers/filter.go @@ -6,6 +6,7 @@ import ( "github.com/go-logr/logr" "github.com/kyverno/kyverno/pkg/config" + wildcard "github.com/kyverno/kyverno/pkg/utils/wildcard" webhookutils "github.com/kyverno/kyverno/pkg/webhooks/utils" admissionv1 "k8s.io/api/admission/v1" "k8s.io/apimachinery/pkg/util/sets" @@ -25,14 +26,25 @@ func (inner AdmissionHandler) WithSubResourceFilter(subresources ...string) Admi func (inner AdmissionHandler) withFilter(c config.Configuration) AdmissionHandler { return func(ctx context.Context, logger logr.Logger, request *admissionv1.AdmissionRequest, startTime time.Time) *admissionv1.AdmissionResponse { - if c.ToFilter(request.Kind.Kind, request.Namespace, request.Name) { - return nil - } - for _, username := range c.GetExcludeUsername() { - if request.UserInfo.Username == username { + // filter by username + for _, username := range c.GetExcludedUsernames() { + if wildcard.Match(username, request.UserInfo.Username) { return nil } } + // filter by groups + for _, group := range c.GetExcludedGroups() { + for _, candidate := range request.UserInfo.Groups { + if wildcard.Match(group, candidate) { + return nil + } + } + } + // filter by resource filters + if c.ToFilter(request.Kind.Kind, request.Namespace, request.Name) { + return nil + } + // filter kyverno resources if webhookutils.ExcludeKyvernoResources(request.Kind.Kind) { return nil } diff --git a/pkg/webhooks/server.go b/pkg/webhooks/server.go index b3588ffc39..69cf285dc6 100644 --- a/pkg/webhooks/server.go +++ b/pkg/webhooks/server.go @@ -96,7 +96,7 @@ func NewServer( return handler. WithFilter(configuration). WithProtection(toggle.ProtectManagedResources.Enabled()). - WithDump(debugModeOpts.DumpPayload, rbLister, crbLister, configuration). + WithDump(debugModeOpts.DumpPayload, rbLister, crbLister). WithOperationFilter(admissionv1.Create, admissionv1.Update, admissionv1.Connect). WithMetrics(resourceLogger, metricsConfig.Config(), metrics.WebhookMutating). WithAdmission(resourceLogger.WithName("mutate")) @@ -111,7 +111,7 @@ func NewServer( return handler. WithFilter(configuration). WithProtection(toggle.ProtectManagedResources.Enabled()). - WithDump(debugModeOpts.DumpPayload, rbLister, crbLister, configuration). + WithDump(debugModeOpts.DumpPayload, rbLister, crbLister). WithMetrics(resourceLogger, metricsConfig.Config(), metrics.WebhookValidating). WithAdmission(resourceLogger.WithName("validate")) }, @@ -120,7 +120,7 @@ func NewServer( "POST", config.PolicyMutatingWebhookServicePath, handlers.FromAdmissionFunc("MUTATE", policyHandlers.Mutate). - WithDump(debugModeOpts.DumpPayload, rbLister, crbLister, configuration). + WithDump(debugModeOpts.DumpPayload, rbLister, crbLister). WithMetrics(policyLogger, metricsConfig.Config(), metrics.WebhookMutating). WithAdmission(policyLogger.WithName("mutate")). ToHandlerFunc(), @@ -129,7 +129,7 @@ func NewServer( "POST", config.PolicyValidatingWebhookServicePath, handlers.FromAdmissionFunc("VALIDATE", policyHandlers.Validate). - WithDump(debugModeOpts.DumpPayload, rbLister, crbLister, configuration). + WithDump(debugModeOpts.DumpPayload, rbLister, crbLister). WithSubResourceFilter(). WithMetrics(policyLogger, metricsConfig.Config(), metrics.WebhookValidating). WithAdmission(policyLogger.WithName("validate")). @@ -139,7 +139,7 @@ func NewServer( "POST", config.ExceptionValidatingWebhookServicePath, handlers.FromAdmissionFunc("VALIDATE", exceptionHandlers.Validate). - WithDump(debugModeOpts.DumpPayload, rbLister, crbLister, configuration). + WithDump(debugModeOpts.DumpPayload, rbLister, crbLister). WithSubResourceFilter(). WithMetrics(exceptionLogger, metricsConfig.Config(), metrics.WebhookValidating). WithAdmission(exceptionLogger.WithName("validate")). diff --git a/pkg/webhooks/utils/policy_context_builder.go b/pkg/webhooks/utils/policy_context_builder.go index ee7bed49e4..14f950128a 100644 --- a/pkg/webhooks/utils/policy_context_builder.go +++ b/pkg/webhooks/utils/policy_context_builder.go @@ -39,7 +39,7 @@ func (b *policyContextBuilder) Build(request *admissionv1.AdmissionRequest) (*en userRequestInfo := kyvernov1beta1.RequestInfo{ AdmissionUserInfo: *request.UserInfo.DeepCopy(), } - if roles, clusterRoles, err := userinfo.GetRoleRef(b.rbLister, b.crbLister, request, b.configuration); err != nil { + if roles, clusterRoles, err := userinfo.GetRoleRef(b.rbLister, b.crbLister, request); err != nil { return nil, fmt.Errorf("failed to fetch RBAC information for request: %w", err) } else { userRequestInfo.Roles = roles