All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Smalley <stephen.smalley.work@gmail.com>
To: David Howells <dhowells@redhat.com>
Cc: Paul Moore <paul@paul-moore.com>,
	keyrings@vger.kernel.org, SElinux list <selinux@vger.kernel.org>,
	LSM List <linux-security-module@vger.kernel.org>
Subject: Re: [PATCH] selinux: Fix use of KEY_NEED_* instead of KEY__* perms [v2]
Date: Tue, 28 Apr 2020 16:19:02 +0000	[thread overview]
Message-ID: <CAEjxPJ6pFdDfm55pv9bT3CV5DTFF9VqzRmG_Xi5bKNxPaGuOLg@mail.gmail.com> (raw)
In-Reply-To: <1072935.1588089479@warthog.procyon.org.uk>

On Tue, Apr 28, 2020 at 11:58 AM David Howells <dhowells@redhat.com> wrote:
>
> Stephen Smalley <stephen.smalley.work@gmail.com> wrote:
>
> > 1) Are we guaranteed that the caller only ever passes a single
> > KEY_NEED_* perm at a time (i.e. hook is never called with a bitmask
> > of multiple permissions)?  Where is that guarantee enforced?
>
> Currently it's the case that only one perm is ever used at once.  I'm tempted
> to enforce this by switching the KEY_NEED_* to an enum rather than a bitmask.
>
> I'm not sure how I would actually define the meaning of two perms being OR'd
> together.  Either okay?  Both required?

Both required is the usual convention in functions like
inode_permission() or avc_has_perm().
But if you know that you'll never use combinations, we should just
prohibit it up front, e.g.
key_task_permission() or whatever can reject them before they reach
the hook call.  Then the
hook code doesn't have to revisit the issue.

>
> > 2) We had talked about adding a BUILD_BUG_ON() or other build-time
> > guard
>
> That doesn't help you trap unallowed perm combinations, though.

I think we want both.

>
> > to ensure that new KEY_NEED_* permissions
> > are not added without updating SELinux.  We already have similar
> > constructs for catching new capabilities (#if CAP_LAST_CAP > 63 #error
> > ...), socket address families (#if PF_MAX > 45 #error ...),  RTM_* and
> > XFRM_MSG* values.
>
> David
>

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Smalley <stephen.smalley.work@gmail.com>
To: David Howells <dhowells@redhat.com>
Cc: Paul Moore <paul@paul-moore.com>,
	keyrings@vger.kernel.org, SElinux list <selinux@vger.kernel.org>,
	LSM List <linux-security-module@vger.kernel.org>
Subject: Re: [PATCH] selinux: Fix use of KEY_NEED_* instead of KEY__* perms [v2]
Date: Tue, 28 Apr 2020 12:19:02 -0400	[thread overview]
Message-ID: <CAEjxPJ6pFdDfm55pv9bT3CV5DTFF9VqzRmG_Xi5bKNxPaGuOLg@mail.gmail.com> (raw)
In-Reply-To: <1072935.1588089479@warthog.procyon.org.uk>

On Tue, Apr 28, 2020 at 11:58 AM David Howells <dhowells@redhat.com> wrote:
>
> Stephen Smalley <stephen.smalley.work@gmail.com> wrote:
>
> > 1) Are we guaranteed that the caller only ever passes a single
> > KEY_NEED_* perm at a time (i.e. hook is never called with a bitmask
> > of multiple permissions)?  Where is that guarantee enforced?
>
> Currently it's the case that only one perm is ever used at once.  I'm tempted
> to enforce this by switching the KEY_NEED_* to an enum rather than a bitmask.
>
> I'm not sure how I would actually define the meaning of two perms being OR'd
> together.  Either okay?  Both required?

Both required is the usual convention in functions like
inode_permission() or avc_has_perm().
But if you know that you'll never use combinations, we should just
prohibit it up front, e.g.
key_task_permission() or whatever can reject them before they reach
the hook call.  Then the
hook code doesn't have to revisit the issue.

