From 8e42253413b578926b8fbb9df171cd8e5ee556be Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Mon, 7 Sep 2026 16:04:43 +0100 Subject: [PATCH] Add Sippy discovered exceptions for RBAC monitor test --- .../rbacadminescalationtests/monitortest.go | 85 +++++++++++++++++++ .../monitortest_test.go | 23 +++++ 2 files changed, 108 insertions(+) diff --git a/pkg/monitortests/authentication/rbacadminescalationtests/monitortest.go b/pkg/monitortests/authentication/rbacadminescalationtests/monitortest.go index e8e0d3f40333..39f95a153593 100644 --- a/pkg/monitortests/authentication/rbacadminescalationtests/monitortest.go +++ b/pkg/monitortests/authentication/rbacadminescalationtests/monitortest.go @@ -131,6 +131,13 @@ var trackedExceptions = []bindingException{ subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-cluster-storage-operator", Name: "cluster-storage-operator"}}, note: "TODO", }, + { + name: "cluster-version-operator", + checkID: clusterAdminCheckID, + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-cluster-version", Name: "default"}}, + note: "TODO", + }, { name: "cluster-version-operator-1", checkID: clusterAdminCheckID, @@ -145,6 +152,20 @@ var trackedExceptions = []bindingException{ subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-machine-config-operator", Name: "machine-config-operator"}}, note: "TODO", }, + { + name: "istiod-clusterrole-openshift-gateway-openshift-ingress", + checkID: admissionWebhooksCheckID, + roleRef: "istiod-clusterrole-openshift-gateway-openshift-ingress", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-ingress", Name: "istiod-openshift-gateway"}}, + note: "TODO", + }, + { + name: "kubemacpool-manager-rolebinding", + checkID: admissionWebhooksCheckID, + roleRef: "kubemacpool-manager-role", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-cnv", Name: "kubemacpool-sa"}}, + note: "TODO", + }, { name: "machine-api-operator", checkID: admissionWebhooksCheckID, @@ -152,6 +173,13 @@ var trackedExceptions = []bindingException{ subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-machine-api", Name: "machine-api-operator"}}, note: "TODO", }, + { + name: "nmstate-handler", + checkID: admissionWebhooksCheckID, + roleRef: "nmstate-handler", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-nmstate", Name: "nmstate-handler"}}, + note: "TODO", + }, { name: "olm-operator-binding-openshift-operator-lifecycle-manager", checkID: admissionWebhooksCheckID, @@ -166,6 +194,13 @@ var trackedExceptions = []bindingException{ subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-operator-lifecycle-manager", Name: "olm-operator-serviceaccount"}}, note: "TODO", }, + { + name: "olm-operator-binding-openshift-operator-lifecycle-manager", + checkID: impersonateCheckID, + roleRef: "system:controller:operator-lifecycle-manager", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-operator-lifecycle-manager", Name: "olm-operator-serviceaccount"}}, + note: "TODO", + }, { name: "openshift-dns-operator", checkID: impersonateCheckID, @@ -222,6 +257,20 @@ var trackedExceptions = []bindingException{ subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "kube-system", Name: "generic-garbage-collector"}}, note: "TODO", }, + { + name: "system:openshift:capi-installer", + checkID: admissionWebhooksCheckID, + roleRef: "openshift-capi-installer", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-cluster-api-operator", Name: "capi-installer"}}, + note: "TODO", + }, + { + name: "system:openshift:capi-installer", + checkID: escalateRBACCheckID, + roleRef: "openshift-capi-installer", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-cluster-api-operator", Name: "capi-installer"}}, + note: "TODO", + }, { name: "system:openshift:controller:service-ca", checkID: admissionWebhooksCheckID, @@ -229,6 +278,13 @@ var trackedExceptions = []bindingException{ subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-service-ca", Name: "service-ca"}}, note: "TODO", }, + { + name: "system:openshift:controller:serviceaccount-pull-secrets-controller", + checkID: impersonateCheckID, + roleRef: "system:openshift:controller:serviceaccount-pull-secrets-controller", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-infra", Name: "serviceaccount-pull-secrets-controller"}}, + note: "TODO", + }, { name: "system:openshift:controller:template-instance-controller:admin", checkID: impersonateCheckID, @@ -264,6 +320,13 @@ var trackedExceptions = []bindingException{ subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-authentication", Name: "oauth-openshift"}}, note: "TODO", }, + { + name: "system:openshift:openshift-compatibility-requirements-operator:compatibility-requirements-controllers", + checkID: admissionWebhooksCheckID, + roleRef: "system:openshift:openshift-compatibility-requirements-operator:compatibility-requirements-controllers", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-compatibility-requirements-operator", Name: "compatibility-requirements-controllers"}}, + note: "TODO", + }, { name: "system:openshift:operator:authentication", checkID: clusterAdminCheckID, @@ -669,6 +732,21 @@ func bindingInScope(binding rbacv1.ClusterRoleBinding) bool { return false } +// olmOwnerKindLabel is the label OLM stamps on the RBAC it generates for an operator, recording the +// kind of the owning object. For an operator's own cluster permissions this is ClusterServiceVersion. +const ( + olmOwnerKindLabel = "olm.owner.kind" + clusterServiceVersionKind = "ClusterServiceVersion" +) + +// isOLMManaged reports whether the binding was generated by OLM on behalf of a ClusterServiceVersion, +// i.e. it belongs to an OLM-installed operator rather than the release payload. Such bindings are out +// of scope: they are layered-product RBAC, and OLM names them with a per-install permissions hash so +// an allowlist entry keyed on name could never match a future install. We skip auditing them entirely. +func isOLMManaged(binding rbacv1.ClusterRoleBinding) bool { + return binding.Labels[olmOwnerKindLabel] == clusterServiceVersionKind +} + // evaluateBinding runs every escalation check against a single ClusterRoleBinding and returns the // resulting JUnit cases. Only checks that fire produce cases, and the outcome depends on the // exception class of the (binding, check) pair and whether the check is in flake mode (see @@ -687,6 +765,13 @@ func evaluateBinding(binding rbacv1.ClusterRoleBinding, rolesByName map[string][ return nil } + // Skip RBAC generated by OLM for installed operators: it is layered-product RBAC, not part of the + // release payload, and its names carry a per-install hash so it cannot be allowlisted (see + // isOLMManaged). + if isOLMManaged(binding) { + return nil + } + // A ClusterRoleBinding's RoleRef always references a ClusterRole. A dangling reference grants // nothing, so there is nothing to evaluate. roleRules, ok := rolesByName[binding.RoleRef.Name] diff --git a/pkg/monitortests/authentication/rbacadminescalationtests/monitortest_test.go b/pkg/monitortests/authentication/rbacadminescalationtests/monitortest_test.go index 8eb6787971b7..bdd58b31715a 100644 --- a/pkg/monitortests/authentication/rbacadminescalationtests/monitortest_test.go +++ b/pkg/monitortests/authentication/rbacadminescalationtests/monitortest_test.go @@ -410,6 +410,29 @@ func TestCheckFlakeMode(t *testing.T) { } } +// TestOLMManagedSkipped verifies that a binding generated by OLM (labelled with olm.owner.kind: +// ClusterServiceVersion) is skipped entirely, even when its role would otherwise trip a check. +func TestOLMManagedSkipped(t *testing.T) { + saSubject := rbacv1.Subject{Kind: "ServiceAccount", Namespace: "openshift-cnv", Name: "operator"} + + b := binding("kubevirt-hyperconverged--somehash", "kubevirt-hyperconverged--somehash", saSubject) + b.Labels = map[string]string{"olm.owner.kind": "ClusterServiceVersion"} + + rolesByName := map[string][]rbacv1.PolicyRule{ + "kubevirt-hyperconverged--somehash": {rule([]string{"*"}, []string{"*"}, []string{"*"})}, + } + + if junits := evaluateBinding(b, rolesByName); len(junits) != 0 { + t.Errorf("expected OLM-managed binding to be skipped (no cases), got %d", len(junits)) + } + + // The same binding without the OLM label is audited normally. + b.Labels = nil + if junits := evaluateBinding(b, rolesByName); len(junits) == 0 { + t.Errorf("expected non-OLM binding to be audited, got no cases") + } +} + func TestRoleGrantsAny(t *testing.T) { // A role with resource wildcard should be detected as covering escalate on clusterroles. wildcard := []rbacv1.PolicyRule{rule([]string{"*"}, []string{rbacv1.GroupName}, []string{"*"})}