Linux-Fsdevel Archive on lore.kernel.org help / color / mirror / Atom feed
From: Adrian Reber <areber@redhat.com> To: "Christian Brauner" <christian.brauner@ubuntu.com>, "Eric Biederman" <ebiederm@xmission.com>, "Pavel Emelyanov" <ovzxemul@gmail.com>, "Oleg Nesterov" <oleg@redhat.com>, "Dmitry Safonov" <0x7f454c46@gmail.com>, "Andrei Vagin" <avagin@gmail.com>, "Nicolas Viennot" <Nicolas.Viennot@twosigma.com>, "Michał Cłapiński" <mclapinski@google.com>, "Kamil Yurtsever" <kyurtsever@google.com>, "Dirk Petersen" <dipeit@gmail.com>, "Christine Flood" <chf@redhat.com>, "Casey Schaufler" <casey@schaufler-ca.com> Cc: Mike Rapoport <rppt@linux.ibm.com>, Radostin Stoyanov <rstoyanov1@gmail.com>, Adrian Reber <areber@redhat.com>, Cyrill Gorcunov <gorcunov@openvz.org>, Serge Hallyn <serge@hallyn.com>, Stephen Smalley <stephen.smalley.work@gmail.com>, Sargun Dhillon <sargun@sargun.me>, Arnd Bergmann <arnd@arndb.de>, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, selinux@vger.kernel.org, Eric Paris <eparis@parisplace.org>, Jann Horn <jannh@google.com>, linux-fsdevel@vger.kernel.org Subject: [PATCH v6 5/7] prctl: Allow local CAP_CHECKPOINT_RESTORE to change /proc/self/exe Date: Sun, 19 Jul 2020 12:04:15 +0200 [thread overview] Message-ID: <20200719100418.2112740-6-areber@redhat.com> (raw) In-Reply-To: <20200719100418.2112740-1-areber@redhat.com> From: Nicolas Viennot <Nicolas.Viennot@twosigma.com> Originally, only a local CAP_SYS_ADMIN could change the exe link, making it difficult for doing checkpoint/restore without CAP_SYS_ADMIN. This commit adds CAP_CHECKPOINT_RESTORE in addition to CAP_SYS_ADMIN for permitting changing the exe link. The following describes the history of the /proc/self/exe permission checks as it may be difficult to understand what decisions lead to this point. * [1] May 2012: This commit introduces the ability of changing /proc/self/exe if the user is CAP_SYS_RESOURCE capable. In the related discussion [2], no clear thread model is presented for what could happen if the /proc/self/exe changes multiple times, or why would the admin be at the mercy of userspace. * [3] Oct 2014: This commit introduces a new API to change /proc/self/exe. The permission no longer checks for CAP_SYS_RESOURCE, but instead checks if the current user is root (uid=0) in its local namespace. In the related discussion [4] it is said that "Controlling exe_fd without privileges may turn out to be dangerous. At least things like tomoyo examine it for making policy decisions (see tomoyo_manager())." * [5] Dec 2016: This commit removes the restriction to change /proc/self/exe at most once. The related discussion [6] informs that the audit subsystem relies on the exe symlink, presumably audit_log_d_path_exe() in kernel/audit.c. * [7] May 2017: This commit changed the check from uid==0 to local CAP_SYS_ADMIN. No discussion. * [8] July 2020: A PoC to spoof any program's /proc/self/exe via ptrace is demonstrated Overall, the concrete points that were made to retain capability checks around changing the exe symlink is that tomoyo_manager() and audit_log_d_path_exe() uses the exe_file path. Christian Brauner said that relying on /proc/<pid>/exe being immutable (or guarded by caps) in a sake of security is a bit misleading. It can only be used as a hint without any guarantees of what code is being executed once execve() returns to userspace. Christian suggested that in the future, we could call audit_log() or similar to inform the admin of all exe link changes, instead of attempting to provide security guarantees via permission checks. However, this proposed change requires the understanding of the security implications in the tomoyo/audit subsystems. [1] b32dfe377102 ("c/r: prctl: add ability to set new mm_struct::exe_file") [2] https://lore.kernel.org/patchwork/patch/292515/ [3] f606b77f1a9e ("prctl: PR_SET_MM -- introduce PR_SET_MM_MAP operation") [4] https://lore.kernel.org/patchwork/patch/479359/ [5] 3fb4afd9a504 ("prctl: remove one-shot limitation for changing exe link") [6] https://lore.kernel.org/patchwork/patch/697304/ [7] 4d28df6152aa ("prctl: Allow local CAP_SYS_ADMIN changing exe_file") [8] https://github.com/nviennot/run_as_exe Signed-off-by: Nicolas Viennot <Nicolas.Viennot@twosigma.com> Signed-off-by: Adrian Reber <areber@redhat.com> --- kernel/sys.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/kernel/sys.c b/kernel/sys.c index 00a96746e28a..a3f4ef0bbda3 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -2007,11 +2007,14 @@ static int prctl_set_mm_map(int opt, const void __user *addr, unsigned long data if (prctl_map.exe_fd != (u32)-1) { /* - * Make sure the caller has the rights to - * change /proc/pid/exe link: only local sys admin should - * be allowed to. + * Check if the current user is checkpoint/restore capable. + * At the time of this writing, it checks for CAP_SYS_ADMIN + * or CAP_CHECKPOINT_RESTORE. + * Note that a user with access to ptrace can masquerade an + * arbitrary program as any executable, even setuid ones. + * This may have implications in the tomoyo subsystem. */ - if (!ns_capable(current_user_ns(), CAP_SYS_ADMIN)) + if (!checkpoint_restore_ns_capable(current_user_ns())) return -EINVAL; error = prctl_set_mm_exe_file(mm, prctl_map.exe_fd); -- 2.26.2
next prev parent reply other threads:[~2020-07-19 10:06 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-19 10:04 [PATCH v6 0/7] capabilities: Introduce CAP_CHECKPOINT_RESTORE Adrian Reber 2020-07-19 10:04 ` [PATCH v6 1/7] " Adrian Reber 2020-07-19 10:04 ` [PATCH v6 2/7] pid: use checkpoint_restore_ns_capable() for set_tid Adrian Reber 2020-07-19 10:04 ` [PATCH v6 3/7] pid_namespace: use checkpoint_restore_ns_capable() for ns_last_pid Adrian Reber 2020-07-19 10:04 ` [PATCH v6 4/7] proc: allow access in init userns for map_files with CAP_CHECKPOINT_RESTORE Adrian Reber 2020-07-19 16:50 ` Serge E. Hallyn 2020-07-19 10:04 ` Adrian Reber [this message] 2020-07-19 10:04 ` [PATCH v6 6/7] prctl: exe link permission error changed from -EINVAL to -EPERM Adrian Reber 2020-07-19 17:05 ` Serge E. Hallyn 2020-07-19 10:04 ` [PATCH v6 7/7] selftests: add clone3() CAP_CHECKPOINT_RESTORE test Adrian Reber 2020-07-19 18:17 ` [PATCH v6 0/7] capabilities: Introduce CAP_CHECKPOINT_RESTORE Christian Brauner 2020-07-20 11:54 ` Christian Brauner 2020-07-20 12:46 ` Adrian Reber 2020-07-20 12:58 ` Christian Brauner
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200719100418.2112740-6-areber@redhat.com \ --to=areber@redhat.com \ --cc=0x7f454c46@gmail.com \ --cc=Nicolas.Viennot@twosigma.com \ --cc=arnd@arndb.de \ --cc=avagin@gmail.com \ --cc=casey@schaufler-ca.com \ --cc=chf@redhat.com \ --cc=christian.brauner@ubuntu.com \ --cc=dipeit@gmail.com \ --cc=ebiederm@xmission.com \ --cc=eparis@parisplace.org \ --cc=gorcunov@openvz.org \ --cc=jannh@google.com \ --cc=kyurtsever@google.com \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-security-module@vger.kernel.org \ --cc=mclapinski@google.com \ --cc=oleg@redhat.com \ --cc=ovzxemul@gmail.com \ --cc=rppt@linux.ibm.com \ --cc=rstoyanov1@gmail.com \ --cc=sargun@sargun.me \ --cc=selinux@vger.kernel.org \ --cc=serge@hallyn.com \ --cc=stephen.smalley.work@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).