argoproj / argo-cd

Declarative Continuous Deployment for Kubernetes
https://argo-cd.readthedocs.io
Apache License 2.0
17.4k stars 5.29k forks source link

define minimum necessary rbac permissions #5389

Open andras-babos opened 3 years ago

andras-babos commented 3 years ago

Summary

Please define a more restricted minimum necessary cluster role permissions for the argocd-applicaton-controller and argocd-manager permissions instead of the current cluster wide get, list, watch.

https://argoproj.github.io/argo-cd/getting_started/

"The rules of the argocd-manager-role role can be modified such that it only has create, update, patch, delete privileges to a limited set of namespaces, groups, kinds. However get, list, watch privileges are required at the cluster-scope for Argo CD to function."

Motivation

We would like to secure the cluster as good as possible and the service account have too much privileges. For example the default cluster role will allow the service account to read any other secret and it makes possible for privilege escalation.

Example description there: https://kubernetes.io/docs/reference/access-authn-authz/rbac/ -> view cluster role:

"Allows read-only access to see most objects in a namespace. It does not allow viewing roles or role bindings.

This role does not allow viewing Secrets, since reading the contents of Secrets enables access to ServiceAccount credentials in the namespace, which would allow API access as any ServiceAccount in the namespace (a form of privilege escalation)."

The builtin cluster role view have more restrictions and it will be great if argocd have similar in the documentation.

Proposal

It would be good if a more restricted default cluster role is defined and documented then the end users can define more roles to allow more permissions only on the target objects.

dahrens commented 3 years ago

I thought exactly the same and agree that the default privileges of the service account argocd-applicaton-controller are to elevated.

However argocd allows you to reconfigure this (with a few drawbacks).

[...] configure the list of included resources using the resource.inclusions setting. By default, all resource group/kinds are included.

I used the inclusion and ended up with something that I attach below this comment - which does not include Secrets, ClusterRoleBindings and ClusterRoleBindings and some apiGroups like certificates.k8s.io, crd.projectcalico.org, and few others.

The drawbacks are quite clear:

  1. you can no longer deploy anything that relies on "missing" resources (Secrets, ClusterRoleBindings and ClusterRoleBindings) using argocd - including argocd itself :)
  2. You need to pin the do everything ClusterRole to each namespace - cumbersome - but common when you want to achieve some basic security.

Another interesting issue is #5275 which gives some insight in the reasons behind the elevated sa. From my point of view it is a bit unfortunate that global read access is required for the synchronization - otherwise it would be possible to grant access to secrets using a RoleBinding for each namespace that should be managed by argocd. Not that elegant - but argocd is no longer cluster admin.

In fact cluster wide read access reveals all secrets and grants access to impersonate all other ServiceAccounts. I understand that a software that is meant to be used to actually control (big) parts of the cluster needs high privileges - but following the principle of least privileges is actually quite difficult using argocd - or do I miss something (justed dived in for ~20 hours).

I'd prefer that it does not need to have read access for several namespaces like e.g. kube-system at all. Despite that issue I like argocd a lot - good work :+1: - I hope I missed something in my analysis.

kubectl edit configmap argocd-cm -n argocd
data:
  resource.inclusions: |
    - apiGroups:
      - ""
      kinds:
      - ConfigMap
      - Endpoints
      - LimitRange
      - Namespace
      - PersistentVolumeClaim
      - PersistentVolume
      - Pod
      - PodTemplate
      - ReplicationController
      - ResourceQuota
      - ServiceAccount
      - Service
      clusters:
      - "*"
    - apiGroups:
      - "rbac.authorization.k8s.io/v1"
      kinds:
      - RoleBinding
      - Role
      clusters:
      - "*"
    - apiGroups:
      - "networking.k8s.io"
      - "extensions"
      - "discovery.k8s.io"
      - "cert-manager.io"
      - "batch"
      - "argoproj.io"
      - "apps"
      - "apiextensions.k8s.io"
      - "apiregistration.k8s.io"
      - "acme.cert-manager.io"
      kinds:
      - "*"
      clusters:
      - "*"
kind: ConfigMap

And ClusterRole that looks like that:

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  labels:
    app.kubernetes.io/component: application-controller
    app.kubernetes.io/name: argocd-application-controller
    app.kubernetes.io/part-of: argocd
  name: argocd-application-controller
rules:
- apiGroups:
  - ""
  resources:
  - configmaps
  - endpoints
  - persistentvolumeclaims
  - persistentvolumeclaims/status
  - pods
  - replicationcontrollers
  - replicationcontrollers/scale
  - serviceaccounts
  - services
  - services/status
  - persistentvolumes
  - podtemplates
  - bindings
  - events
  - limitranges
  - namespaces/status
  - pods/log
  - pods/status
  - replicationcontrollers/status
  - resourcequotas
  - resourcequotas/status
  - namespaces
  verbs: [get, list, watch]
- apiGroups:
  - "networking.k8s.io"
  - "extensions"
  - "discovery.k8s.io"
  - "cert-manager.io"
  - "batch"
  - "argoproj.io"
  - "apps"
  - "apiextensions.k8s.io"
  - "apiregistration.k8s.io"
  - "acme.cert-manager.io"
  resources:
  - "*"
  verbs: [get, list, watch]
- nonResourceURLs:
  - '*'
  verbs: [get, list, watch]

Together with the old ClusterRole having a new name argocd-application-controller-namespace-admin:

apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
  labels:
    app.kubernetes.io/name: argocd-application-controller
    app.kubernetes.io/part-of: argocd
    app.kubernetes.io/component: application-controller
  name: argocd-application-controller-namespace-admin
