* Patch "KEYS: prevent keys from being removed from specified keyrings" has been added to the 3.10-stable tree
@ 2016-01-20 16:58 gregkh
2016-01-20 18:15 ` Mimi Zohar
0 siblings, 1 reply; 4+ messages in thread
From: gregkh @ 2016-01-20 16:58 UTC (permalink / raw)
To: zohar, dhowells, gregkh; +Cc: stable, stable-commits
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 <stable@vger.kernel.org> know about it.
>From d3600bcf9d64d88dc1d189a754dcfab960ce751f Mon Sep 17 00:00:00 2001
From: Mimi Zohar <zohar@linux.vnet.ibm.com>
Date: Tue, 10 Nov 2015 08:34:46 -0500
Subject: KEYS: prevent keys from being removed from specified keyrings
From: Mimi Zohar <zohar@linux.vnet.ibm.com>
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 <zohar@linux.vnet.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Patch "KEYS: prevent keys from being removed from specified keyrings" has been added to the 3.10-stable tree
2016-01-20 16:58 Patch "KEYS: prevent keys from being removed from specified keyrings" has been added to the 3.10-stable tree gregkh
@ 2016-01-20 18:15 ` Mimi Zohar
2016-01-20 18:36 ` Greg KH
0 siblings, 1 reply; 4+ messages in thread
From: Mimi Zohar @ 2016-01-20 18:15 UTC (permalink / raw)
To: gregkh; +Cc: dhowells, stable, stable-commits
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 <stable@vger.kernel.org> know about it.
>
>
> From d3600bcf9d64d88dc1d189a754dcfab960ce751f Mon Sep 17 00:00:00 2001
> From: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Date: Tue, 10 Nov 2015 08:34:46 -0500
> Subject: KEYS: prevent keys from being removed from specified keyrings
>
> From: Mimi Zohar <zohar@linux.vnet.ibm.com>
>
> 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 <zohar@linux.vnet.ibm.com>
> Cc: David Howells <dhowells@redhat.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> ---
> 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Patch "KEYS: prevent keys from being removed from specified keyrings" has been added to the 3.10-stable tree
2016-01-20 18:15 ` Mimi Zohar
@ 2016-01-20 18:36 ` Greg KH
2016-01-20 19:23 ` Mimi Zohar
0 siblings, 1 reply; 4+ messages in thread
From: Greg KH @ 2016-01-20 18:36 UTC (permalink / raw)
To: Mimi Zohar; +Cc: dhowells, stable, stable-commits
On Wed, Jan 20, 2016 at 01:15:47PM -0500, Mimi Zohar wrote:
> 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.
Ah, ok, remove this for all stable kernel trees, right? For some reason
I thought this was resolving a different key security "issue" that was
recently reported...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Patch "KEYS: prevent keys from being removed from specified keyrings" has been added to the 3.10-stable tree
2016-01-20 18:36 ` Greg KH
@ 2016-01-20 19:23 ` Mimi Zohar
0 siblings, 0 replies; 4+ messages in thread
From: Mimi Zohar @ 2016-01-20 19:23 UTC (permalink / raw)
To: Greg KH; +Cc: dhowells, stable, stable-commits
On Wed, 2016-01-20 at 10:36 -0800, Greg KH wrote:
> On Wed, Jan 20, 2016 at 01:15:47PM -0500, Mimi Zohar wrote:
> > 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.
>
> Ah, ok, remove this for all stable kernel trees, right? For some reason
> I thought this was resolving a different key security "issue" that was
> recently reported...
Yes, thanks!
Mimi
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-01-20 19:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-20 16:58 Patch "KEYS: prevent keys from being removed from specified keyrings" has been added to the 3.10-stable tree gregkh
2016-01-20 18:15 ` Mimi Zohar
2016-01-20 18:36 ` Greg KH
2016-01-20 19:23 ` Mimi Zohar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).