-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add SELinux support for pods #7902
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rhatdan 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 |
// duplicate the security options from the pod | ||
processLabel, err := pod.ProcessLabel() | ||
if err != nil { | ||
return nil, err |
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.
This should probably be nonfatal, making pods without an infra container is perfectly valid.
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.
There will be no SELinux label though, although we have to figure out how to get one.
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 checked inside of the code where it checks for an infra container, if no infra then it will return "" for process label, which will get SELinux to fall back to default, which is to label the containers differently.
Added a test for this use case.
All containers within a Pod need to run with the same SELinux label, unless overwritten by the user. Also added a bunch of SELinux tests to make sure selinux labels are correct on namespaces. Signed-off-by: Daniel J Walsh <[email protected]>
@containers/podman-maintainers PTAL |
LGTM |
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.
/lgtm
It("podman test --pid=host", func() { | ||
session := podmanTest.Podman([]string{"run", "--pid=host", ALPINE, "cat", "/proc/self/attr/current"}) | ||
session.WaitWithDefaultTimeout() | ||
Expect(session.ExitCode()).To(Equal(0)) | ||
Expect(session.OutputToString()).To(ContainSubstring("spc_t")) | ||
}) |
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 cannot get this to work. I'm trying to write an equivalent system test, and no matter what, I get:
$ ./bin/podman run --pid=host alpine cat /proc/self/attr/current
unconfined_u:system_r:container_runtime_t:s0%
$ ls -lZ bin/podman
-rwxrwxr-x. 1 esm esm system_u:object_r:container_runtime_exec_t:s0 54319936 Oct 5 15:08 bin/podman
(As root, I get spc_t
as expected). master @ f48b163, with container-selinux-2.145.0-1.fc32.
I must be missing something simple. Any suggestions, please?
- images test: add test for 'table' and '\t' formatting - image mount test: check output from 'umount', test repeat umount (NOP), and test invalid-umount - kill test: remove kludgy workaround for crun signal bug ref: containers#5004 -- code is no longer needed (fingers crossed), and the workaround involved pulling an expensive image. - selinux test: add new tests for shared context in: * pods , w/ and w/o infra container (ref: containers#7902) * containers with namespace sharing: --ipc, --pid, --net - selinux test: new test for --pid=host (disabled pending propagation of container-selinux-2.146, ref: containers#7939) Signed-off-by: Ed Santiago <[email protected]>
All containers within a Pod need to run with the same SELinux
label, unless overwritten by the user.
Also added a bunch of SELinux tests to make sure selinux labels
are correct on namespaces.
Fixes: #7886
Signed-off-by: Daniel J Walsh [email protected]