Fix 4.1.7 and 4.1.8 audits: test flag from -z to -n #1467

andypitcher · 2023-06-30T17:47:45Z

Bug Details

Fix 4.1.7 and 4.1.8 audits: test flag from -z to -n (+ missing double quotes).

Target: node.yaml
Test ids: 4.1.7 and 4.1.8
Impacted CIS versions: 1.5 to 1.7

The test -z makes the node.yaml test ids 4.1.7 and 4.1.8 failing, as -z verifies if $CAFILE is zero instead of nonzero, to enter the first if condition and set CAFILE=$kubeletcafile.

As per test man page:

-n STRING
the length of STRING is nonzero

-z STRING
the length of STRING is zero

Inclosing #CAFILE with double quotes, so that it is treated as a single string, even if it contains whitespace or special characters.

Test environment

kube-bench version: 0.6.2
OS distro: Ubuntu 18.04.6 LTS
K8s version: rke1-v1.25.9

Bug Tests

Before patch

Failing commands:

$ CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')
$ echo $CAFILE
/etc/kubernetes/ssl/kube-ca.pem /etc/kubernetes/ssl/kube-ca.pem
$  if test -z $CAFILE; then CAFILE=$kubeletcafile; fi
-bash: test: /etc/kubernetes/ssl/kube-ca.pem: binary operator expected
$ if test -z "$CAFILE"; then CAFILE=/etc/kubernetes/ssl/kube-ca.pem; fi # Adding double quotes
$ echo $?
0
$ echo $CAFILE
/etc/kubernetes/ssl/kube-ca.pem /etc/kubernetes/ssl/kube-ca.pem # CAFILE should be /etc/kubernetes/ssl/kube-ca.pem, condition failed as it's nonzero string (there is content in it).

Failing logs for 4.1.7 caused by missing double quotes :

I0630 17:26:43.069971   29715 check.go:110] -----   Running check 4.1.7   -----
I0630 17:26:43.080767   29715 check.go:299] Command: "CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')\nif test -z $CAFILE; then CAFILE=/etc/kubernetes/ssl/kube-ca.pem; fi\nif test -e $CAFILE; then stat -c permissions=%a $CAFILE; fi\n#if test -n \"$CAFILE\"; then CAFILE=/etc/kubernetes/ssl/kube-ca.pem; fi\n#if test -e \"$CAFILE\"; then stat -c permissions=%a \"$CAFILE\"; fi"
I0630 17:26:43.080791   29715 check.go:300] Output:
 "/bin/sh: 2: test: /etc/kubernetes/ssl/kube-ca.pem: unexpected operator\n/bin/sh: 3: test: /etc/kubernetes/ssl/kube-ca.pem: unexpected operator\n"
I0630 17:26:43.080808   29715 check.go:221] Running 1 test_items
I0630 17:26:43.080817   29715 test.go:151] In flagTestItem.findValue
I0630 17:26:43.080823   29715 test.go:245] Flag 'permissions' does not exist

Failing logs for 4.1.8 caused by missing double quotes:

I0630 17:26:43.080872   29715 check.go:110] -----   Running check 4.1.8   -----
I0630 17:26:43.091479   29715 check.go:299] Command: "CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')\nif test -z $CAFILE; then CAFILE=/etc/kubernetes/ssl/kube-ca.pem; fi\nif test -e $CAFILE; then stat -c %U:%G $CAFILE; fi\n#if test -n \"$CAFILE\"; then CAFILE=/etc/kubernetes/ssl/kube-ca.pem; fi\n#if test -e \"$CAFILE\"; then stat -c %U:%G \"$CAFILE\"; fi"
I0630 17:26:43.091503   29715 check.go:300] Output:
 "/bin/sh: 2: test: /etc/kubernetes/ssl/kube-ca.pem: unexpected operator\n/bin/sh: 3: test: /etc/kubernetes/ssl/kube-ca.pem: unexpected operator\n"
I0630 17:26:43.091515   29715 check.go:221] Running 1 test_items
I0630 17:26:43.091523   29715 test.go:151] In flagTestItem.findValue
I0630 17:26:43.091529   29715 test.go:245] Flag 'root:root' does not exist

Failing logs for 4.1.7 with fixed double quotes, but test `-z` present :

