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: Move permissions checking decisions into the checking code
Date: Fri, 15 May 2020 11:06:19 -0400
Message-ID: <CAEjxPJ5wW2qHYDsqKr5rjnRJ++4f2LXobCQkKZvWCSb_j0WN6w@mail.gmail.com> (raw)
In-Reply-To: <3999877.1589475539@warthog.procyon.org.uk>

On Thu, May 14, 2020 at 12:59 PM David Howells <dhowells@redhat.com> wrote:
>
> How about this then?
>
> David
> ---
> commit fa37b6c7e2f86d16ede1e0e3cb73857152d51825
> Author: David Howells <dhowells@redhat.com>
> Date:   Thu May 14 17:48:55 2020 +0100
>
>     keys: Move permissions checking decisions into the checking code
>
>     Overhaul the permissions checking, moving the decisions of which permits to
>     request for what operation and what overrides to allow into the permissions
>     checking functions in keyrings, SELinux and Smack.
>
>     To this end, the KEY_NEED_* constants are turned into an enum and expanded
>     in number to cover operation types individually.
>
>     Note that some more tweaking is probably needed to separate kernel uses,
>     e.g. AFS using rxrpc keys, from direct userspace users.
>
>     Some overrides are available and this needs to be handled specially:
>
>      (1) KEY_FLAG_KEEP in key->flags - The key may not be deleted and/or things
>          may not be removed from the keyring.

Why can't they be deleted / removed?  They can't ever be deleted or
removed or for some period of time?

>      (2) KEY_FLAG_ROOT_CAN_CLEAR in key->flags - The keyring can be cleared by
>          CAP_SYS_ADMIN.

Why do some keyrings get this flag and others do not?  Under what
conditions?  Why is CAP_SYS_ADMIN the right capability for this?

>      (3) KEY_FLAG_ROOT_CAN_INVAL in key->flags - The key can be invalidated by
>          CAP_SYS_ADMIN.

Ditto.

>      (4) An appropriate auth token being set in cred->request_key_auth that
>          gives a process transient permission to view and instantiate a key.
>          This is used by the kernel to delegate instantiation to userspace.

Is this ever allowed across different credentials?  When?  Why?  Is
there a check between the different credentials before the auth token
is created?

>     Note that this requires some tweaks to the testsuite as some of the error
>     codes change.

Which testsuite?  keyring or selinux or both?  What error codes change
(from what to what)?  Does this constitute an ABI change?

I like moving more of the permission checking logic into the security
modules and giving them greater visibility and control.  That said, I
am somewhat concerned by the scale of this change, by the extent to
which you are exposing keyring internals inside the security modules,
and by the extent to which logic is getting duplicated in each
security module.  I'd suggest a more incremental approach, e.g. start
with just the enum patch, then migrate the easy cases, then consider
the more complicated cases.  And possibly we need multiple different
security hooks for the keyring subsystem that are more specialized for
the complicated cases.  If we authorize the delegation up front, we
don't need to check it later.

  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
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 [this message]
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=CAEjxPJ5wW2qHYDsqKr5rjnRJ++4f2LXobCQkKZvWCSb_j0WN6w@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