All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Djalal Harouni <tixxdz@opendz.org>,
	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: Fri, 4 Oct 2013 15:59:51 -0700	[thread overview]
Message-ID: <CALCETrVFNcGtsm3N50sZRDWmBOJ3VWn=BidAOV4Q+eDwYhJ+vw@mail.gmail.com> (raw)
In-Reply-To: <87fvsgy7cn.fsf@xmission.com>

On Fri, Oct 4, 2013 at 3:55 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
> Andy Lutomirski <luto@amacapital.net> writes:
>
>> On Fri, Oct 4, 2013 at 12:41 PM, Djalal Harouni <tixxdz@opendz.org> wrote:
>>> On Fri, Oct 04, 2013 at 12:32:09PM -0700, Andy Lutomirski wrote:
>>>> On Fri, Oct 4, 2013 at 12:27 PM, Djalal Harouni <tixxdz@opendz.org> wrote:
>
>>>> > So sorry Andy, I don't follow what you are describing.
>>>>
>>>> And what parameters are you passing to security_ptrace_access_check?
>>>> It's supposed to be f_cred, right?  Because you want to make sure
>>>> that, if the opener had some low-privilege label, the target has
>>>> execed and gotten a more secure label, and the reader has a
>>>> high-privilege label, that the opener's label is checked against the
>>>> target's new label.
>>> The current's cred each time.
>>
>> Exactly.  Hence the NAK.
>>
>>>
>>> Is there some mechanism to check what you describe?
>>>
>>
>> No.  You could try to add one, but getting it to be compatible with
>> YAMA might be really messy.
>>
>> Or you could see if destroying and recreating all the inodes on exec
>> or some other revoke-like approach would work.
>
> This is a revoke like approach, and yes proc has a fully functional
> revoke infrastructure.  Right now that revoke is based on the process
> going away.  The problem challenge is that the process is morphing.
>
> The practical question is which runtime checks do we want to perform.
>
> If we can say in no uncertain terms that short of a suid exec that
> no calls (such as setuid) can change the process permissions beyond
> our ability to access the file, we can detect and exec and use that
> as a signal.

If you could ptrace a process before it calls setuid and you can't
after it calls setuid, then this is stupid and doesn't matter -- once
you've pwned a process, you retain your pwnership at least until exec.

So yes, except that it's not just suid exec.  It's any exec that any
LSM would not do if no_new_privs were set.

>
> Alternatively we may to look at a processes credentials and in all
> cases where those change use that as a signal that the file must
> be reopened.

Hmm.  So why don't we just do a revoke whenever an exec that changes
cred happens?  (This will have some false positives, like unsharing
userns, I think, but we probably don't care.)

>
> Right now the model that we do a full permission check at every system
> call because the morphing process may cause problems.  If analysis can
> be done to show that we can use a simpler check than a full permission
> check that would be grand.
>
> The problem is not lack of techinical infrastructure (revoke).  The
> problem is a question of which tests are sufficient.

Can you point us at that infrastructure?  My limited grepping skills
didn't spot it.

I'd really like a solution where there are no read or write
implementations in the entire kernel that check permissions.  Failing
that, just getting it for procfs would be nice.  (uid_map, etc will
probably need to be revoked on unshare for this to work.)

--Andy

WARNING: multiple messages have this Message-ID (diff)
From: Andy Lutomirski <luto@amacapital.net>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Djalal Harouni <tixxdz@opendz.org>,
	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: Fri, 4 Oct 2013 15:59:51 -0700	[thread overview]
Message-ID: <CALCETrVFNcGtsm3N50sZRDWmBOJ3VWn=BidAOV4Q+eDwYhJ+vw@mail.gmail.com> (raw)
In-Reply-To: <87fvsgy7cn.fsf@xmission.com>

On Fri, Oct 4, 2013 at 3:55 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
> Andy Lutomirski <luto@amacapital.net> writes:
>
>> On Fri, Oct 4, 2013 at 12:41 PM, Djalal Harouni <tixxdz@opendz.org> wrote:
>>> On Fri, Oct 04, 2013 at 12:32:09PM -0700, Andy Lutomirski wrote:
>>>> On Fri, Oct 4, 2013 at 12:27 PM, Djalal Harouni <tixxdz@opendz.org> wrote:
>
>>>> > So sorry Andy, I don't follow what you are describing.
>>>>
>>>> And what parameters are you passing to security_ptrace_access_check?
>>>> It's supposed to be f_cred, right?  Because you want to make sure
>>>> that, if the opener had some low-privilege label, the target has
>>>> execed and gotten a more secure label, and the reader has a
>>>> high-privilege label, that the opener's label is checked against the
>>>> target's new label.
>>> The current's cred each time.
>>
>> Exactly.  Hence the NAK.
>>
>>>
>>> Is there some mechanism to check what you describe?
>>>
>>
>> No.  You could try to add one, but getting it to be compatible with
>> YAMA might be really messy.
>>
>> Or you could see if destroying and recreating all the inodes on exec
>> or some other revoke-like approach would work.
>
> This is a revoke like approach, and yes proc has a fully functional
> revoke infrastructure.  Right now that revoke is based on the process
> going away.  The problem challenge is that the process is morphing.
>
> The practical question is which runtime checks do we want to perform.
>
> If we can say in no uncertain terms that short of a suid exec that
> no calls (such as setuid) can change the process permissions beyond
> our ability to access the file, we can detect and exec and use that
> as a signal.

If you could ptrace a process before it calls setuid and you can't
after it calls setuid, then this is stupid and doesn't matter -- once
you've pwned a process, you retain your pwnership at least until exec.

So yes, except that it's not just suid exec.  It's any exec that any
LSM would not do if no_new_privs were set.

>
> Alternatively we may to look at a processes credentials and in all
> cases where those change use that as a signal that the file must
> be reopened.

Hmm.  So why don't we just do a revoke whenever an exec that changes
cred happens?  (This will have some false positives, like unsharing
userns, I think, but we probably don't care.)

>
> Right now the model that we do a full permission check at every system
> call because the morphing process may cause problems.  If analysis can
> be done to show that we can use a simpler check than a full permission
> check that would be grand.
>
> The problem is not lack of techinical infrastructure (revoke).  The
> problem is a question of which tests are sufficient.

Can you point us at that infrastructure?  My limited grepping skills
didn't spot it.

I'd really like a solution where there are no read or write
implementations in the entire kernel that check permissions.  Failing
that, just getting it for procfs would be nice.  (uid_map, etc will
probably need to be revoked on unshare for this to work.)

--Andy

  reply	other threads:[~2013-10-04 23:00 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 [this message]
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
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='CALCETrVFNcGtsm3N50sZRDWmBOJ3VWn=BidAOV4Q+eDwYhJ+vw@mail.gmail.com' \
    --to=luto@amacapital.net \
    --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=mingo@kernel.org \
    --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.