[10/11] selinux: Implement the watch_key security hook [ver #6]
diff mbox series

Message ID 156710348066.10009.17986469867635955040.stgit@warthog.procyon.org.uk
State New
Headers show
Series
  • Keyrings, Block and USB notifications [ver #6]
Related show

Commit Message

David Howells Aug. 29, 2019, 6:31 p.m. UTC
Implement the watch_key security hook to make sure that a key grants the
caller View permission in order to set a watch on a key.

For the moment, the watch_devices security hook is left unimplemented as
it's not obvious what the object should be since the queue is global and
didn't previously exist.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 security/selinux/hooks.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Stephen Smalley Aug. 29, 2019, 6:44 p.m. UTC | #1
On 8/29/19 2:31 PM, David Howells wrote:
> Implement the watch_key security hook to make sure that a key grants the
> caller View permission in order to set a watch on a key.
> 
> For the moment, the watch_devices security hook is left unimplemented as
> it's not obvious what the object should be since the queue is global and
> didn't previously exist.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
> 
>   security/selinux/hooks.c |   17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 74dd46de01b6..371f2ebc879b 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6533,6 +6533,20 @@ static int selinux_key_getsecurity(struct key *key, char **_buffer)
>   	*_buffer = context;
>   	return rc;
>   }
> +
> +#ifdef CONFIG_KEY_NOTIFICATIONS
> +static int selinux_watch_key(struct watch *watch, struct key *key)
> +{
> +	struct key_security_struct *ksec;
> +	u32 sid;
> +
> +	sid = cred_sid(watch->cred);

Can watch->cred ever differ from current's cred here?  If not, why can't 
we just use current_sid() here and why do we need the watch object at all?

> +	ksec = key->security;
> +
> +	return avc_has_perm(&selinux_state,
> +			    sid, ksec->sid, SECCLASS_KEY, KEY_NEED_VIEW, NULL);
> +}
> +#endif
>   #endif
>   
>   #ifdef CONFIG_SECURITY_INFINIBAND
> @@ -6965,6 +6979,9 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>   	LSM_HOOK_INIT(key_free, selinux_key_free),
>   	LSM_HOOK_INIT(key_permission, selinux_key_permission),
>   	LSM_HOOK_INIT(key_getsecurity, selinux_key_getsecurity),
> +#ifdef CONFIG_KEY_NOTIFICATIONS
> +	LSM_HOOK_INIT(watch_key, selinux_watch_key),
> +#endif
>   #endif
>   
>   #ifdef CONFIG_AUDIT
>
David Howells Aug. 29, 2019, 7:11 p.m. UTC | #2
Stephen Smalley <sds@tycho.nsa.gov> wrote:

> Can watch->cred ever differ from current's cred here?  If not, why can't we
> just use current_sid() here

Um.  Not currently.  I'm not sure whether its ever likely to be otherwise.
Probably we could just use that and fix it up later if we do find otherwise.

> and why do we need the watch object at all?

It carries more than just the creds for the caller of keyctl_watch_key(), it
also carries information about the queue to which notifications will be
written, including the creds that were active when that was set up.

Note that there's no requirement that the process that opened /dev/watch_queue
be the one that sets the watch.  In the keyutils testsuite, I 'leak' a file
descriptor from the session wrangler into the program that it runs so that
tests running inside the test script can add watches to it.

David

Patch
diff mbox series

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 74dd46de01b6..371f2ebc879b 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6533,6 +6533,20 @@  static int selinux_key_getsecurity(struct key *key, char **_buffer)
 	*_buffer = context;
 	return rc;
 }
+
+#ifdef CONFIG_KEY_NOTIFICATIONS
+static int selinux_watch_key(struct watch *watch, struct key *key)
+{
+	struct key_security_struct *ksec;
+	u32 sid;
+
+	sid = cred_sid(watch->cred);
+	ksec = key->security;
+
+	return avc_has_perm(&selinux_state,
+			    sid, ksec->sid, SECCLASS_KEY, KEY_NEED_VIEW, NULL);
+}
+#endif
 #endif
 
 #ifdef CONFIG_SECURITY_INFINIBAND
@@ -6965,6 +6979,9 @@  static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(key_free, selinux_key_free),
 	LSM_HOOK_INIT(key_permission, selinux_key_permission),
 	LSM_HOOK_INIT(key_getsecurity, selinux_key_getsecurity),
+#ifdef CONFIG_KEY_NOTIFICATIONS
+	LSM_HOOK_INIT(watch_key, selinux_watch_key),
+#endif
 #endif
 
 #ifdef CONFIG_AUDIT