From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from e23smtp08.au.ibm.com ([202.81.31.141]:41132 "EHLO e23smtp08.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934613AbcATSRw (ORCPT ); Wed, 20 Jan 2016 13:17:52 -0500 Received: from localhost by e23smtp08.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 21 Jan 2016 04:17:49 +1000 Message-ID: <1453313747.4396.5.camel@linux.vnet.ibm.com> Subject: Re: Patch "KEYS: prevent keys from being removed from specified keyrings" has been added to the 3.10-stable tree From: Mimi Zohar To: gregkh@linuxfoundation.org Cc: dhowells@redhat.com, stable@vger.kernel.org, stable-commits@vger.kernel.org Date: Wed, 20 Jan 2016 13:15:47 -0500 In-Reply-To: <145330913814483@kroah.com> References: <145330913814483@kroah.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org List-ID: Hi Greg, The concept of not being able to remove a key from a keyring was introduced to prevent keys from being removed from the blacklist keyring. The blacklist keyring was just upstreamed in the current open window. I don't see a need to backport either this patch or the "KEYS: refcount bug fix" patch. Mimi On Wed, 2016-01-20 at 08:58 -0800, gregkh@linuxfoundation.org wrote: > This is a note to let you know that I've just added the patch titled > > KEYS: prevent keys from being removed from specified keyrings > > to the 3.10-stable tree which can be found at: > http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary > > The filename of the patch is: > keys-prevent-keys-from-being-removed-from-specified-keyrings.patch > and it can be found in the queue-3.10 subdirectory. > > If you, or anyone else, feels it should not be added to the stable tree, > please let know about it. > > > From d3600bcf9d64d88dc1d189a754dcfab960ce751f Mon Sep 17 00:00:00 2001 > From: Mimi Zohar > Date: Tue, 10 Nov 2015 08:34:46 -0500 > Subject: KEYS: prevent keys from being removed from specified keyrings > > From: Mimi Zohar > > commit d3600bcf9d64d88dc1d189a754dcfab960ce751f upstream. > > Userspace should not be allowed to remove keys from certain keyrings > (eg. blacklist), though the keys themselves can expire. > > This patch defines a new key flag named KEY_FLAG_KEEP to prevent > userspace from being able to unlink, revoke, invalidate or timed > out a key on a keyring. When this flag is set on the keyring, all > keys subsequently added are flagged. > > In addition, when this flag is set, the keyring itself can not be > cleared. > > Signed-off-by: Mimi Zohar > Cc: David Howells > Signed-off-by: Greg Kroah-Hartman > > --- > include/linux/key.h | 1 > security/keys/key.c | 6 ++++- > security/keys/keyctl.c | 56 ++++++++++++++++++++++++++++++++++++++++--------- > 3 files changed, 52 insertions(+), 11 deletions(-) > > --- a/include/linux/key.h > +++ b/include/linux/key.h > @@ -162,6 +162,7 @@ struct key { > #define KEY_FLAG_NEGATIVE 5 /* set if key is negative */ > #define KEY_FLAG_ROOT_CAN_CLEAR 6 /* set if key can be cleared by root without permission */ > #define KEY_FLAG_INVALIDATED 7 /* set if key has been invalidated */ > +#define KEY_FLAG_KEEP 12 /* set if key should not be removed */ > > /* the description string > * - this is used to match a key against search criteria > --- a/security/keys/key.c > +++ b/security/keys/key.c > @@ -434,8 +434,12 @@ static int __key_instantiate_and_link(st > awaken = 1; > > /* and link it into the destination keyring */ > - if (keyring) > + if (keyring) { > + if (test_bit(KEY_FLAG_KEEP, &keyring->flags)) > + set_bit(KEY_FLAG_KEEP, &key->flags); > + > __key_link(keyring, key, _prealloc); > + } > > /* disable the authorisation key */ > if (authkey) > --- a/security/keys/keyctl.c > +++ b/security/keys/keyctl.c > @@ -358,11 +358,14 @@ error: > * and any links to the key will be automatically garbage collected after a > * certain amount of time (/proc/sys/kernel/keys/gc_delay). > * > + * Keys with KEY_FLAG_KEEP set should not be revoked. > + * > * If successful, 0 is returned. > */ > long keyctl_revoke_key(key_serial_t id) > { > key_ref_t key_ref; > + struct key *key; > long ret; > > key_ref = lookup_user_key(id, 0, KEY_WRITE); > @@ -377,8 +380,13 @@ long keyctl_revoke_key(key_serial_t id) > } > } > > - key_revoke(key_ref_to_ptr(key_ref)); > - ret = 0; > + key = key_ref_to_ptr(key_ref); > + if (test_bit(KEY_FLAG_KEEP, &key->flags)) > + return -EPERM; > + else { > + key_revoke(key); > + ret = 0; > + } > > key_ref_put(key_ref); > error: > @@ -392,11 +400,14 @@ error: > * The key and any links to the key will be automatically garbage collected > * immediately. > * > + * Keys with KEY_FLAG_KEEP set should not be invalidated. > + * > * If successful, 0 is returned. > */ > long keyctl_invalidate_key(key_serial_t id) > { > key_ref_t key_ref; > + struct key *key; > long ret; > > kenter("%d", id); > @@ -407,8 +418,13 @@ long keyctl_invalidate_key(key_serial_t > goto error; > } > > - key_invalidate(key_ref_to_ptr(key_ref)); > - ret = 0; > + key = key_ref_to_ptr(key_ref); > + if (test_bit(KEY_FLAG_KEEP, &key->flags)) > + ret = -EPERM; > + else { > + key_invalidate(key); > + ret = 0; > + } > > key_ref_put(key_ref); > error: > @@ -420,12 +436,13 @@ error: > * Clear the specified keyring, creating an empty process keyring if one of the > * special keyring IDs is used. > * > - * The keyring must grant the caller Write permission for this to work. If > - * successful, 0 will be returned. > + * The keyring must grant the caller Write permission and not have > + * KEY_FLAG_KEEP set for this to work. If successful, 0 will be returned. > */ > long keyctl_keyring_clear(key_serial_t ringid) > { > key_ref_t keyring_ref; > + struct key *keyring; > long ret; > > keyring_ref = lookup_user_key(ringid, KEY_LOOKUP_CREATE, KEY_WRITE); > @@ -447,7 +464,11 @@ long keyctl_keyring_clear(key_serial_t r > } > > clear: > - ret = keyring_clear(key_ref_to_ptr(keyring_ref)); > + keyring = key_ref_to_ptr(keyring_ref); > + if (test_bit(KEY_FLAG_KEEP, &keyring->flags)) > + ret = -EPERM; > + else > + ret = keyring_clear(keyring); > error_put: > key_ref_put(keyring_ref); > error: > @@ -498,11 +519,14 @@ error: > * itself need not grant the caller anything. If the last link to a key is > * removed then that key will be scheduled for destruction. > * > + * Keys or keyrings with KEY_FLAG_KEEP set should not be unlinked. > + * > * If successful, 0 will be returned. > */ > long keyctl_keyring_unlink(key_serial_t id, key_serial_t ringid) > { > key_ref_t keyring_ref, key_ref; > + struct key *keyring, *key; > long ret; > > keyring_ref = lookup_user_key(ringid, 0, KEY_WRITE); > @@ -517,7 +541,13 @@ long keyctl_keyring_unlink(key_serial_t > goto error2; > } > > - ret = key_unlink(key_ref_to_ptr(keyring_ref), key_ref_to_ptr(key_ref)); > + keyring = key_ref_to_ptr(keyring_ref); > + key = key_ref_to_ptr(key_ref); > + if (test_bit(KEY_FLAG_KEEP, &keyring->flags) && > + test_bit(KEY_FLAG_KEEP, &key->flags)) > + ret = -EPERM; > + else > + ret = key_unlink(keyring, key); > > key_ref_put(key_ref); > error2: > @@ -1306,6 +1336,8 @@ error: > * the current time. The key and any links to the key will be automatically > * garbage collected after the timeout expires. > * > + * Keys with KEY_FLAG_KEEP set should not be timed out. > + * > * If successful, 0 is returned. > */ > long keyctl_set_timeout(key_serial_t id, unsigned timeout) > @@ -1337,10 +1369,14 @@ long keyctl_set_timeout(key_serial_t id, > > okay: > key = key_ref_to_ptr(key_ref); > - key_set_timeout(key, timeout); > + if (test_bit(KEY_FLAG_KEEP, &key->flags)) > + ret = -EPERM; > + else { > + key_set_timeout(key, timeout); > + ret = 0; > + } > key_put(key); > > - ret = 0; > error: > return ret; > } > > > Patches currently in stable-queue which might be from zohar@linux.vnet.ibm.com are > > queue-3.10/keys-refcount-bug-fix.patch > queue-3.10/keys-prevent-keys-from-being-removed-from-specified-keyrings.patch >