Replace custom k3s etcd script checks with vanilla grep checks #1601

dereknola · 2024-04-22T19:24:43Z

Background:
When #1523 was merged, it pulled the cfg checks directly from https://github.com/rancher/security-scan/tree/master/package/cfg.

Problem:

These checks utilizes a custom sonobuoy plugin to deploy kube-bench + additional helper scripts. These additional helper scripts were not accounted for when copying over the cfg. K3s utilizes kine which allows sqlite and other non etcd databases. The helper scripts allowed CIS check to pass on these other DB. These helper scripts do not exist in upstream kubebench.

Solution:

This PR fixes the k3s 1.23, 1.24, and 1.7 benchmarks, restoring the correct etcd checks

Note: This means that running these benchmarks will only pass these checks is K3s is utilizing etcd (via the --cluster-init embedded etcd flag). It does not solve the original problem the custom scripts rancher implemented do.

Validation:

k3s version v1.29.4+k3s1, with hardening applied using https://docs.k3s.io/security/hardening-guide

main:

== Summary total ==
42 checks PASS
8 checks FAIL
45 checks WARN
36 checks INFO

this PR:

== Summary total ==
51 checks PASS
0 checks FAIL
44 checks WARN
36 checks INFO

Signed-off-by: Derek Nola <derek.nola@suse.com>

andypitcher · 2024-05-03T15:05:13Z

Test case 1: auto-tls and peer-auto-tls set to false

[root@k3s-standalone]# cat /etc/rancher/k3s/config.yaml.d/50-rancher.yaml | tail -5
  "etcd-arg": [
    "auto-tls=true",
    "peer-auto-tls=true"
  ]
}

Results:

[root@k3s-standalone]# ./kube-bench run --targets etcd --config-dir ./security-scan-0.2.14/package/cfg --config ./security-scan-0.2.14/package/cfg/config.yaml  --benchmark k3s-cis-1.7 --group 2 --include-test-output
[INFO] 2 Etcd Node Configuration
[INFO] 2 Etcd Node Configuration
[PASS] 2.1 Ensure that the --cert-file and --key-file arguments are set as appropriate (Automated)
[PASS] 2.2 Ensure that the --client-cert-auth argument is set to true (Automated)
[PASS] 2.3 Ensure that the --auto-tls argument is not set to true (Automated)
[PASS] 2.4 Ensure that the --peer-cert-file and --peer-key-file arguments are set as appropriate (Automated)
[PASS] 2.5 Ensure that the --peer-client-cert-auth argument is set to true (Automated)
[PASS] 2.6 Ensure that the --peer-auto-tls argument is not set to true (Automated)
[PASS] 2.7 Ensure that a unique Certificate Authority is used for etcd (Automated)

== Summary etcd ==
7 checks PASS
0 checks FAIL
0 checks WARN
0 checks INFO

== Summary total ==
7 checks PASS
0 checks FAIL
0 checks WARN
0 checks INFO

[root@k3s-standalone]# grep auto-tls /var/lib/rancher/k3s/server/db/etcd/config
auto-tls: false
peer-auto-tls: false

Test case 2: auto-tls and peer-auto-tls set to true

Set configuration in /etc/rancher/k3s/config.yaml.d/50-rancher.yaml and restart k3s unit.

[root@k3s-standalone]# cat /etc/rancher/k3s/config.yaml.d/50-rancher.yaml | tail -5
  "etcd-arg": [
    "auto-tls=true",
    "peer-auto-tls=true"
  ]
}

Results:

[root@k3s-standalone]# ./kube-bench run --targets etcd --config-dir ./security-scan-0.2.14/package/cfg --config ./security-scan-0.2.14/package/cfg/config.yaml  --benchmark k3s-cis-1.7 --group 2 --include-test-output
[INFO] 2 Etcd Node Configuration
[INFO] 2 Etcd Node Configuration
[PASS] 2.1 Ensure that the --cert-file and --key-file arguments are set as appropriate (Automated)
[PASS] 2.2 Ensure that the --client-cert-auth argument is set to true (Automated)
[FAIL] 2.3 Ensure that the --auto-tls argument is not set to true (Automated)
	 auto-tls: true
[PASS] 2.4 Ensure that the --peer-cert-file and --peer-key-file arguments are set as appropriate (Automated)
[PASS] 2.5 Ensure that the --peer-client-cert-auth argument is set to true (Automated)
[FAIL] 2.6 Ensure that the --peer-auto-tls argument is not set to true (Automated)
	 peer-auto-tls: true
