-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
feat: add kubelet systemd service hardening option #9194
Conversation
Hi @alegrey91. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
{% if kubelet_systemd_hardening %} | ||
# Hardening setup | ||
IPAddressDeny=any | ||
IPAddressAllow={{ groups['kube_control_plane'] | map('extract', hostvars, ['ansible_host']) | join(' ') }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may be dealing with multi-homed hosts which have dedicated management interfaces (think ssh access only) and use a separate interface for kubelet traffic. This hardening setup should allow to bind to dedicated interfaces IMHO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @cristicalin,
tell me know if I understand correctly.
Actually you don't need to bind a dedicated interface for kubelet
.
The systemd service set a dedicated firewall just for the running binary (in this case kubelet
).
If you try to access ssh, you are free to do this because the firewall does not intervene on the ssh
incoming packets.
Here's the example:
From the control plane node, we run a python -m http.server 9999
(not managed by the systemd service), and from an external machine, we run this:
curl --max-time 5 http://10.0.126.110:9999/
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<title>Directory listing for /</title>
</head>
<body>
<h1>Directory listing for /</h1>
<hr>
<ul>
</ul>
<hr>
</body>
</html>
Then, always from the control plane node, we start the systemd service for kubelet
. From an external machine we run this:
curl --max-time 5 -k https://10.0.126.110:10250/pods/
curl: (28) Connection timed out after 5001 milliseconds
As you can see, in the second example, the systemd firewall is intervening only on kubelet
service, blocking the incoming packets. The other packets, instead, are not interested by the firewall.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I meant was on this line:
{{ groups['kube_control_plane'] | map('extract', hostvars, ['ansible_host']) | join(' ') }}
You are making the assumption that only the main interface is used where the deployer may want to have the kubelet interface set to a different interface. ansible_host
is the IP on which the ansible controller connects to the host which may not be the IP that the deployer wants to use for this type of traffic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, now I got your point.
So, essentially we should list the IPs used by the kube-apiserver
, instead of the ansible_host
variable.
This because the kube-apiserver
is the only component that communicates with the kubelet
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cristicalin do you have suggestions on how to retrieve this information? I'm stuck here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we set the kubelet_secure_address
as {{ groups['kube_control_plane'] | map('extract', hostvars, ['ansible_host'] | join(' ') }}
by default, so the user should not deal with this variable in a common case. Instead, when needed, it can be changed.
In case the user bound the kube-apiserver
to a different interface (that is not the one used by ansible), it should just define the variable in this way: kubelet_secure_address="172.16.23.10 172.16.23.11 172.16.23.12"
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be mindful of variable precedence if doing that but I'd like to see an example of how the code would look like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the code. Take a look and let me know what do you think :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable that you suggested is defined like so:
kubelet_secure_address: "{{ groups['kube_control_plane'] | map('extract', hostvars, ['ansible_host']) | join(' ') }}"
You can just override it in this way:
kubelet_secure_address: "172.16.23.10 172.16.23.11 172.16.23.12"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @cristicalin, did you take a look at the code?
4f643e7
to
c15d5b7
Compare
Nice work /ok-to-test |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alegrey91, oomichi The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
docs/hardening.md
Outdated
# in case you have multiple interfaces in your | ||
# control plane nodes and you want to specify the right | ||
# IP addresses | ||
kubelet_secure_address: "192.168.10.110 192.168.10.111 192.168.10.112" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable name is singular and the example is in the plural, this is confusing, the example should make it clear how to set these either on a per node basis and change the way the list is compiled or rename the variable to the plural and explain that the variable is a list of addresses from the secure
interface of each node.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, probably the variable should be renamed in kubelet_secure_addresses
.
Since this patch can affect the way |
Hi @alegrey91 , please rebase this PR on latest state of the master branch to be able to pass the CI tests. You can place the diagram as an image in |
Co-authored-by: Cristian Calin <6627509+cristicalin@users.noreply.github.com>
Hi @cristicalin, I just rebased and added the diagram. Thanks for your help with the PR :) |
Thanks @alegrey91 /lgtm |
Thanks @cristicalin and @oomichi |
[CI] remove opensuse Leap from molecule test blocking CI (kubernetes-sigs#9229) Fixes for calico_datastore: etcd (kubernetes-sigs#9228) It seems that PR kubernetes-sigs#8839 broke `calico_datastore: etcd` when it removed ipamconfig support for etcd mode. This PR fixes some failing tasks when `calico_datastore == etcd`, but it does not restore ipamconfig support for calico in etcd mode. If someone wants to restore ipamconfig support for `calico_datastore: etcd` please submit a follow up PR for that. kube-vip shoud fail if kube_proxy_strict_arp is false in arp mod (kubernetes-sigs#9223) * fix-kube-vip-strict-arp * fix-kube-vip-strict-arp add runc v1.1.4 (kubernetes-sigs#9230) Fix typo. Fix kube_ovn_hw_offload value (kubernetes-sigs#9218) Fix cloud_init files for different distros (kubernetes-sigs#9232) Fix abort because calicoctl.sh is not a full path (kubernetes-sigs#9217) feat: add kubelet systemd service hardening option (kubernetes-sigs#9194) * feat: add kubelet systemd service hardening option * refactor: move variable name to kubelet_secure_addresses Co-authored-by: Cristian Calin <6627509+cristicalin@users.noreply.github.com> * docs: add diagram about kubelet_secure_addresses variable Co-authored-by: Cristian Calin <6627509+cristicalin@users.noreply.github.com> make calico installation more stable (kubernetes-sigs#9227) Signed-off-by: hang.jiang <hang.jiang@daocloud.io> Signed-off-by: hang.jiang <hang.jiang@daocloud.io> Update security contacts file (kubernetes-sigs#9235) disable kubelet_authorization_mode_webhook by default (kubernetes-sigs#9238) add-yankay-to-reviewers (kubernetes-sigs#9247)
[CI] remove opensuse Leap from molecule test blocking CI (kubernetes-sigs#9229) Fixes for calico_datastore: etcd (kubernetes-sigs#9228) It seems that PR kubernetes-sigs#8839 broke `calico_datastore: etcd` when it removed ipamconfig support for etcd mode. This PR fixes some failing tasks when `calico_datastore == etcd`, but it does not restore ipamconfig support for calico in etcd mode. If someone wants to restore ipamconfig support for `calico_datastore: etcd` please submit a follow up PR for that. kube-vip shoud fail if kube_proxy_strict_arp is false in arp mod (kubernetes-sigs#9223) * fix-kube-vip-strict-arp * fix-kube-vip-strict-arp add runc v1.1.4 (kubernetes-sigs#9230) Fix typo. Fix kube_ovn_hw_offload value (kubernetes-sigs#9218) Fix cloud_init files for different distros (kubernetes-sigs#9232) Fix abort because calicoctl.sh is not a full path (kubernetes-sigs#9217) feat: add kubelet systemd service hardening option (kubernetes-sigs#9194) * feat: add kubelet systemd service hardening option * refactor: move variable name to kubelet_secure_addresses Co-authored-by: Cristian Calin <6627509+cristicalin@users.noreply.github.com> * docs: add diagram about kubelet_secure_addresses variable Co-authored-by: Cristian Calin <6627509+cristicalin@users.noreply.github.com> make calico installation more stable (kubernetes-sigs#9227) Signed-off-by: hang.jiang <hang.jiang@daocloud.io> Signed-off-by: hang.jiang <hang.jiang@daocloud.io> Update security contacts file (kubernetes-sigs#9235) disable kubelet_authorization_mode_webhook by default (kubernetes-sigs#9238) add-yankay-to-reviewers (kubernetes-sigs#9247)
) * feat: add kubelet systemd service hardening option * refactor: move variable name to kubelet_secure_addresses Co-authored-by: Cristian Calin <6627509+cristicalin@users.noreply.github.com> * docs: add diagram about kubelet_secure_addresses variable Co-authored-by: Cristian Calin <6627509+cristicalin@users.noreply.github.com>
) * feat: add kubelet systemd service hardening option * refactor: move variable name to kubelet_secure_addresses Co-authored-by: Cristian Calin <6627509+cristicalin@users.noreply.github.com> * docs: add diagram about kubelet_secure_addresses variable Co-authored-by: Cristian Calin <6627509+cristicalin@users.noreply.github.com>
What type of PR is this?
What this PR does / why we need it:
Improve security of
kubelet
service, minimizing access through systemd.Which issue(s) this PR fixes:
Fixes #9107
Special notes for your reviewer:
Does this PR introduce a user-facing change?: