Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: "Serge E. Hallyn" <serge@hallyn.com>
To: Casey Schaufler <casey@schaufler-ca.com>
Cc: Micah Morton <mortonm@chromium.org>,
	serge@hallyn.com, jmorris@namei.org,
	Kees Cook <keescook@chromium.org>,
	linux-security-module@vger.kernel.org
Subject: Re: [PATCH] LSM: add SafeSetID module that gates setid calls
Date: Fri, 2 Nov 2018 18:30:56 +0000
Message-ID: <20181102183056.GA20738@mail.hallyn.com> (raw)
In-Reply-To: <5c7e1a80-c534-6adb-be19-58bb0d97084f@schaufler-ca.com>

Quoting Casey Schaufler (casey@schaufler-ca.com):
> On 11/2/2018 10:12 AM, Micah Morton wrote:
> > On Fri, Nov 2, 2018 at 9:05 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> On 11/1/2018 12:52 PM, Micah Morton wrote:
> >>> On Thu, Nov 1, 2018 at 10:08 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >>>> On 11/1/2018 9:11 AM, Micah Morton wrote:
> >>>>> On Wed, Oct 31, 2018 at 11:07 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> >>>>>> On Wed, Oct 31, 2018 at 09:02:45PM +0000, Serge E. Hallyn wrote:
> >>>>>>> Quoting mortonm@chromium.org (mortonm@chromium.org):
> >>>>>>>> From: Micah Morton <mortonm@chromium.org>
> >>>>>>>>
> >>>>>>>> SafeSetID gates the setid family of syscalls to restrict UID/GID
> >>>>>>>> transitions from a given UID/GID to only those approved by a
> >>>>>>>> system-wide whitelist. These restrictions also prohibit the given
> >>>>>>>> UIDs/GIDs from obtaining auxiliary privileges associated with
> >>>>>>>> CAP_SET{U/G}ID, such as allowing a user to set up user namespace UID
> >>>>>>>> mappings. For now, only gating the set*uid family of syscalls is
> >>>>>>>> supported, with support for set*gid coming in a future patch set.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Micah Morton <mortonm@chromium.org>
> >>>>>>>> ---
> >>>>>>>>
> >>>>>>>> NOTE: See the TODO above setuid_syscall() in lsm.c for an aspect of this
> >>>>>>>> code that likely needs improvement before being an acceptable approach.
> >>>>>>>> I'm specifically interested to see if there are better ideas for how
> >>>>>>>> this could be done.
> >>>>>>>>
> >>>>>>>>  Documentation/admin-guide/LSM/SafeSetID.rst |  94 ++++++
> >>>>>>>>  Documentation/admin-guide/LSM/index.rst     |   1 +
> >>>>>>>>  arch/Kconfig                                |   5 +
> >>>>>>>>  arch/arm/Kconfig                            |   1 +
> >>>>>>>>  arch/arm64/Kconfig                          |   1 +
> >>>>>>>>  arch/x86/Kconfig                            |   1 +
> >>>>>>>>  security/Kconfig                            |   1 +
> >>>>>>>>  security/Makefile                           |   2 +
> >>>>>>>>  security/safesetid/Kconfig                  |  13 +
> >>>>>>>>  security/safesetid/Makefile                 |   7 +
> >>>>>>>>  security/safesetid/lsm.c                    | 334 ++++++++++++++++++++
> >>>>>>>>  security/safesetid/lsm.h                    |  30 ++
> >>>>>>>>  security/safesetid/securityfs.c             | 189 +++++++++++
> >>>>>>>>  13 files changed, 679 insertions(+)
> >>>>>>>>  create mode 100644 Documentation/admin-guide/LSM/SafeSetID.rst
> >>>>>>>>  create mode 100644 security/safesetid/Kconfig
> >>>>>>>>  create mode 100644 security/safesetid/Makefile
> >>>>>>>>  create mode 100644 security/safesetid/lsm.c
> >>>>>>>>  create mode 100644 security/safesetid/lsm.h
> >>>>>>>>  create mode 100644 security/safesetid/securityfs.c
> >>>>>>>>
> >>>>>>>> diff --git a/Documentation/admin-guide/LSM/SafeSetID.rst b/Documentation/admin-guide/LSM/SafeSetID.rst
> >>>>>>>> new file mode 100644
> >>>>>>>> index 000000000000..e7d072124424
> >>>>>>>> --- /dev/null
> >>>>>>>> +++ b/Documentation/admin-guide/LSM/SafeSetID.rst
> >>>>>>>> @@ -0,0 +1,94 @@
> >>>>>>>> +=========
> >>>>>>>> +SafeSetID
> >>>>>>>> +=========
> >>>>>>>> +SafeSetID is an LSM module that gates the setid family of syscalls to restrict
> >>>>>>>> +UID/GID transitions from a given UID/GID to only those approved by a
> >>>>>>>> +system-wide whitelist. These restrictions also prohibit the given UIDs/GIDs
> >>>>>>>> +from obtaining auxiliary privileges associated with CAP_SET{U/G}ID, such as
> >>>>>>>> +allowing a user to set up user namespace UID mappings.
> >>>>>>>> +
> >>>>>>>> +
> >>>>>>>> +Background
> >>>>>>>> +==========
> >>>>>>>> +In absence of file capabilities, processes spawned on a Linux system that need
> >>>>>>>> +to switch to a different user must be spawned with CAP_SETUID privileges.
> >>>>>>>> +CAP_SETUID is granted to programs running as root or those running as a non-root
> >>>>>>>> +user that have been explicitly given the CAP_SETUID runtime capability. It is
> >>>>>>>> +often preferable to use Linux runtime capabilities rather than file
> >>>>>>>> +capabilities, since using file capabilities to run a program with elevated
> >>>>>>>> +privileges opens up possible security holes since any user with access to the
> >>>>>>>> +file can exec() that program to gain the elevated privileges.
> >>>>>>> Not true, see inheritable capabilities.  You also might look at ambient
> >>>>>>> capabilities.
> >>>>>> So for example with pam_cap.so you could have your N uids each be given
> >>>>>> the desired pI, and assign the corrsponding fIs to the files they should
> >>>>>> be able to exec with privilege.  No other uids will run those files with
> >>>>>> privilege.  *1
> >>>>> Sorry, what are "pl" and "fls" here? "Privilege level" and "files"?
> >>>>>
> >>>>>> Can you give some more details about exactly how you see SafeSetID being
> >>>>>> used?
> >>>>> Sure. The main use case for this LSM is to allow a non-root program to
> >>>>> transition to other untrusted uids without full blown CAP_SETUID
> >>>>> capabilities. The non-root program would still need CAP_SETUID to do
> >>>>> any kind of transition, but the additional restrictions imposed by
> >>>>> this LSM would mean it is a "safer" version of CAP_SETUID since the
> >>>>> non-root program cannot take advantage of CAP_SETUID to do any
> >>>>> unapproved actions (i.e. setuid to uid 0 or create/enter new user
> >>>>> namespace). The higher level goal is to allow for uid-based sandboxing
> >>>>> of system services without having to give out CAP_SETUID all over the
> >>>>> place just so that non-root programs can drop to
> >>>>> even-further-non-privileged uids. This is especially relevant when one
> >>>>> non-root daemon on the system should be allowed to spawn other
> >>>>> processes as different uids, but its undesirable to give the daemon a
> >>>>> basically-root-equivalent CAP_SETUID.
> >>>> I don't want to sound stupid(er than usual), but it sounds like
> >>>> you could do all this using setuid bits prudently. Based on this
> >>>> description, I don't see that anything new is needed.
> >>> There are situations where setuid bits don't get the job done, as
> >>> there are many situations where a program just wants to call setuid as
> >>> part of its execution (or fork + setuid without exec), instead of
> >>> fork/exec'ing a setuid binary.
> >> Yes, I understand that.
> >>
> >>> Take the following scenario for
> >>> example: init script (as root) spawns a network manager program as uid
> >>> 1000
> >> So far, so good.
> >>
> >>> and then the network manager spawns OpenVPN. The common mode of
> >>> operation for OpenVPN is to start running as the uid it was spawned
> >>> with (1000) at startup, but then drop to a lesser-privileged uid (e.g.
> >>> 2000) after initialization/setup by calling setuid.
> >> OK. That's an operation that does and ought to require privilege.
> > Sure, but the idea behind this LSM is that full CAP_SETUID
> > capabilities are a lot more privilege than is necessary in this
> > scenario.
> 
> I'll start by pointing out that CAP_SETUID is about the finest grained
> capability there is. It's very precise in what it allows. I think that
> your concern is about the worst case scenario, which is setting the
> effective UID to 0, and hence gaining all privilege.
> 
> 
> >>> This is something
> >>> setuid bits wouldn't help with, without refactoring OpenVPN.
> >> You're correct.
> >>
> >>> So one
> >>> option here is to give the network manager CAP_SETUID, which will be
> >>> inherited by OpenVPN, and then OpenVPN drops to uid 2000 and drops
> >>> CAP_SETUID (would probably require patching OpenVPN for the capability
> >>> dropping).
> >> Or, you put CAP_SETUID on the file capabilities for OpenVPN,
> >> which is the way the P1003.1e DRAFT specification would have
> >> you accomplish this. Unfortunately, with all the changes made
> >> to capabilities for namespaces and all I'm not 100% sure I
> >> could say exactly how to set that.
> >>
> >>> The problem here is that if the network manager itself is
> >>> untrusted and exploitable, then giving it unrestricted CAP_SETUID is a
> >>> big security risk.
> >> Right. That's why you set the file capabilities on OpenVPN.
> > So it seems like you're suggesting that any time a program needs to
> > switch user by calling setuid,
> 
> ... in a way that requires CAP_SETUID ...
> 
> > that it should get full CAP_SETUID
> > capabilities (whether that's through setting file capabilities on the
> > binary or inheriting CAP_SETUID from a parent process or otherwise).
> 
> Yup. That's correct. With all the duties and responsibilities associated
> with the dangers of UID management. Changing UIDs shouldn't be done
> lightly and needs to be done carefully.
> 
> > But that brings us back to the basic problem this LSM is trying to
> > solve. Namely, we don't want to sprinkle unrestricted CAP_SETUID privs
> > all over the system for binaries that just want to switch to specific
> > uid[s] and don't need any of the root-equivalent privileges provided
> > by CAP_SETUID.
> 
> I would see marking a program with a list of UIDs it can run with or
> that its children can run with as a better solution. You get much
> better locality of reference that way.
> 
> >>> Even just sticking with the network manager / VPN
> >>> example, strongSwan VPN also uses the same drop-to-user-through-setuid
> >>> setup, as do other Linux applications.
> >> Same solution.
> >>
> >>> Refactoring these applications
> >>> to fork/exec setuid binaries instead of simply calling setuid is often
> >>> infeasible. So a direct call to setuid is often necessary/expected,
> >>> and setuid bits don't help here.
> >> What is it with kids these days, that they are so
> >> afraid of fixing code that needs fixing? But that's
> >> not necessary in this example.
> >>
> >>> Also, use of setuid bits precludes the use of the no_new_privs bit,
> >>> which is usually at least a nice-to-have (if not need-to-have) for
> >>> sandboxed processes on the system.
> >> But you've already said that you *want* to change the security state,
> >> "drop to a lesser-privileged uid", so you're already mucking with the
> >> sandbox. If you're going to say that changing UIDs doesn't count for
> >> sandboxing I'll point out that you brought up the notion of a
> >> lesser-privileged UID.
> > There are plenty of ways that non-root processes further restrict
> > especially vulnerable parts of their code to even lesser-privileged
> > contexts. But its often easier to reason about the security of such
> > applications if the no_new_privs bit is set and file capabilities are
> > avoided, so the application can have full control of which privileges
> > are given to spawned processes without having to worry about which
> > privileges are attached to which files. Granted, the no_new_privs
> > issue is less central to the LSM being proposed here compared to the
> > discussion above.
> 
> Let me suggest a change to the way your LSM works
> that would reduce my concerns. Rather than refusing to
> make a UID change that isn't on your whitelist, kill a
> process that makes a prohibited request. This mitigates
> the problem where a process doesn't check for an error
> return. Sure, your system will be harder to get running
> until your whitelist is complete, but you'll avoid a
> whole category of security bugs.

Might also consider not restricting CAP_SETUID, but instead adding a
new CAP_SETUID_RANGE capability.  That way you can be sure there will be
no regressions with any programs which run with CAP_SETUID.

Though that violates what Casey was just arguing halfway up the email.

-serge

  reply index

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-31 15:28 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
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 [this message]
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 publically 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=20181102183056.GA20738@mail.hallyn.com \
    --to=serge@hallyn.com \
    --cc=casey@schaufler-ca.com \
    --cc=jmorris@namei.org \
    --cc=keescook@chromium.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mortonm@chromium.org \
    /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

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org linux-security-module@archiver.kernel.org
	public-inbox-index linux-security-module


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/ public-inbox