Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: Stephen Smalley <stephen.smalley.work@gmail.com>
To: David Howells <dhowells@redhat.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	Paul Moore <paul@paul-moore.com>,
	Casey Schaufler <casey@schaufler-ca.com>,
	keyrings@vger.kernel.org, SElinux list <selinux@vger.kernel.org>,
	LSM List <linux-security-module@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] keys: Make the KEY_NEED_* perms an enum rather than a mask
Date: Thu, 14 May 2020 10:45:40 -0400
Message-ID: <CAEjxPJ72Y+G5wJEzYT_Ekth07nHzTS5Oh+8ss7Xc90-_Qq6wWw@mail.gmail.com> (raw)
In-Reply-To: <CAEjxPJ5Dw0Lf6DnUL01XSJZF9zu-ACnmxOkC0VHxGTzGcmbpRQ@mail.gmail.com>

On Thu, May 14, 2020 at 8:08 AM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Wed, May 13, 2020 at 7:13 PM David Howells <dhowells@redhat.com> wrote:
> >
> > Stephen Smalley <stephen.smalley.work@gmail.com> wrote:
> >
> > > >  (3) An override due to CAP_SYS_ADMIN.
> > >
> > > CAP_SYS_ADMIN should never skip SELinux checking.  Even for Smack,
> > > there is a separate capability (CAP_MAC_ADMIN) for that purpose.
> >
> > The LSM doesn't get consulted at the moment.  With this patch, it will get
> > consulted.
> >
> > > >  (4) An override due to an instantiation token being present.
> > >
> > > Not sure what this means but again we shouldn't skip SELinux checking
> > > based on mere possession of an object capability (not a POSIX
> > > capability).
> >
> > The kernel has delegated the instantiation of a key to the calling process and
> > has given it a temporary key of type ".request_key_auth" which it has put into
> > force with keyctl(KEYCTL_ASSUME_AUTHORITY).
> >
> > This authorisation token grants the caller the ability to (a) perform
> > operations on the key it wouldn't otherwise have permission to do, (b) use the
> > key instantiation keyctls and (c) temporarily search the keyrings of the
> > caller of request_key() using the creds of that caller and to read/use the
> > keys found therein if the caller was permitted to do so.
> >
> > > It would be better if the permission indicated the actual operation
> > > (e.g. KEY_NEED_INVALIDATE_SPECIAL), and the decision whether to permit
> > > CAP_SYS_ADMIN processes to override was left to the security modules.
> > > SELinux doesn't automatically allow CAP_SYS_ADMIN processes to do
> > > everything.
> >
> > These individual permissions don't exist yet.  I have an ACL patchset that
> > allows me to add a greater range - though there's issues with SELinux there
> > also.
> >
> > Also, the keyrings are specially marked to say that the sysadmin is allowed to
> > flush them at the moment - but that can go away with the ACL stuff.
> >
> > > > +       switch (need_perm) {
> > > > +       case KEY_NEED_UNLINK:
> > > > +       case KEY_SYSADMIN_OVERRIDE:
> > > > +       case KEY_AUTHTOKEN_OVERRIDE:
> > > > +       case KEY_DEFER_PERM_CHECK:
> > > >                 return 0;
> > >
> > > We really shouldn't be skipping any/all checking on CAP_SYS_ADMIN or
> > > an AUTHTOKEN; those should still be subject to MAC policy.
> >
> > I'm not sure how to do that.
> >
> > Note that KEY_NEED_UNLINK *must not* be overruled by the MAC policy.  The
> > value is only there because lookup_user_key() requires something to be put
> > into that parameter - it's more of a courtesy thing, I suppose.
> >
> > Why should AUTHTOKEN be subject to MAC policy?  The kernel has told the
> > process to go and instantiate a key.  It shouldn't really then turn around and
> > tell the process "oh, but you're not actually allowed to do that".
>
> On what basis did the kernel authorize the process to instantiate the
> key?  At what point did a security module get involved in the decision
> as to what process(es) are authorize to instantiate a key,
> particularly for a process with a different credential/security
> context?

BTW I'm not saying you have to change this patch since IIUC it merely
preserves the existing behavior for these special cases.  But we will
want to address these gaps (or clarify that no real gap exists) in LSM
control over key operations going forward.

  reply index

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-17 15:48 Problem with 9ba09998baa9 ("selinux: Implement the watch_key security hook") in linux-next Paul Moore
2020-04-17 16:32 ` Richard Haines
2020-04-17 16:59   ` Paul Moore
2020-04-21 12:29 ` David Howells
2020-04-22 19:20   ` Paul Moore
2020-04-22 21:09     ` Paul Moore
2020-04-24 23:43   ` David Howells
2020-04-26 20:53     ` Paul Moore
2020-04-27 14:12     ` [PATCH] selinux: Fix use of KEY_NEED_* instead of KEY__* perms David Howells
2020-04-27 14:36       ` Stephen Smalley
2020-04-27 15:24         ` Paul Moore
2020-04-27 17:02       ` Stephen Smalley
2020-04-27 22:17         ` Paul Moore
2020-04-28 12:54 ` [PATCH] selinux: Fix use of KEY_NEED_* instead of KEY__* perms [v2] David Howells
2020-04-28 14:32   ` Stephen Smalley
2020-04-28 15:57   ` David Howells
2020-04-28 16:19     ` Stephen Smalley
2020-05-01 16:37       ` Paul Moore
2020-05-12 22:33       ` [PATCH] keys: Make the KEY_NEED_* perms an enum rather than a mask David Howells
2020-05-13  1:04         ` Paul Moore
2020-05-13 12:58         ` Stephen Smalley
2020-05-13 15:25         ` Casey Schaufler
2020-05-13 23:13         ` David Howells
2020-05-14 12:08           ` Stephen Smalley
2020-05-14 14:45             ` Stephen Smalley [this message]
2020-05-13 23:16         ` David Howells
2020-05-13 23:25         ` David Howells
2020-05-14 11:00         ` Jarkko Sakkinen
2020-05-14 16:58         ` [PATCH] keys: Move permissions checking decisions into the checking code David Howells
2020-05-14 17:06           ` Casey Schaufler
2020-05-15 15:06           ` Stephen Smalley
2020-05-15 16:45           ` David Howells
2020-05-15 18:55             ` Stephen Smalley
2020-05-15 19:10               ` Casey Schaufler
2020-05-15 22:27             ` David Howells

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=CAEjxPJ72Y+G5wJEzYT_Ekth07nHzTS5Oh+8ss7Xc90-_Qq6wWw@mail.gmail.com \
    --to=stephen.smalley.work@gmail.com \
    --cc=casey@schaufler-ca.com \
    --cc=dhowells@redhat.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=selinux@vger.kernel.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
	public-inbox-index linux-security-module

Example config snippet for mirrors

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.git