linux-fsdevel.vger.kernel.org archive mirror
 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

  reply	other threads:[~2013-10-04 22:59 UTC|newest]

Thread overview: 68+ 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 ` [PATCH v2 1/9] procfs: add proc_same_open_cred() to check if the cred have changed 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-02  1:36   ` Andy Lutomirski
2013-10-02 14:55     ` Djalal Harouni
2013-10-02 16:44       ` Andy Lutomirski
2013-10-03 14:36         ` Djalal Harouni
2013-10-03 15:12           ` Andy Lutomirski
2013-10-03 19:29             ` Djalal Harouni
2013-10-03 19:37               ` Andy Lutomirski
2013-10-03 20:13                 ` Djalal Harouni
2013-10-03 21:09                   ` Andy Lutomirski
2013-10-04  8:59                     ` Djalal Harouni
2013-10-04 15:40                       ` Andy Lutomirski
2013-10-04 18:23                         ` Djalal Harouni
2013-10-04 18:34                           ` Andy Lutomirski
2013-10-04 19:11                             ` Djalal Harouni
2013-10-04 19:16                               ` Andy Lutomirski
2013-10-04 19:27                                 ` Djalal Harouni
2013-10-04 19:32                                   ` Andy Lutomirski
2013-10-04 19:41                                     ` Djalal Harouni
2013-10-04 22:17                                       ` Andy Lutomirski
2013-10-04 22:55                                         ` Eric W. Biederman
2013-10-04 22:59                                           ` Andy Lutomirski [this message]
2013-10-04 23:08                                             ` Andy Lutomirski
2013-10-05  0:35                                             ` Eric W. Biederman
2013-10-09 10:35                                               ` Djalal Harouni
2013-10-05 13:23                                         ` Djalal Harouni
2013-10-07 21:41                                           ` Andy Lutomirski
2013-10-09 10:54                                             ` Djalal Harouni
2013-10-09 11:15                                               ` Djalal Harouni
2013-10-09 17:27                                               ` Andy Lutomirski
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 ` [PATCH v2 4/9] procfs: make /proc/*/{stack,syscall} 0400 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 ` [PATCH v2 6/9] procfs: add permission checks on the file's opener of /proc/*/stat Djalal Harouni
2013-10-02  1:39   ` Andy Lutomirski
2013-10-02 15:14     ` Djalal Harouni
2013-10-02 16:46       ` Andy Lutomirski
2013-10-02 19:00         ` 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 ` [PATCH v2 8/9] procfs: improve permission checks on /proc/*/stack Djalal Harouni
2013-10-01 20:26 ` [PATCH v2 9/9] procfs: improve permission checks on /proc/*/syscall 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 14:37   ` Djalal Harouni
2013-10-02 16:51     ` Andy Lutomirski
2013-10-02 17:48       ` Kees Cook
2013-10-02 18:00         ` Andy Lutomirski
2013-10-02 18:07           ` Kees Cook
2013-10-03 23:14             ` Julien Tinnes
2013-10-02 18:26           ` Djalal Harouni
2013-10-02 18:41             ` Djalal Harouni
2013-10-02 18:22         ` Djalal Harouni
2013-10-02 18:35           ` Kees Cook
2013-10-02 18:48             ` Djalal Harouni
2013-10-02 19:43               ` Kees Cook
2013-10-03  6:12               ` Ingo Molnar
2013-10-03 12:29                 ` Djalal Harouni
2013-10-03 15:15                   ` Andy Lutomirski
2013-10-03 15:40                     ` Djalal Harouni
2013-10-03 15:50                       ` Andy Lutomirski
2013-10-03 18:37                         ` Djalal Harouni
2013-10-04  9:05                 ` Djalal Harouni
2013-10-02 18:12       ` Djalal Harouni
2013-10-03  6:22         ` Ingo Molnar
2013-10-03 12:56           ` Djalal Harouni
2013-10-03 13:39             ` 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 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).