public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: David Laight <David.Laight@ACULAB.COM>
To: 'Linus Torvalds' <linus@torvalds.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>
Cc: "Linux API" <linux-api@vger.kernel.org>,
	"Etienne Dechamps" <etienne@edechamps.fr>,
	"Alexey Gladkov" <legion@kernel.org>,
	"Kees Cook" <keescook@chromium.org>,
	"Shuah Khan" <shuah@kernel.org>,
	"Christian Brauner" <brauner@kernel.org>,
	"Solar Designer" <solar@openwall.com>,
	"Ran Xiaokai" <ran.xiaokai@zte.com.cn>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"open list:KERNEL SELFTEST FRAMEWORK"
	<linux-kselftest@vger.kernel.org>,
	"Linux Containers" <containers@lists.linux-foundation.org>,
	"Michal Koutný" <mkoutny@suse.com>,
	"Security Officers" <security@kernel.org>,
	"Neil Brown" <neilb@cse.unsw.edu.au>, NeilBrown <neilb@suse.de>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	"Jann Horn" <jannh@google.com>,
	"Andy Lutomirski" <luto@kernel.org>, "Willy Tarreau" <w@1wt.eu>
Subject: RE: How should rlimits, suid exec, and capabilities interact?
Date: Thu, 24 Feb 2022 03:00:49 +0000	[thread overview]
Message-ID: <a56fcc390a374b4da628ea55eb5dd6cc@AcuMS.aculab.com> (raw)
In-Reply-To: <CAHk-=wgW8+vmqhx4t+uFiZL==8Ac5VWTqCm_oshA0e47B73qPw@mail.gmail.com>

From: Linus Torvalds
> Sent: 24 February 2022 01:42
> 
> On Wed, Feb 23, 2022 at 5:24 PM Eric W. Biederman <ebiederm@xmission.com> wrote:
> >
> > Question: Running a suid program today charges the activity of that
> > program to the user who ran that program, not to the user the program
> > runs as.  Does anyone see a problem with charging the user the program
> > runs as?
> 
> So I think that there's actually two independent issues with limits
> when you have situations like this where the actual user might be
> ambiguous.
> 
>  - the "who to charge" question
> 
>  - the "how do we *check* the limit" question
> 
> and honestly, I think that when it comes to suid binaries, the first
> question is fundamentally ambiguous, because it almost certainly
> depends on the user.

Doesn't the rlimit check happen during the fork.
At which time you don't know that a suid exec might follow?

The problem with changing the uid is that when the process exits
you need to "uncharge" the correct uid.
So either you need to remember the original uid or setuid
has to transfer the charge (whichever uid is used).
If you transfer the charge then the setuid system call can't fail.
But a later exec can fail.

Any check will always be done against the process's own rlimit value.
Set that to zero and fork should fail regardless of which uid's
process count is checked.

Now a normal suid program only changes the effective uid.
So keeping the process charged against the real uid makes sense.

If a process changes its real uid you could change the charged uid
but you can't error if over the rlimit value.
OTOH during a later exec you can test things and exec can fail.

At least one unix I've used has three uids for each process.
The 'real uid', 'effective uid' and 'saved by exec uid'.
I suspect the process is always "charged" against the latter.
I think that exec compares the 'real' and 'saved by exec' uids
and, if different, moves the charge to the real uid (which will
check rlimit) then sets the 'saved by exec uid' to the real uid.

So an exec after a setuid() can be allowed to fail if the real user
has too many processes.
But in all other cases exec just works regardless of the process
count for any user.

> 
> Which to me implies that there probably isn't an answer that is always
> right, and that what you should look at is that second option.
> 
> So I would actually suggest that the "execute a suid binary" should
> charge the real user, but *because* it is suid, it should then not
> check the limit (or, perhaps, should check the hard limit?).
> 
> You have to charge somebody, but at that point it's a bit ambiguous
> whether it should be allowed.
> 
> Exactly so that if you're over a process limit (or something similar -
> think "too many files open" or whatever because you screwed up and
> opened everything) you could still log in as yourself (ssh/login
> charges some admin thing, which probably has high limits or is
> unlimited), and hopefully get shell access, and then be able to "exec
> sudo" to actually get admin access that should be disabled from the
> network.