I0630 17:31:08.022889     944 check.go:110] -----   Running check 4.1.7   -----
I0630 17:31:08.033643     944 check.go:299] Command: "CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')\nif test -z \"$CAFILE\"; then CAFILE=/etc/kubernetes/ssl/kube-ca.pem; fi\nif test -e \"$CAFILE\"; then stat -c permissions=%a \"$CAFILE\"; fi\n#if test -n \"$CAFILE\"; then CAFILE=/etc/kubernetes/ssl/kube-ca.pem; fi\n#if test -e \"$CAFILE\"; then stat -c permissions=%a \"$CAFILE\"; fi"
I0630 17:31:08.033700     944 check.go:300] Output:
 ""
I0630 17:31:08.033712     944 check.go:221] Running 1 test_items
I0630 17:31:08.033734     944 test.go:245] Flag 'permissions' does not exist

Failing logs for 4.1.8 with fixed double quotes, but test `-z` present:

I0630 17:31:08.033782     944 check.go:110] -----   Running check 4.1.12   -----
I0630 17:31:08.044812     944 check.go:299] Command: "CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')\nif test -z \"$CAFILE\"; then CAFILE=/etc/kubernetes/ssl/kube-ca.pem; fi\nif test -e \"$CAFILE\"; then stat -c %U:%G \"$CAFILE\"; fi\n#if test -n \"$CAFILE\"; then CAFILE=/etc/kubernetes/ssl/kube-ca.pem; fi\n#if test -e \"$CAFILE\"; then stat -c %U:%G \"$CAFILE\"; fi"
I0630 17:31:08.044868     944 check.go:300] Output:
 ""
I0630 17:31:08.044887     944 check.go:221] Running 1 test_items
I0630 17:31:08.044901     944 test.go:245] Flag 'root:root' does not exist

After patch (`double quotes + test -n`)

Successful commands:

$ CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')
$ echo $CAFILE
/etc/kubernetes/ssl/kube-ca.pem /etc/kubernetes/ssl/kube-ca.pem
$ if test -n "$CAFILE"; then CAFILE=/etc/kubernetes/ssl/kube-ca.pem; fi
$ echo $?
0
$ echo $CAFILE
/etc/kubernetes/ssl/kube-ca.pem

Successful logs for 4.1.7:

I0630 16:27:42.986053   12490 check.go:110] -----   Running check 4.1.7   -----
I0630 16:27:42.997158   12490 check.go:299] Command: "CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')\nif test -n \"$CAFILE\"; then CAFILE=/etc/kubernetes/ssl/kube-ca.pem; fi\nif test -e \"$CAFILE\"; then stat -c permissions=%a \"$CAFILE\"; fi"
I0630 16:27:42.997187   12490 check.go:300] Output:
 "permissions=600\n"
I0630 16:27:42.997198   12490 check.go:221] Running 1 test_items
I0630 16:27:42.997229   12490 test.go:151] In flagTestItem.findValue 600
I0630 16:27:42.997237   12490 test.go:245] Flag 'permissions' exists

Successful logs for 4.1.8:

I0630 16:27:42.997282   12490 check.go:110] -----   Running check 4.1.12   -----
I0630 16:27:43.009269   12490 check.go:299] Command: "CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')\nif test -n \"$CAFILE\"; then CAFILE=/etc/kubernetes/ssl/kube-ca.pem; fi\nif test -e \"$CAFILE\"; then stat -c %U:%G \"$CAFILE\"; fi"
I0630 16:27:43.009302   12490 check.go:300] Output:
 "root:root\n"
I0630 16:27:43.009316   12490 check.go:221] Running 1 test_items
I0630 16:27:43.009385   12490 test.go:151] In flagTestItem.findValue root:root
I0630 16:27:43.009394   12490 test.go:245] Flag 'root:root' exists

The test flag -z makes the node.yaml checks 4.1.7 and 4.1.8 to fail: as -z verifies if $CAFILE is zero instead of nonzero, to enter the first if condition and set CAFILE=$kubeletcafile. As per test man page: -n STRING the length of STRING is nonzero -z STRING the length of STRING is zero

CLAassistant · 2023-06-30T17:47:50Z

All committers have signed the CLA.

mozillazg · 2023-07-01T02:05:02Z

@andypitcher

The test -z makes the node.yaml test ids 4.1.7 and 4.1.8 failing, as -z verifies if $CAFILE is zero instead of nonzero, to enter the first if condition and set CAFILE=$kubeletcafile.

The use of test -z is as expected, we need $CAFILE to fallback to using $kubeletcafile when it is empty. The root cause of this issue is that the value of $CAFILE may contain multiple file paths but we do not wrap it with double quotes.

