argoproj / argo-helm

ArgoProj Helm Charts
https://argoproj.github.io/argo-helm/
Apache License 2.0
1.75k stars 1.87k forks source link

Argo CD Helm Chart:Unnecessary RBAC permissions #2719

Closed Yseona closed 1 month ago

Yseona commented 5 months ago

Describe the bug

The bug is that the Deployment argo-cd-argocd-server and StatefulSet argo-cd-argocd-application-controller in the charts have too much RBAC permission than they need. The service account of argo-cd-argocd-server is bound to a clusterrole (argocd-server/clusterrole.yaml) with the following permissions:

And the service account of argo-cd-argocd-application-controller is also bound to a clusterrole (argocd-application-controller/clusterrole.yaml) with the following permissions:

After reading the source code of argo-cd, I didn't find any Kubernetes API usages using these permissions. Besides, some of these unused permissions may have potential risks. For example, if a malicious user gains control of a Kubernetes node running an argo-cd-argocd-server pod, they can use the "patch jobs" permission to edit existing workloads, create privileged containers with malicious container images, or use the "patch nodes" permission to modify node specifications so that system-critical components with high privileges will run on the controlled node.

Related helm chart

argo-cd

Helm chart version

6.10

To Reproduce

Install the Argo CD helm chart with default values.

Expected behavior

Therefore, these permissions should be rechecked to determine if they are truly unnecessary. If they are, the issue should be fixed by removing the unnecessary permissions or by using methods from some similar CVEs (e.g., CVE-2023-26484, CVE-2023-30840).

Screenshots

No response

Additional context

No response

yu-croco commented 5 months ago

Hi @Yseona , thank you for opening an issue.

After reading the source code of argo-cd, I didn't find any Kubernetes API usages using these permissions.

Since argo-helm follows upstream's manifest, can you please open an issue in upstream as well? Once upstream's manifests are updated, we will also follow.

e.g.) argocd-application-controller clister role

FYI: As a workaround, we can configure clusterRoleRules.rules to adjust permissions. e.g.) https://github.com/argoproj/argo-helm/blob/main/charts/argo-cd/values.yaml#L889-L893

Yseona commented 5 months ago

OK

dan-m8t commented 4 months ago

We had the same issue for a dedicated customer instance on a shared cluster. We ended up with a custom clusterrole like @yu-croco mentioned, like this:

rules:
- apiGroups:
  - '*'
  resources:
  - '*'
  verbs:
  - get
  - list
  - watch

Afaik Argo CD needs at least get,list,watch on all ressources. To narrow down the permissions we deploy a RoleBinding into the namespaces Argo CD is about to manage and bind that onto a ClusterRole for doing things inside that namespace. In our case it is the default "admin" ClusterRole of OpenShift which allows Argo CD to act like a project admin and thus can create all namespaced stuff but no clusterwide ressources. This comes with the drawback that the application-controller service account is able to read secrets from other namespaces, so this needs an additional layer of security, e.g. no access on the namespace where argocd is running and forbid impersonation for users. I hope the project will take more effort in multitenancy as I read alot of issues about that here.

github-actions[bot] commented 2 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.