Bug: Unnecessary permissions

[Yseona]

Is there an existing issue for this?

• [X] I have searched the existing issues

Current Behavior

The bug is that the Deployment kubewarden-controller in the charts has too much RBAC permission than it needs. The service account of kubewarden-controller is bound to a role (rbac.yaml#L46) with the following permissions:

• update verb of the deployments resource (Role)
• list verb of the secrets resource (Role)
• ptach verb of the services resource (Role)

After reading the source code of kubewarden-controller, I didn't find any Kubernetes API usages that require these permissions. Besides, some of these unused permissions may have potential risks. For example, if malicious users gain control of a Kubernetes node running a kubewarden-controller pod, they can use the "update deployments" permission to edit existing workloads, create privileged containers with malicious container images.

Therefore, for security reasons, I suggest checking these permissions to determine if they are truly unnecessary. If they are, the issue should be fixed by removing the unnecessary permission or other feasible methods.

Expected Behavior

No response

Steps To Reproduce

Use helm charts with default values.

Environment

- OS:
- Architecture:

Anything else?

No response

[flavio]

Thanks for the report. We've recently changed the code a bit and this lead to some of these permissions to be not needed.

I've already verified the list permission for Deployment is not needed. As for the Secret and Service permissions, their permissions are defined inside of the same RBAC block as ConfigMaps (see here)

I think these resources should have similar verbs. We have to do some double checking and probably replace some CreateOrUpdate with CreateOrPatch statements.

@Yseona: I've seen you've opened similar issues against other open source projects. What kind of tool are you using to automate the auditing? It would be useful for us too

[flavio]

Listing of secrets and other resources is required by the k8s' client-go cache

[flavio]

We have a mix of CreateOrUpdate and CreateOrPatch, I've made sure we are only using CreateOrPatch and then I removed the update verb from deployments, configmaps, secrets and services. Everything seems to be working fine.

However... if you have patch verb you can still change the contents of one of these resources, hence there's no real benefit in removing the update verb.

To recap:

• list: needed for the client-go cache to work as expected
• update: theoretically not needed with a small changes, but being pragmatic it doesn't make any difference from the security POV since we must leave the patch verb around. Hence, it's still possible to alter the contents of these resources

I would personally close this issue without changing the RBAC criteria we currently have. I would just consolidate the codebase to use only CreateOrPatch.

What do you think?

[jvanz]

I'm fine with the standardization of the 'CreateOrPatch' usage. I like it. Furthermore, keep in mind that the actual RBAC roles deploy is in the helm-chart repository. And they can be out of sync sometimes because of the manual process of updating it. I've recently updated the controller repo to follow the helm-chart values (at least, most of them). Therefore, if we did this change, remember to update both places.

[flavio]

Closing after having discussed the implications with the rest of the team.