andypitcher · 2023-07-02T16:04:02Z

@mozillazg makes sense, the nature of rke makes the test audit returning the $kubeletcafile path twice, which is in fact the main root cause of the inital fix proposed. Also by default, only one instance of kubelet is running per node in k8s.

Here is an example of the two processes that are returned (ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file=), in the case of a rke node:

root@node1-rke1-1259:~# ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file=
root     17000 16973  0 Jun29 ?        00:00:00 /bin/bash /opt/rke-tools/entrypoint.sh kubelet --v=2 --streaming-connection-idle-timeout=30m --client-ca-file=/etc/kubernetes/ssl/kube-ca.pem --pod-infra-container-image=rancher/mirrored-pause:3.7 --container-runtime=remote --container-runtime-endpoint=unix:///var/run/cri-dockerd.sock --read-only-port=0 --address=0.0.0.0 --tls-cipher-suites=TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305 --cloud-provider= --hostname-override=node1-rke1-1259 --make-iptables-util-chains=true --anonymous-auth=false --cluster-dns=10.43.0.10 --fail-swap-on=false --authentication-token-webhook=true --volume-plugin-dir=/var/lib/kubelet/volumeplugins --cgroups-per-qos=True --cluster-domain=cluster.local --kubeconfig=/etc/kubernetes/ssl/kubecfg-kube-node.yaml --root-dir=/var/lib/kubelet --event-qps=0 --authorization-mode=Webhook --resolv-conf=/etc/resolv.conf
root     17411 17000  1 Jun29 ?        01:20:51 kubelet --v=2 --streaming-connection-idle-timeout=30m --client-ca-file=/etc/kubernetes/ssl/kube-ca.pem --pod-infra-container-image=rancher/mirrored-pause:3.7 --container-runtime=remote --container-runtime-endpoint=unix:///var/run/cri-dockerd.sock --read-only-port=0 --address=0.0.0.0 --tls-cipher-suites=TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305 --cloud-provider= --hostname-override=node1-rke1-1259 --make-iptables-util-chains=true --anonymous-auth=false --cluster-dns=10.43.0.10 --fail-swap-on=false --authentication-token-webhook=true --volume-plugin-dir=/var/lib/kubelet/volumeplugins --cgroups-per-qos=True --cluster-domain=cluster.local --kubeconfig=/etc/kubernetes/ssl/kubecfg-kube-node.yaml --root-dir=/var/lib/kubelet --event-qps=0 --authorization-mode=Webhook --resolv-conf=/etc/resolv.conf --cgroup-driver=cgroupfs --resolv-conf=/run/systemd/resolve/resolv.conf

As rke runs entirely within Docker containers, the first process is the entrypoint and parent (17000) of the second process kubelet (17411).

Thus, in order to fix the 4.1.7 and 4.1.8, for rke and any other distribution with this issue (two file locations returned), we could piped the full command to uniq or sort -u:

Before:

root@node1-rke1-1259:~# ps -ef | grep 'kubelet' | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}' 
/etc/kubernetes/ssl/kube-ca.pem
/etc/kubernetes/ssl/kube-ca.pem

After:

root@node1-rke1-1259:~# ps -ef | grep 'kubelet' | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}' | uniq
/etc/kubernetes/ssl/kube-ca.pem

This said, I would lean towards implementing this improvement in the dedicated rke cis profiles, as it would imply changing all previous cis versions otherwise.

Let me know what you think, thanks !

mozillazg · 2023-07-03T14:36:41Z

Thus, in order to fix the 4.1.7 and 4.1.8, for rke and any other distribution with this issue (two file locations returned), we could piped the full command to uniq or sort -u

@andypitcher SGTM.

mozillazg · 2023-07-03T14:41:05Z

This said, I would lean towards implementing this improvement in the dedicated rke cis profiles, as it would imply changing all previous cis versions otherwise.

IMHO, Changing all previous cis versions is better, but implementing this improvement in the dedicated rke cis profiles is ok for me too.

andypitcher · 2023-07-05T13:48:12Z

I agree to apply this change against all CIS versions (the ones that use this audit in 4.1.7/4.1.8).

CIS profiles to improve are cis-1.5, cis-1.6, cis-1.20, cis-1.23, cis-1.24, cis-1.7, ack-1.0 :