[PASS] 2.7 Ensure that a unique Certificate Authority is used for etcd (Automated)

== Remediations etcd ==
2.3 Edit the etcd pod specification file /var/lib/rancher/k3s/server/db/etcd/config on the master
node and either remove the --auto-tls parameter or set it to false.
  --auto-tls=false

2.6 Edit the etcd pod specification file /var/lib/rancher/k3s/server/db/etcd/config on the master
node and either remove the --peer-auto-tls parameter or set it to false.
--peer-auto-tls=false


== Summary etcd ==
5 checks PASS
2 checks FAIL
0 checks WARN
0 checks INFO

== Summary total ==
5 checks PASS
2 checks FAIL
0 checks WARN
0 checks INFO

Test case 3: auto-tls and peer-auto-tls not set (default value is false)

Set configuration in /etc/rancher/k3s/config.yaml.d/50-rancher.yaml and restart k3s unit.

[root@k3s-standalone]# grep auto-tls /var/lib/rancher/k3s/server/db/etcd/config
[root@k3s-standalone]# echo $?
1

Results:

[root@k3s-standalone]#./kube-bench run --targets etcd --config-dir ./security-scan-0.2.14/package/cfg --config ./security-scan-0.2.14/package/cfg/config.yaml  --benchmark k3s-cis-1.7 --group 2 --include-test-output
[INFO] 2 Etcd Node Configuration
[INFO] 2 Etcd Node Configuration
[PASS] 2.1 Ensure that the --cert-file and --key-file arguments are set as appropriate (Automated)
[PASS] 2.2 Ensure that the --client-cert-auth argument is set to true (Automated)
[PASS] 2.3 Ensure that the --auto-tls argument is not set to true (Automated)
[PASS] 2.4 Ensure that the --peer-cert-file and --peer-key-file arguments are set as appropriate (Automated)
[PASS] 2.5 Ensure that the --peer-client-cert-auth argument is set to true (Automated)
[PASS] 2.6 Ensure that the --peer-auto-tls argument is not set to true (Automated)
[PASS] 2.7 Ensure that a unique Certificate Authority is used for etcd (Automated)

== Summary etcd ==
7 checks PASS
0 checks FAIL
0 checks WARN
0 checks INFO

== Summary total ==
7 checks PASS
0 checks FAIL
0 checks WARN
0 checks INFO

andypitcher

@dereknola feel free to apply the changes. Regarding the $etcddatadir, we need to incorporate it in all k3s-cis-* configmap, and append it in the kube-bench's global configmap.
With regards to the env: test_items we could get rid of them since k8s environment variables are not usable with k3s (configuration are passed through etcd-arg etc).

cfg/k3s-cis-1.23/etcd.yaml

cfg/k3s-cis-1.7/etcd.yaml

cfg/k3s-cis-1.7/master.yaml

…ddatadir Signed-off-by: Derek Nola <derek.nola@suse.com>

dereknola · 2024-05-06T19:10:52Z

Lint appears to be failing for unrelated reasons to this PR:

