linux-kernel - Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve

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]

Date:	Wed, 13 Jul 2011 10:31:42 +0400
From:	Solar Designer <solar@...nwall.com>
To:	NeilBrown <neilb@...e.de>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Vasiliy Kulikov <segoon@...nwall.com>,
	linux-kernel@...r.kernel.org, Greg Kroah-Hartman <gregkh@...e.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"David S. Miller" <davem@...emloft.net>,
	kernel-hardening@...ts.openwall.com, Jiri Slaby <jslaby@...e.cz>,
	James Morris <jmorris@...ei.org>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	linux-fsdevel@...r.kernel.org,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
Subject: Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common()

Linus, Neil, Motohiro - thank you for your comments!

On Wed, Jul 13, 2011 at 09:14:08AM +1000, NeilBrown wrote:
> The contrast is really "failing when trying to use reduced privileges is
> safer than failing to reduce privileges - if the reduced privileges are not
> available".

Right.

> Note that there is room for a race that could have unintended consequences.
> 
> Between the 'setuid(ordinary-user)' and a subsequent 'exit()' after execve()
> has failed, any other process owned by the same user (and we know where are
> quite a few) would fail an execve() where it really should not.

It is not obvious to me that this is unintended, and that dealing with
it in some way makes much of a difference.  (Also, it's not exactly "any
other process owned by the same user" - this only affects processes that
also run with similar or lower RLIMIT_NPROC.  So, for example, if a web
server is set to use RLIMIT_NPROC of 30, but interactive logins use 40,
then the latter may succeed and allow for shell commands to succeed.
This is actually a common combination of settings that we've been using
on some systems for years.)

> I think it would be safer to add a test for PF_SUPERPRIV and PF_FORKNOEXEC
> in current->flags and only fail the execve if both are set.
> i.e.
>     (current->flags & (PF_SUPERPRIV|PF_FORKNOEXEC)) == (PF_SUPERPRIV|PF_FORKNOEXEC)
> 
> That should narrow it down to only failing in the particular case that we are
> interested in.

That's a curious idea, and apparently this is what NetBSD does, but
unfortunately it does not match a common use case that we are interested
in - specifically, Apache with suEXEC (which is part of the Apache
distribution).  Here's what happens:

httpd runs as non-root.  It forks, execs suexec (SUID root).  suexec
calls setuid() to the target non-root user and execve() on the CGI
program (script, interpreter, whatever).

Notice how the fork() and the setuid() are separated by execve() of
suexec itself.  Thus, we need to apply the RLIMIT_NPROC check on
execve() unconditionally (well, we may allow processes with
CAP_SYS_RESOURCE to proceed despite of the failed check, like it's
done in -ow patches), or at least not on the condition proposed above.

Alexander
--
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/