Need to setup labeling of kernel keyrings. #2012
Conversation
Work is ongoing in the kernel to support different kernel keyrings per user namespace. We want to allow SELinux to manage kernel keyrings inside of the container. Currently when runc creates the kernel keyring it gets the label which runc is running with ususally `container_runtime_t`, with this change the kernel keyring will be labeled with the container process label container_t:s0:C1,c2. Container running as container_t:s0:c1,c2 can manage keyrings with the same label. This change required a revendoring or the SELinux go bindings. github.com/opencontainers/selinux. Signed-off-by: Daniel J Walsh <[email protected]>
|
@giuseppe FYI |
| @@ -5,7 +5,7 @@ github.com/opencontainers/runtime-spec 29686dbc5559d93fb1ef402eeda3e35c38d75af4 | |||
| # Core libcontainer functionality. | |||
| github.com/checkpoint-restore/go-criu v3.11 | |||
| github.com/mrunalp/fileutils ed869b029674c0e9ce4c0dfa781405c2d9946d08 | |||
| github.com/opencontainers/selinux v1.0.0-rc1 | |||
| github.com/opencontainers/selinux v1.2 | |||
There was a problem hiding this comment.
(not directly related to this PR); I noticed that these tags miss a trailing .0, and because of that are following the SemVer syntax (which may complicate using them with Go mod)
There was a problem hiding this comment.
I can add another .0 if necessary. in the next version.
There was a problem hiding this comment.
Thanks! I noticed it earlier this week, and this PR made me remember that I saw that 🤗
| @@ -48,6 +48,10 @@ func (l *linuxStandardInit) Init() error { | |||
| runtime.LockOSThread() | |||
| defer runtime.UnlockOSThread() | |||
| if !l.config.Config.NoNewKeyring { | |||
| if err := label.SetKeyLabel(l.config.ProcessLabel); err != nil { | |||
There was a problem hiding this comment.
Just to double-check -- are you sure this needs to be done before we create a new session? (Is SetKeyLabel setting what the label will be for all future keys or the label for the current key?)
There was a problem hiding this comment.
Yes this says to the kernel, any new keyrings that get created by this process and its children should get created with the specified label.
When the new keyring gets created it gets created with the correct label.
Later on the defaulf SetKeyLabel("") tells the kernel to go back to the default labeling.
There was a problem hiding this comment.
BTW I have been testing this patch with the updated kernel, which allows sepate kernel keyrings for each new UserNamespace.
There was a problem hiding this comment.
I am not clear why we need to reset it back. Shouldn't it pass down to the container process?
There was a problem hiding this comment.
Yes it will, the reset is on defer. Not really important for runc, but if people vendor in this code, then we would want to make sure other code paths don't accidently set the label to this container.
There was a problem hiding this comment.
Yup, we've discussed the defer handling for SELinux in the past. Even though it doesn't really make sense for runc it avoids people vendoring it and shooting themselves in the foot (not that SELinux is the only thing that is a massive foot-gun in libcontainer).
|
LGTM. I haven't tested on a kernel with keyring labeling support, but I take @rhatdan's word that this works as it should. |
|
LGTM |
full diff: opencontainers/runc@2b18fe1...v1.0.0-rc7 changes included: - opencontainers/runc#2012 Need to setup labeling of kernel keyrings - opencontainers/runc#2014 Add $RUNC_USE_SYSTEMD to run tests using systemd cgroup driver - opencontainers/runc#2015 Use getenv not secure_getenv - fixes opencontainers/runc#2013 build fails with musl libc - opencontainers/runc#2023 Fixes regression causing zombie runc:[1:CHILD] processes Signed-off-by: Sebastiaan van Stijn <[email protected]>
full diff: opencontainers/runc@2b18fe1...v1.0.0-rc7 changes included: - opencontainers/runc#2012 Need to setup labeling of kernel keyrings - opencontainers/runc#2014 Add $RUNC_USE_SYSTEMD to run tests using systemd cgroup driver - opencontainers/runc#2015 Use getenv not secure_getenv - fixes opencontainers/runc#2013 build fails with musl libc - opencontainers/runc#2023 Fixes regression causing zombie runc:[1:CHILD] processes Signed-off-by: Sebastiaan van Stijn <[email protected]> (cherry picked from commit bc6ac08) Signed-off-by: Sebastiaan van Stijn <[email protected]>
full diff: opencontainers/runc@2b18fe1...v1.0.0-rc7 changes included: - opencontainers/runc#2012 Need to setup labeling of kernel keyrings - opencontainers/runc#2014 Add $RUNC_USE_SYSTEMD to run tests using systemd cgroup driver - opencontainers/runc#2015 Use getenv not secure_getenv - fixes opencontainers/runc#2013 build fails with musl libc - opencontainers/runc#2023 Fixes regression causing zombie runc:[1:CHILD] processes Signed-off-by: Sebastiaan van Stijn <[email protected]> (cherry picked from commit bc6ac08) Signed-off-by: Sebastiaan van Stijn <[email protected]>
Work is ongoing in the kernel to support different kernel
keyrings per user namespace. We want to allow SELinux to manage
kernel keyrings inside of the container.
Currently when runc creates the kernel keyring it gets the label which runc is
running with ususally
container_runtime_t, with this change the kernel keyringwill be labeled with the container process label container_t:s0:C1,c2.
Container running as container_t:s0:c1,c2 can manage keyrings with the same label.
This change required a revendoring or the SELinux go bindings.
github.com/opencontainers/selinux.
Signed-off-by: Daniel J Walsh [email protected]