All of lore.kernel.org
 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: [kernel-hardening] 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

WARNING: multiple messages have this Message-ID (diff)
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: 76+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-12 13:09 [kernel-hardening] RLIMIT_NPROC check in set_user() Vasiliy Kulikov
2011-06-12 13:09 ` Vasiliy Kulikov
2011-07-06 17:36 ` [kernel-hardening] " Vasiliy Kulikov
2011-07-06 17:36   ` Vasiliy Kulikov
2011-07-06 18:01   ` [kernel-hardening] " Linus Torvalds
2011-07-06 18:01     ` Linus Torvalds
2011-07-06 18:59     ` [kernel-hardening] " Vasiliy Kulikov
2011-07-07  7:56       ` Vasiliy Kulikov
2011-07-07  8:19         ` Vasiliy Kulikov
2011-07-12 13:27           ` [kernel-hardening] [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common() Vasiliy Kulikov
2011-07-12 13:27             ` Vasiliy Kulikov
2011-07-12 21:16             ` [kernel-hardening] " Linus Torvalds
2011-07-12 21:16               ` Linus Torvalds
2011-07-12 23:14               ` [kernel-hardening] " NeilBrown
2011-07-12 23:14                 ` NeilBrown
2011-07-13  6:31                 ` [kernel-hardening] " Solar Designer
2011-07-13  6:31                   ` Solar Designer
2011-07-13  7:06                   ` [kernel-hardening] " NeilBrown
2011-07-13  7:06                     ` NeilBrown
2011-07-13 20:46                     ` [kernel-hardening] " Linus Torvalds
2011-07-13 20:46                       ` Linus Torvalds
2011-07-14  0:11                       ` [kernel-hardening] " James Morris
2011-07-14  0:11                         ` James Morris
2011-07-14  1:27                         ` [kernel-hardening] " NeilBrown
2011-07-14  1:27                           ` NeilBrown
2011-07-14 15:06                           ` Solar Designer [this message]
2011-07-14 15:06                             ` Solar Designer
2011-07-15  3:30                             ` [kernel-hardening] " NeilBrown
2011-07-15  3:30                               ` NeilBrown
2011-07-15  5:35                               ` [kernel-hardening] " Willy Tarreau
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:06                                   ` NeilBrown
2011-07-15  7:38                                   ` Vasiliy Kulikov
2011-07-15 13:04                                     ` Solar Designer
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 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                                                             ` [kernel-hardening] [patch v2] " Vasiliy Kulikov
2011-07-26 14:48                                                               ` Vasiliy Kulikov
2011-07-27  2:15                                                               ` [kernel-hardening] " NeilBrown
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:06                                                                 ` Vasiliy Kulikov
2011-07-29  8:11                                                                 ` [kernel-hardening] [patch v3] " Vasiliy Kulikov
2011-07-29  8:11                                                                   ` Vasiliy Kulikov
2011-07-29  8:17                                                                   ` [kernel-hardening] " James Morris
2011-07-29  8:17                                                                     ` James Morris
2011-07-24 14:32                                               ` [kernel-hardening] [PATCH] " Solar Designer
2011-07-24 18:02                                                 ` Vasiliy Kulikov
2011-07-14  1:30                         ` [kernel-hardening] " KOSAKI Motohiro
2011-07-14  1:30                           ` KOSAKI Motohiro
2011-07-13  5:36             ` [kernel-hardening] " KOSAKI Motohiro
2011-07-13  5:36               ` KOSAKI Motohiro
2011-07-14 15:22             ` [kernel-hardening] " Solar Designer
2011-07-14 15:55               ` Vasiliy Kulikov
2011-07-11 16:59       ` [kernel-hardening] RLIMIT_NPROC check in set_user() Solar Designer
2011-07-11 18:56         ` Vasiliy Kulikov
2011-07-13  9:48           ` Solar Designer
2011-07-14 14:15             ` Solar Designer
2011-07-14 14:27               ` Vasiliy Kulikov
2011-07-14 15:14                 ` Solar Designer
2011-07-14 16:31                   ` [kernel-hardening] compile time warnings in libc for setuid() unused result (was: RLIMIT_NPROC check in set_user()) Vasiliy Kulikov

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.