rules:
- apiGroups:
  - '*'
  resources:
  - '*'
  verbs:
  - '*'
- nonResourceURLs:
  - '*'
  verbs:
  - '*'

Bound to several namespaces like that:

apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
  name: argocd-application-controller-foo
  namespace: foo
roleRef:
  apiGroup: rbac.authorization.k8s.io
  kind: ClusterRole
  name: argocd-application-controller-namespace-admin
subjects:
- kind: ServiceAccount
  name: argocd-application-controller
  namespace: argocd
aelbarkani commented 3 years ago

I support this proposal. Otherwise it's a big security issue, especially in multi-tenant clusters, where ArgoCD is completely unusable because of the privilege escalation vulnerability. https://github.com/argoproj-labs/argocd-operator/issues/154

loreleimccollum-work commented 3 years ago

@dahrens we are implementing this right now to lock down our cluster, was wondering if you had implemented what you said on your own? I am seeing an issue with terraform and the patching of the clusterrole, because another roll out of argo sets it back to whats in the chart and wipes out my changes.. so trying to figure out the best way to avoid that

dahrens commented 3 years ago

@loreleimccollum-work I did - not in production, but in my "playground" cluster.

I was not using helm, but the plain manifests referenced in the argocd's Getting started guide. Therefore this was no issue for me.

The manifests folder of this repository (e.g. v2.0.1) together with kustomize might be an approach to be able to handle this in a real world scenario.

loreleimccollum-work commented 3 years ago

@dahrens thanks, yeah we are looking to do in production. Kustomize is probably the best option right now because the helm chart doesn't support this customization. I created https://github.com/argoproj/argo-helm/issues/721, I may propose an option to allow for the customization this week, because we are using terraform and the helm provider right now, there are not many options for us to get this configured this way

loreleimccollum-work commented 3 years ago

My PR https://github.com/argoproj/argo-helm/pull/730 to the helm chart now allows for customizing this cluster role, which is all that was necessary. We are now able to with Terraform achive the idea behind this gitissue

aelbarkani commented 2 years ago

Hi!

Any progress on this proposal ?

ArgoCD currently needs cluster-wide read permissions to work. That can lead to a privilege escalation in a multi-tenant context. So imho these wide permissions shouldn't be necessary because some ArgoCD instance need only to read and write in specific namespaces. In a multi-tenant and self-service context, each team has its own ArgoCD, provisioned through ArgoCD operator. Basically through ArgoCD operator we achieve ArgoCD as a Service. However, since Kubernetes/OpenShift cluster are multi-tenant, it means that each team has cluster-wide read permissions on all ressources, which is a security issue. It leads even to a potential privilege escalation since ArgoCD administrators can have read access to all secrets of the clusters.

Related issue: https://github.com/argoproj-labs/argocd-operator/issues/154

lgoral commented 2 years ago

Hi!

Any progress on this proposal ?

lorelei-rupp-imprivata commented 2 years ago

Hi!

Any progress on this proposal ?

you can already implement the things above FYI, there is support to make argo least priv today its just not really documented other then this ticket

aelbarkani commented 2 years ago

Good news @lorelei-rupp-imprivata ! Any hint about where we can find how to do that ?

lorelei-rupp-imprivata commented 2 years ago

Good news @lorelei-rupp-imprivata ! Any hint about where we can find how to do that

You can do this with the helm chart, they expose a way to customize the things outlined in the description of this issue. See my PR add as well, which allowed the cluster role configuration customization

lorelei-rupp-imprivata commented 1 year ago

another thing to add here, this works well for an argocd that is managing a cluster its deployed within. I am not sure of the solution when Argocd is managing remove clusters. When you do argocd add cluster it gives a argocd-manager-role in the other clusters with full admin cluster access. I am wondering if there is a way to make that least priv too?

ronmegini commented 1 year ago

We are facing the same issue described here and followed the instructions to create a limited cluster-role. The problem is that ArgoCD requires cluster read (get, list, watch) permissions in order to function. From my understanding, they seem to be used for creating an initial cache of the cluster.

For example, I don't want ArgoCD to have cluster wide permissions on all the secrets in the cluster, but I do want it to manage secrets on namespaces it has permissions on.

What is the reason ArgoCD requires these permissions? Does this initial cache have to be cluster wide? Is it possible to change ArgoCD to avoid this behavior?

rbizzell-collibra commented 1 year ago

Good news @lorelei-rupp-imprivata ! Any hint about where we can find how to do that

You can do this with the helm chart, they expose a way to customize the things outlined in the description of this issue. See my PR add as well, which allowed the cluster role configuration customization

ogunleye0720 commented 9 months ago

Hello, have anyone of you found a solution yet? I'm currently working on a project, but learning argocd has a cluster-wide privilege has got me thinking about my choice of integrating Argo in my pipeline.

ivan-cai commented 8 months ago

Using auto respect rbac feature, and ArgoCD release >= v2.9.0. https://argo-cd.readthedocs.io/en/stable/operator-manual/declarative-setup/#auto-respect-rbac-for-controller

Pionerd commented 6 months ago

Does anyone have a (working) example of a more restricted ClusterRole? I'm a bit confused by @dahrens example vs. @ronmegini's remark that ArgoCD will not operate without cluster-wide permissions.

Basically what I was hoping for that we can (at least) get ArgoCD working without any permissions on the kube-system namespace. I think that would be a great step forward.

@ivan-cai I think the documentation on the feature you mention is a bit limited as well. Can you explain how that helps this case?