CVE-2022-24348 fix broke Helm downloader plugins interface

[Enrico204]

Checklist:

• I've searched in the docs and FAQ for my answer: https://bit.ly/argocd-faq.
• I've included steps to reproduce the bug.
• I've pasted the output of argocd version.

Describe the bug

The fix for CVE-2022-24348 introduced a check for helm remote protocols (see allowedHelmRemoteProtocols, isURLSchemeAllowed and others in 78c2084 ). This check broke helm secrets integration with ArgoCD as helm secrets is using URL schemas like secrets+age-import:// in valueFiles to indicate the tool to use (age, gpg, etc).

See jkroepke/helm-secrets#185

To Reproduce

Create an Application like:

kind: Application
apiVersion: argoproj.io/v1alpha1
metadata:
  name: mariadb
  namespace: argocd
spec:
  project: default
  source:
    path: helm/mariadb/
    repoURL: 'git@gitlab.com:repo'
    targetRevision: HEAD
    helm:
      version: v3
      releaseName: mariadb
      valueFiles:
        - "secrets+age-import://argocd-age-key/keys.txt?secrets.yml"
  destination:
    namespace: default
    server: 'https://kubernetes.default.svc'

A detailed example is here: https://github.com/jkroepke/helm-secrets/wiki/ArgoCD-Integration

Expected behavior

To keep using helm secrets.

Version

argocd version
argocd: v2.2.4+78d749e
  BuildDate: 2022-02-03T20:41:00Z
  GitCommit: 78d749ec88bea5a5e851fc48b9404fdf067192b6
  GitTreeState: clean
  GoVersion: go1.16.11
  Compiler: gc
  Platform: linux/amd64

Logs

The error in ArgoCD is :rpc error: code = Unknown desc = Manifest generation error (cached): the URL scheme 'secrets+age-import' is not allowed

[jkroepke]

Hi,

the fix breaks the downloader plugin feature in at all.

Consider to allow to configure the allowedHelmRemoteProtocols propterty, e.g. through argocd-cm

[Enrico204]

Be aware that allowing custom schemes in allowedHelmRemoteProtocols might reintroduce the CVE bug via plugins. For example, in this case (helm secrets), the "secrets" YAML is loaded via a relative URL that can go outside the project dir, hence allowing path traversal vulnerabilities.

I think that a very prominent warning should be placed in doc and other places.

Another solution may be somehow chrooting the execution (this will eradicate the issue once for all), but I don't know if it's feasible to do safely inside containers.

[jkroepke]

Be aware that allowing custom schemes in allowedHelmRemoteProtocols might reintroduce the CVE bug via plugins.

Thats abolute correct. Someone needs to be faked the file protocol and the issue appears again. While the standard installation might be safe for now, custom schemes needs to implements first by an administrator (by installing the plugin inside the container)

For example, in this case (helm secrets), the "secrets" YAML is loaded via a relative URL that can go outside the project dir, hence allowing path traversal vulnerabilities.

This is a good catch. I will add a switch here to disable following symlinks, Since the plugin doesn't know the boundaries, I can only deny symlinks by default.

[Enrico204]

A more general solution for path traversal vulnerabilities in ArgoCD (and this issue) might be proot. It doesn't require root privileges, and it's able to restrict the access to directories (regardless of the tool):

$ proot -b /dev -b /proc -b /sys -b /lib -b /bin -b /lib64 -b /usr -b "$(pwd):/src" -r ~/testproot/ -w /src `which ls` / -la
total 152
drwxr-xr-x  11 1000 1000   4096 feb  6 13:50 .
drwxr-xr-x  11 1000 1000   4096 feb  6 13:50 ..
drwxr-xr-x   2    0    0 114688 feb  6 13:42 bin
drwxr-xr-x  20    0    0   4080 feb  4 15:53 dev
drwxr-xr-x   3 1000 1000   4096 feb  6 13:49 home
drwxr-xr-x 165    0    0  12288 gen 30 21:37 lib
drwxr-xr-x   3    0    0   4096 dic 26  2019 lib64
dr-xr-xr-x 418    0    0      0 nov 10 18:21 proc
drwxr-xr-x   4 1000 1000   4096 feb  6 12:48 src
dr-xr-xr-x  13    0    0      0 nov 12 22:10 sys
drwxr-xr-x  15    0    0   4096 dic 28 18:36 usr

