diff --git a/build/selectorsyncset.yaml b/build/selectorsyncset.yaml index a2ba30dd..6226b5fb 100644 --- a/build/selectorsyncset.yaml +++ b/build/selectorsyncset.yaml @@ -274,6 +274,36 @@ objects: scope: Cluster sideEffects: None timeoutSeconds: 2 + - apiVersion: admissionregistration.k8s.io/v1 + kind: ValidatingWebhookConfiguration + metadata: + annotations: + service.beta.openshift.io/inject-cabundle: "true" + creationTimestamp: null + name: sre-clusterroles-validation + webhooks: + - admissionReviewVersions: + - v1 + clientConfig: + service: + name: validation-webhook + namespace: openshift-validation-webhook + path: /clusterroles-validation + failurePolicy: Ignore + matchPolicy: Equivalent + name: clusterroles-validation.managed.openshift.io + rules: + - apiGroups: + - rbac.authorization.k8s.io + apiVersions: + - v1 + operations: + - DELETE + resources: + - clusterroles + scope: Cluster + sideEffects: None + timeoutSeconds: 2 - apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: diff --git a/pkg/webhooks/add_clusterrole.go b/pkg/webhooks/add_clusterrole.go new file mode 100644 index 00000000..b58eeba0 --- /dev/null +++ b/pkg/webhooks/add_clusterrole.go @@ -0,0 +1,9 @@ +package webhooks + +import ( + "github.com/openshift/managed-cluster-validating-webhooks/pkg/webhooks/clusterrole" +) + +func init() { + Register(clusterrole.WebhookName, func() Webhook { return clusterrole.NewWebhook() }) +} diff --git a/pkg/webhooks/clusterrole/clusterrole.go b/pkg/webhooks/clusterrole/clusterrole.go new file mode 100644 index 00000000..9c2be77e --- /dev/null +++ b/pkg/webhooks/clusterrole/clusterrole.go @@ -0,0 +1,244 @@ +package clusterrole + +import ( + "fmt" + "net/http" + "os" + "slices" + "strings" + + "github.com/openshift/managed-cluster-validating-webhooks/pkg/webhooks/utils" + admissionv1 "k8s.io/api/admission/v1" + admissionregv1 "k8s.io/api/admissionregistration/v1" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + logf "sigs.k8s.io/controller-runtime/pkg/log" + admissionctl "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +const ( + WebhookName string = "clusterroles-validation" + backplanePrefix string = "backplane-" + docString string = `Managed OpenShift Customers may not delete protected ClusterRoles including cluster-admin, view, edit, admin, specific system roles (system:admin, system:node, system:node-proxier, system:kube-scheduler, system:kube-controller-manager), and backplane-* roles` +) + +var ( + timeout int32 = 2 + log = logf.Log.WithName(WebhookName) + scope = admissionregv1.ClusterScope + rules = []admissionregv1.RuleWithOperations{ + { + Operations: []admissionregv1.OperationType{"DELETE"}, + Rule: admissionregv1.Rule{ + APIGroups: []string{"rbac.authorization.k8s.io"}, + APIVersions: []string{"v1"}, + Resources: []string{"clusterroles"}, + Scope: &scope, + }, + }, + } + + // Protected ClusterRoles that should not be deleted + protectedClusterRoles = []string{ + "cluster-admin", + "view", + "edit", + "admin", + "system:admin", + "system:node", + "system:kube-scheduler", + "system:kube-controller-manager", + } + + // Users allowed to delete protected ClusterRoles + allowedUsers = []string{ + "backplane-cluster-admin", + } + + // Groups allowed to delete protected ClusterRoles + allowedGroups = []string{ + "system:serviceaccounts:openshift-backplane-srep", + } +) + +type ClusterRoleWebHook struct { + s runtime.Scheme +} + +// NewWebhook creates the new webhook +func NewWebhook() *ClusterRoleWebHook { + scheme := runtime.NewScheme() + err := admissionv1.AddToScheme(scheme) + if err != nil { + log.Error(err, "Fail adding admissionsv1 scheme to ClusterRoleWebHook") + os.Exit(1) + } + err = corev1.AddToScheme(scheme) + if err != nil { + log.Error(err, "Fail adding corev1 scheme to ClusterRoleWebHook") + os.Exit(1) + } + + return &ClusterRoleWebHook{ + s: *scheme, + } +} + +// Authorized implements Webhook interface +func (s *ClusterRoleWebHook) Authorized(request admissionctl.Request) admissionctl.Response { + return s.authorized(request) +} + +func (s *ClusterRoleWebHook) authorized(request admissionctl.Request) admissionctl.Response { + var ret admissionctl.Response + + if request.AdmissionRequest.UserInfo.Username == "system:unauthenticated" { + log.Info("system:unauthenticated made a webhook request. Check RBAC rules", "request", request.AdmissionRequest) + ret = admissionctl.Denied("Unauthenticated") + ret.UID = request.AdmissionRequest.UID + return ret + } + + if strings.HasPrefix(request.AdmissionRequest.UserInfo.Username, "system:") && request.AdmissionRequest.UserInfo.Username != "system:admin" { + ret = admissionctl.Allowed("authenticated system: users are allowed") + ret.UID = request.AdmissionRequest.UID + return ret + } + + if strings.HasPrefix(request.AdmissionRequest.UserInfo.Username, "kube:") { + ret = admissionctl.Allowed("kube: users are allowed") + ret.UID = request.AdmissionRequest.UID + return ret + } + + clusterRole, err := s.renderClusterRole(request) + if err != nil { + log.Error(err, "Couldn't render a ClusterRole from the incoming request") + return admissionctl.Errored(http.StatusBadRequest, err) + } + + log.Info(fmt.Sprintf("Found clusterrole: %v", clusterRole.Name)) + + if isProtectedClusterRole(clusterRole) && !isAllowedUserGroup(request) { + switch request.Operation { + case admissionv1.Delete: + log.Info(fmt.Sprintf("Deleting operation detected on ClusterRole: %v", clusterRole.Name)) + + ret = admissionctl.Denied(fmt.Sprintf("Deleting ClusterRole %v is not allowed", clusterRole.Name)) + ret.UID = request.AdmissionRequest.UID + return ret + } + } + + ret = admissionctl.Allowed("Request is allowed") + ret.UID = request.AdmissionRequest.UID + return ret +} + +// renderClusterRole renders the ClusterRole object from the request +func (s *ClusterRoleWebHook) renderClusterRole(request admissionctl.Request) (*rbacv1.ClusterRole, error) { + decoder := admissionctl.NewDecoder(&s.s) + clusterRole := &rbacv1.ClusterRole{} + + var err error + if len(request.OldObject.Raw) > 0 { + err = decoder.DecodeRaw(request.OldObject, clusterRole) + } + if err != nil { + return nil, err + } + + return clusterRole, nil +} + +// isAllowedUserGroup checks if the user or group is allowed to perform the action +func isAllowedUserGroup(request admissionctl.Request) bool { + if slices.Contains(allowedUsers, request.UserInfo.Username) { + return true + } + for _, group := range allowedGroups { + if slices.Contains(request.UserInfo.Groups, group) { + return true + } + } + return false +} + +// isProtectedClusterRole returns true if the ClusterRole is in the protected list or matches protected patterns +func isProtectedClusterRole(clusterRole *rbacv1.ClusterRole) bool { + // Check if it's in the explicit protected list (includes specific system roles) + if slices.Contains(protectedClusterRoles, clusterRole.Name) { + return true + } + // Check if it matches backplane pattern + if strings.HasPrefix(clusterRole.Name, backplanePrefix) { + return true + } + return false +} + +// GetURI implements Webhook interface +func (s *ClusterRoleWebHook) GetURI() string { + return "/" + WebhookName +} + +// Validate implements Webhook interface +func (s *ClusterRoleWebHook) Validate(request admissionctl.Request) bool { + valid := true + valid = valid && (request.UserInfo.Username != "") + valid = valid && (request.Kind.Kind == "ClusterRole") + + return valid +} + +// Name implements Webhook interface +func (s *ClusterRoleWebHook) Name() string { + return WebhookName +} + +// FailurePolicy implements Webhook interface +func (s *ClusterRoleWebHook) FailurePolicy() admissionregv1.FailurePolicyType { + return admissionregv1.Ignore +} + +// MatchPolicy implements Webhook interface +func (s *ClusterRoleWebHook) MatchPolicy() admissionregv1.MatchPolicyType { + return admissionregv1.Equivalent +} + +// Rules implements Webhook interface +func (s *ClusterRoleWebHook) Rules() []admissionregv1.RuleWithOperations { + return rules +} + +// ObjectSelector implements Webhook interface +func (s *ClusterRoleWebHook) ObjectSelector() *metav1.LabelSelector { + return nil +} + +// SideEffects implements Webhook interface +func (s *ClusterRoleWebHook) SideEffects() admissionregv1.SideEffectClass { + return admissionregv1.SideEffectClassNone +} + +// TimeoutSeconds implements Webhook interface +func (s *ClusterRoleWebHook) TimeoutSeconds() int32 { + return timeout +} + +// Doc implements Webhook interface +func (s *ClusterRoleWebHook) Doc() string { + return docString +} + +// SyncSetLabelSelector returns the label selector to use in the SyncSet. +// Return utils.DefaultLabelSelector() to stick with the default +func (s *ClusterRoleWebHook) SyncSetLabelSelector() metav1.LabelSelector { + return utils.DefaultLabelSelector() +} + +func (s *ClusterRoleWebHook) ClassicEnabled() bool { return true } + +func (s *ClusterRoleWebHook) HypershiftEnabled() bool { return true } diff --git a/pkg/webhooks/clusterrole/clusterrole_test.go b/pkg/webhooks/clusterrole/clusterrole_test.go new file mode 100644 index 00000000..f474d4cf --- /dev/null +++ b/pkg/webhooks/clusterrole/clusterrole_test.go @@ -0,0 +1,171 @@ +package clusterrole + +import ( + "testing" + + "github.com/openshift/managed-cluster-validating-webhooks/pkg/testutils" + admissionv1 "k8s.io/api/admission/v1" + authenticationv1 "k8s.io/api/authentication/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + admissionctl "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +type ClusterRoleTestSuites struct { + testID string + targetClusterRole string + username string + operation admissionv1.Operation + userGroups []string + shouldBeAllowed bool +} + +var ( + // testClusterRoleJSON represents a minimal ClusterRole JSON object for testing + testClusterRoleJSON = `{ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRole", + "metadata": { + "name": "cluster-admin" + }, + "rules": [ + { + "apiGroups": ["*"], + "resources": ["*"], + "verbs": ["*"] + } + ] + }` + + // testOtherClusterRoleJSON represents a non-protected ClusterRole + testOtherClusterRoleJSON = `{ + "apiVersion": "rbac.authorization.k8s.io/v1", + "kind": "ClusterRole", + "metadata": { + "name": "some-custom-role" + }, + "rules": [ + { + "apiGroups": [""], + "resources": ["pods"], + "verbs": ["get", "list"] + } + ] + }` +) + +func runClusterRoleTests(t *testing.T, tests []ClusterRoleTestSuites) { + for _, test := range tests { + gvk := metav1.GroupVersionKind{ + Group: "rbac.authorization.k8s.io", + Version: "v1", + Kind: "ClusterRole", + } + gvr := metav1.GroupVersionResource{ + Group: "rbac.authorization.k8s.io", + Version: "v1", + Resource: "clusterroles", + } + + var clusterRoleJSON string + if test.targetClusterRole == "cluster-admin" { + clusterRoleJSON = testClusterRoleJSON + } else { + clusterRoleJSON = testOtherClusterRoleJSON + } + + rawOldObject := []byte(clusterRoleJSON) + req := admissionctl.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + UID: "test-uid", + Kind: gvk, + Resource: gvr, + Operation: admissionv1.Delete, + UserInfo: authenticationv1.UserInfo{ + Username: test.username, + Groups: test.userGroups, + }, + OldObject: runtime.RawExtension{ + Raw: rawOldObject, + }, + }, + } + + hook := NewWebhook() + response := hook.Authorized(req) + + if response.Allowed != test.shouldBeAllowed { + t.Fatalf("Mismatch: %s (groups=%s) %s %s the clusterrole. Test's expectation is that the user %s", test.username, test.userGroups, testutils.CanCanNot(response.Allowed), test.operation, testutils.CanCanNot(test.shouldBeAllowed)) + } + } +} + +func TestClusterRoleDeletionNegative(t *testing.T) { + tests := []ClusterRoleTestSuites{ + { + testID: "regular-user-deny", + username: "test-user", + userGroups: []string{"system:authenticated"}, + operation: admissionv1.Delete, + shouldBeAllowed: false, + targetClusterRole: "cluster-admin", + }, + { + testID: "cluster-admin-user-deny", + username: "cluster-admin", + userGroups: []string{"system:authenticated"}, + operation: admissionv1.Delete, + shouldBeAllowed: false, + targetClusterRole: "cluster-admin", + }, + { + testID: "customer-admin-deny", + username: "customer-user", + userGroups: []string{"system:authenticated", "customer-admin"}, + operation: admissionv1.Delete, + shouldBeAllowed: false, + targetClusterRole: "cluster-admin", + }, + } + + runClusterRoleTests(t, tests) +} + +func TestClusterRoleDeletionPositive(t *testing.T) { + tests := []ClusterRoleTestSuites{ + { + testID: "backplane-admin-allow", + username: "backplane-cluster-admin", + userGroups: []string{"system:authenticated"}, + operation: admissionv1.Delete, + shouldBeAllowed: true, + targetClusterRole: "cluster-admin", + }, + { + testID: "backplane-srep-allow", + username: "test-user", + userGroups: []string{"system:authenticated", "system:serviceaccounts:openshift-backplane-srep"}, + operation: admissionv1.Delete, + shouldBeAllowed: true, + targetClusterRole: "cluster-admin", + }, + { + testID: "other-role-allow", + username: "regular-user", + userGroups: []string{"system:authenticated"}, + operation: admissionv1.Delete, + shouldBeAllowed: true, + targetClusterRole: "some-custom-role", + }, + { + testID: "system-user-allow", + username: "system:kube-controller-manager", + userGroups: []string{"system:authenticated"}, + operation: admissionv1.Delete, + shouldBeAllowed: true, + targetClusterRole: "cluster-admin", + }, + } + + runClusterRoleTests(t, tests) +}