Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kubelet: Restrict access to TLS private key #2639

Merged
merged 2 commits into from
Jan 4, 2023

Conversation

stmcginnis
Copy link
Contributor

Issue number:

Closes #2638

Description of changes:

This moves the tlsPrivateKeyFile to be under a private subdirectory. This allows us to keep the public cert files readable for non-system processes that expect to be able to read them while limiting access to the more sensitive private key.

Testing done:

Built and deployed cluster.
Ran kubectl apply -f job.yaml with contents of the kube-bench sample job with the exception of commenting out the mount of /srv/kubernetes.
Verified job completed successfully and no errors in the kube-bench logs:

...
== Summary total ==
14 checks PASS
0 checks FAIL
37 checks WARN
0 checks INFO

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@stmcginnis stmcginnis self-assigned this Dec 6, 2022
Copy link
Contributor

@jpmcb jpmcb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! 👏🏼

webern
webern previously approved these changes Dec 8, 2022
@webern webern dismissed their stale review December 8, 2022 14:34

I missed the migration requirement

@stmcginnis stmcginnis force-pushed the k8s-pki branch 2 times, most recently from 8c9936b to 1d1f935 Compare December 8, 2022 19:52
Copy link
Contributor

@etungsten etungsten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

Small nit: I think it's better to separate out a commit just for the migration for maintenance and debugging but it's not a big deal.

@stmcginnis
Copy link
Contributor Author

I think it's better to separate out a commit just for the migration for maintenance and debugging but it's not a big deal.

Makes sense, I've separated it out into a second commit.

Copy link
Contributor

@bcressey bcressey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM.

What were all the warnings from kube-bench? That seems like a lot.

IIRC we should run the EKS-specific job-eks.yaml for aws-k8s-* to avoid some warnings about TLS certs and keys that aren't used.

Not sure if the same is true for vmware-k8s-* or metal-k8s-*, might be good to check.

README.md Outdated Show resolved Hide resolved
packages/kubernetes-1.21/etc-kubernetes-pki.mount Outdated Show resolved Hide resolved
This moves the `tlsPrivateKeyFile` to be under a `private` subdirectory.
This allows us to keep the public cert files readable for non-system
processes that expect to be able to read them while limiting access to
the more sensitive private key.

Signed-off-by: Sean McGinnis <[email protected]>
This adds a migration to update the `kubelet-server.key` file location
used for Kubernetes PKI. This was moved from the common location with
the public key to a separate private location so users would still be
able to read the public key if needed.

Signed-off-by: Sean McGinnis <[email protected]>
Copy link
Contributor Author

@stmcginnis stmcginnis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What were all the warnings from kube-bench? That seems like a lot.

IIRC we should run the EKS-specific job-eks.yaml for aws-k8s-* to avoid some warnings about TLS certs and keys that aren't used.

I didn't realize there was an EKS-specific option. I will run that and verify there are no errors. I was only looking to make sure there are no certificate related failures, so at least that part appears to be fine after this change. Will report back if there are any issues running the EKS tests.

README.md Outdated Show resolved Hide resolved
packages/kubernetes-1.21/etc-kubernetes-pki.mount Outdated Show resolved Hide resolved
@stmcginnis
Copy link
Contributor Author

Full output of running jobs-eks.yaml:

[INFO] 3 Worker Node Security Configuration
[INFO] 3.1 Worker Node Configuration Files
[PASS] 3.1.1 Ensure that the kubeconfig file permissions are set to 644 or more restrictive (Manual)
[PASS] 3.1.2 Ensure that the kubelet kubeconfig file ownership is set to root:root (Manual)
[PASS] 3.1.3 Ensure that the kubelet configuration file has permissions set to 644 or more restrictive (Manual)
[PASS] 3.1.4 Ensure that the kubelet configuration file ownership is set to root:root (Manual)
[INFO] 3.2 Kubelet
[PASS] 3.2.1 Ensure that the --anonymous-auth argument is set to false (Automated)
[PASS] 3.2.2 Ensure that the --authorization-mode argument is not set to AlwaysAllow (Automated)
[PASS] 3.2.3 Ensure that the --client-ca-file argument is set as appropriate (Manual)
[PASS] 3.2.4 Ensure that the --read-only-port argument is set to 0 (Manual)
[PASS] 3.2.5 Ensure that the --streaming-connection-idle-timeout argument is not set to 0 (Manual)
[PASS] 3.2.6 Ensure that the --protect-kernel-defaults argument is set to true (Automated)
[PASS] 3.2.7 Ensure that the --make-iptables-util-chains argument is set to true (Automated) 
[PASS] 3.2.8 Ensure that the --hostname-override argument is not set (Manual)
[WARN] 3.2.9 Ensure that the --eventRecordQPS argument is set to 0 or a level which ensures appropriate event capture (Automated)
[PASS] 3.2.10 Ensure that the --rotate-certificates argument is not set to false (Manual)
[PASS] 3.2.11 Ensure that the RotateKubeletServerCertificate argument is set to true (Manual)
[INFO] 3.3 Container Optimized OS
[WARN] 3.3.1 Prefer using Container-Optimized OS when possible (Manual)

== Remediations node ==
3.2.9 If using a Kubelet config file, edit the file to set eventRecordQPS: to an appropriate level.
If using command line arguments, edit the kubelet service file
/etc/systemd/system/kubelet.service.d/10-kubeadm.conf on each worker node and
set the below parameter in KUBELET_SYSTEM_PODS_ARGS variable.
Based on your system, restart the kubelet service. For example:
systemctl daemon-reload
systemctl restart kubelet.service

3.3.1 audit test did not run: No tests defined

== Summary node ==
14 checks PASS
0 checks FAIL
2 checks WARN
0 checks INFO

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

I may investigate why this line shows up as a WARN, but that is not related to this change:

[WARN] 3.3.1 Prefer using Container-Optimized OS when possible (Manual)

@stmcginnis
Copy link
Contributor Author

It appears the test blindly warns on the container optimized OS regardless of the nodes being used: https://github.com/aquasecurity/kube-bench/blob/07e01cf38c54583cb21f281b97a67b0b05c8dc76/cfg/eks-1.1.0/node.yaml#L328

So very safe to ignore that. Would be nice if there was a consistent way to test and show that condition though.

@stmcginnis stmcginnis merged commit f4eedcf into bottlerocket-os:develop Jan 4, 2023
@stmcginnis stmcginnis deleted the k8s-pki branch January 4, 2023 20:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to run kube-bench job in Bottlerocket
6 participants