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

fix failure when selinux enabled in old kernel #2033

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lifubang
Copy link
Member

@lifubang lifubang commented Apr 4, 2019

Signed-off-by: lifubang lifubang@acmcoder.com

I think #2032 fixed the problem on disabled SELinux Machines.
But on enabled SELinux Machines with some old kernels, it still be fail when selinuxLabel is empty.

So, I think we should add selinuxLabel check in runc, it will be more safe to run.

And I will send a PR to selinux project soon.

Signed-off-by: lifubang <lifubang@acmcoder.com>
@lifubang
Copy link
Member Author

lifubang commented Apr 4, 2019

Please see opencontainers/selinux#50
The fail kernel is: 3.10.0-862.el7.x86_64 in RedHat Enterprise Linux 7.5
I think 3.10.0 kernel is still used by lots of users, especially in some cloud.

@lifubang
Copy link
Member Author

lifubang commented Apr 4, 2019

@rhatdan I would've suggested doing both checks, but if you're okay with failures on (very) old SELinux systems that's fine.

And I think @cyphar has pointed it out in opencontainers/selinux#49 (comment).

@lifubang
Copy link
Member Author

lifubang commented Apr 4, 2019

As PR opencontainers/selinux#50 is closed by @rhatdan . Please see opencontainers/selinux#52

@cyphar PTAL

@cyphar
Copy link
Member

cyphar commented Apr 5, 2019

This patch doesn't make much sense to me -- old kernels will give you failures because /proc/self/attr/keycreate doesn't exist. So, if you run runc with an SELinux label you will still end up with -ENOENT. This would only fix cases where you are running containers without an explicit SELinux label on an SELinux-enabled system -- which is not the default under Docker/cri-o/podman/etc (by default they all have SELinux labels).

I would prefer to just wait for opencontainers/selinux#52 to be resolved.

@lifubang
Copy link
Member Author

lifubang commented Apr 5, 2019

So, if you run runc with an SELinux label you will still end up with -ENOENT.

Oh, my god, too many things to consider.
So, we should not be hurry, just think more.

I would prefer to just wait for opencontainers/selinux#52 to be resolved.

Ok, let's wait. Thanks.

ansilh added a commit to ansilh/k8s-vagrant that referenced this pull request Apr 7, 2019
Rolling back to rc6 as rc7 shown issues while starting pod
"init caused \"write /proc/self/attr/keycreate: invalid argument\"": unknown""

opencontainers/runc#2033
@kolyshkin
Copy link
Contributor

OK this is what I just saw on CentOS 7 system (with the latest CentOS 7 kernel, 3.10.0-957.12.2.el7.x86_64). This is from strace on containerd:

20396 openat(AT_FDCWD, "/proc/self/attr/keycreate", O_WRONLY|O_CLOEXEC) = 7
20396 epoll_ctl(6, EPOLL_CTL_ADD, 7, {EPOLLIN|EPOLLOUT|EPOLLRDHUP|EPOLLET, {u32=3687386888, u64=139826347708168}}) = -1 EPERM (Operation not permitted)
20396 epoll_ctl(6, EPOLL_CTL_DEL, 7, 0xc00014aedc) = -1 EPERM (Operation not permitted)
20396 write(7, "", 0)                   = -1 EACCES (Permission denied)
20396 close(7)                          = 0
20396 write(3, "{\"type\":\"procError\"}", 20) = 20

this EACCES error on write() is kinda flaky, it sometimes disappears when I upgrade or downgrade container-selinux package (apparently its version doesn't play any role which is the weirdest part...)

@rhatdan
Copy link
Contributor

rhatdan commented Jun 10, 2019

What AVC's are you seeing?

ausearch -m avc -ts recent

@kolyshkin
Copy link
Contributor

[root@kir-ce7-selinux-02 ~]# docker run hello-world
docker: Error response from daemon: OCI runtime create failed: container_linux.go:345: starting container process caused "process_linux.go:430: container init caused \"write /proc/self/attr/keycreate: permission denied\"": unknown.

[root@kir-ce7-selinux-02 ~]# ausearch -m avc -ts recent
----
time->Tue Jun 11 00:50:06 2019
type=PROCTITLE msg=audit(1560214206.491:3314): proctitle=72756E6300696E6974
type=SYSCALL msg=audit(1560214206.491:3314): arch=c000003e syscall=1 success=no exit=-13 a0=7 a1=5579f5a61158 a2=0 a3=0 items=0 ppid=20958 pid=20967 auid=4294967295 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=(none) ses=4294967295 comm="runc:[2:INIT]" exe="/" subj=system_u:system_r:unconfined_service_t:s0 key=(null)
type=AVC msg=audit(1560214206.491:3314): avc:  denied  { create } for  pid=20967 comm="runc:[2:INIT]" scontext=system_u:system_r:unconfined_service_t:s0 tcontext=system_u:object_r:unlabeled_t:s0 tclass=key permissive=0

[root@kir-ce7-selinux-02 ~]# rpm -q container-selinux
container-selinux-2.95-2.el7_6.noarch

@rhatdan
Copy link
Contributor

rhatdan commented Jun 11, 2019

So you are running runc directly as a service in a systemd unit file?

Or are you running this under docker? Either way it should not be running as unconfined_service_t, it should be running as container_runtime_t. (Not sure if this would work either with that policy.)

In Fedora we currently have
sesearch -A -s container_runtime_t -t unlabeled_t -c key
allow container_runtime_t unlabeled_t:key { create link read search setattr view write };

@kolyshkin
Copy link
Contributor

kolyshkin commented Jun 12, 2019

@lifubang what version of docker are you running?

It might be the case that your containerd and/or runc binaries are mislabeled.

To test:

ls -Z /usr/{s,}bin/{containerd,runc,docker}
ps axZ | grep -E 'containerd|dockerd'

Both commands should have container_runtime_exec_t in every line corresponding to containerd, dockerd, and runc binaries, and container_runtime_t for processes.

If this is not the case,

yum update container-selinux
restorecon /usr/{s,}bin/{containerd,runc,docker}
systemctl restart docker
systemctl restart containerd

should fix the issue.

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.

None yet

4 participants