All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Djalal Harouni <tixxdz@opendz.org>
Cc: Kees Cook <keescook@chromium.org>,
	Andy Lutomirski <luto@amacapital.net>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.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 0/9] procfs: protect /proc/<pid>/* files with file->f_cred
Date: Thu, 3 Oct 2013 08:12:44 +0200	[thread overview]
Message-ID: <20131003061244.GC25345@gmail.com> (raw)
In-Reply-To: <20131002184844.GB3393@dztty>


* Djalal Harouni <tixxdz@opendz.org> wrote:

> > Regardless, glibc uses /proc/self/maps, which would be fine here, right?
>
> I did not touch /proc/self/maps and others, but I'm planning to fix them 
> if this solution is accepted.
> 
> I'll do the same thing as in /proc/*/stat for maps, let it be 0444, and 
> try to delay the check to ->read(). So during ->read() perform 
> ptrace_may_access() on currenct's cred and process_allow_access() on 
> file's opener cred. This should work.

Aren't read() time checks fundamentally unrobust? We generally don't do 
locking on read()s, so such checks - in the general case - are pretty 
racy.

Now procfs might be special, as by its nature of a pseudofilesystem it's 
far more atomic than other filesystems, but still IMHO it's a lot more 
robust security design wise to revoke access to objects you should not 
have a reference to when a security boundary is crossed, than letting 
stuff leak through and sprinking all the potential cases that may leak 
information with permission checks ...

It's also probably faster: security boundary crossings are relatively rare 
slowpaths, while read()s are much more common...

So please first get consensus on this fundamental design question before 
spreading your solution to more areas.

Thanks,

	Ingo

WARNING: multiple messages have this Message-ID (diff)
From: Ingo Molnar <mingo@kernel.org>
To: Djalal Harouni <tixxdz@opendz.org>
Cc: Kees Cook <keescook@chromium.org>,
	Andy Lutomirski <luto@amacapital.net>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.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 0/9] procfs: protect /proc/<pid>/* files with file->f_cred
Date: Thu, 3 Oct 2013 08:12:44 +0200	[thread overview]
Message-ID: <20131003061244.GC25345@gmail.com> (raw)
In-Reply-To: <20131002184844.GB3393@dztty>


* Djalal Harouni <tixxdz@opendz.org> wrote:

> > Regardless, glibc uses /proc/self/maps, which would be fine here, right?
>
> I did not touch /proc/self/maps and others, but I'm planning to fix them 
> if this solution is accepted.
> 
> I'll do the same thing as in /proc/*/stat for maps, let it be 0444, and 
> try to delay the check to ->read(). So during ->read() perform 
> ptrace_may_access() on currenct's cred and process_allow_access() on 
> file's opener cred. This should work.

Aren't read() time checks fundamentally unrobust? We generally don't do 
locking on read()s, so such checks - in the general case - are pretty 
racy.

Now procfs might be special, as by its nature of a pseudofilesystem it's 
far more atomic than other filesystems, but still IMHO it's a lot more 
robust security design wise to revoke access to objects you should not 
have a reference to when a security boundary is crossed, than letting 
stuff leak through and sprinking all the potential cases that may leak 
information with permission checks ...

It's also probably faster: security boundary crossings are relatively rare 
slowpaths, while read()s are much more common...

So please first get consensus on this fundamental design question before 
spreading your solution to more areas.

Thanks,

	Ingo

  parent reply	other threads:[~2013-10-03  6:12 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
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 [this message]
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=20131003061244.GC25345@gmail.com \
    --to=mingo@kernel.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=rientjes@google.com \
    --cc=serge.hallyn@ubuntu.com \
    --cc=tixxdz@gmail.com \
    --cc=tixxdz@opendz.org \
    --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: link
Be 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.