From 25e3dd69e412ed6ca76eb06f7cb9e34cdc4fefec Mon Sep 17 00:00:00 2001 From: Eljees <3.14hell@gmail.com> Date: Fri, 31 Jul 2026 13:35:17 +0300 Subject: [PATCH 1/2] fix(cis-1.12): do not report FAIL for file permission checks when the file is absent The file permission and ownership checks of the cis-1.12 benchmark wrap their audit command in a file existence guard, for example /bin/sh -c 'if test -e $apiserverconf; then stat -c permissions=%a $apiserverconf; fi' When the file is absent that command produces no output at all, so the test_item that looks for the "permissions" flag cannot match and the check is reported as FAIL. The guard therefore has no effect: without it stat would fail and the check would be reported as FAIL as well. cis-1.12/node.yaml 4.1.2 already handles this correctly by echoing a sentinel in the else branch and accepting it with bin_op: or. This change applies the same pattern to the remaining 21 file permission and ownership checks of the benchmark. Checks where the absence of the file is the finding itself are left untouched: 1.2.28 (encryption provider configuration) and 4.1.7 / 4.1.8 (client certificate authority file). Fixes #1881 Signed-off-by: Eljees <3.14hell@gmail.com> --- cfg/cis-1.12/master.yaml | 56 +++++++++++++++----- cfg/cis-1.12/node.yaml | 26 +++++++--- check/check_test.go | 47 +++++++++++++++++ check/controls_test.go | 109 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 217 insertions(+), 21 deletions(-) diff --git a/cfg/cis-1.12/master.yaml b/cfg/cis-1.12/master.yaml index b0cc78129..ffc3a3007 100644 --- a/cfg/cis-1.12/master.yaml +++ b/cfg/cis-1.12/master.yaml @@ -10,13 +10,15 @@ groups: checks: - id: 1.1.1 text: "Ensure that the API server pod specification file permissions are set to 600 or more restrictive (Automated)" - audit: "/bin/sh -c 'if test -e $apiserverconf; then stat -c permissions=%a $apiserverconf; fi'" + audit: "/bin/sh -c 'if test -e $apiserverconf; then stat -c permissions=%a $apiserverconf; else echo \"File not found\"; fi'" tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the control plane node. @@ -25,10 +27,12 @@ groups: - id: 1.1.2 text: "Ensure that the API server pod specification file ownership is set to root:root (Automated)" - audit: "/bin/sh -c 'if test -e $apiserverconf; then stat -c %U:%G $apiserverconf; fi'" + audit: "/bin/sh -c 'if test -e $apiserverconf; then stat -c %U:%G $apiserverconf; else echo \"File not found\"; fi'" tests: + bin_op: or test_items: - flag: "root:root" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, chown root:root $apiserverconf @@ -36,13 +40,15 @@ groups: - id: 1.1.3 text: "Ensure that the controller manager pod specification file permissions are set to 600 or more restrictive (Automated)" - audit: "/bin/sh -c 'if test -e $controllermanagerconf; then stat -c permissions=%a $controllermanagerconf; fi'" + audit: "/bin/sh -c 'if test -e $controllermanagerconf; then stat -c permissions=%a $controllermanagerconf; else echo \"File not found\"; fi'" tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, chmod 600 $controllermanagerconf @@ -50,10 +56,12 @@ groups: - id: 1.1.4 text: "Ensure that the controller manager pod specification file ownership is set to root:root (Automated)" - audit: "/bin/sh -c 'if test -e $controllermanagerconf; then stat -c %U:%G $controllermanagerconf; fi'" + audit: "/bin/sh -c 'if test -e $controllermanagerconf; then stat -c %U:%G $controllermanagerconf; else echo \"File not found\"; fi'" tests: + bin_op: or test_items: - flag: "root:root" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, chown root:root $controllermanagerconf @@ -61,13 +69,15 @@ groups: - id: 1.1.5 text: "Ensure that the scheduler pod specification file permissions are set to 600 or more restrictive (Automated)" - audit: "/bin/sh -c 'if test -e $schedulerconf; then stat -c permissions=%a $schedulerconf; fi'" + audit: "/bin/sh -c 'if test -e $schedulerconf; then stat -c permissions=%a $schedulerconf; else echo \"File not found\"; fi'" tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, chmod 600 $schedulerconf @@ -75,10 +85,12 @@ groups: - id: 1.1.6 text: "Ensure that the scheduler pod specification file ownership is set to root:root (Automated)" - audit: "/bin/sh -c 'if test -e $schedulerconf; then stat -c %U:%G $schedulerconf; fi'" + audit: "/bin/sh -c 'if test -e $schedulerconf; then stat -c %U:%G $schedulerconf; else echo \"File not found\"; fi'" tests: + bin_op: or test_items: - flag: "root:root" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, chown root:root $schedulerconf @@ -86,14 +98,16 @@ groups: - id: 1.1.7 text: "Ensure that the etcd pod specification file permissions are set to 600 or more restrictive (Automated)" - audit: "/bin/sh -c 'if test -e $etcdconf; then find $etcdconf -name '*etcd*' | xargs stat -c permissions=%a; fi'" + audit: "/bin/sh -c 'if test -e $etcdconf; then find $etcdconf -name '*etcd*' | xargs stat -c permissions=%a; else echo \"File not found\"; fi'" use_multiple_values: true tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, @@ -102,11 +116,13 @@ groups: - id: 1.1.8 text: "Ensure that the etcd pod specification file ownership is set to root:root (Automated)" - audit: "/bin/sh -c 'if test -e $etcdconf; then find $etcdconf -name '*etcd*' | xargs stat -c %U:%G; fi'" + audit: "/bin/sh -c 'if test -e $etcdconf; then find $etcdconf -name '*etcd*' | xargs stat -c %U:%G; else echo \"File not found\"; fi'" use_multiple_values: true tests: + bin_op: or test_items: - flag: "root:root" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, @@ -189,14 +205,16 @@ groups: - id: 1.1.13 text: "Ensure that the default administrative credential file permissions are set to 600 (Automated)" audit: | - for adminconf in /etc/kubernetes/admin.conf /etc/kubernetes/super-admin.conf; do if test -e $adminconf; then stat -c "permissions=%a %n" $adminconf; fi; done + found=""; for adminconf in /etc/kubernetes/admin.conf /etc/kubernetes/super-admin.conf; do if test -e $adminconf; then found="yes"; stat -c "permissions=%a %n" $adminconf; fi; done; if test -z "$found"; then echo "File not found"; fi use_multiple_values: true tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, chmod 600 /etc/kubernetes/admin.conf @@ -207,14 +225,16 @@ groups: - id: 1.1.14 text: "Ensure that the default administrative credential file ownership is set to root:root (Automated)" audit: | - for adminconf in /etc/kubernetes/admin.conf /etc/kubernetes/super-admin.conf; do if test -e $adminconf; then stat -c "ownership=%U:%G %n" $adminconf; fi; done + found=""; for adminconf in /etc/kubernetes/admin.conf /etc/kubernetes/super-admin.conf; do if test -e $adminconf; then found="yes"; stat -c "ownership=%U:%G %n" $adminconf; fi; done; if test -z "$found"; then echo "File not found"; fi use_multiple_values: true tests: + bin_op: or test_items: - flag: "ownership" compare: op: eq value: "root:root" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, chown root:root /etc/kubernetes/admin.conf @@ -224,13 +244,15 @@ groups: - id: 1.1.15 text: "Ensure that the scheduler.conf file permissions are set to 600 or more restrictive (Automated)" - audit: "/bin/sh -c 'if test -e $schedulerkubeconfig; then stat -c permissions=%a $schedulerkubeconfig; fi'" + audit: "/bin/sh -c 'if test -e $schedulerkubeconfig; then stat -c permissions=%a $schedulerkubeconfig; else echo \"File not found\"; fi'" tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, @@ -239,10 +261,12 @@ groups: - id: 1.1.16 text: "Ensure that the scheduler.conf file ownership is set to root:root (Automated)" - audit: "/bin/sh -c 'if test -e $schedulerkubeconfig; then stat -c %U:%G $schedulerkubeconfig; fi'" + audit: "/bin/sh -c 'if test -e $schedulerkubeconfig; then stat -c %U:%G $schedulerkubeconfig; else echo \"File not found\"; fi'" tests: + bin_op: or test_items: - flag: "root:root" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, @@ -251,13 +275,15 @@ groups: - id: 1.1.17 text: "Ensure that the controller-manager.conf file permissions are set to 600 or more restrictive (Automated)" - audit: "/bin/sh -c 'if test -e $controllermanagerkubeconfig; then stat -c permissions=%a $controllermanagerkubeconfig; fi'" + audit: "/bin/sh -c 'if test -e $controllermanagerkubeconfig; then stat -c permissions=%a $controllermanagerkubeconfig; else echo \"File not found\"; fi'" tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, @@ -266,10 +292,12 @@ groups: - id: 1.1.18 text: "Ensure that the controller-manager.conf file ownership is set to root:root (Automated)" - audit: "/bin/sh -c 'if test -e $controllermanagerkubeconfig; then stat -c %U:%G $controllermanagerkubeconfig; fi'" + audit: "/bin/sh -c 'if test -e $controllermanagerkubeconfig; then stat -c %U:%G $controllermanagerkubeconfig; else echo \"File not found\"; fi'" tests: + bin_op: or test_items: - flag: "root:root" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, diff --git a/cfg/cis-1.12/node.yaml b/cfg/cis-1.12/node.yaml index d7f8a7937..4a976fbf6 100644 --- a/cfg/cis-1.12/node.yaml +++ b/cfg/cis-1.12/node.yaml @@ -10,13 +10,15 @@ groups: checks: - id: 4.1.1 text: "Ensure that the kubelet service file permissions are set to 600 or more restrictive (Automated)" - audit: '/bin/sh -c ''if test -e $kubeletsvc; then stat -c permissions=%a $kubeletsvc; fi'' ' + audit: '/bin/sh -c ''if test -e $kubeletsvc; then stat -c permissions=%a $kubeletsvc; else echo "File not found"; fi'' ' tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the each worker node. For example, chmod 600 $kubeletsvc @@ -38,7 +40,7 @@ groups: - id: 4.1.3 text: "If proxy kubeconfig file exists ensure permissions are set to 600 or more restrictive (Manual)" - audit: '/bin/sh -c ''if test -e $proxykubeconfig; then stat -c permissions=%a $proxykubeconfig; fi'' ' + audit: '/bin/sh -c ''if test -e $proxykubeconfig; then stat -c permissions=%a $proxykubeconfig; else echo "File not found"; fi'' ' tests: bin_op: or test_items: @@ -47,6 +49,7 @@ groups: compare: op: bitmask value: "600" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the each worker node. For example, @@ -55,11 +58,12 @@ groups: - id: 4.1.4 text: "If proxy kubeconfig file exists ensure ownership is set to root:root (Manual)" - audit: '/bin/sh -c ''if test -e $proxykubeconfig; then stat -c %U:%G $proxykubeconfig; fi'' ' + audit: '/bin/sh -c ''if test -e $proxykubeconfig; then stat -c %U:%G $proxykubeconfig; else echo "File not found"; fi'' ' tests: bin_op: or test_items: - flag: root:root + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the each worker node. For example, chown root:root $proxykubeconfig @@ -67,13 +71,15 @@ groups: - id: 4.1.5 text: "Ensure that the --kubeconfig kubelet.conf file permissions are set to 600 or more restrictive (Automated)" - audit: '/bin/sh -c ''if test -e $kubeletkubeconfig; then stat -c permissions=%a $kubeletkubeconfig; fi'' ' + audit: '/bin/sh -c ''if test -e $kubeletkubeconfig; then stat -c permissions=%a $kubeletkubeconfig; else echo "File not found"; fi'' ' tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the each worker node. For example, @@ -82,10 +88,12 @@ groups: - id: 4.1.6 text: "Ensure that the --kubeconfig kubelet.conf file ownership is set to root:root (Automated)" - audit: '/bin/sh -c ''if test -e $kubeletkubeconfig; then stat -c %U:%G $kubeletkubeconfig; fi'' ' + audit: '/bin/sh -c ''if test -e $kubeletkubeconfig; then stat -c %U:%G $kubeletkubeconfig; else echo "File not found"; fi'' ' tests: + bin_op: or test_items: - flag: root:root + - flag: "File not found" remediation: | Run the below command (based on the file location on your system) on the each worker node. For example, @@ -128,13 +136,15 @@ groups: - id: 4.1.9 text: "If the kubelet config.yaml configuration file is being used validate permissions set to 600 or more restrictive (Automated)" - audit: '/bin/sh -c ''if test -e $kubeletconf; then stat -c permissions=%a $kubeletconf; fi'' ' + audit: '/bin/sh -c ''if test -e $kubeletconf; then stat -c permissions=%a $kubeletconf; else echo "File not found"; fi'' ' tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - flag: "File not found" remediation: | Run the following command (using the config file location identified in the Audit step) chmod 600 $kubeletconf @@ -142,10 +152,12 @@ groups: - id: 4.1.10 text: "If the kubelet config.yaml configuration file is being used validate file ownership is set to root:root (Automated)" - audit: '/bin/sh -c ''if test -e $kubeletconf; then stat -c %U:%G $kubeletconf; fi'' ' + audit: '/bin/sh -c ''if test -e $kubeletconf; then stat -c %U:%G $kubeletconf; else echo "File not found"; fi'' ' tests: + bin_op: or test_items: - flag: root:root + - flag: "File not found" remediation: | Run the following command (using the config file location identified in the Audit step) chown root:root $kubeletconf diff --git a/check/check_test.go b/check/check_test.go index 124e6f93c..62e78f675 100644 --- a/check/check_test.go +++ b/check/check_test.go @@ -94,6 +94,53 @@ func TestCheck_Run(t *testing.T) { }, Expected: FAIL, }, + { + // A file permission check whose audit guards against a missing + // file produces no output when the file is absent, which no + // test_item can match. See issue #1881. + name: "File permission check should FAIL for a missing file when the audit is silent about it", + check: Check{ + Scored: true, + Audit: "/bin/sh -c 'if test -e /no/such/file; then stat -c permissions=%a /no/such/file; fi'", + Tests: &tests{TestItems: []*testItem{{ + Flag: "permissions", + Set: true, + Compare: compare{ + Op: "bitmask", + Value: "600", + }, + }}}, + }, + Expected: FAIL, + }, + { + // Echoing a sentinel in the else branch and accepting it with + // bin_op: or makes the same check PASS, because a file that does + // not exist has no permissions to get wrong. + name: "File permission check should PASS for a missing file when the audit reports it", + check: Check{ + Scored: true, + Audit: "/bin/sh -c 'if test -e /no/such/file; then stat -c permissions=%a /no/such/file; else echo \"File not found\"; fi'", + Tests: &tests{ + BinOp: or, + TestItems: []*testItem{ + { + Flag: "permissions", + Set: true, + Compare: compare{ + Op: "bitmask", + Value: "600", + }, + }, + { + Flag: "File not found", + Set: true, + }, + }, + }, + }, + Expected: PASS, + }, } for _, testCase := range testCases { diff --git a/check/controls_test.go b/check/controls_test.go index c2f6ab36e..9ef6cde23 100644 --- a/check/controls_test.go +++ b/check/controls_test.go @@ -22,6 +22,7 @@ import ( "os" "path/filepath" "reflect" + "strings" "testing" "github.com/aws/aws-sdk-go-v2/aws" @@ -72,6 +73,114 @@ func TestYamlFiles(t *testing.T) { } } +// benchmarksGuardingFileExistence lists the benchmark directories that have +// been reviewed for https://github.com/aquasecurity/kube-bench/issues/1881. +// Older benchmarks share the same defect and can be added here as they are +// fixed. +var benchmarksGuardingFileExistence = []string{"cis-1.12"} + +// missingFileSentinel is echoed by an audit command when the file it inspects +// does not exist. +const missingFileSentinel = "File not found" + +// checksWhereMissingFileIsAFinding lists the checks that intentionally keep +// reporting FAIL when the audited file is absent, because its absence is the +// finding rather than a property of a file that is not there. +var checksWhereMissingFileIsAFinding = map[string]string{ + // The file is the encryption provider configuration: if it is missing, + // encryption of secrets at rest is not configured at all. + "cis-1.12/master.yaml 1.2.28": "no encryption provider configuration means encryption is not configured", + // The file is the client certificate authority: if it is missing, the + // kubelet has no CA to authenticate API server clients against. + "cis-1.12/node.yaml 4.1.7": "a missing client CA file is a finding of its own", + "cis-1.12/node.yaml 4.1.8": "a missing client CA file is a finding of its own", +} + +// A check whose audit command guards against a missing file, for example +// +// /bin/sh -c 'if test -e $apiserverconf; then stat -c permissions=%a $apiserverconf; fi' +// +// produces no output at all when the file is absent. A test_item that looks +// for a flag in that output can never match, so the check is reported as FAIL +// for a file that simply does not exist. The guard is then pointless: without +// it stat would fail and the check would be reported as FAIL as well. +// +// Such checks must tolerate the empty output, the way cis-1.12/node.yaml 4.1.2 +// does: the audit echoes a sentinel in its else branch and the tests accept +// either the real value or that sentinel via bin_op: or. +func TestChecksGuardingFileExistenceTolerateMissingFile(t *testing.T) { + for _, benchmark := range benchmarksGuardingFileExistence { + dir := filepath.Join(cfgDir, benchmark) + files, err := filepath.Glob(filepath.Join(dir, "*.yaml")) + if err != nil { + t.Fatalf("failure globbing %q: %v", dir, err) + } + if len(files) == 0 { + t.Fatalf("no benchmark files found in %q", dir) + } + + for _, path := range files { + in, err := os.ReadFile(path) + if err != nil { + t.Fatalf("error opening file %s: %v", path, err) + } + + c := new(Controls) + if err := yaml.Unmarshal(in, c); err != nil { + t.Fatalf("failed to load YAML from %s: %v", path, err) + } + + for _, group := range c.Groups { + for _, check := range group.Checks { + if !auditGuardsFileExistence(check.Audit) { + continue + } + key := fmt.Sprintf("%s/%s %s", benchmark, filepath.Base(path), check.ID) + if _, ok := checksWhereMissingFileIsAFinding[key]; ok { + continue + } + if !toleratesEmptyAuditOutput(check.Tests) { + t.Errorf("%s: check %s guards against a missing file but reports FAIL when it is missing: "+ + "the audit should echo a sentinel in its else branch and the tests should accept it with bin_op: or", + path, check.ID) + } + } + } + } + } +} + +func auditGuardsFileExistence(audit string) bool { + for _, guard := range []string{"test -e", "test -f", "[[ -e", "[ -e"} { + if strings.Contains(audit, guard) { + return true + } + } + return false +} + +func toleratesEmptyAuditOutput(ts *tests) bool { + if ts == nil || len(ts.TestItems) == 0 { + return true + } + satisfied := false + for _, item := range ts.TestItems { + // A test_item that requires a flag to be absent, or that matches the + // sentinel echoed when the file does not exist, is satisfied by the + // empty output. + if !item.Set || item.Flag == missingFileSentinel { + satisfied = true + break + } + } + if !satisfied { + return false + } + // The remaining test_items are not satisfied by the empty output, so they + // must not be combined with the tolerant one by the default AND. + return len(ts.TestItems) == 1 || ts.BinOp == or +} + func TestNewControls(t *testing.T) { t.Run("Should return error when node type is not specified", func(t *testing.T) { // given From 681a90a9254debd078b49bd6255fa3e91e0d9459 Mon Sep 17 00:00:00 2001 From: Eljees <3.14hell@gmail.com> Date: Sat, 1 Aug 2026 13:28:31 +0300 Subject: [PATCH 2/2] fixup(cis-1.12): use bare stat with || true instead of existence guards Apply andypitcher's review suggestion to every wrapped check: drop the 'if test -e ... else echo' guards and let stat report a missing file itself. stat alone exits non-zero, and runAudit turns that into an error that fails the check before any test_item is evaluated, so the audits keep '|| true' to stay on the flag-matching path; the tests accept stat's own 'No such file or directory' (captured from stderr) via bin_op: or. The 4.1.2 upstream sentinel check is aligned to the same pattern, the multi-file 1.1.13/1.1.14 loops drop their found-sentinel, and the guard meta-test now enforces the new shape. A new unit test documents why the bare stat without '|| true' would regress to FAIL. Signed-off-by: Eljees <3.14hell@gmail.com> --- cfg/cis-1.12/master.yaml | 56 ++++++++++++++++++++-------------------- cfg/cis-1.12/node.yaml | 32 +++++++++++------------ check/check_test.go | 43 +++++++++++++++++++++++++----- check/controls_test.go | 44 +++++++++++++++++++++---------- 4 files changed, 112 insertions(+), 63 deletions(-) diff --git a/cfg/cis-1.12/master.yaml b/cfg/cis-1.12/master.yaml index ffc3a3007..a1acf8602 100644 --- a/cfg/cis-1.12/master.yaml +++ b/cfg/cis-1.12/master.yaml @@ -10,7 +10,7 @@ groups: checks: - id: 1.1.1 text: "Ensure that the API server pod specification file permissions are set to 600 or more restrictive (Automated)" - audit: "/bin/sh -c 'if test -e $apiserverconf; then stat -c permissions=%a $apiserverconf; else echo \"File not found\"; fi'" + audit: "/bin/sh -c 'stat -c permissions=%a $apiserverconf || true'" tests: bin_op: or test_items: @@ -18,7 +18,7 @@ groups: compare: op: bitmask value: "600" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the control plane node. @@ -27,12 +27,12 @@ groups: - id: 1.1.2 text: "Ensure that the API server pod specification file ownership is set to root:root (Automated)" - audit: "/bin/sh -c 'if test -e $apiserverconf; then stat -c %U:%G $apiserverconf; else echo \"File not found\"; fi'" + audit: "/bin/sh -c 'stat -c %U:%G $apiserverconf || true'" tests: bin_op: or test_items: - flag: "root:root" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, chown root:root $apiserverconf @@ -40,7 +40,7 @@ groups: - id: 1.1.3 text: "Ensure that the controller manager pod specification file permissions are set to 600 or more restrictive (Automated)" - audit: "/bin/sh -c 'if test -e $controllermanagerconf; then stat -c permissions=%a $controllermanagerconf; else echo \"File not found\"; fi'" + audit: "/bin/sh -c 'stat -c permissions=%a $controllermanagerconf || true'" tests: bin_op: or test_items: @@ -48,7 +48,7 @@ groups: compare: op: bitmask value: "600" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, chmod 600 $controllermanagerconf @@ -56,12 +56,12 @@ groups: - id: 1.1.4 text: "Ensure that the controller manager pod specification file ownership is set to root:root (Automated)" - audit: "/bin/sh -c 'if test -e $controllermanagerconf; then stat -c %U:%G $controllermanagerconf; else echo \"File not found\"; fi'" + audit: "/bin/sh -c 'stat -c %U:%G $controllermanagerconf || true'" tests: bin_op: or test_items: - flag: "root:root" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, chown root:root $controllermanagerconf @@ -69,7 +69,7 @@ groups: - id: 1.1.5 text: "Ensure that the scheduler pod specification file permissions are set to 600 or more restrictive (Automated)" - audit: "/bin/sh -c 'if test -e $schedulerconf; then stat -c permissions=%a $schedulerconf; else echo \"File not found\"; fi'" + audit: "/bin/sh -c 'stat -c permissions=%a $schedulerconf || true'" tests: bin_op: or test_items: @@ -77,7 +77,7 @@ groups: compare: op: bitmask value: "600" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, chmod 600 $schedulerconf @@ -85,12 +85,12 @@ groups: - id: 1.1.6 text: "Ensure that the scheduler pod specification file ownership is set to root:root (Automated)" - audit: "/bin/sh -c 'if test -e $schedulerconf; then stat -c %U:%G $schedulerconf; else echo \"File not found\"; fi'" + audit: "/bin/sh -c 'stat -c %U:%G $schedulerconf || true'" tests: bin_op: or test_items: - flag: "root:root" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, chown root:root $schedulerconf @@ -98,7 +98,7 @@ groups: - id: 1.1.7 text: "Ensure that the etcd pod specification file permissions are set to 600 or more restrictive (Automated)" - audit: "/bin/sh -c 'if test -e $etcdconf; then find $etcdconf -name '*etcd*' | xargs stat -c permissions=%a; else echo \"File not found\"; fi'" + audit: "/bin/sh -c 'stat -c permissions=%a $etcdconf || true'" use_multiple_values: true tests: bin_op: or @@ -107,7 +107,7 @@ groups: compare: op: bitmask value: "600" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, @@ -116,13 +116,13 @@ groups: - id: 1.1.8 text: "Ensure that the etcd pod specification file ownership is set to root:root (Automated)" - audit: "/bin/sh -c 'if test -e $etcdconf; then find $etcdconf -name '*etcd*' | xargs stat -c %U:%G; else echo \"File not found\"; fi'" + audit: "/bin/sh -c 'stat -c %U:%G $etcdconf || true'" use_multiple_values: true tests: bin_op: or test_items: - flag: "root:root" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, @@ -205,7 +205,7 @@ groups: - id: 1.1.13 text: "Ensure that the default administrative credential file permissions are set to 600 (Automated)" audit: | - found=""; for adminconf in /etc/kubernetes/admin.conf /etc/kubernetes/super-admin.conf; do if test -e $adminconf; then found="yes"; stat -c "permissions=%a %n" $adminconf; fi; done; if test -z "$found"; then echo "File not found"; fi + for adminconf in /etc/kubernetes/admin.conf /etc/kubernetes/super-admin.conf; do stat -c "permissions=%a %n" $adminconf; done || true use_multiple_values: true tests: bin_op: or @@ -214,7 +214,7 @@ groups: compare: op: bitmask value: "600" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, chmod 600 /etc/kubernetes/admin.conf @@ -225,7 +225,7 @@ groups: - id: 1.1.14 text: "Ensure that the default administrative credential file ownership is set to root:root (Automated)" audit: | - found=""; for adminconf in /etc/kubernetes/admin.conf /etc/kubernetes/super-admin.conf; do if test -e $adminconf; then found="yes"; stat -c "ownership=%U:%G %n" $adminconf; fi; done; if test -z "$found"; then echo "File not found"; fi + for adminconf in /etc/kubernetes/admin.conf /etc/kubernetes/super-admin.conf; do stat -c "ownership=%U:%G %n" $adminconf; done || true use_multiple_values: true tests: bin_op: or @@ -234,7 +234,7 @@ groups: compare: op: eq value: "root:root" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, chown root:root /etc/kubernetes/admin.conf @@ -244,7 +244,7 @@ groups: - id: 1.1.15 text: "Ensure that the scheduler.conf file permissions are set to 600 or more restrictive (Automated)" - audit: "/bin/sh -c 'if test -e $schedulerkubeconfig; then stat -c permissions=%a $schedulerkubeconfig; else echo \"File not found\"; fi'" + audit: "/bin/sh -c 'stat -c permissions=%a $schedulerkubeconfig || true'" tests: bin_op: or test_items: @@ -252,7 +252,7 @@ groups: compare: op: bitmask value: "600" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, @@ -261,12 +261,12 @@ groups: - id: 1.1.16 text: "Ensure that the scheduler.conf file ownership is set to root:root (Automated)" - audit: "/bin/sh -c 'if test -e $schedulerkubeconfig; then stat -c %U:%G $schedulerkubeconfig; else echo \"File not found\"; fi'" + audit: "/bin/sh -c 'stat -c %U:%G $schedulerkubeconfig || true'" tests: bin_op: or test_items: - flag: "root:root" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, @@ -275,7 +275,7 @@ groups: - id: 1.1.17 text: "Ensure that the controller-manager.conf file permissions are set to 600 or more restrictive (Automated)" - audit: "/bin/sh -c 'if test -e $controllermanagerkubeconfig; then stat -c permissions=%a $controllermanagerkubeconfig; else echo \"File not found\"; fi'" + audit: "/bin/sh -c 'stat -c permissions=%a $controllermanagerkubeconfig || true'" tests: bin_op: or test_items: @@ -283,7 +283,7 @@ groups: compare: op: bitmask value: "600" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, @@ -292,12 +292,12 @@ groups: - id: 1.1.18 text: "Ensure that the controller-manager.conf file ownership is set to root:root (Automated)" - audit: "/bin/sh -c 'if test -e $controllermanagerkubeconfig; then stat -c %U:%G $controllermanagerkubeconfig; else echo \"File not found\"; fi'" + audit: "/bin/sh -c 'stat -c %U:%G $controllermanagerkubeconfig || true'" tests: bin_op: or test_items: - flag: "root:root" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the control plane node. For example, diff --git a/cfg/cis-1.12/node.yaml b/cfg/cis-1.12/node.yaml index 4a976fbf6..4c9991945 100644 --- a/cfg/cis-1.12/node.yaml +++ b/cfg/cis-1.12/node.yaml @@ -10,7 +10,7 @@ groups: checks: - id: 4.1.1 text: "Ensure that the kubelet service file permissions are set to 600 or more restrictive (Automated)" - audit: '/bin/sh -c ''if test -e $kubeletsvc; then stat -c permissions=%a $kubeletsvc; else echo "File not found"; fi'' ' + audit: '/bin/sh -c "stat -c permissions=%a $kubeletsvc || true"' tests: bin_op: or test_items: @@ -18,7 +18,7 @@ groups: compare: op: bitmask value: "600" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the each worker node. For example, chmod 600 $kubeletsvc @@ -26,12 +26,12 @@ groups: - id: 4.1.2 text: "Ensure that the kubelet service file ownership is set to root:root (Automated)" - audit: '/bin/sh -c "if test -e $kubeletsvc; then stat -c %U:%G $kubeletsvc; else echo \"File not found\"; fi"' + audit: '/bin/sh -c "stat -c %U:%G $kubeletsvc || true"' tests: bin_op: or test_items: - flag: root:root - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the each worker node. For example, @@ -40,7 +40,7 @@ groups: - id: 4.1.3 text: "If proxy kubeconfig file exists ensure permissions are set to 600 or more restrictive (Manual)" - audit: '/bin/sh -c ''if test -e $proxykubeconfig; then stat -c permissions=%a $proxykubeconfig; else echo "File not found"; fi'' ' + audit: '/bin/sh -c "stat -c permissions=%a $proxykubeconfig || true"' tests: bin_op: or test_items: @@ -49,7 +49,7 @@ groups: compare: op: bitmask value: "600" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the each worker node. For example, @@ -58,12 +58,12 @@ groups: - id: 4.1.4 text: "If proxy kubeconfig file exists ensure ownership is set to root:root (Manual)" - audit: '/bin/sh -c ''if test -e $proxykubeconfig; then stat -c %U:%G $proxykubeconfig; else echo "File not found"; fi'' ' + audit: '/bin/sh -c "stat -c %U:%G $proxykubeconfig || true"' tests: bin_op: or test_items: - flag: root:root - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the each worker node. For example, chown root:root $proxykubeconfig @@ -71,7 +71,7 @@ groups: - id: 4.1.5 text: "Ensure that the --kubeconfig kubelet.conf file permissions are set to 600 or more restrictive (Automated)" - audit: '/bin/sh -c ''if test -e $kubeletkubeconfig; then stat -c permissions=%a $kubeletkubeconfig; else echo "File not found"; fi'' ' + audit: '/bin/sh -c "stat -c permissions=%a $kubeletkubeconfig || true"' tests: bin_op: or test_items: @@ -79,7 +79,7 @@ groups: compare: op: bitmask value: "600" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the each worker node. For example, @@ -88,12 +88,12 @@ groups: - id: 4.1.6 text: "Ensure that the --kubeconfig kubelet.conf file ownership is set to root:root (Automated)" - audit: '/bin/sh -c ''if test -e $kubeletkubeconfig; then stat -c %U:%G $kubeletkubeconfig; else echo "File not found"; fi'' ' + audit: '/bin/sh -c "stat -c %U:%G $kubeletkubeconfig || true"' tests: bin_op: or test_items: - flag: root:root - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the below command (based on the file location on your system) on the each worker node. For example, @@ -136,7 +136,7 @@ groups: - id: 4.1.9 text: "If the kubelet config.yaml configuration file is being used validate permissions set to 600 or more restrictive (Automated)" - audit: '/bin/sh -c ''if test -e $kubeletconf; then stat -c permissions=%a $kubeletconf; else echo "File not found"; fi'' ' + audit: '/bin/sh -c "stat -c permissions=%a $kubeletconf || true"' tests: bin_op: or test_items: @@ -144,7 +144,7 @@ groups: compare: op: bitmask value: "600" - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the following command (using the config file location identified in the Audit step) chmod 600 $kubeletconf @@ -152,12 +152,12 @@ groups: - id: 4.1.10 text: "If the kubelet config.yaml configuration file is being used validate file ownership is set to root:root (Automated)" - audit: '/bin/sh -c ''if test -e $kubeletconf; then stat -c %U:%G $kubeletconf; else echo "File not found"; fi'' ' + audit: '/bin/sh -c "stat -c %U:%G $kubeletconf || true"' tests: bin_op: or test_items: - flag: root:root - - flag: "File not found" + - flag: "No such file or directory" remediation: | Run the following command (using the config file location identified in the Audit step) chown root:root $kubeletconf diff --git a/check/check_test.go b/check/check_test.go index 62e78f675..411690d75 100644 --- a/check/check_test.go +++ b/check/check_test.go @@ -114,13 +114,14 @@ func TestCheck_Run(t *testing.T) { Expected: FAIL, }, { - // Echoing a sentinel in the else branch and accepting it with - // bin_op: or makes the same check PASS, because a file that does - // not exist has no permissions to get wrong. - name: "File permission check should PASS for a missing file when the audit reports it", + // A bare stat exits non-zero for a missing file. runAudit turns + // that into an error and the check FAILs on the error path, + // before any test_item is evaluated - which is why the audit + // needs `|| true`. + name: "File permission check should FAIL for a missing file when the audit exits non-zero", check: Check{ Scored: true, - Audit: "/bin/sh -c 'if test -e /no/such/file; then stat -c permissions=%a /no/such/file; else echo \"File not found\"; fi'", + Audit: "/bin/sh -c 'stat -c permissions=%a /no/such/file'", Tests: &tests{ BinOp: or, TestItems: []*testItem{ @@ -133,7 +134,37 @@ func TestCheck_Run(t *testing.T) { }, }, { - Flag: "File not found", + Flag: "No such file or directory", + Set: true, + }, + }, + }, + }, + Expected: FAIL, + }, + { + // With `|| true` the audit exits zero, kube-bench captures + // stat's own error message (stderr is collected together with + // stdout), and accepting it with bin_op: or makes the check + // PASS: a file that does not exist has no permissions to get + // wrong. + name: "File permission check should PASS for a missing file when stat reports it", + check: Check{ + Scored: true, + Audit: "/bin/sh -c 'stat -c permissions=%a /no/such/file || true'", + Tests: &tests{ + BinOp: or, + TestItems: []*testItem{ + { + Flag: "permissions", + Set: true, + Compare: compare{ + Op: "bitmask", + Value: "600", + }, + }, + { + Flag: "No such file or directory", Set: true, }, }, diff --git a/check/controls_test.go b/check/controls_test.go index 9ef6cde23..170169d83 100644 --- a/check/controls_test.go +++ b/check/controls_test.go @@ -79,9 +79,10 @@ func TestYamlFiles(t *testing.T) { // fixed. var benchmarksGuardingFileExistence = []string{"cis-1.12"} -// missingFileSentinel is echoed by an audit command when the file it inspects -// does not exist. -const missingFileSentinel = "File not found" +// missingFileSentinel is the message stat prints when the file an audit +// command inspects does not exist. kube-bench collects stderr together with +// stdout, so a test_item can match it directly. +const missingFileSentinel = "No such file or directory" // checksWhereMissingFileIsAFinding lists the checks that intentionally keep // reporting FAIL when the audited file is absent, because its absence is the @@ -102,12 +103,18 @@ var checksWhereMissingFileIsAFinding = map[string]string{ // // produces no output at all when the file is absent. A test_item that looks // for a flag in that output can never match, so the check is reported as FAIL -// for a file that simply does not exist. The guard is then pointless: without -// it stat would fail and the check would be reported as FAIL as well. +// for a file that simply does not exist. // -// Such checks must tolerate the empty output, the way cis-1.12/node.yaml 4.1.2 -// does: the audit echoes a sentinel in its else branch and the tests accept -// either the real value or that sentinel via bin_op: or. +// File permission and ownership checks therefore use a bare stat with +// `|| true`: +// +// /bin/sh -c 'stat -c permissions=%a $apiserverconf || true' +// +// stat's own error message ("No such file or directory", collected from +// stderr) is accepted alongside the real value via bin_op: or, and `|| true` +// keeps the audit's exit code zero so the check is not failed on the error +// path before the test_items are evaluated. Existence guards remain only in +// the checks where a missing file is the finding itself. func TestChecksGuardingFileExistenceTolerateMissingFile(t *testing.T) { for _, benchmark := range benchmarksGuardingFileExistence { dir := filepath.Join(cfgDir, benchmark) @@ -132,16 +139,21 @@ func TestChecksGuardingFileExistenceTolerateMissingFile(t *testing.T) { for _, group := range c.Groups { for _, check := range group.Checks { - if !auditGuardsFileExistence(check.Audit) { + key := fmt.Sprintf("%s/%s %s", benchmark, filepath.Base(path), check.ID) + if auditGuardsFileExistence(check.Audit) { + if _, ok := checksWhereMissingFileIsAFinding[key]; !ok { + t.Errorf("%s: check %s guards against a missing file: "+ + "use a bare stat with `|| true` and accept its error message with bin_op: or", + path, check.ID) + } continue } - key := fmt.Sprintf("%s/%s %s", benchmark, filepath.Base(path), check.ID) - if _, ok := checksWhereMissingFileIsAFinding[key]; ok { + if !auditToleratesMissingFile(check.Audit) { continue } if !toleratesEmptyAuditOutput(check.Tests) { - t.Errorf("%s: check %s guards against a missing file but reports FAIL when it is missing: "+ - "the audit should echo a sentinel in its else branch and the tests should accept it with bin_op: or", + t.Errorf("%s: check %s tolerates a missing file in its audit but reports FAIL when it is missing: "+ + "the tests should accept stat's error message with bin_op: or", path, check.ID) } } @@ -159,6 +171,12 @@ func auditGuardsFileExistence(audit string) bool { return false } +// auditToleratesMissingFile reports whether the audit lets a stat on a +// missing file surface its error message instead of failing the command. +func auditToleratesMissingFile(audit string) bool { + return strings.Contains(audit, "stat -c") && strings.Contains(audit, "|| true") +} + func toleratesEmptyAuditOutput(ts *tests) bool { if ts == nil || len(ts.TestItems) == 0 { return true