diff --git a/cfg/cis-1.12/master.yaml b/cfg/cis-1.12/master.yaml index b0cc78129..a1acf8602 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 'stat -c permissions=%a $apiserverconf || true'" tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - flag: "No such file or directory" 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 'stat -c %U:%G $apiserverconf || true'" tests: + bin_op: or test_items: - flag: "root:root" + - 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 @@ -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 'stat -c permissions=%a $controllermanagerconf || true'" tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - 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 @@ -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 'stat -c %U:%G $controllermanagerconf || true'" tests: + bin_op: or test_items: - flag: "root:root" + - 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 @@ -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 'stat -c permissions=%a $schedulerconf || true'" tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - 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 @@ -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 'stat -c %U:%G $schedulerconf || true'" tests: + bin_op: or test_items: - flag: "root:root" + - 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 @@ -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 'stat -c permissions=%a $etcdconf || true'" use_multiple_values: true tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - 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, @@ -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 'stat -c %U:%G $etcdconf || true'" use_multiple_values: true tests: + bin_op: or test_items: - flag: "root:root" + - 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, @@ -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 + 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 test_items: - flag: "permissions" compare: op: bitmask value: "600" + - 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 @@ -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 + 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 test_items: - flag: "ownership" compare: op: eq value: "root:root" + - 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 @@ -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 'stat -c permissions=%a $schedulerkubeconfig || true'" tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - 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, @@ -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 'stat -c %U:%G $schedulerkubeconfig || true'" tests: + bin_op: or test_items: - flag: "root:root" + - 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, @@ -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 'stat -c permissions=%a $controllermanagerkubeconfig || true'" tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - 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, @@ -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 'stat -c %U:%G $controllermanagerkubeconfig || true'" tests: + bin_op: or test_items: - flag: "root:root" + - 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 d7f8a7937..4c9991945 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 "stat -c permissions=%a $kubeletsvc || true"' tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - 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 @@ -24,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, @@ -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 "stat -c permissions=%a $proxykubeconfig || true"' tests: bin_op: or test_items: @@ -47,6 +49,7 @@ groups: compare: op: bitmask value: "600" + - 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, @@ -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 "stat -c %U:%G $proxykubeconfig || true"' tests: bin_op: or test_items: - flag: root:root + - 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 @@ -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 "stat -c permissions=%a $kubeletkubeconfig || true"' tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - 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, @@ -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 "stat -c %U:%G $kubeletkubeconfig || true"' tests: + bin_op: or test_items: - flag: root:root + - 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, @@ -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 "stat -c permissions=%a $kubeletconf || true"' tests: + bin_op: or test_items: - flag: "permissions" compare: op: bitmask value: "600" + - flag: "No such file or directory" 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 "stat -c %U:%G $kubeletconf || true"' tests: + bin_op: or test_items: - flag: root:root + - 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 124e6f93c..411690d75 100644 --- a/check/check_test.go +++ b/check/check_test.go @@ -94,6 +94,84 @@ 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, + }, + { + // 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 'stat -c permissions=%a /no/such/file'", + Tests: &tests{ + BinOp: or, + TestItems: []*testItem{ + { + Flag: "permissions", + Set: true, + Compare: compare{ + Op: "bitmask", + Value: "600", + }, + }, + { + 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, + }, + }, + }, + }, + Expected: PASS, + }, } for _, testCase := range testCases { diff --git a/check/controls_test.go b/check/controls_test.go index c2f6ab36e..170169d83 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,132 @@ 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 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 +// 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. +// +// 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) + 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 { + 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 + } + if !auditToleratesMissingFile(check.Audit) { + continue + } + if !toleratesEmptyAuditOutput(check.Tests) { + 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) + } + } + } + } + } +} + +func auditGuardsFileExistence(audit string) bool { + for _, guard := range []string{"test -e", "test -f", "[[ -e", "[ -e"} { + if strings.Contains(audit, guard) { + return true + } + } + 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 + } + 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