$ proot -b /dev -b /proc -b /sys -b /lib -b /bin -b /lib64 -b /usr -b "$(pwd):/src" -r ~/testproot/ -w /src `which ls` /src -la
total 32
drwxr-xr-x  4 1000 1000 4096 feb  6 12:48 .
drwxr-xr-x 11 1000 1000 4096 feb  6 13:50 ..
drwxr-xr-x  2 1000 1000 4096 dic 20 07:07 charts
-rw-r--r--  1 1000 1000 1146 dic 20 07:08 Chart.yaml
-rw-r--r--  1 1000 1000  349 dic 20 07:07 .helmignore
-rw-r--r--  1 1000 1000 3622 feb  6 12:48 secrets.yaml
drwxr-xr-x  2 1000 1000 4096 gen 25 17:18 templates
-rw-r--r--  1 1000 1000   23 feb  6 09:39 values.yaml

Apparently, proot is able to bind directories from the "host" (without using mount), e.g. -b /dev (or even remap, like docker/podman: -b "/host/dir:/inside/proot/dir"). -r ~/testproot/ specifies the path for files created inside the "chroot-like" environment (an empty dir is sufficient, can be discarded at each execution).

I was able to run helm in a way that it's restricted to ~/test (r/w) and other system dirs like /usr/bin (r/o) inside an ArgoCD container (running as argocd user):

proot -b /dev -b /proc -b /sys -b "/usr/lib:/lib" -b "/usr/lib64:/lib64" -b "/usr/lib32:/lib32" -b /usr -b ~/test/ -r /tmp/proot -w ~/test/ /usr/local/bin/helm template test .

Note that proot is not in the default image. I rebuilt the container image to add proot.

I'll try to do a full PoC if you (maintainer) think that it's a potential solution :-)

[jkroepke]

Restrict the directory with something like proot increase the complexity in general. Who knows, if ptrace is allowed by AppArmor/SELinux.

This issue that other build directory can access, exists in Jenkins, Gitlab and other CI systems, too. To separate build directories and denied the access from each other, separate or non peristent runner/agents would archive this. Something like multiple repo-server which can be bound to Applications and restricted by Projects) can a long termin solution.

Restriction the helm call to the helm call may introduce a lot of incompabilties.

I would strongly recommend to move the dicussion "What the best way to solve this long term" into a GitHub discussion.

As short-term goal to fix this issue, I a conguration for allowedHelmRemoteProtocols would still the best.

[deechella]

Can we update the argocd version directly to 2.2.4 from 2.0.0 to overcome this https://nvd.nist.gov/vuln/detail/CVE-2022-24348

[stephaneetje]

Hello,

Any news with this issue ? SInce i updated argocd, i cant use helm secrets anymore which is very problematic. Is there any workaround ?

[MeNsaaH]

@stephaneetje none for now aside downgrading to the vulnerable version..

[alexmt]

Working on fixing it. Once the fix is ready will cherry-pick it into 2.3, 2.2 and 2.1. Please check vulnerability limitations in https://medium.com/argo-project/argo-cd-deals-with-our-first-zero-day-cve-86e8fb158e8f . The vulnerability is there but it is not applicable to every organization.

[jkroepke]

@stephaneetje none for now aside downgrading to the vulnerable version..

There is no invulnerable version, yet. If you take a look at this chart, you are still able to access files from everywhere.

[jkroepke]

@alexmt Hi, is there a chance to backport this into the 2.2.x branch? In case if not, is there any release of ArgoCD 2.3 planned?

[petr4]

@alexmt Hey here, we need helm-secrets feature, is any chance to support it on 2.3.x ?

[alexmt]

hello @jkroepke , @petr4 . Sorry, I forgot to mention. The fix was cherry-picked to v2.3~2.1 . You should be able to use helm.valuesFileSchemes setting to "whitelist" additional schemes. Can you please try it?

[jkroepke]

Hi @alexmt ,

it is already document on my side and users confirmed it works.

[petr4]

@jkroepke Yes, confirmed - it works. I'd marked clearly in documentation:

• which services needed custom image and configs with helm-secrets
• example integrations with providers/clouds, in my case - AWS kms

[jkroepke]

@petr4 Feel free to send a PR https://github.com/jkroepke/helm-secrets/blob/main/docs/ArgoCD%20Integration.md

I'm happy to see KMS examples aside the existing GCP one.