Re: [PATCH v2 0/9] procfs: protect /proc//* files with file->f_cred

Djalal Harouni
2013-10-02T18:13:07Z

On Wed, Oct 02, 2013 at 05:51:15PM +0100, Andy Lutomirski wrote:
> On Wed, Oct 2, 2013 at 3:37 PM, Djalal Harouni <tixxdz@opendz.org> wrote:
> > On Tue, Oct 01, 2013 at 06:40:41PM -0700, Andy Lutomirski wrote:
> >> On 10/01/2013 01:26 PM, Djalal Harouni wrote:
> >> > /proc/<pid>/* entries varies at runtime, appropriate permission checks
> >> > need to happen during each system call.
> >> >
> >> > Currently some of these sensitive entries are protected by performing
> >> > the ptrace_may_access() check. However even with that the /proc file
> >> > descriptors can be passed to a more privileged process
> >> > (e.g. a suid-exec) which will pass the classic ptrace_may_access()
> >> > check. In general the ->open() call will be issued by an unprivileged
> >> > process while the ->read(),->write() calls by a more privileged one.
> >> >
> >> > Example of these files are:
> >> > /proc/*/syscall, /proc/*/stack etc.
> >> >
> >> > And any open(/proc/self/*) then suid-exec to read()/write() /proc/self/*
> >> >
> >> >
> >> > These files are protected during read() by the ptrace_may_access(),
> >> > however the file descriptor can be passed to a suid-exec which can be
> >> > used to read data and bypass ASLR. Of course this was discussed several
> >> > times on LKML.
> >>
> >> Can you elaborate on what it is that you're fixing? That is, can you
> >> give a concrete example of what process opens what file and passes the
> >> fd to what process?
> > Yes, the references were already given in this email:
> > https://lkml.org/lkml/2013/8/31/209
> >
> > This has been discussed several times on lkml:
> > https://lkml.org/lkml/2013/8/28/544
> >
> > https://lkml.org/lkml/2013/8/28/564 (check Kees's references)
> >
> >
> >> I'm having trouble following your description.
> > Process open a /proc file and pass the fd to a more privilaged process
> > that will pass the ptrace_may_access() check, while the original process
> > that opened that file should fail at the ptrace_may_access()
>
> So we're talking about two kinds of attacks, right?
Yes,

> Type 1: Unprivileged process does something like open("/proc/1/maps",
> O_RDONLY) and then passes the resulting fd to something privileged.
Yes,

> Type 2: Unprivileged process does something like
> open("/proc/self/maps", O_RDONLY) and then forks. The parent calls
> execve on something privileged.
Correct

>
> Can we really not get away with fixing type 1 by preventing these
> files from being opened in the first place and type 2 by revoking all
> of these fds when a privilege-changing exec happens?
Yes for 1) and that's what exactly this series try to do
* ptrace checks during ->open() for /proc/*/{stack,syscall} and 0400
* You can't do it for /proc/*/stat otherwise you will break userspace
"ps"..., ps must access /proc/1/stat etc... so the proposed solution
will work without any side effect.

And for /proc/*/maps you will perhaps break glibc under certain
situations... so just hold it for the moment and test it
later. There have been reports in the past about it.

So for 1) this series will fix it

For type 2) yes this series will also handle it, since there were a cred
change.

For fd revoking, yes that perhaps will work and it would be great, but
it does not exist :-/

Not to mention if the process do not want to revoke and pass the fd ?
is this compliant with standards ?

> I'm not objecting to your patches so much as thinking that read(2) has
> no business looking at current->cred *at all*. But maybe that ship
> has already sailed.
No that's not correct.

read(2) and during each system call current->cred must be checked,
the /proc varies at runtime, you are not sure about what are you
reading,writing...

> --Andy
>
> >
> >
> >> --Andy
> >>
> >
> > --
> > Djalal Harouni
> > http://opendz.org
>
>
>
> --
> Andy Lutomirski
> AMA Capital Management, LLC

--
Djalal Harouni