[BUG] request.clusterRoles variable not resolving when a RoleBinding references a ClusterRole

[chipzoller]

Software version numbers
State the version numbers of applications involved in the bug.

• Kubernetes version: 1.22.2, 1.23.3
• Kubernetes platform (if applicable; ex., EKS, GKE, OpenShift): K3d
• Kyverno version: 1.5.8, 1.6.0-rc3

Describe the bug
The variable request.clusterRoles is resolving to null when a ServiceAccount (Namespaced) is bound to a ClusterRole.

To Reproduce
Steps to reproduce the behavior:

1. Create a Namespace called test
2. Create a ServiceAccount inside test called developer
3. Create a ClusterRole

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  name: developer
rules:
- apiGroups:
  - '*'
  resources:
  - configmaps
  - cronjobs
  - daemonsets
  - deployments
  - endpoints
  - horizontalpodautoscalers
  - ingresses
  - jobs
  - networkpolicies
  - persistentvolumeclaims
  - pods
  - pods/exec
  - pods/portforward
  - poddisruptionbudgets
  - replicationcontrollers
  - roles
  - rolebindings
  - secrets
  - serviceaccounts
  - services
  - services/portforward
  - replicasets
  - replicasets/scale
  - statefulsets
  verbs:
  - '*'

4. Create a RoleBinding

apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
  name: developer
  namespace: test
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: developer
subjects:
- kind: ServiceAccount
  name: developer

5. Create a NetworkPolicy in test called default-deny-ingress

apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
  name: default-deny-ingress
  namespace: test
spec:
  podSelector: {}
  policyTypes:
  - Ingress

6. Create the ClusterPolicy

apiVersion: kyverno.io/v1
kind: ClusterPolicy
metadata:
  name: block-default-deny-netpol-modifications
  annotations:
    policies.kyverno.io/title: Block Modifications to Default Deny Network Policy
    policies.kyverno.io/category: Security
    policies.kyverno.io/subject: RBAC
    policies.kyverno.io/description: >-
      This policy restricts updates and deletes to the default-deny-ingress NetworkPolicy
      unless by any user except for a cluster-admin.      
spec:
  validationFailureAction: enforce
  background: false
  rules:
  - name: block-default-deny-netpol-modifications
    match:
      any:
      - resources:
          kinds:
          - NetworkPolicy
          names:
          - default-deny-ingress
    exclude:
      clusterRoles:
        - cluster-admin
    validate:
      message: "Denied. clusterRoles are {{request.clusterRoles}}"
      deny:
        conditions:
          any:
          - key: "{{request.operation}}"
            operator: In
            value:
            - DELETE
            - UPDATE

7. Verify both allowed and denied resource types are working properly (Pods are allowed because they're in the ClusterRole while csistoragecapacities are disallowed because they're absent [yet also Namespaced]):

$ k -n test auth can-i create pods --as system:serviceaccount:test:developer
yes
$ k -n test auth can-i create csistoragecapacities --as system:serviceaccount:test:developer
no

8. Edit the above NetworkPolicy as that Service Account and see the edit is blocked:

k -n test edit networkpolicy/default-deny-ingress --as system:serviceaccount:test:developer
error: networkpolicies.networking.k8s.io "default-deny-ingress" could not be patched: admission webhook "validate.kyverno.svc-fail" denied the request: 

resource NetworkPolicy/test/default-deny-ingress was blocked due to the following policies

block-default-deny-netpol-modifications:
  block-default-deny-netpol-modifications: Denied. clusterRoles are null

9. Check the logs and see that the value for request.clusterRoles is null

EngineValidate "msg"="variable substituted" "kind"="NetworkPolicy" "name"="default-deny-ingress" "namespace"="test" "policy"="block-default-deny-netpol-modifications" "rule"="block-default-deny-netpol-modifications" "path"="" "value"=null "variable"="{{request.clusterRoles}}"

Although the edit is blocked as expected, the request.clusterRoles is not resolved and therefore printed as null in the error message.

Expected behavior

The edit is blocked (as it is now) but the array of strings substituted for request.clusterRoles contains, at minimum, the string developer.

Additional context
Slack convo here.

[realshuting]

Hi @chipzoller - how did you bind the ClusterRole to this user system:serviceaccount:test:developer?

From the steps you provided above, the clusterrole developer was bound to the serviceaccount developer but I can see the namespace is missing, and I don't see how it relates to the user system:serviceaccount:test:developer.

From the definition below, the rolebinding.subjects.namespace can be empty if the kind is User or Group. Since this is a rolebinding, does it automatically binds to the serviceaccount inside the same namespace when it's missing?

✗ k explain rolebinding.subjects
KIND:     RoleBinding
VERSION:  rbac.authorization.k8s.io/v1

RESOURCE: subjects <[]Object>

