From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753964AbXLEUjs (ORCPT ); Wed, 5 Dec 2007 15:39:48 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751270AbXLEUjg (ORCPT ); Wed, 5 Dec 2007 15:39:36 -0500 Received: from mummy.ncsc.mil ([144.51.88.129]:46918 "EHLO mummy.ncsc.mil" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753227AbXLEUjf (ORCPT ); Wed, 5 Dec 2007 15:39:35 -0500 Subject: Re: [PATCH 4/7] KEYS: Add keyctl function to get a security label From: Stephen Smalley To: casey@schaufler-ca.com Cc: David Howells , viro@ftp.linux.org.uk, hch@infradead.org, Trond.Myklebust@netapp.com, linux-kernel@vger.kernel.org, selinux@tycho.nsa.gov, linux-security-module@vger.kernel.org In-Reply-To: <138670.64371.qm@web36609.mail.mud.yahoo.com> References: <138670.64371.qm@web36609.mail.mud.yahoo.com> Content-Type: text/plain Organization: National Security Agency Date: Wed, 05 Dec 2007 15:38:28 -0500 Message-Id: <1196887108.16006.121.camel@moss-spartans.epoch.ncsc.mil> Mime-Version: 1.0 X-Mailer: Evolution 2.10.3 (2.10.3-4.fc7) Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2007-12-05 at 12:12 -0800, Casey Schaufler wrote: > --- David Howells wrote: > > > Add a keyctl() function to get the security label of a key. > > > > The following is added to Documentation/keys.txt: > > > > (*) Get the LSM security context attached to a key. > > > > long keyctl(KEYCTL_GET_SECURITY, key_serial_t key, char *buffer, > > size_t buflen) > > > > This function returns a string that represents the LSM security context > > attached to a key in the buffer provided. > > > > Unless there's an error, it always returns the amount of data it could > > produce, even if that's too big for the buffer, but it won't copy more > > than requested to userspace. If the buffer pointer is NULL then no copy > > will take place. > > > > A NUL character is included at the end of the string if the buffer is > > sufficiently big. This is included in the returned count. If no LSM is > > in force then an empty string will be returned. > > > > A process must have view permission on the key for this function to be > > successful. > > > > Signed-off-by: David Howells > > --- > > > > Documentation/keys.txt | 21 +++++++++++++++ > > include/linux/keyctl.h | 1 + > > include/linux/security.h | 20 +++++++++++++- > > security/dummy.c | 8 ++++++ > > security/keys/compat.c | 3 ++ > > security/keys/keyctl.c | 66 > > ++++++++++++++++++++++++++++++++++++++++++++++ > > security/security.c | 5 +++ > > security/selinux/hooks.c | 21 +++++++++++++-- > > 8 files changed, 141 insertions(+), 4 deletions(-) > > > > diff --git a/Documentation/keys.txt b/Documentation/keys.txt > > index b82d38d..be424b0 100644 > > --- a/Documentation/keys.txt > > +++ b/Documentation/keys.txt > > @@ -711,6 +711,27 @@ The keyctl syscall functions are: > > The assumed authoritative key is inherited across fork and exec. > > > > > > + (*) Get the LSM security context attached to a key. > > + > > + long keyctl(KEYCTL_GET_SECURITY, key_serial_t key, char *buffer, > > + size_t buflen) > > + > > + This function returns a string that represents the LSM security context > > + attached to a key in the buffer provided. > > + > > + Unless there's an error, it always returns the amount of data it could > > + produce, even if that's too big for the buffer, but it won't copy more > > + than requested to userspace. If the buffer pointer is NULL then no copy > > + will take place. > > + > > + A NUL character is included at the end of the string if the buffer is > > + sufficiently big. This is included in the returned count. If no LSM > > is > > + in force then an empty string will be returned. > > + > > + A process must have view permission on the key for this function to be > > + successful. > > + > > + > > =============== > > KERNEL SERVICES > > =============== > > diff --git a/include/linux/keyctl.h b/include/linux/keyctl.h > > index 3365945..656ee6b 100644 > > --- a/include/linux/keyctl.h > > +++ b/include/linux/keyctl.h > > @@ -49,5 +49,6 @@ > > #define KEYCTL_SET_REQKEY_KEYRING 14 /* set default request-key keyring */ > > #define KEYCTL_SET_TIMEOUT 15 /* set key timeout */ > > #define KEYCTL_ASSUME_AUTHORITY 16 /* assume request_key() authorisation */ > > +#define KEYCTL_GET_SECURITY 17 /* get key security label */ > > > > #endif /* _LINUX_KEYCTL_H */ > > diff --git a/include/linux/security.h b/include/linux/security.h > > index ac05083..8d9e946 100644 > > --- a/include/linux/security.h > > +++ b/include/linux/security.h > > @@ -959,6 +959,17 @@ struct request_sock; > > * @perm describes the combination of permissions required of this key. > > * Return 1 if permission granted, 0 if permission denied and -ve it the > > * normal permissions model should be effected. > > + * @key_getsecurity: > > If I contradict what I said last time around, well, I've seen > more of what's going on since then. (insert smiley here) > > I think that this should be key_getsecctx(). That would get you > what you need from SELinux and Smack. Unfortunately, there is no > precedent for interfaces that return the secctx, while the are > places in networking and audit (audit needs to be converted off > of SELinux calls to LSM calls) that call getsecid() interfaces > and later use security_secid_to_secctx() to get the "printable" > version. So, key_getsecid() is the interface to introduce if you > want to be consistant, and pass the result to > security_secid_to_secctx() to get the data you're looking for. inode_getsecurity and getprocattr directly return the strings. Admittedly, the whole interface could be cleaned up and made far more consistent, but I don't think he necessarily has to go through the getsecid + secid_to_secctx sequence if he only wants the secctx. > > + * Get a textual representation of the security context attached to a key > > + * for the purposes of honouring KEYCTL_GETSECURITY. This function > > + * allocates the storage for the NUL-terminated string and the caller > > + * should free it. > > + * @key points to the key to be queried. > > + * @_buffer points to a pointer that should be set to point to the > > + * resulting string (if no label or an error occurs). > > + * Return the length of the string (including terminating NUL) or -ve if > > + * an error. > > + * May also return 0 (and a NULL buffer pointer) if there is no label. > > * > > * Security hooks affecting all System V IPC operations. > > * > > @@ -1437,7 +1448,7 @@ struct security_operations { > > int (*key_permission)(key_ref_t key_ref, > > struct task_struct *context, > > key_perm_t perm); > > - > > + int (*key_getsecurity)(struct key *key, char **_buffer); > > #endif /* CONFIG_KEYS */ > > > > }; > > @@ -2567,6 +2578,7 @@ int security_key_alloc(struct key *key, struct > > task_struct *tsk, unsigned long f > > void security_key_free(struct key *key); > > int security_key_permission(key_ref_t key_ref, > > struct task_struct *context, key_perm_t perm); > > +int security_key_getsecurity(struct key *key, char **_buffer); > > > > #else > > > > @@ -2588,6 +2600,12 @@ static inline int security_key_permission(key_ref_t > > key_ref, > > return 0; > > } > > > > +static inline int security_key_getsecurity(struct key *key, char **_buffer) > > +{ > > + *_buffer = NULL; > > + return 0; > > +} > > + > > #endif > > #endif /* CONFIG_KEYS */ > > > > diff --git a/security/dummy.c b/security/dummy.c > > index 6d895ad..7993b30 100644 > > --- a/security/dummy.c > > +++ b/security/dummy.c > > @@ -949,6 +949,13 @@ static inline int dummy_key_permission(key_ref_t > > key_ref, > > { > > return 0; > > } > > + > > +static int dummy_key_getsecurity(struct key *key, char **_buffer) > > +{ > > + *_buffer = NULL; > > + return 0; > > +} > > + > > #endif /* CONFIG_KEYS */ > > > > struct security_operations dummy_security_ops; > > @@ -1133,6 +1140,7 @@ void security_fixup_ops (struct security_operations > > *ops) > > set_to_dummy_if_null(ops, key_alloc); > > set_to_dummy_if_null(ops, key_free); > > set_to_dummy_if_null(ops, key_permission); > > + set_to_dummy_if_null(ops, key_getsecurity); > > #endif /* CONFIG_KEYS */ > > > > } > > diff --git a/security/keys/compat.c b/security/keys/compat.c > > index e10ec99..c766c68 100644 > > --- a/security/keys/compat.c > > +++ b/security/keys/compat.c > > @@ -79,6 +79,9 @@ asmlinkage long compat_sys_keyctl(u32 option, > > case KEYCTL_ASSUME_AUTHORITY: > > return keyctl_assume_authority(arg2); > > > > + case KEYCTL_GET_SECURITY: > > + return keyctl_get_security(arg2, compat_ptr(arg3), arg4); > > + > > default: > > return -EOPNOTSUPP; > > } > > diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c > > index 1698bf9..56e963b 100644 > > --- a/security/keys/keyctl.c > > +++ b/security/keys/keyctl.c > > @@ -20,6 +20,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include "internal.h" > > > > @@ -1080,6 +1081,66 @@ error: > > > > } /* end keyctl_assume_authority() */ > > > > +/* > > + * get the security label of a key > > + * - the key must grant us view permission > > + * - if there's a buffer, we place up to buflen bytes of data into it > > + * - unless there's an error, we return the amount of information available, > > + * irrespective of how much we may have copied (including the terminal > > NUL) > > + * - implements keyctl(KEYCTL_GET_SECURITY) > > + */ > > +long keyctl_get_security(key_serial_t keyid, > > + char __user *buffer, > > + size_t buflen) > > +{ > > + struct key *key, *instkey; > > + key_ref_t key_ref; > > + char *context; > > + long ret; > > + > > + key_ref = lookup_user_key(NULL, keyid, 0, 1, KEY_VIEW); > > + if (IS_ERR(key_ref)) { > > + if (PTR_ERR(key_ref) != -EACCES) > > + return PTR_ERR(key_ref); > > + > > + /* viewing a key under construction is also permitted if we > > + * have the authorisation token handy */ > > + instkey = key_get_instantiation_authkey(keyid); > > + if (IS_ERR(instkey)) > > + return PTR_ERR(key_ref); > > + key_put(instkey); > > + > > + key_ref = lookup_user_key(NULL, keyid, 0, 1, 0); > > + if (IS_ERR(key_ref)) > > + return PTR_ERR(key_ref); > > + } > > + > > + key = key_ref_to_ptr(key_ref); > > + ret = security_key_getsecurity(key, &context); > > + if (ret == 0) { > > + /* if no information was returned, give userspace an empty > > + * string */ > > + ret = 1; > > + if (buffer && buflen > 0 && > > + copy_to_user(buffer, "", 1) != 0) > > + ret = -EFAULT; > > + } else if (ret > 0) { > > + /* return as much data as there's room for */ > > + if (buffer && buflen > 0) { > > + if (buflen > ret) > > + buflen = ret; > > + > > + if (copy_to_user(buffer, context, buflen) != 0) > > + ret = -EFAULT; > > + } > > + > > + kfree(context); > > + } > > + > > + key_ref_put(key_ref); > > + return ret; > > +} > > + > > > > > /*****************************************************************************/ > > /* > > * the key control system call > > @@ -1160,6 +1221,11 @@ asmlinkage long sys_keyctl(int option, unsigned long > > arg2, unsigned long arg3, > > case KEYCTL_ASSUME_AUTHORITY: > > return keyctl_assume_authority((key_serial_t) arg2); > > > > + case KEYCTL_GET_SECURITY: > > + return keyctl_get_security((key_serial_t) arg2, > > + (char *) arg3, > > + (size_t) arg4); > > + > > default: > > return -EOPNOTSUPP; > > } > > diff --git a/security/security.c b/security/security.c > > index 0e1f1f1..16213e3 100644 > > --- a/security/security.c > > +++ b/security/security.c > > @@ -1079,4 +1079,9 @@ int security_key_permission(key_ref_t key_ref, > > return security_ops->key_permission(key_ref, context, perm); > > } > > > > +int security_key_getsecurity(struct key *key, char **_buffer) > > +{ > > + return security_ops->key_getsecurity(key, _buffer); > > +} > > + > > #endif /* CONFIG_KEYS */ > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > > index 9f3124b..bd4cfab 100644 > > --- a/security/selinux/hooks.c > > +++ b/security/selinux/hooks.c > > @@ -4768,6 +4768,20 @@ static int selinux_key_permission(key_ref_t key_ref, > > SECCLASS_KEY, perm, NULL); > > } > > > > +static int selinux_key_getsecurity(struct key *key, char **_buffer) > > +{ > > + struct key_security_struct *ksec = key->security; > > + char *context = NULL; > > + unsigned len; > > + int rc; > > + > > + rc = security_sid_to_context(ksec->sid, &context, &len); > > + if (!rc) > > + rc = len; > > + *_buffer = context; > > + return rc; > > +} > > + > > #endif > > > > static struct security_operations selinux_ops = { > > @@ -4943,9 +4957,10 @@ static struct security_operations selinux_ops = { > > #endif > > > > #ifdef CONFIG_KEYS > > - .key_alloc = selinux_key_alloc, > > - .key_free = selinux_key_free, > > - .key_permission = selinux_key_permission, > > + .key_alloc = selinux_key_alloc, > > + .key_free = selinux_key_free, > > + .key_permission = selinux_key_permission, > > + .key_getsecurity = selinux_key_getsecurity, > > #endif > > }; > > > > > > > > > > > Casey Schaufler > casey@schaufler-ca.com > > -- > This message was distributed to subscribers of the selinux mailing list. > If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with > the words "unsubscribe selinux" without quotes as the message. -- Stephen Smalley National Security Agency