* [PATCH v3 0/1] Unprivileged chroot @ 2021-03-11 10:52 Mickaël Salaün 2021-03-11 10:52 ` [PATCH v3 1/1] fs: Allow no_new_privs tasks to call chroot(2) Mickaël Salaün 0 siblings, 1 reply; 4+ messages in thread From: Mickaël Salaün @ 2021-03-11 10:52 UTC (permalink / raw) To: Al Viro, James Morris, Serge Hallyn Cc: Mickaël Salaün, Andy Lutomirski, Casey Schaufler, Christian Brauner, Christoph Hellwig, David Howells, Dominik Brodowski, Eric W . Biederman, John Johansen, Kees Cook, Kentaro Takeda, Tetsuo Handa, kernel-hardening, linux-fsdevel, linux-kernel, linux-security-module Hi, This new patch replaces the path_is_under() check with current_chrooted() as it is done with user namespaces. Indeed, it is much more simple to check the current root instead of limiting access to a subset of files. The chroot system call is currently limited to be used by processes with the CAP_SYS_CHROOT capability. This protects against malicious procesess willing to trick SUID-like binaries. The following patch allows unprivileged users to safely use chroot(2), which may be complementary to the use of user namespaces. This patch is a follow-up of a previous one sent by Andy Lutomirski some time ago: https://lore.kernel.org/lkml/0e2f0f54e19bff53a3739ecfddb4ffa9a6dbde4d.1327858005.git.luto@amacapital.net/ This patch can be applied on top of v5.12-rc2 . I would really appreciate constructive reviews. Previous version: https://lore.kernel.org/r/20210310181857.401675-1-mic@digikod.net Regards, Mickaël Salaün (1): fs: Allow no_new_privs tasks to call chroot(2) fs/open.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) base-commit: a38fd8748464831584a19438cbb3082b5a2dab15 -- 2.30.2 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v3 1/1] fs: Allow no_new_privs tasks to call chroot(2) 2021-03-11 10:52 [PATCH v3 0/1] Unprivileged chroot Mickaël Salaün @ 2021-03-11 10:52 ` Mickaël Salaün 2021-03-15 21:17 ` Kees Cook 0 siblings, 1 reply; 4+ messages in thread From: Mickaël Salaün @ 2021-03-11 10:52 UTC (permalink / raw) To: Al Viro, James Morris, Serge Hallyn Cc: Mickaël Salaün, Andy Lutomirski, Casey Schaufler, Christian Brauner, Christoph Hellwig, David Howells, Dominik Brodowski, Eric W . Biederman, John Johansen, Kees Cook, Kentaro Takeda, Tetsuo Handa, kernel-hardening, linux-fsdevel, linux-kernel, linux-security-module, Mickaël Salaün From: Mickaël Salaün <mic@linux.microsoft.com> Being able to easily change root directories enable to ease some development workflow and can be used as a tool to strengthen unprivileged security sandboxes. chroot(2) is not an access-control mechanism per se, but it can be used to limit the absolute view of the filesystem, and then limit ways to access data and kernel interfaces (e.g. /proc, /sys, /dev, etc.). Users may not wish to expose namespace complexity to potentially malicious processes, or limit their use because of limited resources. The chroot feature is much more simple (and limited) than the mount namespace, but can still be useful. As for containers, users of chroot(2) should take care of file descriptors or data accessible by other means (e.g. current working directory, leaked FDs, passed FDs, devices, mount points, etc.). There is a lot of literature that discuss the limitations of chroot, and users of this feature should be aware of the multiple ways to bypass it. Using chroot(2) for security purposes can make sense if it is combined with other features (e.g. dedicated user, seccomp, LSM access-controls, etc.). One could argue that chroot(2) is useless without a properly populated root hierarchy (i.e. without /dev and /proc). However, there are multiple use cases that don't require the chrooting process to create file hierarchies with special files nor mount points, e.g.: * A process sandboxing itself, once all its libraries are loaded, may not need files other than regular files, or even no file at all. * Some pre-populated root hierarchies could be used to chroot into, provided for instance by development environments or tailored distributions. * Processes executed in a chroot may not require access to these special files (e.g. with minimal runtimes, or by emulating some special files with a LD_PRELOADed library or seccomp). Allowing a task to change its own root directory is not a threat to the system if we can prevent confused deputy attacks, which could be performed through execution of SUID-like binaries. This can be prevented if the calling task sets PR_SET_NO_NEW_PRIVS on itself with prctl(2). To only affect this task, its filesystem information must not be shared with other tasks, which can be achieved by not passing CLONE_FS to clone(2). A similar no_new_privs check is already used by seccomp to avoid the same kind of security issues. Furthermore, because of its security use and to avoid giving a new way for attackers to get out of a chroot (e.g. using /proc/<pid>/root, or chroot/chdir), an unprivileged chroot is only allowed if the calling process is not already chrooted. This limitation is the same as for creating user namespaces. This change may not impact systems relying on other permission models than POSIX capabilities (e.g. Tomoyo). Being able to use chroot(2) on such systems may require to update their security policies. Only the chroot system call is relaxed with this no_new_privs check; the init_chroot() helper doesn't require such change. Allowing unprivileged users to use chroot(2) is one of the initial objectives of no_new_privs: https://www.kernel.org/doc/html/latest/userspace-api/no_new_privs.html This patch is a follow-up of a previous one sent by Andy Lutomirski: https://lore.kernel.org/lkml/0e2f0f54e19bff53a3739ecfddb4ffa9a6dbde4d.1327858005.git.luto@amacapital.net/ Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Andy Lutomirski <luto@amacapital.net> Cc: Christian Brauner <christian.brauner@ubuntu.com> Cc: Christoph Hellwig <hch@lst.de> Cc: David Howells <dhowells@redhat.com> Cc: Dominik Brodowski <linux@dominikbrodowski.net> Cc: Eric W. Biederman <ebiederm@xmission.com> Cc: James Morris <jmorris@namei.org> Cc: John Johansen <john.johansen@canonical.com> Cc: Kees Cook <keescook@chromium.org> Cc: Kentaro Takeda <takedakn@nttdata.co.jp> Cc: Serge Hallyn <serge@hallyn.com> Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com> Link: https://lore.kernel.org/r/20210311105242.874506-2-mic@digikod.net --- Changes since v2: * Replace path_is_under() check with current_chrooted() to gain the same protection as create_user_ns() (suggested by Jann Horn). See commit 3151527ee007 ("userns: Don't allow creation if the user is chrooted") Changes since v1: * Replace custom is_path_beneath() with existing path_is_under(). --- fs/open.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/fs/open.c b/fs/open.c index e53af13b5835..1eb086ed324b 100644 --- a/fs/open.c +++ b/fs/open.c @@ -22,6 +22,7 @@ #include <linux/slab.h> #include <linux/uaccess.h> #include <linux/fs.h> +#include <linux/path.h> #include <linux/personality.h> #include <linux/pagemap.h> #include <linux/syscalls.h> @@ -546,8 +547,18 @@ SYSCALL_DEFINE1(chroot, const char __user *, filename) if (error) goto dput_and_out; + /* + * Changing the root directory for the calling task (and its future + * children) requires that this task has CAP_SYS_CHROOT in its + * namespace, or be running with no_new_privs and not sharing its + * fs_struct and not escaping its current root (cf. create_user_ns()). + * As for seccomp, checking no_new_privs avoids scenarios where + * unprivileged tasks can affect the behavior of privileged children. + */ error = -EPERM; - if (!ns_capable(current_user_ns(), CAP_SYS_CHROOT)) + if (!ns_capable(current_user_ns(), CAP_SYS_CHROOT) && + !(task_no_new_privs(current) && current->fs->users == 1 + && !current_chrooted())) goto dput_and_out; error = security_path_chroot(&path); if (error) -- 2.30.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3 1/1] fs: Allow no_new_privs tasks to call chroot(2) 2021-03-11 10:52 ` [PATCH v3 1/1] fs: Allow no_new_privs tasks to call chroot(2) Mickaël Salaün @ 2021-03-15 21:17 ` Kees Cook 2021-03-16 8:17 ` Mickaël Salaün 0 siblings, 1 reply; 4+ messages in thread From: Kees Cook @ 2021-03-15 21:17 UTC (permalink / raw) To: Mickaël Salaün Cc: Al Viro, James Morris, Serge Hallyn, Andy Lutomirski, Casey Schaufler, Christian Brauner, Christoph Hellwig, David Howells, Dominik Brodowski, Eric W . Biederman, John Johansen, Kentaro Takeda, Tetsuo Handa, kernel-hardening, linux-fsdevel, linux-kernel, linux-security-module, Mickaël Salaün On Thu, Mar 11, 2021 at 11:52:42AM +0100, Mickaël Salaün wrote: > [...] > This change may not impact systems relying on other permission models > than POSIX capabilities (e.g. Tomoyo). Being able to use chroot(2) on > such systems may require to update their security policies. > > Only the chroot system call is relaxed with this no_new_privs check; the > init_chroot() helper doesn't require such change. > > Allowing unprivileged users to use chroot(2) is one of the initial > objectives of no_new_privs: > https://www.kernel.org/doc/html/latest/userspace-api/no_new_privs.html > This patch is a follow-up of a previous one sent by Andy Lutomirski: > https://lore.kernel.org/lkml/0e2f0f54e19bff53a3739ecfddb4ffa9a6dbde4d.1327858005.git.luto@amacapital.net/ I liked it back when Andy first suggested it, and I still like it now. :) I'm curious, do you have a specific user in mind for this feature? > [...] > @@ -546,8 +547,18 @@ SYSCALL_DEFINE1(chroot, const char __user *, filename) > if (error) > goto dput_and_out; > > + /* > + * Changing the root directory for the calling task (and its future > + * children) requires that this task has CAP_SYS_CHROOT in its > + * namespace, or be running with no_new_privs and not sharing its > + * fs_struct and not escaping its current root (cf. create_user_ns()). > + * As for seccomp, checking no_new_privs avoids scenarios where > + * unprivileged tasks can affect the behavior of privileged children. > + */ > error = -EPERM; > - if (!ns_capable(current_user_ns(), CAP_SYS_CHROOT)) > + if (!ns_capable(current_user_ns(), CAP_SYS_CHROOT) && > + !(task_no_new_privs(current) && current->fs->users == 1 > + && !current_chrooted())) > goto dput_and_out; > error = security_path_chroot(&path); > if (error) I think the logic here needs to be rearranged to avoid setting PF_SUPERPRIV, and I find the many negations hard to read. Perhaps: static inline int current_chroot_allowed(void) { /* comment here */ if (task_no_new_privs(current) && current->fs->users == 1 && !current_chrooted()) return 0; if (ns_capable(current_user_ns(), CAP_SYS_CHROOT)) return 0; return -EPERM; } ... error = current_chroot_allowed(); if (error) goto dput_and_out; I can't think of a way to race current->fs->users ... -- Kees Cook ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3 1/1] fs: Allow no_new_privs tasks to call chroot(2) 2021-03-15 21:17 ` Kees Cook @ 2021-03-16 8:17 ` Mickaël Salaün 0 siblings, 0 replies; 4+ messages in thread From: Mickaël Salaün @ 2021-03-16 8:17 UTC (permalink / raw) To: Kees Cook Cc: Al Viro, James Morris, Serge Hallyn, Andy Lutomirski, Casey Schaufler, Christian Brauner, Christoph Hellwig, David Howells, Dominik Brodowski, Eric W . Biederman, John Johansen, Kentaro Takeda, Tetsuo Handa, kernel-hardening, linux-fsdevel, linux-kernel, linux-security-module, Mickaël Salaün On 15/03/2021 22:17, Kees Cook wrote: > On Thu, Mar 11, 2021 at 11:52:42AM +0100, Mickaël Salaün wrote: >> [...] >> This change may not impact systems relying on other permission models >> than POSIX capabilities (e.g. Tomoyo). Being able to use chroot(2) on >> such systems may require to update their security policies. >> >> Only the chroot system call is relaxed with this no_new_privs check; the >> init_chroot() helper doesn't require such change. >> >> Allowing unprivileged users to use chroot(2) is one of the initial >> objectives of no_new_privs: >> https://www.kernel.org/doc/html/latest/userspace-api/no_new_privs.html >> This patch is a follow-up of a previous one sent by Andy Lutomirski: >> https://lore.kernel.org/lkml/0e2f0f54e19bff53a3739ecfddb4ffa9a6dbde4d.1327858005.git.luto@amacapital.net/ > > I liked it back when Andy first suggested it, and I still like it now. > :) I'm curious, do you have a specific user in mind for this feature? Except for development and test purposes, being able to use root (without CAP_SYS_CHROOT) would now enable to easily remove ambient filesystem access. Indeed, thanks to openat2 with RESOLVE_BENEATH or RESOLVE_IN_ROOT, it would be simple for most processes to chroot/chdir into an empty directory after opening (or being given) file descriptors opened with RESOLVE_BENEATH (e.g. configuration directory, cache directory, data directory, etc.). We can get something really close to a security capability system (different than POSIX capabilities), which wasn't possible when Andy posted the previous patches, and can help improve the state of userspace security. It is already possible to limit ptrace-like attacks, even when multiple processes are running with the same UID, with the help of SELinux, or even simply with Yama. This already enables sysadmins or distros to harden their system, and this kind of restrictions (with additional access-control bits) will be available to userspace developers thanks to Landlock. > >> [...] >> @@ -546,8 +547,18 @@ SYSCALL_DEFINE1(chroot, const char __user *, filename) >> if (error) >> goto dput_and_out; >> >> + /* >> + * Changing the root directory for the calling task (and its future >> + * children) requires that this task has CAP_SYS_CHROOT in its >> + * namespace, or be running with no_new_privs and not sharing its >> + * fs_struct and not escaping its current root (cf. create_user_ns()). >> + * As for seccomp, checking no_new_privs avoids scenarios where >> + * unprivileged tasks can affect the behavior of privileged children. >> + */ >> error = -EPERM; >> - if (!ns_capable(current_user_ns(), CAP_SYS_CHROOT)) >> + if (!ns_capable(current_user_ns(), CAP_SYS_CHROOT) && >> + !(task_no_new_privs(current) && current->fs->users == 1 >> + && !current_chrooted())) >> goto dput_and_out; >> error = security_path_chroot(&path); >> if (error) > > I think the logic here needs to be rearranged to avoid setting > PF_SUPERPRIV, and I find the many negations hard to read. Perhaps: > > static inline int current_chroot_allowed(void) > { > /* comment here */ > if (task_no_new_privs(current) && current->fs->users == 1 && > !current_chrooted()) > return 0; > > if (ns_capable(current_user_ns(), CAP_SYS_CHROOT)) > return 0; > > return -EPERM; > } > > ... > > error = current_chroot_allowed(); > if (error) > goto dput_and_out; > > > I can't think of a way to race current->fs->users ... > OK, I would be a bit bigger patch but easier to read. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-16 8:17 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-11 10:52 [PATCH v3 0/1] Unprivileged chroot Mickaël Salaün 2021-03-11 10:52 ` [PATCH v3 1/1] fs: Allow no_new_privs tasks to call chroot(2) Mickaël Salaün 2021-03-15 21:17 ` Kees Cook 2021-03-16 8:17 ` Mickaël Salaün
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.