linux-kernel - Re: [PATCH 1/2] procfs: restore 0400 permissions on /proc/*/{syscall,stack,personality}

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGXu5jJtiji1KPPLSr5SXKXGwBbb8McWZY3DqCZFWXKtP=KEzA@mail.gmail.com>
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/