[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 29 Aug 2013 15:14:32 -0700
From: Kees Cook <keescook@...omium.org>
To: Djalal Harouni <tixxdz@...ndz.org>
Cc: "Eric W. Biederman" <ebiederm@...ssion.com>, Al Viro <viro@...iv.linux.org.uk>,
Andrew Morton <akpm@...ux-foundation.org>, Solar Designer <solar@...nwall.com>,
Vasiliy Kulikov <segoon@...nwall.com>, Linus Torvalds <torvalds@...ux-foundation.org>,
Ingo Molnar <mingo@...nel.org>, LKML <linux-kernel@...r.kernel.org>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>
Subject: Re: [PATCH 1/2] procfs: restore 0400 permissions on /proc/*/{syscall,stack,personality}
On Thu, Aug 29, 2013 at 2:11 AM, Djalal Harouni <tixxdz@...ndz.org> wrote:
> Hi Eric,
>
> On Wed, Aug 28, 2013 at 05:26:56PM -0700, Eric W. Biederman wrote:
>>
>> I have take a moment and read this thread, and have been completely
>> unenlightend. People are upset but it is totally unclear why.
>>
>> There is no explanation why it is ok to ignore the suid-exec case, as
>> the posted patches do. Which ultimately means the patches provide
> Please, did you take a look at the patches ?
> - INF("syscall", S_IRUGO, proc_pid_syscall),
> + INF("syscall", S_IRUSR, proc_pid_syscall),
>
> Can you please tell me how did you come to the conclusion that the
> patches "ignore the suid-exec case as the posted patches do" ?
There are a few conditions that need to be handled. The original fix
that Al landed was to stop this:
create IPC
fork child
child opens /proc/self/syscall
child sends fd to parent over IPC
child execs setuid process
parent reads setuid process's "syscall" file
The solution was to check perms of reader (in this case parent wasn't
privileged, so it gets denied).
The new problem is:
open /proc/$target/syscall
dup to stdin
exec setuid process that reports contents of stdin
So, changing perms to 0400 doesn't actually fix what we want to fix,
since it can still by bypassed under more limited situations:
open /proc/self/syscall
dup to stdin
exec setuid process that reports contents of stdin
So, changing to 0400 means only setuid programs that aren't already
running will have their ASLR leaked.
>
> I just did s/0444/0400/ which is pretty obvious and did not remove
> that ptrace check at read() added by Al.
>
>> little to no security benefit, and that the posted patches as written
>> are broken.
> They are correct. Perhaps you didn't take a closer look
Maybe I'm lacking imagination, but changing to 0400 does reduce the
scope of the leak from all processes to "just" what was execed. This
still needs to be addressed, but I don't see a way to handle this
without explicitly invalidating the /proc handle across exec.
-Kees
--
Kees Cook
Chrome OS Security
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