/tmp/kube-bench/cfg$ grep -ri -A3  '4.1.7\|4.1.8' . | grep 'apiserver' -B4 | grep 'id\|apiserver' 
./cis-1.20/node.yaml:      - id: 4.1.7
./cis-1.20/node.yaml-          CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')
./cis-1.20/node.yaml:      - id: 4.1.8
./cis-1.20/node.yaml-          CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')
./cis-1.24/node.yaml:      - id: 4.1.7
./cis-1.24/node.yaml-          CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')
./cis-1.24/node.yaml:      - id: 4.1.8
./cis-1.24/node.yaml-          CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')
./cis-1.6/node.yaml:      - id: 4.1.7
./cis-1.6/node.yaml-          CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')
./cis-1.6/node.yaml:      - id: 4.1.8
./cis-1.6/node.yaml-          CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')
./cis-1.23/node.yaml:      - id: 4.1.7
./cis-1.23/node.yaml-          CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')
./cis-1.23/node.yaml:      - id: 4.1.8
./cis-1.23/node.yaml-          CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')
./cis-1.5/node.yaml:      - id: 4.1.7
./cis-1.5/node.yaml-          CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')
./cis-1.5/node.yaml:      - id: 4.1.8
./cis-1.5/node.yaml-          CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')
./ack-1.0/node.yaml:      - id: 4.1.7
./ack-1.0/node.yaml-          CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')
./ack-1.0/node.yaml:      - id: 4.1.8
./ack-1.0/node.yaml-          CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')
./cis-1.7/node.yaml:      - id: 4.1.7
./cis-1.7/node.yaml-          CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')
./cis-1.7/node.yaml:      - id: 4.1.8
./cis-1.7/node.yaml-          CAFILE=$(ps -ef | grep kubelet | grep -v apiserver | grep -- --client-ca-file= | awk -F '--client-ca-file=' '{print $2}' | awk '{print $1}')

Should I create a new PR with the updated files ^ (+ uniq), and link to this one ?

mozillazg · 2023-07-05T14:40:42Z

@andypitcher

Should I create a new PR with the updated files ^ (+ uniq), and link to this #1467 ?

Yes. Thanks for your hard work.

andypitcher · 2023-07-11T12:13:00Z

I'm closing this PR, following #1472 merge.

Merge branch 'main' into node-4.1.7-4.1.8-audit-patch

024dbf9

andypitcher mentioned this pull request Jul 11, 2023

Fix node.yaml - 4.1.7 and 4.1.8 audit by adding uniq #1472

Merged

andypitcher closed this Jul 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix 4.1.7 and 4.1.8 audits: test flag from -z to -n #1467

Fix 4.1.7 and 4.1.8 audits: test flag from -z to -n #1467

andypitcher commented Jun 30, 2023

CLAassistant commented Jun 30, 2023 •

edited

Loading

mozillazg commented Jul 1, 2023

andypitcher commented Jul 2, 2023

mozillazg commented Jul 3, 2023

mozillazg commented Jul 3, 2023

andypitcher commented Jul 5, 2023

mozillazg commented Jul 5, 2023

andypitcher commented Jul 11, 2023

Fix 4.1.7 and 4.1.8 audits: test flag from -z to -n #1467

Fix 4.1.7 and 4.1.8 audits: test flag from -z to -n #1467

Conversation

andypitcher commented Jun 30, 2023

Bug Details

Fix 4.1.7 and 4.1.8 audits: test flag from -z to -n (+ missing double quotes).

Test environment

Bug Tests

Before patch

Failing commands:

Failing logs for 4.1.7 caused by missing double quotes :

Failing logs for 4.1.8 caused by missing double quotes:

Failing logs for 4.1.7 with fixed double quotes, but test -z present :

Failing logs for 4.1.8 with fixed double quotes, but test -z present:

After patch (double quotes + test -n)

Successful commands:

Successful logs for 4.1.7:

Successful logs for 4.1.8:

CLAassistant commented Jun 30, 2023 • edited Loading

mozillazg commented Jul 1, 2023

andypitcher commented Jul 2, 2023

Before:

After:

mozillazg commented Jul 3, 2023

mozillazg commented Jul 3, 2023

andypitcher commented Jul 5, 2023

mozillazg commented Jul 5, 2023

andypitcher commented Jul 11, 2023

Failing logs for 4.1.7 with fixed double quotes, but test `-z` present :

Failing logs for 4.1.8 with fixed double quotes, but test `-z` present:

After patch (`double quotes + test -n`)

CLAassistant commented Jun 30, 2023 •

edited

Loading