linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Solar Designer <solar@openwall.com>
Cc: NeilBrown <neilb@suse.de>, Stephen Smalley <sds@tycho.nsa.gov>,
	Vasiliy Kulikov <segoon@openwall.com>,
	kernel-hardening@lists.openwall.com,
	James Morris <jmorris@namei.org>,
	linux-kernel@vger.kernel.org, Greg Kroah-Hartman <gregkh@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	"David S. Miller" <davem@davemloft.net>,
	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>, Willy Tarreau <w@1wt.eu>,
	Sebastian Krahmer <krahmer@suse.de>
Subject: Re: [kernel-hardening] Re: [PATCH] move RLIMIT_NPROC check from set_user() to do_execve_common()
Date: Thu, 21 Jul 2011 11:21:07 -0700	[thread overview]
Message-ID: <CA+55aFy4ybatb0McQr2f4A56ympZVLqK2sbp2=73idu58_4RxQ@mail.gmail.com> (raw)
In-Reply-To: <20110721124830.GA1325@openwall.com>

On Thu, Jul 21, 2011 at 5:48 AM, Solar Designer <solar@openwall.com> wrote:
>
> Maybe, and if so I think that one I proposed above falls in this
> category as well, but it closes more vulnerabilities (and/or does so
> more fully).

I think we could have a pretty simple approach that "works in
practice": retain the check at setuid() time, but make it a higher
limit.

IOW, the logic is that we have two competing pressures:

 (a) we should try to avoid failing on setuid(), because there is a
real risk that the setuid caller doesn't really check the failure case
and opens itself up for a security problem

and

 (b) never failing setuid at all is in itself a security problem,
since it can lead to DoS attacks in the form of excessive resource use
by one user.

But the sane (intelligent) solution to that is to say that we *PREFER*
to fail in execve(), but that at some point a failure in setuid() is
preferable to no failure at all. After all, we have no hard knowledge
that there is any actual setuid() issue. Neither generally does the
user (iow, look at this whole discussion where intelligent people
simply have different inputs depending on "what could happen").

So it really seems like the natural approach would be to simply fail
*earlier* on execve() and fork(). That will catch most cases, and has
no downsides. But if we notice that we are in a situation where some
privileged user can be tricked into forking a lot and doing setuid(),
then at that point the setuid() path becomes relevant.

IOW, I'd suggest simply making the rule be that "setuid() allows 10%
more users than the limit technically says". It's not a guarantee, but
it means that in order to hit the problem, you need to have *both* a
setuid application that allows unconstrained user forking *and*
doesn't check the setuid() return value.

Put another way: a user cannot force the "we're at the edge of the
setuid() limit" on its own by just forking - the user will be stopped
10% before the setuid() failure case can ever trigger.

Is this some "guarantee of nothing bad can ever happen"? No. If you
have bad setuid applications, you will have problems. But it's a "you
need to really work harder at it and you need to find more things to
go wrong", which is after all what real security is all about.

No?

                Linus

  reply	other threads:[~2011-07-21 18:21 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
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 [this message]
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='CA+55aFy4ybatb0McQr2f4A56ympZVLqK2sbp2=73idu58_4RxQ@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --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=solar@openwall.com \
    --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).