>
> > 2) We had talked about adding a BUILD_BUG_ON() or other build-time
> > guard
>
> That doesn't help you trap unallowed perm combinations, though.

I think we want both.

>
> > to ensure that new KEY_NEED_* permissions
> > are not added without updating SELinux.  We already have similar
> > constructs for catching new capabilities (#if CAP_LAST_CAP > 63 #error
> > ...), socket address families (#if PF_MAX > 45 #error ...),  RTM_* and
> > XFRM_MSG* values.
>
> David
>

  reply	other threads:[~2020-04-28 16:19 UTC|newest]

Thread overview: 70+ 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 15:48 ` Paul Moore
2020-04-17 16:32 ` Richard Haines
2020-04-17 16:32   ` Richard Haines
2020-04-17 16:59   ` Paul Moore
2020-04-17 16:59     ` Paul Moore
2020-04-21 12:29 ` David Howells
2020-04-21 12:29   ` David Howells
2020-04-22 19:20   ` Paul Moore
2020-04-22 19:20     ` Paul Moore
2020-04-22 21:09     ` Paul Moore
2020-04-22 21:09       ` Paul Moore
2020-04-24 23:43   ` David Howells
2020-04-24 23:43     ` David Howells
2020-04-26 20:53     ` Paul Moore
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:12       ` David Howells
2020-04-27 14:36       ` Stephen Smalley
2020-04-27 14:36         ` Stephen Smalley
2020-04-27 15:24         ` Paul Moore
2020-04-27 15:24           ` Paul Moore
2020-04-27 17:02       ` Stephen Smalley
2020-04-27 17:02         ` Stephen Smalley
2020-04-27 22:17         ` Paul Moore
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 12:54   ` David Howells
2020-04-28 14:32   ` Stephen Smalley
2020-04-28 14:32     ` Stephen Smalley
2020-04-28 15:57   ` David Howells
2020-04-28 15:57     ` David Howells
2020-04-28 16:19     ` Stephen Smalley [this message]
2020-04-28 16:19       ` Stephen Smalley
2020-05-01 16:37       ` Paul Moore
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-12 22:33         ` David Howells
2020-05-13  1:04         ` Paul Moore
2020-05-13  1:04           ` Paul Moore
2020-05-13 12:58         ` Stephen Smalley
2020-05-13 12:58           ` Stephen Smalley
2020-05-13 15:25         ` Casey Schaufler
2020-05-13 15:25           ` Casey Schaufler
2020-05-13 23:13         ` David Howells
2020-05-13 23:13           ` David Howells
2020-05-14 12:08           ` Stephen Smalley
2020-05-14 12:08             ` Stephen Smalley
2020-05-14 14:45             ` Stephen Smalley
2020-05-14 14:45               ` Stephen Smalley
2020-05-13 23:16         ` David Howells
2020-05-13 23:16           ` David Howells
2020-05-13 23:25         ` David Howells
2020-05-13 23:25           ` David Howells
2020-05-14 11:00         ` Jarkko Sakkinen
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 16:58           ` David Howells
2020-05-14 17:06           ` Casey Schaufler
2020-05-14 17:06             ` Casey Schaufler
2020-05-15 15:06           ` Stephen Smalley
2020-05-15 15:06             ` Stephen Smalley
2020-05-15 16:45           ` David Howells
2020-05-15 16:45             ` David Howells
2020-05-15 18:55             ` Stephen Smalley
2020-05-15 18:55               ` Stephen Smalley
2020-05-15 19:10               ` Casey Schaufler
2020-05-15 19:10                 ` Casey Schaufler
2020-05-15 22:27             ` David Howells
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=CAEjxPJ6pFdDfm55pv9bT3CV5DTFF9VqzRmG_Xi5bKNxPaGuOLg@mail.gmail.com \
    --to=stephen.smalley.work@gmail.com \
    --cc=dhowells@redhat.com \
    --cc=keyrings@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
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.