linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Casey Schaufler <casey@schaufler-ca.com>
To: "Serge E. Hallyn" <serge@hallyn.com>,
	Micah Morton <mortonm@chromium.org>
Cc: Kees Cook <keescook@chromium.org>,
	jmorris@namei.org, linux-security-module@vger.kernel.org
Subject: Re: [PATCH] LSM: add SafeSetID module that gates setid calls
Date: Thu, 1 Nov 2018 08:39:34 -0700	[thread overview]
Message-ID: <2d8b8ee8-0087-b9c2-f20d-90108e0b18bd@schaufler-ca.com> (raw)
In-Reply-To: <20181101061322.GB7132@mail.hallyn.com>

On 10/31/2018 11:13 PM, Serge E. Hallyn wrote:
> On Wed, Oct 31, 2018 at 06:12:46PM -0700, Micah Morton wrote:
>> On Wed, Oct 31, 2018 at 3:37 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>> On 10/31/2018 2:57 PM, Kees Cook wrote:
>>>> On Wed, Oct 31, 2018 at 2:02 PM, Serge E. Hallyn <serge@hallyn.com> wrote:
>>>>> Just to be sure - your end-goal is to have a set of tasks which have
>>>>> some privileges, including CAP_SETUID, but which cannot transition to
>>>>> certain uids, perhaps including root?
>> Correct, only whitelisted uids can be switched to. This only pertains
>> to CAP_SETUID, other capabilities are not affected.
>>
>>>> AIUI, the issue is that CAP_SETUID is TOO permissive. Instead, run
>>>> _without_ CAP_SETUID and still allow whitelisted uid transitions.
>> Kees is right that this LSM only pertains to a single capability:
>> CAP_SETUID (future work could tackle CAP_SETGID in the same fashion)
>> -- although the idea here is to put in per-user limitations on what a
>> process running as that user can do even when it _has_ CAP_SETUID. So
>> it doesn't grant any extra privileges to processes that don't have
>> CAP_SETUID, only restricts processes that _do_ have CAP_SETUID if the
>> user they are running under is restricted.
>>
>>> I don't like that thought at all at all. You need CAP_SETUID for
>>> some transitions but not all. I can call setreuid() and restore
>>> the saved UID to the effective UID. If this LSM works correctly
>>> (I haven't examined it carefully yet) it should prevent restoring
>>> the effective UID if there isn't an appropriate whitelist entry.
>> Yep, thats how it works. The idea here is that you still need
>> CAP_SETUID for all transitions, regardless of whether whitelist
>> policies exist or not.
>>
>>> It also violates the "additional restriction" model of LSMs.
> Does it, or does the fact that CAP_SETUID is still required in order
> to change uids address that?

Yes, it does. Reading Kees' response had me a little concerned.

>>> That has the potential to introduce a failure when a process tries
>>> to give up privilege. If 0:1000 isn't on the whitelist but 1000:0
>> As above, if a process drops CAP_SETUID it wouldn't be able to do any
>> transitions (if this is what you mean by give up privilege). The
>> whitelist is a one-way policy so if one wanted to restrict user 123
>> but let it switch to 456 and back, 2 policies would need to be added:
>> 123 -> 456 and 456 -> 123.
>>
>>> is Bad Things can happen. A SUID root program would be unable to
>>> give up its privilege by going back to the real UID in this case.
> Yes, this was the root cause of the "sendmail capabilities bug"

I'm very familiar with that particular bug, as Bob Mende's
work to convert sendmail to using capabilities was done for
a project I owned. The blowback against all things security
was pretty intense.

>  - a
> privileged daemon which could be made to run with slightly less
> privilege in such a way that it failed to drop privilege, then continued
> ot run with some privilege.
>
> But the key trigger there was that an unprivileged task could prevent
> the more privileged task from dropping its privilege.
>
> Is that the case here?

I think it is reasonably safe to assume that there
are many instances of programs that don't handle errors
from setreuid() in the reset case. Without privilege
setreuid() can be used to swap effective and real UIDs.

>   It might be...  If one of the uid-restricted
> tasks running with CAP_SETUID runs a filter over some malicious data
> which forces it to run a program which intends to change its uid and
> fails to detect that that failed.  It's not quite as cut-and-dried
> though, and if we simply do not allow uid 0 to be in the set of uids,
> that may prevent any such cases.

Alas, UID 0 is not the only case we have to worry about.
If I run a program owned by tss (Trousers) with the setuid
bit set it will change the effective UID to tss. If this
program expects to switch effective UID back to me and
the SafeSetID whitelist prevents it, Bad Things may happen
even though no capabilities or root privilege where ever
involved.

It would be easy for an inexperienced or malicious admin to
include cschaufler:tss in the whitelist but miss on adding
tss:cschaufler.


  reply	other threads:[~2018-11-01 15:39 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-31 15:28 [PATCH] LSM: add SafeSetID module that gates setid calls mortonm
2018-10-31 21:02 ` Serge E. Hallyn
2018-10-31 21:57   ` Kees Cook
2018-10-31 22:37     ` Casey Schaufler
2018-11-01  1:12       ` Micah Morton
2018-11-01  6:13         ` Serge E. Hallyn
2018-11-01 15:39           ` Casey Schaufler [this message]
2018-11-01 15:56             ` Serge E. Hallyn
2018-11-01 16:18             ` Micah Morton
2018-11-01  6:07   ` Serge E. Hallyn
2018-11-01 16:11     ` Micah Morton
2018-11-01 16:22       ` Micah Morton
2018-11-01 16:41       ` Micah Morton
2018-11-01 17:08       ` Casey Schaufler
2018-11-01 19:52         ` Micah Morton
2018-11-02 16:05           ` Casey Schaufler
2018-11-02 17:12             ` Micah Morton
2018-11-02 18:19               ` Casey Schaufler
2018-11-02 18:30                 ` Serge E. Hallyn
2018-11-02 19:02                   ` Casey Schaufler
2018-11-02 19:22                     ` Serge E. Hallyn
2018-11-08 20:53                       ` Micah Morton
2018-11-08 21:34                         ` Casey Schaufler
2018-11-09  0:30                           ` Micah Morton
2018-11-09 23:21                             ` [PATCH] LSM: generalize flag passing to security_capable mortonm
2018-11-21 16:54                             ` [PATCH] LSM: add SafeSetID module that gates setid calls mortonm
2018-12-06  0:08                               ` Kees Cook
2018-12-06 17:51                                 ` Micah Morton
2019-01-11 17:13                                 ` [PATCH v2] " mortonm
2019-01-15  0:38                                   ` Kees Cook
2019-01-15 18:04                                     ` [PATCH v3 1/2] LSM: mark all set*uid call sites in kernel/sys.c mortonm
2019-01-15 19:34                                       ` Kees Cook
2019-01-15 18:04                                     ` [PATCH v3 2/2] LSM: add SafeSetID module that gates setid calls mortonm
2019-01-15 19:44                                       ` Kees Cook
2019-01-15 21:50                                         ` [PATCH v4 " mortonm
2019-01-15 22:32                                           ` Kees Cook
2019-01-16 15:46                                             ` [PATCH v5 " mortonm
2019-01-16 16:10                                               ` Casey Schaufler
2019-01-22 20:40                                                 ` Micah Morton
2019-01-22 22:28                                                   ` James Morris
2019-01-22 22:40                                                     ` Micah Morton
2019-01-22 22:42                                                       ` [PATCH v3 1/2] " mortonm
2019-01-25 15:51                                                         ` Micah Morton
2019-01-25 20:15                                               ` [PATCH v5 2/2] " James Morris
2019-01-25 21:06                                                 ` Micah Morton
2019-01-28 19:47                                                   ` Micah Morton
2019-01-28 19:56                                                     ` Kees Cook
2019-01-28 20:09                                                       ` James Morris
2019-01-28 20:19                                                       ` Micah Morton
2019-01-28 20:30                                                         ` [PATCH] LSM: Add 'name' field for SafeSetID in DEFINE_LSM mortonm
2019-01-28 22:12                                                           ` James Morris
2019-01-28 22:33                                                         ` [PATCH v5 2/2] LSM: add SafeSetID module that gates setid calls Micah Morton
2019-01-29 17:25                                                           ` James Morris
2019-01-29 21:14                                                             ` Micah Morton
2019-01-30  7:15                                                               ` Kees Cook
2019-02-06 19:03                                                                 ` [PATCH] LSM: SafeSetID: add selftest mortonm
2019-02-06 19:26                                                                   ` Edwin Zimmerman
2019-02-07 21:54                                                                     ` Micah Morton
2019-02-12 19:01                                                                   ` James Morris
2019-01-15 21:58                                         ` [PATCH v3 2/2] LSM: add SafeSetID module that gates setid calls Micah Morton
2019-01-15 19:49                                     ` [PATCH v2] " Micah Morton
2019-01-15 19:53                                       ` Kees Cook
2019-01-15  4:07                                   ` James Morris
2019-01-15 19:42                                     ` Micah Morton
2018-11-02 19:28                 ` [PATCH] " Micah Morton
2018-11-06 19:09                 ` [PATCH v2] " mortonm
2018-11-06 20:59       ` [PATCH] " James Morris
2018-11-06 21:21         ` [PATCH v3] " mortonm
2018-11-02 18:07 ` [PATCH] " Stephen Smalley
2018-11-02 19:13   ` Micah Morton
2018-11-19 18:54   ` [PATCH] [PATCH] LSM: generalize flag passing to security_capable mortonm
2018-12-13 22:29     ` Micah Morton
2018-12-13 23:09       ` Casey Schaufler
2018-12-14  0:05         ` Micah Morton
2018-12-18 22:37         ` [PATCH v2] " mortonm
2019-01-07 17:55           ` Micah Morton
2019-01-07 18:16             ` Casey Schaufler
2019-01-07 18:36               ` Micah Morton
2019-01-07 18:46                 ` Casey Schaufler
2019-01-07 19:02                   ` Micah Morton
2019-01-07 22:57                     ` [PATCH v3] " mortonm
2019-01-07 23:13           ` [PATCH v2] " Kees Cook
2019-01-08  0:10             ` [PATCH v4] " mortonm
2019-01-08  0:20               ` Kees Cook
2019-01-09 18:39                 ` Micah Morton
2019-01-10 22:31               ` James Morris
2019-01-10 23:03                 ` Micah Morton
2019-01-08  0:10             ` [PATCH v2] " Micah Morton

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=2d8b8ee8-0087-b9c2-f20d-90108e0b18bd@schaufler-ca.com \
    --to=casey@schaufler-ca.com \
    --cc=jmorris@namei.org \
    --cc=keescook@chromium.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mortonm@chromium.org \
    --cc=serge@hallyn.com \
    /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).