DESCRIPTION:
     Subjects holds references to the objects the role applies to.

     Subject contains a reference to the object or user identities a role
     binding applies to. This can either hold a direct API object reference, or
     a value for non-objects such as user and group names.

FIELDS:
   apiGroup     <string>
     APIGroup holds the API group of the referenced subject. Defaults to "" for
     ServiceAccount subjects. Defaults to "rbac.authorization.k8s.io" for User
     and Group subjects.

   kind <string> -required-
     Kind of object being referenced. Values defined by this API group are
     "User", "Group", and "ServiceAccount". If the Authorizer does not
     recognized the kind value, the Authorizer should report an error.

   name <string> -required-
     Name of the object being referenced.

   namespace    <string>
     Namespace of the referenced object. If the object kind is non-namespace,
     such as "User" or "Group", and this value is not empty the Authorizer
     should report an error.

This is how I bind a clusterrole to a particular user.

kubectl create clusterrolebinding pod-manager --clusterrole=pod-manager --user=nancy

[chipzoller]

Since a RoleBinding is a Namespaced resource, and the Namespace is called out in the metadata field, that should tie the ServiceAccount (which is also Namespaced) in that test Namespace to the ClusterRole.

[realshuting]

Since a RoleBinding is a Namespaced resource, and the Namespace is called out in the metadata field, that should tie the ServiceAccount (which is also Namespaced) in that test Namespace to the ClusterRole.

OK.

And how does this user system:serviceaccount:test:developer bind to that ClusterRole? Via ServiceAccount?

[chipzoller]

developer is a ServiceAccount and not a user. The binding to the ClusterRole is via the RoleBinding.

[realshuting]

I see. I thought --as only takes username.

      --as='': Username to impersonate for the operation

I just noticed that request.clusterRoles is not a variable in the admission request, this is extracted by Kyverno and added internally while processing the policy.

After I changed the rule message to request.userInfo, I can see the following:

✗ k -n test edit networkpolicy/default-deny-ingress --as system:serviceaccount:test:developer
error: networkpolicies.networking.k8s.io "default-deny-ingress" could not be patched: admission webhook "validate.kyverno.svc-fail" denied the request:
# Please edit the object below. Lines beginning with a '#' will be ignored,

resource NetworkPolicy/test/default-deny-ingress was blocked due to the following policies

block-default-deny-netpol-modifications:
  block-default-deny-netpol-modifications: Denied. UserInfo in the admission request
    is {"groups":["system:serviceaccounts","system:serviceaccounts:test","system:authenticated"],"username":"system:serviceaccount:test:developer"}

Since the request was actually sent from the serviceaccount, after I excluded SA test/developer in the policy, I was able to edit the networkpolicy. Otherwise the request was blocked.

    exclude:
      clusterRoles:
        - cluster-admin
      subjects:
      - kind: ServiceAccount
        name: developer
        namespace: test

✗ k -n test edit networkpolicy/default-deny-ingress --as system:serviceaccount:test:developer
networkpolicy.networking.k8s.io/default-deny-ingress edited

[chipzoller]

I just noticed that request.clusterRoles is not a variable in the admission request, this is extracted by Kyverno and added internally while processing the policy.

Yes, that is correct. The problem captured in this issue is that the request.clusterRoles variable, which we document here, does not work in this combination. It would appear that Kyverno's internal logic used to map the username principals to ClusterRoles is not functioning properly. The exclude statement should not necessitate naming the exact ServiceAccount name because the exclude.clusterRoles should be sufficient. That's my interpretation of the problem, anyway.

[realshuting]

The exclude statement should not necessitate naming the exact ServiceAccount name because the exclude.clusterRoles should be sufficient.

We used to do this additional lookup from serviceaccount to clusterrole/role.

@mritunjaysharma394 - can you please confirm? We can also test sending admission requests via a CluserRole/Role and check request.clusterRoles variable.

[mritunjaysharma394]

Sure @realshuting , working on it

[mritunjaysharma394]

So it seems, they are not directly a bug but basically it seems an issue to make the logic crispier so that the exclude statement should not necessitate naming the exact ServiceAccount name because the exclude.clusterRoles should be sufficient, moving it to 1.7 milestone

[chipzoller]

IMHO, Kyverno being able to lookup and derive these variables which aren't present in the API server's AdmissionReview resource (that is, request.roles. and request.clusterRoles) is a big value add and differentiator as it makes matching and excluding way more flexible and useful. We need to ensure those variables always work in all possible combinations, and we should be testing for them in our E2E tests somehow.

[realshuting]

IMHO, Kyverno being able to lookup and derive these variables which aren't present in the API server's AdmissionReview resource (that is, request.roles. and request.clusterRoles) is a big value add and differentiator as it makes matching and excluding way more flexible and useful. We need to ensure those variables always work in all possible combinations, and we should be testing for them in our E2E tests somehow.

Agreed! Let's investigate possible solutions and propose for 1.7.0.