[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 1 Sep 2013 08:04:06 -0700
From: Kees Cook <keescook@...omium.org>
To: "Eric W. Biederman" <ebiederm@...ssion.com>
Cc: Djalal Harouni <tixxdz@...ndz.org>, 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 Sat, Aug 31, 2013 at 6:44 PM, Eric W. Biederman
<ebiederm@...ssion.com> wrote:
> Djalal Harouni <tixxdz@...ndz.org> writes:
>
>> (Sorry for my late response)
>>
>> On Thu, Aug 29, 2013 at 03:14:32PM -0700, Kees Cook wrote:
>>> 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).
>> Yes, of course
>>
>>
>>> 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.
>> Yes I do realize. That change was only to block leaks against already
>> running processes and *restore* the old permissions.
>>
>>
>>> [...]
>>> 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.
>> Yes Kees,
>>
>> I did try a year ago to adapt the exec_id from grsecurity and failed
>> (and failed again to resend - not enough resources):
>> https://lkml.org/lkml/2012/3/10/174
>>
>>
>> Kees IMHO the right solution is to invalidate the fd across exec as
>> you suggest
>>
>> Alan Cox's thread which describe the problem correctly:
>> https://lkml.org/lkml/2012/1/29/35
>>
>> Alan suggested to revoke() the file handles.
>
> That was in particular with respect to /dev/mem.
>
> In the general case calling setuid or any of it's cousins can cause the
> same problem. So a revoke that only works at exec time is insufficient.
>
> The problem we are examining is what happens when the file descriptor is
> passed to a more privileged process that will pass the ptrace_may_access
> check while the original process that opened the file did not.
>
> We have file->f_cred that has the permissions of the process at open
> time, and likely that should factor into the calculations somehow.
>
> Alternatively we may simply be able to call get_task_cred() at the time
> we open the file and if the cred on the process changes fail. I know
> Linus was looking at something like that recently, but ran into problmes
> with Chromes sandbox. (Sigh). Although I think he was talking about
> file->f_cred...
>
> This is most definitely a solvable problem with current mechanisms, but
> it is going to take some grunt work to make it happen.
Well, in the meantime, it seems we should restore the 0400 perms,
since that at least covers the ASLR leak of running processes.
-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.
