linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Solar Designer <solar@openwall.com>
To: NeilBrown <neilb@suse.de>
Cc: James Morris <jmorris@namei.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Vasiliy Kulikov <segoon@openwall.com>,
	linux-kernel@vger.kernel.org, Greg Kroah-Hartman <gregkh@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	"David S. Miller" <davem@davemloft.net>,
	kernel-hardening@lists.openwall.com, Jiri Slaby <jslaby@suse.cz>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Eric Paris <eparis@redhat.com>,
	Stephen Smalley <sds@tycho.nsa.gov>, Willy Tarreau <w@1wt.eu>,
	Sebastian Krahmer <krahmer@suse.de>
Subject: Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common()
Date: Thu, 14 Jul 2011 19:06:02 +0400	[thread overview]
Message-ID: <20110714150602.GA30019@openwall.com> (raw)
In-Reply-To: <20110714112751.1bfd998f@notabene.brown>

On Thu, Jul 14, 2011 at 11:27:51AM +1000, NeilBrown wrote:
> I'm still trying to understand the full consequences, but I agree that there
> is no rush - the code has been this way for quite a while and their is no
> obvious threat that would expedite things (as far as I know).

I don't insist on getting this in sooner than in the next merge window,
although I would have liked that.  Relevant userspace vulnerabilities
are being found quite often - I'll include some examples below.

> However I'm not convinced that testing will help all that much - if there are
> problems they will be is rare corner cases that testing is unlikely to find.

This makes sense.

> The only case where this change will improve safety is where:
>  1/ a process has RLIMIT_NPROC set
>  2/ the process is running with root privileges
>  3/ the process calls setuid() and doesn't handle errors.

Yes, and this is a pretty common case.

> I think the only times that a root process would have RLIMIT_NPROC set are:
>  1/ if it explicitly set up rlimits before calling setuid.  In this case
>       we should be able to expect that the process checks setuid .. maybe
>       this is naive(?)

RLIMIT_NPROC could be set by the parent process or by pam_limits.  The
machine I am typing this on has:

*       hard    nproc   200

(as well as other limits) in /etc/security/limits.conf, so if this
machine's kernel let setuid() fail on RLIMIT_NPROC, I would be taking
extra risk of a security compromise by reducing the risk of system
crashes from inadvertent excessive resource consumption by runaway
processes - a tradeoff I'd rather avoid.

>  2/ if the process was setuid-root and inherited rlimits from before, and
>       never re-set them.  In this case it is easy to imagine that a setuid()
>       would not be checked.

Right.  (In practice, all kinds of programs tend to forget to check
setuid() return value, though.)

Actually, for the problem to apply to setuid-root programs, they need to
switch their real uid first (more fully become root), then try to switch
to a user - but this is common.

Here are some examples for 2011-2010:

"... missing setuid() retval check in opielogin which leads to easy root
compromise":

http://www.openwall.com/lists/oss-security/2011/06/22/6

"The /usr/lib/libgnomesu/gnomesu-pam-backend suid binary which belongs
to the libgnomesu package is not checking setuid() return values.

As a result, two cooperating users, or users with access to guest,
cgi or web accounts can run arbitrary commands as root very easily."

http://www.openwall.com/lists/oss-security/2011/05/30/2

pam_xauth (exploitable if pam_limits is also used):

http://www.openwall.com/lists/oss-security/2010/08/16/2

A collection of examples from 2006:

http://lists.openwall.net/linux-kernel/2006/08/20/156

> So maybe an alternate 'fix' would be to reset all rlimits to match init_task
> when a setuid-root happens.  There are other corner cases where inappropriate
> rlimits can cause setuid programs to behave in ways they don't expect.
> Obviously such programs are buggy, but so are programs that don't check
> 'setuid'.  (There is a CVE about mount potentially corrupting mtab.)

Right, but to me possibly resetting rlimits is not an "alternative" to
moving the RLIMIT_NPROC check.  setuid-root exec is not the only case
where having setuid() fail on RLIMIT_NPROC is undesirable.  We also
don't want such failures with pam_limits, nor on daemon startup:

http://www.openwall.com/lists/oss-security/2009/07/14/2

As to resetting rlimits on SUID/SGID exec, I think this would make
sense for RLIMIT_FSIZE, which would mitigate the mount mtab issue
(thank you for bringing it up!)  But it's to be discussed separately.

Alexander

  reply	other threads:[~2011-07-14 15:06 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20110612130953.GA3709@albatros>
     [not found] ` <20110706173631.GA5431@albatros>
     [not found]   ` <CA+55aFyfjG1h2zkkGai_VPM8p5bhWhvNXs1HvuWMgxv4RSywYw@mail.gmail.com>
     [not found]     ` <20110706185932.GB3299@albatros>
     [not found]       ` <20110707075610.GA3411@albatros>
     [not found]         ` <20110707081930.GA4393@albatros>
2011-07-12 13:27           ` [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() Vasiliy Kulikov
2011-07-12 21:16             ` Linus Torvalds
2011-07-12 23:14               ` NeilBrown
2011-07-13  6:31                 ` Solar Designer
2011-07-13  7:06                   ` NeilBrown
2011-07-13 20:46                     ` Linus Torvalds
2011-07-14  0:11                       ` James Morris
2011-07-14  1:27                         ` NeilBrown
2011-07-14 15:06                           ` Solar Designer [this message]
2011-07-15  3:30                             ` NeilBrown
2011-07-15  5:35                               ` Willy Tarreau
2011-07-15  6:31                               ` [kernel-hardening] " Vasiliy Kulikov
2011-07-15  7:06                                 ` NeilBrown
2011-07-15  7:38                                   ` Vasiliy Kulikov
2011-07-15 13:04                                     ` Solar Designer
2011-07-15 13:58                                     ` [kernel-hardening] " Stephen Smalley
2011-07-15 15:26                                       ` Vasiliy Kulikov
2011-07-15 19:54                                         ` Stephen Smalley
2011-07-21  4:09                                           ` NeilBrown
2011-07-21 12:48                                             ` Solar Designer
2011-07-21 18:21                                               ` Linus Torvalds
2011-07-21 19:39                                                 ` [kernel-hardening] " Solar Designer
2011-07-25 17:14                                                   ` Vasiliy Kulikov
2011-07-25 23:40                                                     ` [kernel-hardening] " Solar Designer
2011-07-26  0:47                                                       ` NeilBrown
2011-07-26  1:16                                                         ` Solar Designer
2011-07-26  4:11                                                           ` NeilBrown
2011-07-26 14:48                                                             ` [patch v2] " Vasiliy Kulikov
2011-07-27  2:15                                                               ` NeilBrown
2011-07-29  7:07                                                                 ` [kernel-hardening] " Vasiliy Kulikov
2011-07-29  8:06                                                               ` Vasiliy Kulikov
2011-07-29  8:11                                                                 ` [patch v3] " Vasiliy Kulikov
2011-07-29  8:17                                                                   ` James Morris
2011-07-14  1:30                         ` [PATCH] " KOSAKI Motohiro
2011-07-13  5:36             ` KOSAKI Motohiro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110714150602.GA30019@openwall.com \
    --to=solar@openwall.com \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=eparis@redhat.com \
    --cc=gregkh@suse.de \
    --cc=jmorris@namei.org \
    --cc=jslaby@suse.cz \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=krahmer@suse.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=sds@tycho.nsa.gov \
    --cc=segoon@openwall.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=w@1wt.eu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).