You usually have to use 'rsh machine sh -i' to avoid the shell
running all its startup scripts.
But I doubt that will get you past a fork bomb.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

  parent reply	other threads:[~2022-02-24  3:00 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-07 12:17 [RFC PATCH 0/6] RLIMIT_NPROC in ucounts fixups Michal Koutný
2022-02-07 12:17 ` [RFC PATCH 1/6] set_user: Perform RLIMIT_NPROC capability check against new user credentials Michal Koutný
2022-02-10  1:14   ` Solar Designer
2022-02-10  1:57     ` Eric W. Biederman
2022-02-11 20:32     ` Eric W. Biederman
2022-02-12 22:14       ` Solar Designer
2022-02-15 11:55     ` Michal Koutný
2022-02-07 12:17 ` [RFC PATCH 2/6] set*uid: Check RLIMIT_PROC against new credentials Michal Koutný
2022-02-07 12:17 ` [RFC PATCH 3/6] cred: Count tasks by their real uid into RLIMIT_NPROC Michal Koutný
2022-02-07 12:17 ` [RFC PATCH 4/6] ucounts: Allow root to override RLIMIT_NPROC Michal Koutný
2022-02-10  0:21   ` Eric W. Biederman
2022-02-07 12:17 ` [RFC PATCH 5/6] selftests: Challenge RLIMIT_NPROC in user namespaces Michal Koutný
2022-02-10  1:22   ` Shuah Khan
2022-02-15  9:45     ` Michal Koutný
2022-02-07 12:18 ` [RFC PATCH 6/6] selftests: Test RLIMIT_NPROC in clone-created " Michal Koutný
2022-02-10  1:25   ` Shuah Khan
2022-02-15  9:34     ` Michal Koutný
2022-02-08 13:54 ` [RFC PATCH 0/6] RLIMIT_NPROC in ucounts fixups Eric W. Biederman
2022-02-11  2:01 ` [PATCH 0/8] ucounts: RLIMIT_NPROC fixes Eric W. Biederman
2022-02-11  2:13   ` [PATCH 1/8] ucounts: Fix RLIMIT_NPROC regression Eric W. Biederman
2022-02-14 18:37     ` Michal Koutný
2022-02-16 15:22       ` Eric W. Biederman
2022-02-11  2:13   ` [PATCH 2/8] ucounts: Fix set_cred_ucounts Eric W. Biederman
2022-02-15 11:10     ` Michal Koutný
2022-02-11  2:13   ` [PATCH 3/8] ucounts: Fix and simplify RLIMIT_NPROC handling during setuid()+execve Eric W. Biederman
2022-02-12 23:17     ` Solar Designer
2022-02-14 15:10       ` Eric W. Biederman
2022-02-14 17:43         ` Eric W. Biederman
2022-02-15 10:25         ` Michal Koutný
2022-02-16 15:35           ` Eric W. Biederman
2022-02-11  2:13   ` [PATCH 4/8] ucounts: Only except the root user in init_user_ns from RLIMIT_NPROC Eric W. Biederman
2022-02-15 10:54     ` Michal Koutný
2022-02-16 15:41       ` Eric W. Biederman
2022-02-11  2:13   ` [PATCH 5/8] ucounts: Handle wrapping in is_ucounts_overlimit Eric W. Biederman
2022-02-12 22:36     ` Solar Designer
2022-02-14 15:23       ` Eric W. Biederman
2022-02-14 15:23         ` Eric W. Biederman
2022-02-15 11:25         ` Michal Koutný
2022-02-14 17:16       ` David Laight
2022-02-11  2:13   ` [PATCH 6/8] ucounts: Handle inc_rlimit_ucounts wrapping in fork Eric W. Biederman
2022-02-11 11:34     ` Alexey Gladkov
2022-02-11 17:50       ` Eric W. Biederman
2022-02-11 18:32         ` Shuah Khan
2022-02-11 18:40         ` Alexey Gladkov
2022-02-11 19:56           ` Eric W. Biederman
2022-02-11  2:13   ` [PATCH 7/8] rlimit: For RLIMIT_NPROC test the child not the parent for capabilites Eric W. Biederman
2022-02-11  2:13   ` [PATCH 8/8] ucounts: Use the same code to enforce RLIMIT_NPROC in fork and exec Eric W. Biederman
2022-02-11 18:22   ` [PATCH 0/8] ucounts: RLIMIT_NPROC fixes Shuah Khan
2022-02-11 19:23     ` Eric W. Biederman
2022-02-15 11:37     ` Michal Koutný
2022-02-16 15:56   ` [PATCH v2 0/5] " Eric W. Biederman
2022-02-16 15:58     ` [PATCH v2 1/5] rlimit: Fix RLIMIT_NPROC enforcement failure caused by capability calls in set_user Eric W. Biederman
2022-02-16 17:42       ` Solar Designer
2022-02-16 15:58     ` [PATCH v2 2/5] ucounts: Enforce RLIMIT_NPROC not RLIMIT_NPROC+1 Eric W. Biederman
2022-02-16 15:58     ` [PATCH v2 3/5] ucounts: Base set_cred_ucounts changes on the real user Eric W. Biederman
2022-02-16 15:58     ` [PATCH v2 4/5] ucounts: Move RLIMIT_NPROC handling after set_user Eric W. Biederman
2022-02-16 15:58     ` [PATCH v2 5/5] ucounts: Handle wrapping in is_ucounts_overlimit Eric W. Biederman
2022-02-16 17:28       ` Shuah Khan
2022-02-18 15:34     ` [GIT PULL] ucounts: RLIMIT_NPROC fixes for v5.17 Eric W. Biederman
2022-02-20 19:05       ` pr-tracker-bot
2022-03-03  0:12       ` [GIT PULL] ucounts: Regression fix " Eric W. Biederman
2022-03-03  0:30         ` pr-tracker-bot
2022-02-12 15:32 ` [RFC PATCH 0/6] RLIMIT_NPROC in ucounts fixups Etienne Dechamps
2022-02-15 10:11   ` Michal Koutný
2022-02-23  0:57     ` Eric W. Biederman
2022-02-23 18:00       ` How should rlimits, suid exec, and capabilities interact? Eric W. Biederman
2022-02-23 19:44         ` Andy Lutomirski
2022-02-23 21:28           ` Willy Tarreau
2022-02-23 19:50         ` Linus Torvalds
2022-02-24  1:24           ` Eric W. Biederman
2022-02-24  1:41             ` Linus Torvalds
2022-02-24  2:12               ` Eric W. Biederman
2022-02-24 15:41                 ` [PATCH] ucounts: Fix systemd LimigtNPROC with private users regression Eric W. Biederman
2022-02-24 16:28                   ` Kees Cook
2022-02-24 18:53                     ` Michal Koutný
2022-02-25  0:29                     ` Eric W. Biederman
2022-02-24  3:00               ` David Laight [this message]
2022-02-24  1:32           ` How should rlimits, suid exec, and capabilities interact? Eric W. Biederman

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=a56fcc390a374b4da628ea55eb5dd6cc@AcuMS.aculab.com \
    --to=david.laight@aculab.com \
    --cc=brauner@kernel.org \
    --cc=containers@lists.linux-foundation.org \
    --cc=ebiederm@xmission.com \
    --cc=etienne@edechamps.fr \
    --cc=jannh@google.com \
    --cc=keescook@chromium.org \
    --cc=legion@kernel.org \
    --cc=linus@torvalds.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mkoutny@suse.com \
    --cc=neilb@cse.unsw.edu.au \
    --cc=neilb@suse.de \
    --cc=ran.xiaokai@zte.com.cn \
    --cc=security@kernel.org \
    --cc=serge@hallyn.com \
    --cc=shuah@kernel.org \
    --cc=solar@openwall.com \
    --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