Add documentation about kernel module autoloading security #15451
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
Deploy preview for kubernetes-io-master-staging ready! Built with commit 9dfde6c https://deploy-preview-15451--kubernetes-io-master-staging.netlify.com |
| blacklist sctp | ||
| ``` | ||
|
|
||
| To block module loading more generically, a Linux Security Module (such as SELinux) can be |
There was a problem hiding this comment.
How about: “…you can use a Linux Security Module (such as SELinux)…” (active rather than passive voice)
danwinship
force-pushed
the
securing-kernel-modules
branch
from
be39fa2 to
645fe31
Compare
|
|
||
| To block module loading more generically, you can use a Linux Security Module (such as | ||
| SELinux) to completely deny the `module_request` permission to containers, preventing the | ||
| kernel from loading modules for them under any circumstances. (Pods would still be able to |
There was a problem hiding this comment.
nit: what is the for them under any circumstances?
nit: Do you need the sentence Pods would still ... contained in parens?
There was a problem hiding this comment.
"them" = "containers". I guess it wouldn't be too unwieldy to just repeat "containers"
The parenthetical statement is parenthetical because the reader ought to understand that without us having to say it. But in the context of making security recommendations, overexplaining seems better than underexplaining.
|
Thanks for the style comments, though I'd like this to get some security expert review too. I'm not sure who the appropriate reviewers would be. |
|
This advice seems fine, but it also seems out of place in the Kubernetes docs. I'm not sure how much linux hardening advice we really want to provide and maintain here, and why we'd focus on kernel module blacklisting in particular. The relationship to Kubernetes for the vuln notification seems like it was already fairly tenuous (i.e. not a vulnerability in a part of Kubernetes). Perhaps @philips has an opinion here since he did the vuln response. |
|
@danwinship , I agree. For a security-related review, possibly assign a label such as sig/network, sig/node, sig/auth? |
The issue is that it's a somewhat non-obvious place where containers are less-contained than people might expect. It's particularly a problem for people running "Kubernetes-as-a-Service". But maybe people doing that already need to be thinking about a lot of other security issues we don't warn about. I'm fine if people think this doesn't belong here. |
tengqm
added
Needs Tech Review
sig/network
|
@kubernetes/sig-network-pr-reviews 👋 Thanks in advance for your review! It's still valuable even though the PR is closed (hopefully it's only temporary). |
|
@danwinship 👋 Thanks for this PR. Please feel free to |
|
This seems like fine advice to have in our docs as it is a fairly common, standardized, and easy best practice. |
|
/cc @liggitt |
danwinship
force-pushed
the
securing-kernel-modules
branch
from
645fe31 to
9dfde6c
Compare
|
I'll defer to sig-node and sig-network reviewers on this one |
|
cc @kubernetes/sig-node-pr-reviews |
k8s-ci-robot
added
the
sig/node
|
I'll also defer to sig-node and sig-network reviewers on this one |
|
/close |
|
@zacharysarah: Closed this PR. In response to this:
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. |
|
@zacharysarah Can you provide some reasoning on why this was closed? |
|
@philips Thanks for the prompt; I missed that we're waiting on SIG review, not on author response to reviewer feedback. @kubernetes/sig-network-pr-reviews Ping /reopen |
|
@zacharysarah: Reopened this PR. In response to this:
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. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: zparnold 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 |
k8s-ci-robot
added
the
approved
This came out of a discussion of moving the SCTP KEP forward in sig-network; some people might be thinking that since Kubernetes doesn't (currently) support SCTP, that this means they are protected from CVEs involving the SCTP kernel module, but that's not true at all. (See also https://discuss.kubernetes.io/t/kubernetes-security-announcement-linux-kernel-memory-cgroups-escape-via-sctp-cve-2019-3874/5594.)
I'm not sure how specific we should get about recommendations here... People should definitely be blacklisting dccp if their distro still ships it, and a lot of people will want to blacklist sctp. But like, you could make a good argument for blacklisting bluetooth too; it's not like pods are going to need to be talking to Bluetooth devices... I think the answer probably comes down to "if you spend more than 5 minutes thinking about it, you should probably just run selinux". (Or maybe someone should improve the cgroups APIs so that Kubernetes can control whether module loading is available or blocked via PodSecurityPolicy. But that's way outside the scope of this PR...)