From: Djalal Harouni <tixxdz@opendz.org> To: Andy Lutomirski <luto@amacapital.net> Cc: "Eric W. Biederman" <ebiederm@xmission.com>, Kees Cook <keescook@chromium.org>, Al Viro <viro@zeniv.linux.org.uk>, Andrew Morton <akpm@linux-foundation.org>, Linus Torvalds <torvalds@linux-foundation.org>, Ingo Molnar <mingo@kernel.org>, "Serge E. Hallyn" <serge.hallyn@ubuntu.com>, Cyrill Gorcunov <gorcunov@openvz.org>, David Rientjes <rientjes@google.com>, LKML <linux-kernel@vger.kernel.org>, Linux FS Devel <linux-fsdevel@vger.kernel.org>, "kernel-hardening@lists.openwall.com" <kernel-hardening@lists.openwall.com>, Djalal Harouni <tixxdz@gmail.com> Subject: Re: [PATCH v2 2/9] procfs: add proc_allow_access() to check if file's opener may access task Date: Sun, 13 Oct 2013 11:18:34 +0100 [thread overview] Message-ID: <20131013101834.GA4404@dztty> (raw) In-Reply-To: <CALCETrUTppUmPVy-GZg1xBimb95Xq-vD6qkJFrkT4qUKx0-iSA@mail.gmail.com> On Wed, Oct 09, 2013 at 06:27:22PM +0100, Andy Lutomirski wrote: > On Wed, Oct 9, 2013 at 11:54 AM, Djalal Harouni <tixxdz@opendz.org> wrote: > > On Mon, Oct 07, 2013 at 02:41:33PM -0700, Andy Lutomirski wrote: > >> On Sat, Oct 5, 2013 at 6:23 AM, Djalal Harouni <tixxdz@opendz.org> wrote: > >> > On Fri, Oct 04, 2013 at 03:17:08PM -0700, Andy Lutomirski wrote: > >> >> > >> >> Exactly. Hence the NAK. > >> > But Having two LSM Hooks there is really not practical! > >> > >> It'd doable *if* it turns out that it's the right solution. > >> > >> But revoke seems much more likely to be simple, comprehensible, and > >> obviously correct to me. > > Yes Andy, I agree, revoke is much better! > > > > But it will not handle or fix all the situations, as I've said what if > > revoke is not invloved here? there is no an execve from the target task! > > > > > > Remember: > > /proc/*/{stat,maps} and perhaps others have 0444 and don't have ptrace > > checks during ->open() to not break some userspace... especially > > /proc/*/stat file > > For /proc/*/stat: check permissions when opening. If the opener > passes the ptrace check, set a flag in file->private_data indicating > that this struct file has permission. That will fix it, but it will need some extra work since file->private_data is already used to store seq_file structs! > For /proc/*/maps: either fail the open if the check fails or set a > flag that causes the resulting struct file to be useless. Not sure about this, we need to inspect glibc > > > > > > So you will have an fd on these privileged files! > > > > Current will execve a privileged process, and pass ptrace_may_access() > > checks during ->read()... > > > > Here revoke is not involved at all! so it will not fix these files and > > they will continue to be vulnerable. > > > > IMO to fix them, we must have the correct ptrace_may_access() check and > > this involves: current doing an execve + current's cred > > > > > > > > BTW, Andy we already return 0 (end of file) for /proc/*/mem > > ->read() > > ->mem_read() > > ->mem_rw() > > if (!atomic_inc_not_zero(&mm->mm_users)) > > return 0 > > > > So can this be considered some sort of simple revoke? > > > > Apparently not. I haven't looked at the code, but I just tried it. > The fd from /proc/<pid>/maps survives an exec of the process it's > pointing at. That means that either the mm changing has no effect on > the struct file or that an unshared mm survives exec. yes the old mm (during ->open()) will survive but not vma, ->read() will return zero In the mean time, to close some of these vulnerabilities, I'll submit another patch to prevent open() arbitrary /proc/*/{stack,syscall} 1) Make them 0400 2) Add the ptrace_may_access() during ->open() for ptrace capabilities and LSM checks It would be nice to get your Acked-by Andy! -- Djalal Harouni http://opendz.org
WARNING: multiple messages have this Message-ID (diff)
From: Djalal Harouni <tixxdz@opendz.org> To: Andy Lutomirski <luto@amacapital.net> Cc: "Eric W. Biederman" <ebiederm@xmission.com>, Kees Cook <keescook@chromium.org>, Al Viro <viro@zeniv.linux.org.uk>, Andrew Morton <akpm@linux-foundation.org>, Linus Torvalds <torvalds@linux-foundation.org>, Ingo Molnar <mingo@kernel.org>, "Serge E. Hallyn" <serge.hallyn@ubuntu.com>, Cyrill Gorcunov <gorcunov@openvz.org>, David Rientjes <rientjes@google.com>, LKML <linux-kernel@vger.kernel.org>, Linux FS Devel <linux-fsdevel@vger.kernel.org>, "kernel-hardening@lists.openwall.com" <kernel-hardening@lists.openwall.com>, Djalal Harouni <tixxdz@gmail.com> Subject: [kernel-hardening] Re: [PATCH v2 2/9] procfs: add proc_allow_access() to check if file's opener may access task Date: Sun, 13 Oct 2013 11:18:34 +0100 [thread overview] Message-ID: <20131013101834.GA4404@dztty> (raw) In-Reply-To: <CALCETrUTppUmPVy-GZg1xBimb95Xq-vD6qkJFrkT4qUKx0-iSA@mail.gmail.com> On Wed, Oct 09, 2013 at 06:27:22PM +0100, Andy Lutomirski wrote: > On Wed, Oct 9, 2013 at 11:54 AM, Djalal Harouni <tixxdz@opendz.org> wrote: > > On Mon, Oct 07, 2013 at 02:41:33PM -0700, Andy Lutomirski wrote: > >> On Sat, Oct 5, 2013 at 6:23 AM, Djalal Harouni <tixxdz@opendz.org> wrote: > >> > On Fri, Oct 04, 2013 at 03:17:08PM -0700, Andy Lutomirski wrote: > >> >> > >> >> Exactly. Hence the NAK. > >> > But Having two LSM Hooks there is really not practical! > >> > >> It'd doable *if* it turns out that it's the right solution. > >> > >> But revoke seems much more likely to be simple, comprehensible, and > >> obviously correct to me. > > Yes Andy, I agree, revoke is much better! > > > > But it will not handle or fix all the situations, as I've said what if > > revoke is not invloved here? there is no an execve from the target task! > > > > > > Remember: > > /proc/*/{stat,maps} and perhaps others have 0444 and don't have ptrace > > checks during ->open() to not break some userspace... especially > > /proc/*/stat file > > For /proc/*/stat: check permissions when opening. If the opener > passes the ptrace check, set a flag in file->private_data indicating > that this struct file has permission. That will fix it, but it will need some extra work since file->private_data is already used to store seq_file structs! > For /proc/*/maps: either fail the open if the check fails or set a > flag that causes the resulting struct file to be useless. Not sure about this, we need to inspect glibc > > > > > > So you will have an fd on these privileged files! > > > > Current will execve a privileged process, and pass ptrace_may_access() > > checks during ->read()... > > > > Here revoke is not involved at all! so it will not fix these files and > > they will continue to be vulnerable. > > > > IMO to fix them, we must have the correct ptrace_may_access() check and > > this involves: current doing an execve + current's cred > > > > > > > > BTW, Andy we already return 0 (end of file) for /proc/*/mem > > ->read() > > ->mem_read() > > ->mem_rw() > > if (!atomic_inc_not_zero(&mm->mm_users)) > > return 0 > > > > So can this be considered some sort of simple revoke? > > > > Apparently not. I haven't looked at the code, but I just tried it. > The fd from /proc/<pid>/maps survives an exec of the process it's > pointing at. That means that either the mm changing has no effect on > the struct file or that an unshared mm survives exec. yes the old mm (during ->open()) will survive but not vma, ->read() will return zero In the mean time, to close some of these vulnerabilities, I'll submit another patch to prevent open() arbitrary /proc/*/{stack,syscall} 1) Make them 0400 2) Add the ptrace_may_access() during ->open() for ptrace capabilities and LSM checks It would be nice to get your Acked-by Andy! -- Djalal Harouni http://opendz.org
next prev parent reply other threads:[~2013-10-13 10:18 UTC|newest] Thread overview: 179+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-10-01 20:26 [PATCH v2 0/9] procfs: protect /proc/<pid>/* files with file->f_cred Djalal Harouni 2013-10-01 20:26 ` [kernel-hardening] " Djalal Harouni 2013-10-01 20:26 ` [PATCH v2 1/9] procfs: add proc_same_open_cred() to check if the cred have changed Djalal Harouni 2013-10-01 20:26 ` [kernel-hardening] " Djalal Harouni 2013-10-01 20:26 ` [PATCH v2 2/9] procfs: add proc_allow_access() to check if file's opener may access task Djalal Harouni 2013-10-01 20:26 ` [kernel-hardening] " Djalal Harouni 2013-10-02 1:36 ` Andy Lutomirski 2013-10-02 1:36 ` [kernel-hardening] " Andy Lutomirski 2013-10-02 14:55 ` Djalal Harouni 2013-10-02 14:55 ` [kernel-hardening] " Djalal Harouni 2013-10-02 16:44 ` Andy Lutomirski 2013-10-02 16:44 ` [kernel-hardening] " Andy Lutomirski 2013-10-03 14:36 ` Djalal Harouni 2013-10-03 14:36 ` [kernel-hardening] " Djalal Harouni 2013-10-03 15:12 ` Andy Lutomirski 2013-10-03 15:12 ` [kernel-hardening] " Andy Lutomirski 2013-10-03 15:12 ` Andy Lutomirski 2013-10-03 19:29 ` Djalal Harouni 2013-10-03 19:29 ` [kernel-hardening] " Djalal Harouni 2013-10-03 19:29 ` Djalal Harouni 2013-10-03 19:37 ` Andy Lutomirski 2013-10-03 19:37 ` [kernel-hardening] " Andy Lutomirski 2013-10-03 19:37 ` Andy Lutomirski 2013-10-03 20:13 ` Djalal Harouni 2013-10-03 20:13 ` [kernel-hardening] " Djalal Harouni 2013-10-03 20:13 ` Djalal Harouni 2013-10-03 21:09 ` Andy Lutomirski 2013-10-03 21:09 ` [kernel-hardening] " Andy Lutomirski 2013-10-03 21:09 ` Andy Lutomirski 2013-10-04 8:59 ` Djalal Harouni 2013-10-04 8:59 ` [kernel-hardening] " Djalal Harouni 2013-10-04 8:59 ` Djalal Harouni 2013-10-04 15:40 ` Andy Lutomirski 2013-10-04 15:40 ` [kernel-hardening] " Andy Lutomirski 2013-10-04 15:40 ` Andy Lutomirski 2013-10-04 18:23 ` Djalal Harouni 2013-10-04 18:23 ` [kernel-hardening] " Djalal Harouni 2013-10-04 18:23 ` Djalal Harouni 2013-10-04 18:34 ` Andy Lutomirski 2013-10-04 18:34 ` [kernel-hardening] " Andy Lutomirski 2013-10-04 18:34 ` Andy Lutomirski 2013-10-04 19:11 ` Djalal Harouni 2013-10-04 19:11 ` [kernel-hardening] " Djalal Harouni 2013-10-04 19:11 ` Djalal Harouni 2013-10-04 19:16 ` Andy Lutomirski 2013-10-04 19:16 ` [kernel-hardening] " Andy Lutomirski 2013-10-04 19:16 ` Andy Lutomirski 2013-10-04 19:27 ` Djalal Harouni 2013-10-04 19:27 ` [kernel-hardening] " Djalal Harouni 2013-10-04 19:27 ` Djalal Harouni 2013-10-04 19:32 ` Andy Lutomirski 2013-10-04 19:32 ` [kernel-hardening] " Andy Lutomirski 2013-10-04 19:32 ` Andy Lutomirski 2013-10-04 19:41 ` Djalal Harouni 2013-10-04 19:41 ` [kernel-hardening] " Djalal Harouni 2013-10-04 19:41 ` Djalal Harouni 2013-10-04 22:17 ` Andy Lutomirski 2013-10-04 22:17 ` [kernel-hardening] " Andy Lutomirski 2013-10-04 22:17 ` Andy Lutomirski 2013-10-04 22:55 ` Eric W. Biederman 2013-10-04 22:55 ` [kernel-hardening] " Eric W. Biederman 2013-10-04 22:55 ` Eric W. Biederman 2013-10-04 22:59 ` Andy Lutomirski 2013-10-04 22:59 ` [kernel-hardening] " Andy Lutomirski 2013-10-04 22:59 ` Andy Lutomirski 2013-10-04 23:08 ` Andy Lutomirski 2013-10-04 23:08 ` [kernel-hardening] " Andy Lutomirski 2013-10-04 23:08 ` Andy Lutomirski 2013-10-05 0:35 ` Eric W. Biederman 2013-10-05 0:35 ` [kernel-hardening] " Eric W. Biederman 2013-10-05 0:35 ` Eric W. Biederman 2013-10-09 10:35 ` Djalal Harouni 2013-10-09 10:35 ` [kernel-hardening] " Djalal Harouni 2013-10-09 10:35 ` Djalal Harouni 2013-10-05 13:23 ` Djalal Harouni 2013-10-05 13:23 ` [kernel-hardening] " Djalal Harouni 2013-10-05 13:23 ` Djalal Harouni 2013-10-07 21:41 ` Andy Lutomirski 2013-10-07 21:41 ` [kernel-hardening] " Andy Lutomirski 2013-10-07 21:41 ` Andy Lutomirski 2013-10-09 10:54 ` Djalal Harouni 2013-10-09 10:54 ` [kernel-hardening] " Djalal Harouni 2013-10-09 10:54 ` Djalal Harouni 2013-10-09 11:15 ` Djalal Harouni 2013-10-09 11:15 ` [kernel-hardening] " Djalal Harouni 2013-10-09 11:15 ` Djalal Harouni 2013-10-09 17:27 ` Andy Lutomirski 2013-10-09 17:27 ` [kernel-hardening] " Andy Lutomirski 2013-10-09 17:27 ` Andy Lutomirski 2013-10-13 10:18 ` Djalal Harouni [this message] 2013-10-13 10:18 ` [kernel-hardening] " Djalal Harouni 2013-10-13 10:18 ` Djalal Harouni 2013-10-01 20:26 ` [PATCH v2 3/9] procfs: Document the proposed solution to protect procfs entries Djalal Harouni 2013-10-01 20:26 ` [kernel-hardening] " Djalal Harouni 2013-10-01 20:26 ` [PATCH v2 4/9] procfs: make /proc/*/{stack,syscall} 0400 Djalal Harouni 2013-10-01 20:26 ` [kernel-hardening] " Djalal Harouni 2013-10-01 20:26 ` [PATCH v2 5/9] procfs: make /proc entries that use seq files able to access file->f_cred Djalal Harouni 2013-10-01 20:26 ` [kernel-hardening] " Djalal Harouni 2013-10-01 20:26 ` [PATCH v2 6/9] procfs: add permission checks on the file's opener of /proc/*/stat Djalal Harouni 2013-10-01 20:26 ` [kernel-hardening] " Djalal Harouni 2013-10-02 1:39 ` Andy Lutomirski 2013-10-02 1:39 ` [kernel-hardening] " Andy Lutomirski 2013-10-02 15:14 ` Djalal Harouni 2013-10-02 15:14 ` [kernel-hardening] " Djalal Harouni 2013-10-02 16:46 ` Andy Lutomirski 2013-10-02 16:46 ` [kernel-hardening] " Andy Lutomirski 2013-10-02 19:00 ` Djalal Harouni 2013-10-02 19:00 ` [kernel-hardening] " Djalal Harouni 2013-10-01 20:26 ` [PATCH v2 7/9] procfs: add permission checks on the file's opener of /proc/*/personality Djalal Harouni 2013-10-01 20:26 ` [kernel-hardening] " Djalal Harouni 2013-10-01 20:26 ` [PATCH v2 8/9] procfs: improve permission checks on /proc/*/stack Djalal Harouni 2013-10-01 20:26 ` [kernel-hardening] " Djalal Harouni 2013-10-01 20:26 ` [PATCH v2 9/9] procfs: improve permission checks on /proc/*/syscall Djalal Harouni 2013-10-01 20:26 ` [kernel-hardening] " Djalal Harouni 2013-10-02 1:40 ` [PATCH v2 0/9] procfs: protect /proc/<pid>/* files with file->f_cred Andy Lutomirski 2013-10-02 1:40 ` [kernel-hardening] " Andy Lutomirski 2013-10-02 14:37 ` Djalal Harouni 2013-10-02 14:37 ` [kernel-hardening] " Djalal Harouni 2013-10-02 16:51 ` Andy Lutomirski 2013-10-02 16:51 ` [kernel-hardening] " Andy Lutomirski 2013-10-02 17:48 ` Kees Cook 2013-10-02 17:48 ` [kernel-hardening] " Kees Cook 2013-10-02 17:48 ` Kees Cook 2013-10-02 18:00 ` Andy Lutomirski 2013-10-02 18:00 ` [kernel-hardening] " Andy Lutomirski 2013-10-02 18:00 ` Andy Lutomirski 2013-10-02 18:07 ` Kees Cook 2013-10-02 18:07 ` [kernel-hardening] " Kees Cook 2013-10-02 18:07 ` Kees Cook 2013-10-03 23:14 ` Julien Tinnes 2013-10-03 23:14 ` [kernel-hardening] " Julien Tinnes 2013-10-03 23:14 ` Julien Tinnes 2013-10-02 18:26 ` Djalal Harouni 2013-10-02 18:26 ` [kernel-hardening] " Djalal Harouni 2013-10-02 18:26 ` Djalal Harouni 2013-10-02 18:41 ` Djalal Harouni 2013-10-02 18:41 ` [kernel-hardening] " Djalal Harouni 2013-10-02 18:41 ` Djalal Harouni 2013-10-02 18:22 ` Djalal Harouni 2013-10-02 18:22 ` [kernel-hardening] " Djalal Harouni 2013-10-02 18:22 ` Djalal Harouni 2013-10-02 18:35 ` Kees Cook 2013-10-02 18:35 ` [kernel-hardening] " Kees Cook 2013-10-02 18:35 ` Kees Cook 2013-10-02 18:48 ` Djalal Harouni 2013-10-02 18:48 ` [kernel-hardening] " Djalal Harouni 2013-10-02 18:48 ` Djalal Harouni 2013-10-02 19:43 ` Kees Cook 2013-10-02 19:43 ` [kernel-hardening] " Kees Cook 2013-10-02 19:43 ` Kees Cook 2013-10-03 6:12 ` Ingo Molnar 2013-10-03 6:12 ` [kernel-hardening] " Ingo Molnar 2013-10-03 6:12 ` Ingo Molnar 2013-10-03 12:29 ` Djalal Harouni 2013-10-03 12:29 ` [kernel-hardening] " Djalal Harouni 2013-10-03 12:29 ` Djalal Harouni 2013-10-03 15:15 ` Andy Lutomirski 2013-10-03 15:15 ` [kernel-hardening] " Andy Lutomirski 2013-10-03 15:15 ` Andy Lutomirski 2013-10-03 15:40 ` Djalal Harouni 2013-10-03 15:40 ` [kernel-hardening] " Djalal Harouni 2013-10-03 15:40 ` Djalal Harouni 2013-10-03 15:50 ` Andy Lutomirski 2013-10-03 15:50 ` [kernel-hardening] " Andy Lutomirski 2013-10-03 15:50 ` Andy Lutomirski 2013-10-03 18:37 ` Djalal Harouni 2013-10-03 18:37 ` [kernel-hardening] " Djalal Harouni 2013-10-03 18:37 ` Djalal Harouni 2013-10-04 9:05 ` Djalal Harouni 2013-10-04 9:05 ` [kernel-hardening] " Djalal Harouni 2013-10-04 9:05 ` Djalal Harouni 2013-10-02 18:12 ` Djalal Harouni 2013-10-02 18:12 ` [kernel-hardening] " Djalal Harouni 2013-10-03 6:22 ` Ingo Molnar 2013-10-03 6:22 ` [kernel-hardening] " Ingo Molnar 2013-10-03 12:56 ` Djalal Harouni 2013-10-03 12:56 ` [kernel-hardening] " Djalal Harouni 2013-10-03 13:39 ` Ingo Molnar 2013-10-03 13:39 ` [kernel-hardening] " Ingo Molnar
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=20131013101834.GA4404@dztty \ --to=tixxdz@opendz.org \ --cc=akpm@linux-foundation.org \ --cc=ebiederm@xmission.com \ --cc=gorcunov@openvz.org \ --cc=keescook@chromium.org \ --cc=kernel-hardening@lists.openwall.com \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=luto@amacapital.net \ --cc=mingo@kernel.org \ --cc=rientjes@google.com \ --cc=serge.hallyn@ubuntu.com \ --cc=tixxdz@gmail.com \ --cc=torvalds@linux-foundation.org \ --cc=viro@zeniv.linux.org.uk \ /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 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.