derek@degion:~/tmp/kube-bench$ golangci-lint run -c ./.golangci.yaml 
WARN [runner] The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. 
WARN [runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. 
check/controls_test.go:43:12: m.Called undefined (type *mockRunner has no field or method Called) (typecheck)
	args := m.Called(c)
	          ^
check/controls_test.go:201:10: runner.On undefined (type *mockRunner has no field or method On) (typecheck)
		runner.On("Run", controls.Groups[0].Checks[0]).Return(PASS)
		       ^
check/controls_test.go:202:10: runner.On undefined (type *mockRunner has no field or method On) (typecheck)
		runner.On("Run", controls.Groups[1].Checks[0]).Return(FAIL)
		       ^
check/controls_test.go:234:10: runner.AssertExpectations undefined (type *mockRunner has no field or method AssertExpectations) (typecheck)
		runner.AssertExpectations(t)

@chen-keinan Should I attempt to include fixes for this lint in this PR?

chen-keinan · 2024-05-07T05:36:28Z

Lint appears to be failing for unrelated reasons to this PR:

derek@degion:~/tmp/kube-bench$ golangci-lint run -c ./.golangci.yaml 
WARN [runner] The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. 
WARN [runner] The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. 
check/controls_test.go:43:12: m.Called undefined (type *mockRunner has no field or method Called) (typecheck)
	args := m.Called(c)
	          ^
check/controls_test.go:201:10: runner.On undefined (type *mockRunner has no field or method On) (typecheck)
		runner.On("Run", controls.Groups[0].Checks[0]).Return(PASS)
		       ^
check/controls_test.go:202:10: runner.On undefined (type *mockRunner has no field or method On) (typecheck)
		runner.On("Run", controls.Groups[1].Checks[0]).Return(FAIL)
		       ^
check/controls_test.go:234:10: runner.AssertExpectations undefined (type *mockRunner has no field or method AssertExpectations) (typecheck)
		runner.AssertExpectations(t)

@chen-keinan Should I attempt to include fixes for this lint in this PR?

could be related to linter version or go version change

Signed-off-by: chenk <hen.keinan@gmail.com>

mozillazg

Thanks for your contribution! I've added some comments. Please check them when you get a chance. Thanks!

cfg/k3s-cis-1.23/master.yaml

cfg/k3s-cis-1.24/master.yaml

cfg/k3s-cis-1.7/master.yaml

Signed-off-by: Derek Nola <derek.nola@suse.com>

mozillazg

LGTM, Thanks for your contribution!

chen-keinan · 2024-05-20T10:47:11Z

lgtm 🚀 thanks for the contributions

Replace custom k3s etcd script checks with vanilla grep checks

2052688

Signed-off-by: Derek Nola <derek.nola@suse.com>

andypitcher suggested changes May 3, 2024

View reviewed changes

Rework etcd grep, remove etcd ENV checks (no-op), add correct k3s etc…

df52f48

…ddatadir Signed-off-by: Derek Nola <derek.nola@suse.com>

dereknola force-pushed the fix_k3s_no_helpers branch from 149f3bd to df52f48 Compare May 6, 2024 19:01

Merge branch 'main' into fix_k3s_no_helpers

687c0e1

chore: update go-linter version

86a42b5

Signed-off-by: chenk <hen.keinan@gmail.com>

chen-keinan requested a review from mozillazg May 16, 2024 07:21

mozillazg requested changes May 16, 2024

View reviewed changes

cfg/k3s-cis-1.23/master.yaml Show resolved Hide resolved

cfg/k3s-cis-1.24/master.yaml Show resolved Hide resolved

cfg/k3s-cis-1.7/master.yaml Outdated Show resolved Hide resolved

cfg/k3s-cis-1.7/master.yaml Show resolved Hide resolved

mozillazg requested a review from andypitcher May 16, 2024 15:26

Use etcddatadir variable

a717453

Signed-off-by: Derek Nola <derek.nola@suse.com>

andypitcher approved these changes May 16, 2024

View reviewed changes

mozillazg approved these changes May 17, 2024

View reviewed changes

mozillazg requested a review from chen-keinan May 17, 2024 00:34

chen-keinan merged commit ed51191 into aquasecurity:main May 20, 2024
5 checks passed

dereknola deleted the fix_k3s_no_helpers branch July 11, 2024 20:49

andypitcher mentioned this pull request Dec 10, 2024

rke-cis-1.7 | Folder is wrong - should be changed to /etc/kubernetes/ssl #1746

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace custom k3s etcd script checks with vanilla grep checks #1601

Replace custom k3s etcd script checks with vanilla grep checks #1601

dereknola commented Apr 22, 2024 •

edited

Loading

andypitcher commented May 3, 2024 •

edited

Loading

andypitcher left a comment •

edited

Loading

dereknola commented May 6, 2024

chen-keinan commented May 7, 2024

mozillazg left a comment

mozillazg left a comment

chen-keinan commented May 20, 2024

Replace custom k3s etcd script checks with vanilla grep checks #1601

Replace custom k3s etcd script checks with vanilla grep checks #1601

Conversation

dereknola commented Apr 22, 2024 • edited Loading

andypitcher commented May 3, 2024 • edited Loading

Test case 1: auto-tls and peer-auto-tls set to false

Test case 2: auto-tls and peer-auto-tls set to true

Test case 3: auto-tls and peer-auto-tls not set (default value is false)

andypitcher left a comment • edited Loading

Choose a reason for hiding this comment

dereknola commented May 6, 2024

chen-keinan commented May 7, 2024

mozillazg left a comment

Choose a reason for hiding this comment

mozillazg left a comment

Choose a reason for hiding this comment

chen-keinan commented May 20, 2024

dereknola commented Apr 22, 2024 •

edited

Loading

andypitcher commented May 3, 2024 •

edited

Loading

andypitcher left a comment •

edited

Loading