diff --git a/pkg/defaultmonitortests/types.go b/pkg/defaultmonitortests/types.go index db766d33c702..f625cda0b55f 100644 --- a/pkg/defaultmonitortests/types.go +++ b/pkg/defaultmonitortests/types.go @@ -6,6 +6,7 @@ import ( "github.com/openshift/origin/pkg/monitortestframework" "github.com/openshift/origin/pkg/monitortests/authentication/legacyauthenticationmonitortests" nodefaultserviceaccountoperatortests "github.com/openshift/origin/pkg/monitortests/authentication/nodefaultserviceaccountoperatortests" + "github.com/openshift/origin/pkg/monitortests/authentication/rbacadminescalationtests" "github.com/openshift/origin/pkg/monitortests/authentication/requiredsccmonitortests" admupgradestatus "github.com/openshift/origin/pkg/monitortests/cli/adm_upgrade/status" azuremetrics "github.com/openshift/origin/pkg/monitortests/cloud/azure/metrics" @@ -126,6 +127,7 @@ func newDefaultMonitorTests(info monitortestframework.MonitorTestInitializationI // Authentication monitorTestRegistry.AddMonitorTestOrDie("legacy-authentication-invariants", "apiserver-auth", legacyauthenticationmonitortests.NewLegacyTests()) monitorTestRegistry.AddMonitorTestOrDie("no-default-service-account-operator-checker", "oauth-apiserver", nodefaultserviceaccountoperatortests.NewAnalyzer()) + monitorTestRegistry.AddMonitorTestOrDie("rbac-cluster-admin-escalation-checker", "kube-apiserver", rbacadminescalationtests.NewAnalyzer()) // Cluster Version Operator monitorTestRegistry.AddMonitorTestOrDie("operator-state-analyzer", "Cluster Version Operator", operatorstateanalyzer.NewAnalyzer()) diff --git a/pkg/monitortests/authentication/rbacadminescalationtests/OWNERS b/pkg/monitortests/authentication/rbacadminescalationtests/OWNERS new file mode 100644 index 000000000000..cb58d4e18d24 --- /dev/null +++ b/pkg/monitortests/authentication/rbacadminescalationtests/OWNERS @@ -0,0 +1,10 @@ +# See the OWNERS docs at https://go.k8s.io/owners + +options: + no_parent_owners: true + +approvers: +- JoelSpeed +- sdodson +- dgoodwin +- enxebre diff --git a/pkg/monitortests/authentication/rbacadminescalationtests/monitortest.go b/pkg/monitortests/authentication/rbacadminescalationtests/monitortest.go new file mode 100644 index 000000000000..68339e89e6d3 --- /dev/null +++ b/pkg/monitortests/authentication/rbacadminescalationtests/monitortest.go @@ -0,0 +1,752 @@ +package rbacadminescalationtests + +import ( + "context" + "fmt" + "sort" + "strings" + "time" + + "github.com/openshift/origin/pkg/monitor/monitorapi" + "github.com/openshift/origin/pkg/monitortestframework" + "github.com/openshift/origin/pkg/test/ginkgo/junitapi" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + rbacvalidation "k8s.io/component-helpers/auth/rbac/validation" + rbacv1helpers "k8s.io/kubernetes/pkg/apis/rbac/v1" + "k8s.io/kubernetes/pkg/registry/rbac/validation" +) + +type escalationChecker struct { + kubeClient kubernetes.Interface +} + +func NewAnalyzer() monitortestframework.MonitorTest { + return &escalationChecker{} +} + +// bindingException describes an approved escalation grant. An exception applies only when the actual +// binding matches the approved grant exactly: same name, same referenced role, and the same set of +// subjects. Pinning roleRef and subjects means that repointing an allowlisted binding at a different +// role, or granting it to a new subject, revokes the exemption and re-triggers the finding for +// re-review — the exemption covers the grant we reviewed, not just the name. +type bindingException struct { + name string + checkID string + roleRef string + subjects []rbacv1.Subject + // note is a tracking Jira for a tracked exception, or a rationale for a permanent one. + note string +} + +// matches reports whether the given (binding, check) pair is exactly the approved grant. +func (e bindingException) matches(binding rbacv1.ClusterRoleBinding, checkID string) bool { + return e.checkID == checkID && + e.name == binding.Name && + e.roleRef == binding.RoleRef.Name && + subjectSet(e.subjects).Equal(subjectSet(binding.Subjects)) +} + +// trackedExceptions are approved escalation grants that are known issues we intend to fix. Each is +// paired with a tracking Jira. These flake (fail + pass) rather than hard-failing, so they stay +// visible in CI and can be burned down. +// +// No new entries should be added to this list without the sign off of an OpenShift Architect. +var trackedExceptions = []bindingException{ + { + name: "cloud-credential-operator-rolebinding", + checkID: "admission-webhooks", + roleRef: "cloud-credential-operator-role", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-cloud-credential-operator", Name: "cloud-credential-operator"}}, + note: "TODO", + }, + { + name: "cloud-credential-operator-rolebinding", + checkID: "escalate-rbac", + roleRef: "cloud-credential-operator-role", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-cloud-credential-operator", Name: "cloud-credential-operator"}}, + note: "TODO", + }, + { + name: "cluster-autoscaler-operator", + checkID: "admission-webhooks", + roleRef: "cluster-autoscaler-operator", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-machine-api", Name: "cluster-autoscaler-operator"}}, + note: "TODO", + }, + { + name: "cluster-baremetal-operator", + checkID: "admission-webhooks", + roleRef: "cluster-baremetal-operator", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-machine-api", Name: "cluster-baremetal-operator"}}, + note: "TODO", + }, + { + name: "cluster-monitoring-operator", + checkID: "admission-webhooks", + roleRef: "cluster-monitoring-operator", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-monitoring", Name: "cluster-monitoring-operator"}}, + note: "TODO", + }, + { + name: "cluster-network-operator", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-network-operator", Name: "cluster-network-operator"}}, + note: "TODO", + }, + { + name: "cluster-olm-operator-role", + checkID: "admission-webhooks", + roleRef: "cluster-olm-operator", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-cluster-olm-operator", Name: "cluster-olm-operator"}}, + note: "TODO", + }, + { + name: "cluster-olm-operator-role", + checkID: "escalate-rbac", + roleRef: "cluster-olm-operator", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-cluster-olm-operator", Name: "cluster-olm-operator"}}, + note: "TODO", + }, + { + name: "cluster-storage-operator-role", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-cluster-storage-operator", Name: "cluster-storage-operator"}}, + note: "TODO", + }, + { + name: "cluster-version-operator-1", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-cluster-version", Name: "cluster-version-operator"}}, + note: "TODO", + }, + { + name: "custom-account-openshift-machine-config-operator", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-machine-config-operator", Name: "machine-config-operator"}}, + note: "TODO", + }, + { + name: "machine-api-operator", + checkID: "admission-webhooks", + roleRef: "machine-api-operator", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-machine-api", Name: "machine-api-operator"}}, + note: "TODO", + }, + { + name: "olm-operator-binding-openshift-operator-lifecycle-manager", + checkID: "admission-webhooks", + roleRef: "system:controller:operator-lifecycle-manager", + 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: "escalate-rbac", + 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: "impersonate", + roleRef: "openshift-dns-operator", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-dns-operator", Name: "dns-operator"}}, + note: "TODO", + }, + { + name: "openshift-ingress-operator", + checkID: "impersonate", + roleRef: "openshift-ingress-operator", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-ingress-operator", Name: "ingress-operator"}}, + note: "TODO", + }, + { + name: "openshift-ingress-operator-sail-library", + checkID: "admission-webhooks", + roleRef: "openshift-ingress-operator-sail-library", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-ingress-operator", Name: "ingress-operator"}}, + note: "TODO", + }, + { + name: "openshift-ingress-operator-sail-library", + checkID: "escalate-rbac", + roleRef: "openshift-ingress-operator-sail-library", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-ingress-operator", Name: "ingress-operator"}}, + note: "TODO", + }, + { + name: "operator-controller-cluster-admin-rolebinding", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-operator-controller", Name: "operator-controller-controller-manager"}}, + note: "TODO", + }, + { + name: "storage-version-migration-migrator", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-kube-storage-version-migrator", Name: "kube-storage-version-migrator-sa"}}, + note: "TODO", + }, + { + name: "system:controller:clusterrole-aggregation-controller", + checkID: "escalate-rbac", + roleRef: "system:controller:clusterrole-aggregation-controller", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "kube-system", Name: "clusterrole-aggregation-controller"}}, + note: "TODO", + }, + { + name: "system:controller:generic-garbage-collector", + checkID: "admission-webhooks", + roleRef: "system:controller:generic-garbage-collector", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "kube-system", Name: "generic-garbage-collector"}}, + note: "TODO", + }, + { + name: "system:openshift:controller:service-ca", + checkID: "admission-webhooks", + roleRef: "system:openshift:controller:service-ca", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-service-ca", Name: "service-ca"}}, + note: "TODO", + }, + { + name: "system:openshift:controller:template-instance-controller:admin", + checkID: "impersonate", + roleRef: "admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-infra", Name: "template-instance-controller"}}, + note: "TODO", + }, + { + name: "system:openshift:controller:template-instance-finalizer-controller:admin", + checkID: "impersonate", + roleRef: "admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-infra", Name: "template-instance-finalizer-controller"}}, + note: "TODO", + }, + { + name: "system:openshift:oauth-apiserver", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-oauth-apiserver", Name: "oauth-apiserver-sa"}}, + note: "TODO", + }, + { + name: "system:openshift:openshift-apiserver", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-apiserver", Name: "openshift-apiserver-sa"}}, + note: "TODO", + }, + { + name: "system:openshift:openshift-authentication", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-authentication", Name: "oauth-openshift"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:authentication", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-authentication-operator", Name: "authentication-operator"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:cluster-kube-scheduler-operator", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-kube-scheduler-operator", Name: "openshift-kube-scheduler-operator"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:etcd-operator", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-etcd-operator", Name: "etcd-operator"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:kube-apiserver-operator", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-kube-apiserver-operator", Name: "kube-apiserver-operator"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:kube-apiserver-recovery", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-kube-apiserver", Name: "localhost-recovery-client"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:kube-controller-manager-operator", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-kube-controller-manager-operator", Name: "kube-controller-manager-operator"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:kube-controller-manager-recovery", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-kube-controller-manager", Name: "localhost-recovery-client"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:kube-scheduler-recovery", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-kube-scheduler", Name: "localhost-recovery-client"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:kube-storage-version-migrator-operator", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-kube-storage-version-migrator-operator", Name: "kube-storage-version-migrator-operator"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:openshift-apiserver-operator", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-apiserver-operator", Name: "openshift-apiserver-operator"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:openshift-config-operator", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-config-operator", Name: "openshift-config-operator"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:openshift-controller-manager-operator", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-controller-manager-operator", Name: "openshift-controller-manager-operator"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:openshift-etcd-installer", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-etcd", Name: "installer-sa"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:openshift-kube-apiserver-installer", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-kube-apiserver", Name: "installer-sa"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:openshift-kube-controller-manager-installer", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-kube-controller-manager", Name: "installer-sa"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:openshift-kube-scheduler-installer", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-kube-scheduler", Name: "installer-sa"}}, + note: "TODO", + }, + { + name: "system:openshift:operator:service-ca-operator", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-service-ca-operator", Name: "service-ca-operator"}}, + note: "TODO", + }, + { + name: "vmware-vsphere-csi-driver-operator-clusterrolebinding", + checkID: "admission-webhooks", + roleRef: "vmware-vsphere-csi-driver-operator-clusterrole", + subjects: []rbacv1.Subject{{Kind: "ServiceAccount", Namespace: "openshift-cluster-csi-drivers", Name: "vmware-vsphere-csi-driver-operator"}}, + note: "TODO", + }, +} + +// permanentExceptions are approved escalation grants that are legitimate and expected forever. These +// are silently accepted: they produce no JUnit result at all. Reserve this list for grants that are +// correct by design and will never be "fixed". +// +// The canonical cluster-admin -> system:masters binding is not listed here: its only subject is a +// cluster-wide Group, so bindingInScope already excludes it (and every other non-namespaced grant). +// +// No new entries should be added to this list without the sign off of an OpenShift Architect. +var permanentExceptions = []bindingException{} + +// escalationCheck is a single way in which a ClusterRoleBinding can hand a subject a path to +// cluster-admin. Each check becomes its own JUnit test (and its own independently flakeable +// exception) so that allowing one binding to hold one dangerous permission does not silently waive +// every other escalation check for that binding. +type escalationCheck struct { + // id is the stable slug used to key exceptions, e.g. "escalate-rbac". + id string + // desc is the human phrase embedded in the test name, e.g. "escalate or bind RBAC roles". + desc string + // rules are the escalation-enabling permissions. The check fires when the bound role grants ANY + // atom of these rules. + rules []rbacv1.PolicyRule +} + +// clusterAdminCheckID is the id of the full cluster-admin check. It must be listed first in +// escalationChecks so that its short-circuit (skip the subsumed, more specific checks) works. +const clusterAdminCheckID = "cluster-admin" + +// clusterAdminRoleName is the name of the built-in cluster-admin ClusterRole. It is the role whose +// `bind` we treat as an escalation path (see the escalate-rbac check). +const clusterAdminRoleName = "cluster-admin" + +// escalationChecks is the curated, tunable set of escalation paths we audit. Broadening this set +// (e.g. cluster-wide secrets read, pod/exec, node proxy) will expand findings and the allowlist, so +// it is deliberately conservative. +var escalationChecks = []escalationCheck{ + { + // All verbs on all resources in all API groups IS cluster-admin: the subject can read every + // secret, mutate any object, and grant itself anything. This is the direct, definitional case + // rather than a path to escalation. + id: clusterAdminCheckID, + desc: "cluster-admin equivalent access (all verbs on all resources)", + rules: []rbacv1.PolicyRule{ + {Verbs: []string{rbacv1.VerbAll}, APIGroups: []string{rbacv1.APIGroupAll}, Resources: []string{rbacv1.ResourceAll}}, + }, + }, + { + // The `escalate` and `bind` verbs are the two ways to bypass Kubernetes' built-in RBAC + // escalation prevention. Plain create/update/patch on clusterroles/clusterrolebindings does + // NOT allow escalation on its own: the rbac policybased storage strategy runs + // ConfirmNoEscalation and rejects writing/binding any permission the caller does not already + // hold, unless the caller has `escalate` (for role rules) or `bind` (for a role reference). + // + // Unlike `escalate`, `bind` is only an escalation path when it can reference a role more + // powerful than the caller. `bind` scoped by resourceNames to a set of roles that does NOT + // include cluster-admin cannot grant anything those roles do not already hold, so it is not + // flagged. Restricting the bind atom to the cluster-admin resourceName makes Covers fire for an + // unrestricted bind (which can bind cluster-admin) and for a bind explicitly scoped to include + // cluster-admin, but not for a bind scoped to unrelated roles. + id: "escalate-rbac", + desc: "escalate or bind RBAC roles", + rules: []rbacv1.PolicyRule{ + rbacv1helpers.NewRule("escalate").Groups(rbacv1.GroupName).Resources("clusterroles", "roles").RuleOrDie(), + rbacv1helpers.NewRule("bind").Groups(rbacv1.GroupName).Resources("clusterroles", "roles").Names(clusterAdminRoleName).RuleOrDie(), + }, + }, + { + // Impersonation lets the subject send requests AS another user or group without needing that + // identity's credentials. Impersonating the `system:masters` group (or any cluster-admin-bound + // user/service account) yields cluster-admin immediately, so this bypasses RBAC entirely. + id: "impersonate", + desc: "impersonate users, groups, or service accounts", + rules: []rbacv1.PolicyRule{ + rbacv1helpers.NewRule("impersonate").Groups("").Resources("users", "groups", "serviceaccounts").RuleOrDie(), + }, + }, + { + // Webhook configurations intercept every write to the API server. Whoever can create or modify + // them can point a webhook at their own endpoint to read the contents of (and, for mutating + // webhooks, rewrite) any admitted object cluster-wide — including secrets and RBAC objects — + // giving them an out-of-band path to cluster-admin. + // + // update/patch scoped to specific resourceNames are NOT an escalation path and are not + // reported: a subject that can only modify a named webhook config it already owns cannot reach + // arbitrary objects, and rbacvalidation.Covers already treats such resourceName-restricted + // rules as not covering the unscoped atoms below (so they never fire). create, on the other + // hand, is always reported even when scoped by resourceName: the API server ignores + // resourceNames for create, so the restriction is ineffective and the subject can still mint an + // arbitrary malicious webhook (see roleGrantsAny / resourceNameIneffectiveVerbs). + id: "admission-webhooks", + desc: "create or modify admission webhook configurations", + rules: []rbacv1.PolicyRule{ + rbacv1helpers.NewRule("create", "update", "patch").Groups("admissionregistration.k8s.io").Resources("mutatingwebhookconfigurations", "validatingwebhookconfigurations").RuleOrDie(), + }, + }, + { + // Control over certificate issuance is control over identity. A subject that can approve a CSR + // and sign it (via the kubernetes.io signers) can mint a client certificate for an arbitrary + // username and group — e.g. group `system:masters` — and authenticate as cluster-admin. + id: "csr-signing", + desc: "approve or sign certificate signing requests", + rules: []rbacv1.PolicyRule{ + rbacv1helpers.NewRule("approve").Groups("certificates.k8s.io").Resources("certificatesigningrequests").RuleOrDie(), + rbacv1helpers.NewRule("sign").Groups("certificates.k8s.io").Resources("signers").RuleOrDie(), + }, + }, + // Intentionally omitted for noise: `create` on pods/pods/exec (token theft via any mounted SA). + // Add here if the escalation surface should be widened. +} + +// matchException reports whether the given (binding, check) pair matches any approved grant in the +// provided list, returning the associated note. +func matchException(list []bindingException, binding rbacv1.ClusterRoleBinding, checkID string) (string, bool) { + for _, exception := range list { + if exception.matches(binding, checkID) { + return exception.note, true + } + } + return "", false +} + +// resourceNameIneffectiveVerbs are verbs for which the API server ignores a rule's resourceNames. +// The restriction is silently ineffective, so a rule that appears to "scope" one of these verbs in +// fact grants it on every object of the resource. Per the Kubernetes RBAC docs, create and +// deletecollection cannot be restricted by resourceName (for create, the object name is not known at +// authorization time). +var resourceNameIneffectiveVerbs = sets.New("create", "deletecollection") + +// verbIgnoresResourceNames reports whether the atom's verb is one whose resourceNames the API server +// ignores. Atoms come from BreakdownRule, so each has exactly one verb. +func verbIgnoresResourceNames(verbs []string) bool { + return resourceNameIneffectiveVerbs.HasAny(verbs...) +} + +// stripResourceNames returns a copy of rules with ResourceNames cleared. Used to test coverage of a +// verb whose resourceNames the API server ignores, so that an ineffective restriction cannot hide an +// escalation path from rbacvalidation.Covers (which otherwise treats a resourceName-scoped owner rule +// as not covering the unscoped atom). +func stripResourceNames(rules []rbacv1.PolicyRule) []rbacv1.PolicyRule { + out := make([]rbacv1.PolicyRule, len(rules)) + for i, r := range rules { + r.ResourceNames = nil + out[i] = r + } + return out +} + +// roleGrantsAny reports whether roleRules cover ANY atomic permission in servantRules, returning the +// matched atoms for reporting. Coverage is wildcard-aware (handled by rbacvalidation.Covers). For +// verbs whose resourceNames the API server ignores (create, deletecollection), the role's rules are +// compared with resourceNames stripped so an ineffective restriction is still flagged. +func roleGrantsAny(roleRules, servantRules []rbacv1.PolicyRule) (bool, []rbacv1.PolicyRule) { + matched := []rbacv1.PolicyRule{} + for _, servantRule := range servantRules { + for _, atom := range rbacvalidation.BreakdownRule(servantRule) { + candidateRules := roleRules + if verbIgnoresResourceNames(atom.Verbs) { + candidateRules = stripResourceNames(roleRules) + } + if covered, _ := rbacvalidation.Covers(candidateRules, []rbacv1.PolicyRule{atom}); covered { + matched = append(matched, atom) + } + } + } + return len(matched) > 0, matched +} + +// subjectSet returns a canonical, order-insensitive set of a binding's subjects for exact matching. +// APIGroup is included so that, e.g., a User and a ServiceAccount of the same name are distinct. +func subjectSet(subjects []rbacv1.Subject) sets.Set[string] { + out := sets.New[string]() + for _, s := range subjects { + out.Insert(fmt.Sprintf("%s/%s/%s/%s", s.APIGroup, s.Kind, s.Namespace, s.Name)) + } + return out +} + +// subjectString renders a binding's subjects for inclusion in a failure message. +func subjectString(subjects []rbacv1.Subject) string { + rendered := sets.NewString() + for _, s := range subjects { + if s.Namespace != "" { + rendered.Insert(fmt.Sprintf("%s/%s/%s", s.Kind, s.Namespace, s.Name)) + continue + } + rendered.Insert(fmt.Sprintf("%s/%s", s.Kind, s.Name)) + } + return strings.Join(rendered.List(), ", ") +} + +// rulesToString renders matched permissions in a compact, human-readable form. +func rulesToString(rules []rbacv1.PolicyRule) string { + compactRules := rules + if compact, err := validation.CompactRules(rules); err == nil { + compactRules = compact + } + descriptions := sets.NewString() + for _, rule := range compactRules { + descriptions.Insert(rbacv1helpers.CompactString(rule)) + } + return strings.Join(descriptions.List(), "\n") +} + +// coreNamespacePrefixes are the namespaces that hold core cluster components. We only audit bindings +// that grant to a ServiceAccount in one of these namespaces. +var coreNamespacePrefixes = []string{"kube-", "openshift-"} + +// bindingInScope reports whether the binding grants to at least one ServiceAccount in a core +// namespace (prefixed kube- or openshift-). Bindings that only grant to subjects outside those +// namespaces are out of scope: transient e2e test namespaces come and go with random names (so an +// allowlist entry could never match), and cluster-wide groups/users (e.g. system:masters) are not +// namespaced. Restricting to core namespaces keeps the audit focused on the payload's own +// components. +func bindingInScope(binding rbacv1.ClusterRoleBinding) bool { + for _, subject := range binding.Subjects { + if subject.Kind != rbacv1.ServiceAccountKind { + continue + } + for _, prefix := range coreNamespacePrefixes { + if strings.HasPrefix(subject.Namespace, prefix) { + return true + } + } + } + return false +} + +// 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: +// - permanent exception: emit nothing (legitimate, by-design grant). +// - tracked exception: emit a fail plus a passing duplicate (a flake) so it stays visible. +// - no exception: emit a hard fail. +func evaluateBinding(binding rbacv1.ClusterRoleBinding, rolesByName map[string][]rbacv1.PolicyRule) []*junitapi.JUnitTestCase { + // Only audit bindings that grant to a core-component ServiceAccount (see bindingInScope). + if !bindingInScope(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] + if !ok { + return nil + } + + junits := []*junitapi.JUnitTestCase{} + for _, check := range escalationChecks { + hit, matched := roleGrantsAny(roleRules, check.rules) + if !hit { + continue + } + + // A full cluster-admin grant covers every more specific escalation check, so evaluating those + // as well would be redundant noise. Whatever we decide for the cluster-admin check, we must + // stop after it — including when it is silently excepted, otherwise the subsumed checks would + // fire and hard-fail. Track that decision here and honor it at the end of the iteration. + isClusterAdmin := check.id == clusterAdminCheckID + + // Permanent exceptions are silently accepted: emit nothing at all. + if _, ok := matchException(permanentExceptions, binding, check.id); ok { + if isClusterAdmin { + break + } + continue + } + + testName := fmt.Sprintf("[sig-auth] clusterrolebinding %q must not grant permission to %s", binding.Name, check.desc) + msg := fmt.Sprintf("clusterrolebinding %q (clusterrole %q) grants permission to %s to subjects [%s] via:\n%s", + binding.Name, binding.RoleRef.Name, check.desc, subjectString(binding.Subjects), rulesToString(matched)) + + jira, tracked := matchException(trackedExceptions, binding, check.id) + if tracked { + msg += fmt.Sprintf("\n(tracked exception: %s)", jira) + + // Temporarily downgrade a tracked exception so that it doesn't produce any JUnit output. + // This will allow us to see only untracked exceptions in sippy and eventually work out whether + // we have caught all exceptions that need to be tracked. + continue + } + + junits = append(junits, &junitapi.JUnitTestCase{ + Name: testName, + SystemOut: msg, + FailureOutput: &junitapi.FailureOutput{Output: msg}, + }) + + // Tracked exceptions flake rather than hard-fail: emit a passing duplicate with the same name. + // Temporarily dowgrading all findings to flakes while we determine if we have caught all exceptions + // that need to be tracked. + // if tracked { + junits = append(junits, &junitapi.JUnitTestCase{Name: testName}) + //} + + if isClusterAdmin { + break + } + } + return junits +} + +// CollectData implements monitortestframework.MonitorTest. +func (e *escalationChecker) CollectData(ctx context.Context, storageDir string, beginning time.Time, end time.Time) (monitorapi.Intervals, []*junitapi.JUnitTestCase, error) { + if e.kubeClient == nil { + return nil, nil, nil + } + + clusterRoles, err := e.kubeClient.RbacV1().ClusterRoles().List(ctx, metav1.ListOptions{}) + if err != nil { + return nil, nil, err + } + rolesByName := make(map[string][]rbacv1.PolicyRule, len(clusterRoles.Items)) + for _, role := range clusterRoles.Items { + rolesByName[role.Name] = role.Rules + } + + bindings, err := e.kubeClient.RbacV1().ClusterRoleBindings().List(ctx, metav1.ListOptions{}) + if err != nil { + return nil, nil, err + } + // Sort for deterministic ordering of emitted test cases. + sort.Slice(bindings.Items, func(i, j int) bool { + return bindings.Items[i].Name < bindings.Items[j].Name + }) + + junits := []*junitapi.JUnitTestCase{} + for _, binding := range bindings.Items { + // Dynamic (generateName) binding names would break static test naming; skip them. + if binding.GenerateName != "" { + continue + } + junits = append(junits, evaluateBinding(binding, rolesByName)...) + } + return nil, junits, nil +} + +// StartCollection implements monitortestframework.MonitorTest. +func (e *escalationChecker) StartCollection(ctx context.Context, adminRESTConfig *rest.Config, recorder monitorapi.RecorderWriter) error { + var err error + e.kubeClient, err = kubernetes.NewForConfig(adminRESTConfig) + if err != nil { + return err + } + return nil +} + +// PrepareCollection implements monitortestframework.MonitorTest. +func (e *escalationChecker) PrepareCollection(ctx context.Context, adminRESTConfig *rest.Config, recorder monitorapi.RecorderWriter) error { + return nil +} + +// ConstructComputedIntervals implements monitortestframework.MonitorTest. +func (e *escalationChecker) ConstructComputedIntervals(ctx context.Context, startingIntervals monitorapi.Intervals, recordedResources monitorapi.ResourcesMap, beginning time.Time, end time.Time) (monitorapi.Intervals, error) { + return nil, nil +} + +// EvaluateTestsFromConstructedIntervals implements monitortestframework.MonitorTest. +func (e *escalationChecker) EvaluateTestsFromConstructedIntervals(ctx context.Context, finalIntervals monitorapi.Intervals) ([]*junitapi.JUnitTestCase, error) { + return nil, nil +} + +// WriteContentToStorage implements monitortestframework.MonitorTest. +func (e *escalationChecker) WriteContentToStorage(ctx context.Context, storageDir string, timeSuffix string, finalIntervals monitorapi.Intervals, finalResourceState monitorapi.ResourcesMap) error { + return nil +} + +// Cleanup implements monitortestframework.MonitorTest. +func (e *escalationChecker) Cleanup(ctx context.Context) error { + return nil +} diff --git a/pkg/monitortests/authentication/rbacadminescalationtests/monitortest_test.go b/pkg/monitortests/authentication/rbacadminescalationtests/monitortest_test.go new file mode 100644 index 000000000000..5cbc804fbfdf --- /dev/null +++ b/pkg/monitortests/authentication/rbacadminescalationtests/monitortest_test.go @@ -0,0 +1,323 @@ +package rbacadminescalationtests + +import ( + "strings" + "testing" + + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func binding(name, roleName string, subjects ...rbacv1.Subject) rbacv1.ClusterRoleBinding { + return rbacv1.ClusterRoleBinding{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + RoleRef: rbacv1.RoleRef{Kind: "ClusterRole", Name: roleName}, + Subjects: subjects, + } +} + +func rule(verbs, groups, resources []string) rbacv1.PolicyRule { + return rbacv1.PolicyRule{Verbs: verbs, APIGroups: groups, Resources: resources} +} + +func ruleWithNames(verbs, groups, resources, resourceNames []string) rbacv1.PolicyRule { + return rbacv1.PolicyRule{Verbs: verbs, APIGroups: groups, Resources: resources, ResourceNames: resourceNames} +} + +// checkID extracts the escalation check that a test case name corresponds to by matching against the +// known checks' descriptions. +func checkIDForName(name string) string { + for _, c := range escalationChecks { + if strings.Contains(name, c.desc) { + return c.id + } + } + return "" +} + +func TestEvaluateBinding(t *testing.T) { + // Subjects must be ServiceAccounts in a core (kube-/openshift-) namespace to be in scope. + saSubject := rbacv1.Subject{Kind: "ServiceAccount", Namespace: "openshift-ns", Name: "sa"} + permSubject := rbacv1.Subject{Kind: "ServiceAccount", Namespace: "openshift-perm", Name: "perm-sa"} + + // Seed a tracked exception for the duration of this test so we can exercise the flake path. It + // pins the exact grant: name, check, roleRef, and subjects. + originalTracked := trackedExceptions + trackedExceptions = append(append([]bindingException{}, originalTracked...), bindingException{ + name: "tracked-esc", + checkID: "escalate-rbac", + roleRef: "escalate-role", + subjects: []rbacv1.Subject{saSubject}, + note: "https://issues.redhat.com/browse/EXAMPLE-1", + }) + defer func() { trackedExceptions = originalTracked }() + + // Seed a permanent exception (in-scope, so it is reached) to exercise the silent-accept path. + originalPermanent := permanentExceptions + permanentExceptions = append(append([]bindingException{}, originalPermanent...), bindingException{ + name: "perm-admin", + checkID: "cluster-admin", + roleRef: "cluster-admin", + subjects: []rbacv1.Subject{permSubject}, + note: "by-design", + }) + defer func() { permanentExceptions = originalPermanent }() + + clusterAdminRule := rule([]string{"*"}, []string{"*"}, []string{"*"}) + escalateRule := rbacv1.PolicyRule{Verbs: []string{"escalate"}, APIGroups: []string{rbacv1.GroupName}, Resources: []string{"clusterroles"}} + impersonateRule := rbacv1.PolicyRule{Verbs: []string{"impersonate"}, APIGroups: []string{""}, Resources: []string{"users"}} + readOnlyRule := rule([]string{"get", "list", "watch"}, []string{""}, []string{"pods"}) + + webhookGroup := []string{"admissionregistration.k8s.io"} + webhookResources := []string{"validatingwebhookconfigurations", "mutatingwebhookconfigurations"} + + tests := []struct { + name string + binding rbacv1.ClusterRoleBinding + rolesByName map[string][]rbacv1.PolicyRule + wantCheckIDs []string // expected failing check ids, in order + wantFlakeChecks map[string]bool + }{ + { + name: "direct cluster-admin", + binding: binding("some-admin", "admin-role", saSubject), + rolesByName: map[string][]rbacv1.PolicyRule{"admin-role": {clusterAdminRule}}, + wantCheckIDs: []string{"cluster-admin"}, + wantFlakeChecks: map[string]bool{"cluster-admin": true}, // Temporary + }, + { + name: "escalate rbac", + binding: binding("escalator", "escalate-role", saSubject), + rolesByName: map[string][]rbacv1.PolicyRule{"escalate-role": {escalateRule}}, + wantCheckIDs: []string{"escalate-rbac"}, + wantFlakeChecks: map[string]bool{"escalate-rbac": true}, // Temporary + }, + { + name: "impersonate", + binding: binding("imp", "imp-role", saSubject), + rolesByName: map[string][]rbacv1.PolicyRule{"imp-role": {impersonateRule}}, + wantCheckIDs: []string{"impersonate"}, + wantFlakeChecks: map[string]bool{"impersonate": true}, // Temporary + }, + { + name: "two checks tripped", + binding: binding("multi", "multi-role", saSubject), + rolesByName: map[string][]rbacv1.PolicyRule{"multi-role": {escalateRule, impersonateRule}}, + // escalate-rbac precedes impersonate in escalationChecks ordering. + wantCheckIDs: []string{"escalate-rbac", "impersonate"}, + wantFlakeChecks: map[string]bool{"escalate-rbac": true, "impersonate": true}, // Temporary + }, + { + name: "benign role emits nothing", + binding: binding("benign", "read-role", saSubject), + rolesByName: map[string][]rbacv1.PolicyRule{"read-role": {readOnlyRule}}, + wantCheckIDs: nil, + }, + { + name: "missing role reference emits nothing", + binding: binding("dangling", "does-not-exist", saSubject), + rolesByName: map[string][]rbacv1.PolicyRule{}, + wantCheckIDs: nil, + }, + { + // A binding that only grants to a ServiceAccount outside the core namespaces (e.g. a + // transient e2e test namespace) is out of scope and emits nothing. + name: "out-of-scope namespace emits nothing", + binding: binding("e2e-test-thing", "admin-role", rbacv1.Subject{Kind: "ServiceAccount", Namespace: "e2e-test-thing", Name: "sa"}), + rolesByName: map[string][]rbacv1.PolicyRule{"admin-role": {clusterAdminRule}}, + wantCheckIDs: nil, + }, + { + // A binding whose only subject is a cluster-wide group (no namespace) is out of scope. + name: "group-only subject emits nothing", + binding: binding("group-admin", "admin-role", rbacv1.Subject{Kind: "Group", APIGroup: rbacv1.GroupName, Name: "system:masters"}), + rolesByName: map[string][]rbacv1.PolicyRule{"admin-role": {clusterAdminRule}}, + wantCheckIDs: nil, + }, + { + // A permanent exception (in scope) is silently accepted: no JUnit result at all, and the + // cluster-admin short-circuit still suppresses the subsumed checks. + name: "permanent exception emits nothing", + binding: binding("perm-admin", "cluster-admin", permSubject), + rolesByName: map[string][]rbacv1.PolicyRule{"cluster-admin": {clusterAdminRule}}, + wantCheckIDs: nil, + }, + { + // A new subject on the permanently-excepted binding no longer matches the approved grant, + // so the exemption is revoked and the finding hard-fails. + name: "permanent exception revoked by new subject", + binding: binding("perm-admin", "cluster-admin", + permSubject, + rbacv1.Subject{Kind: "ServiceAccount", Namespace: "openshift-ns", Name: "sneaky"}), + rolesByName: map[string][]rbacv1.PolicyRule{"cluster-admin": {clusterAdminRule}}, + wantCheckIDs: []string{"cluster-admin"}, + wantFlakeChecks: map[string]bool{"cluster-admin": true}, // Temporary + }, + { + // Modifying webhook configs scoped to a specific resourceName cannot be used to point a + // webhook at an arbitrary target, so it is not an escalation path and emits nothing. + name: "webhook update/patch scoped by resourceName emits nothing", + binding: binding("scoped-webhook", "scoped-webhook-role", saSubject), + rolesByName: map[string][]rbacv1.PolicyRule{"scoped-webhook-role": {ruleWithNames([]string{"update", "patch"}, webhookGroup, webhookResources, []string{"my-webhook"})}}, + wantCheckIDs: nil, + }, + { + // resourceNames are ineffective for create (the API server ignores them), so a role that + // appears to "restrict" create on webhook configs to a name in fact grants create on all of + // them and is still an escalation path. + name: "webhook create scoped by resourceName still fires", + binding: binding("scoped-create-webhook", "scoped-create-webhook-role", saSubject), + rolesByName: map[string][]rbacv1.PolicyRule{"scoped-create-webhook-role": {ruleWithNames([]string{"create"}, webhookGroup, webhookResources, []string{"my-webhook"})}}, + wantCheckIDs: []string{"admission-webhooks"}, + wantFlakeChecks: map[string]bool{"admission-webhooks": true}, // Temporary + }, + { + // Unrestricted create on webhook configs is always an escalation path (create cannot be + // scoped by resourceName), so it fires even when update/patch are scoped. + name: "webhook unrestricted create fires despite scoped update/patch", + binding: binding("open-create-webhook", "open-create-webhook-role", saSubject), + rolesByName: map[string][]rbacv1.PolicyRule{"open-create-webhook-role": { + rule([]string{"create"}, webhookGroup, webhookResources), + ruleWithNames([]string{"update", "patch"}, webhookGroup, webhookResources, []string{"my-webhook"}), + }}, + wantCheckIDs: []string{"admission-webhooks"}, + wantFlakeChecks: map[string]bool{"admission-webhooks": true}, // Temporary + }, + { + // Unrestricted bind can reference the cluster-admin role, so it is an escalation path. + name: "unrestricted bind fires", + binding: binding("binder", "bind-role", saSubject), + rolesByName: map[string][]rbacv1.PolicyRule{"bind-role": {rule([]string{"bind"}, []string{rbacv1.GroupName}, []string{"clusterroles"})}}, + wantCheckIDs: []string{"escalate-rbac"}, + wantFlakeChecks: map[string]bool{"escalate-rbac": true}, // Temporary + }, + { + // bind scoped by resourceName to the cluster-admin role is a direct path to cluster-admin. + name: "bind scoped to cluster-admin fires", + binding: binding("ca-binder", "ca-bind-role", saSubject), + rolesByName: map[string][]rbacv1.PolicyRule{"ca-bind-role": {ruleWithNames([]string{"bind"}, []string{rbacv1.GroupName}, []string{"clusterroles"}, []string{"cluster-admin"})}}, + wantCheckIDs: []string{"escalate-rbac"}, + wantFlakeChecks: map[string]bool{"escalate-rbac": true}, // Temporary + }, + { + // bind scoped to roles other than cluster-admin cannot grant more than those roles already + // hold, so it is not an escalation path and emits nothing. + name: "bind scoped to non-cluster-admin emits nothing", + binding: binding("scoped-binder", "scoped-bind-role", saSubject), + rolesByName: map[string][]rbacv1.PolicyRule{"scoped-bind-role": {ruleWithNames([]string{"bind"}, []string{rbacv1.GroupName}, []string{"clusterroles"}, []string{"view", "edit"})}}, + wantCheckIDs: nil, + }, + { + // A tracked exception flakes: one fail + one pass for that check. + name: "tracked exception flakes", + binding: binding("tracked-esc", "escalate-role", saSubject), + rolesByName: map[string][]rbacv1.PolicyRule{"escalate-role": {escalateRule}}, + // Temporarily disable these checks + // wantCheckIDs: []string{"escalate-rbac"}, + // wantFlakeChecks: map[string]bool{"escalate-rbac": true}, + }, + { + // Same tracked binding+check but repointed at a different escalating role: the roleRef no + // longer matches the approved grant, so it hard-fails instead of flaking. + name: "tracked exception revoked by roleref change", + binding: binding("tracked-esc", "other-escalate-role", saSubject), + rolesByName: map[string][]rbacv1.PolicyRule{"other-escalate-role": {escalateRule}}, + wantCheckIDs: []string{"escalate-rbac"}, + wantFlakeChecks: map[string]bool{"escalate-rbac": true}, // Temporary + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + junits := evaluateBinding(tc.binding, tc.rolesByName) + + // Collect the failing cases (non-nil FailureOutput) and passing cases (nil) per name. + failsByName := map[string]int{} + passesByName := map[string]int{} + failOrder := []string{} + for _, j := range junits { + if j.FailureOutput != nil { + failsByName[j.Name]++ + failOrder = append(failOrder, checkIDForName(j.Name)) + } else { + passesByName[j.Name]++ + } + } + + if len(failOrder) != len(tc.wantCheckIDs) { + t.Fatalf("expected %d failing checks %v, got %d: %v", len(tc.wantCheckIDs), tc.wantCheckIDs, len(failOrder), failOrder) + } + for i, want := range tc.wantCheckIDs { + if failOrder[i] != want { + t.Errorf("failing check %d: expected %q, got %q", i, want, failOrder[i]) + } + } + + // A flaked check has exactly one fail and one pass for the same name; a hard-fail has no + // matching pass. + for _, c := range escalationChecks { + name := "[sig-auth] clusterrolebinding \"" + tc.binding.Name + "\" must not grant permission to " + c.desc + wantFlake := tc.wantFlakeChecks[c.id] + if wantFlake { + if failsByName[name] != 1 || passesByName[name] != 1 { + t.Errorf("check %q expected flake (1 fail + 1 pass), got fail=%d pass=%d", c.id, failsByName[name], passesByName[name]) + } + continue + } + if passesByName[name] != 0 { + t.Errorf("check %q expected no passing (green) case, got %d", c.id, passesByName[name]) + } + } + }) + } +} + +// TestWebhookScopedVerbsNotReported pins the reporting granularity for the admission-webhooks check: +// update/patch scoped by resourceName are not an escalation path and must not appear in a finding, +// while an unrestricted create (which cannot be scoped) still is reported. +func TestWebhookScopedVerbsNotReported(t *testing.T) { + saSubject := rbacv1.Subject{Kind: "ServiceAccount", Namespace: "openshift-ns", Name: "sa"} + webhookGroup := []string{"admissionregistration.k8s.io"} + webhookResources := []string{"validatingwebhookconfigurations", "mutatingwebhookconfigurations"} + + b := binding("mixed-webhook", "mixed-webhook-role", saSubject) + rolesByName := map[string][]rbacv1.PolicyRule{"mixed-webhook-role": { + rule([]string{"create"}, webhookGroup, webhookResources), + ruleWithNames([]string{"update", "patch"}, webhookGroup, webhookResources, []string{"my-webhook"}), + }} + + junits := evaluateBinding(b, rolesByName) + + var failures []string + for _, j := range junits { + if j.FailureOutput != nil { + failures = append(failures, j.SystemOut) + } + } + if len(failures) != 1 { + t.Fatalf("expected exactly one failing case, got %d: %v", len(failures), failures) + } + msg := failures[0] + if !strings.Contains(msg, "create") { + t.Errorf("expected finding to report unrestricted create, got:\n%s", msg) + } + // The scoped update/patch grant must not surface as an escalation path. + if strings.Contains(msg, "update") || strings.Contains(msg, "patch") { + t.Errorf("finding must not report resourceName-scoped update/patch, got:\n%s", msg) + } +} + +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{"*"})} + servant := []rbacv1.PolicyRule{{Verbs: []string{"escalate"}, APIGroups: []string{rbacv1.GroupName}, Resources: []string{"clusterroles"}}} + if hit, matched := roleGrantsAny(wildcard, servant); !hit || len(matched) == 0 { + t.Errorf("expected wildcard role to grant escalate, got hit=%v matched=%v", hit, matched) + } + + // An unrelated role should not match. + unrelated := []rbacv1.PolicyRule{rule([]string{"get"}, []string{""}, []string{"configmaps"})} + if hit, _ := roleGrantsAny(unrelated, servant); hit { + t.Errorf("expected unrelated role not to grant escalate") + } +}