Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: David Howells <dhowells@redhat.com>
To: Stephen Smalley <stephen.smalley.work@gmail.com>
Cc: dhowells@redhat.com,
	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 00:13:31 +0100
Message-ID: <3611507.1589411611@warthog.procyon.org.uk> (raw)
In-Reply-To: <CAEjxPJ4=ZN_jKP2nX5mrMA3OxC8XLsYEmCPCD-78H4XQw=_hCA@mail.gmail.com>

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

David


  parent 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 [this message]
2020-05-14 12:08           ` Stephen Smalley
2020-05-14 14:45             ` Stephen Smalley
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=3611507.1589411611@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=casey@schaufler-ca.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 \
    --cc=stephen.smalley.work@gmail.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

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