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

restore: support restoring threads with SELinux #657

Merged
merged 1 commit into from
Mar 29, 2019

Conversation

adrianreber
Copy link
Member

Restoring a multi-threaded process with CRIU's SELinux support fails
because SELinux does not always support changing the process context of
a multi-threaded process.

Reading the man-page for setcon(), to change the context of a running
process, it states that changing the SELinux context of a multi-threaded
process can only work 'if the new security context is bounded by the old
security context'.

To be able to restore a process without the need to have 'the new
security context [] bounded by the old security context', this sets the
SELinux process context before creating the threads. Thus all threads
are created with the process context of the main process.

Signed-off-by: Adrian Reber areber@redhat.com

@adrianreber
Copy link
Member Author

This is a follow-up to #648

The main problem with #648 is that SELinux only partially supports changing the process context of a threaded process. According to the manpage:

A multi-threaded application can perform a setcon() prior to creating any child threads, in which case all of the child threads will inherit the new context. However, prior to Linux 2.6.28, setcon() would fail if there are any other threads running in the same process since this would yield an inconsistency among the security contexts of threads sharing the same memory space. Since Linux 2.6.28, setcon() is permitted for threads within a multi-threaded process if the new security context is bounded by the old security context, where the bounded relation is defined through typebounds statements in the policy and guarantees that the new security context has a subset of the permissions of the old security context.

For my current use case (getting Podman checkpoint/restore with SELinux working) this pull request works with the following two policies:

allow container_t container_var_lib_t:file append;
allow container_t sysctl_kernel_t:file write;

The first line is to enable the restorer to write to the log-file and should not be a problem. The second line could be a bit more problematic, but is is necessary so that each thread can write to ns_last_pid. As far as I know ns_last_pid requires CAP_SYS_ADMIN and seccomp will be restored later, so that once the container is restored it is not possible to write to ns_last_pid anyway.

The approach in this PR only works if all threads are using the same process context, but if it is not possible to change the context of a single thread during restore, it is also not possible to have a container with threads running with different process context. So the restore is limited, but only in the same way the original process startup is limited.

In parallel I am having the discussion if the approach in this PR with the necessary policy changes can be implemented in the corresponding selinux policy.

CC: @tych0 (just FYI, it's complicated 😉)

criu/pie/restorer.c Outdated Show resolved Hide resolved
Restoring a multi-threaded process with CRIU's SELinux support fails
because SELinux does not always support changing the process context of
a multi-threaded process.

Reading the man-page for setcon(), to change the context of a running
process, it states that changing the SELinux context of a multi-threaded
process can only work 'if the new security context is bounded by the old
security context'.

To be able to restore a process without the need to have 'the new
security context [] bounded by the old security context', this sets the
SELinux process context before creating the threads. Thus all threads
are created with the process context of the main process.

Signed-off-by: Adrian Reber <areber@redhat.com>
@adrianreber
Copy link
Member Author

The feedback I got so far from SELinux experts is that setting the process context of threads is only possible if the security context is bounded and this cannot change. On Fedora bounded security context are rarely used at all.

So this approach to set the process context before creating the threads seems to be the right thing to do and the additional policies, especially the one to write to ns_last_pid, are acceptable.

@avagin avagin closed this Mar 29, 2019
@avagin avagin reopened this Mar 29, 2019
@avagin avagin merged commit e86c2e9 into checkpoint-restore:criu-dev Mar 29, 2019
@adrianreber
Copy link
Member Author

The necessary policy to label ns_last_pid correctly has been merged into Fedora's SELinux policy package: fedora-selinux/selinux-policy@f1ba302

There should also soon be a new container-selinux package which actually allows CRIU to write to ns_last_pid from threads.

@avagin: From my side everything is ready for 3.12.

@adrianreber adrianreber deleted the selinux-threaded branch August 19, 2020 19:22
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.

2 participants