--- /dev/null
+From 237bbd29f7a049d310d907f4b2716a7feef9abf3 Mon Sep 17 00:00:00 2001
+From: Eric Biggers <ebiggers@google.com>
+Date: Mon, 18 Sep 2017 11:37:03 -0700
+Subject: KEYS: prevent creating a different user's keyrings
+
+From: Eric Biggers <ebiggers@google.com>
+
+commit 237bbd29f7a049d310d907f4b2716a7feef9abf3 upstream.
+
+It was possible for an unprivileged user to create the user and user
+session keyrings for another user. For example:
+
+ sudo -u '#3000' sh -c 'keyctl add keyring _uid.4000 "" @u
+ keyctl add keyring _uid_ses.4000 "" @u
+ sleep 15' &
+ sleep 1
+ sudo -u '#4000' keyctl describe @u
+ sudo -u '#4000' keyctl describe @us
+
+This is problematic because these "fake" keyrings won't have the right
+permissions. In particular, the user who created them first will own
+them and will have full access to them via the possessor permissions,
+which can be used to compromise the security of a user's keys:
+
+ -4: alswrv-----v------------ 3000 0 keyring: _uid.4000
+ -5: alswrv-----v------------ 3000 0 keyring: _uid_ses.4000
+
+Fix it by marking user and user session keyrings with a flag
+KEY_FLAG_UID_KEYRING. Then, when searching for a user or user session
+keyring by name, skip all keyrings that don't have the flag set.
+
+Fixes: 69664cf16af4 ("keys: don't generate user and user session keyrings unless they're accessed")
+Signed-off-by: Eric Biggers <ebiggers@google.com>
+Signed-off-by: David Howells <dhowells@redhat.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+
+---
+ include/linux/key.h | 2 ++
+ security/keys/internal.h | 2 +-
+ security/keys/key.c | 2 ++
+ security/keys/keyring.c | 23 ++++++++++++++---------
+ security/keys/process_keys.c | 8 ++++++--
+ 5 files changed, 25 insertions(+), 12 deletions(-)
+
+--- a/include/linux/key.h
++++ b/include/linux/key.h
+@@ -172,6 +172,7 @@ struct key {
+ #define KEY_FLAG_TRUSTED_ONLY 9 /* set if keyring only accepts links to trusted keys */
+ #define KEY_FLAG_BUILTIN 10 /* set if key is builtin */
+ #define KEY_FLAG_ROOT_CAN_INVAL 11 /* set if key can be invalidated by root without permission */
++#define KEY_FLAG_UID_KEYRING 12 /* set if key is a user or user session keyring */
+
+ /* the key type and key description string
+ * - the desc is used to match a key against search criteria
+@@ -223,6 +224,7 @@ extern struct key *key_alloc(struct key_
+ #define KEY_ALLOC_QUOTA_OVERRUN 0x0001 /* add to quota, permit even if overrun */
+ #define KEY_ALLOC_NOT_IN_QUOTA 0x0002 /* not in quota */
+ #define KEY_ALLOC_TRUSTED 0x0004 /* Key should be flagged as trusted */
++#define KEY_ALLOC_UID_KEYRING 0x0010 /* allocating a user or user session keyring */
+
+ extern void key_revoke(struct key *key);
+ extern void key_invalidate(struct key *key);
+--- a/security/keys/internal.h
++++ b/security/keys/internal.h
+@@ -136,7 +136,7 @@ extern key_ref_t keyring_search_aux(key_
+ extern key_ref_t search_my_process_keyrings(struct keyring_search_context *ctx);
+ extern key_ref_t search_process_keyrings(struct keyring_search_context *ctx);
+
+-extern struct key *find_keyring_by_name(const char *name, bool skip_perm_check);
++extern struct key *find_keyring_by_name(const char *name, bool uid_keyring);
+
+ extern int install_user_keyrings(void);
+ extern int install_thread_keyring_to_cred(struct cred *);
+--- a/security/keys/key.c
++++ b/security/keys/key.c
+@@ -298,6 +298,8 @@ struct key *key_alloc(struct key_type *t
+ key->flags |= 1 << KEY_FLAG_IN_QUOTA;
+ if (flags & KEY_ALLOC_TRUSTED)
+ key->flags |= 1 << KEY_FLAG_TRUSTED;
++ if (flags & KEY_ALLOC_UID_KEYRING)
++ key->flags |= 1 << KEY_FLAG_UID_KEYRING;
+
+ #ifdef KEY_DEBUGGING
+ key->magic = KEY_DEBUG_MAGIC;
+--- a/security/keys/keyring.c
++++ b/security/keys/keyring.c
+@@ -961,15 +961,15 @@ found:
+ /*
+ * Find a keyring with the specified name.
+ *
+- * All named keyrings in the current user namespace are searched, provided they
+- * grant Search permission directly to the caller (unless this check is
+- * skipped). Keyrings whose usage points have reached zero or who have been
+- * revoked are skipped.
++ * Only keyrings that have nonzero refcount, are not revoked, and are owned by a
++ * user in the current user namespace are considered. If @uid_keyring is %true,
++ * the keyring additionally must have been allocated as a user or user session
++ * keyring; otherwise, it must grant Search permission directly to the caller.
+ *
+ * Returns a pointer to the keyring with the keyring's refcount having being
+ * incremented on success. -ENOKEY is returned if a key could not be found.
+ */
+-struct key *find_keyring_by_name(const char *name, bool skip_perm_check)
++struct key *find_keyring_by_name(const char *name, bool uid_keyring)
+ {
+ struct key *keyring;
+ int bucket;
+@@ -997,10 +997,15 @@ struct key *find_keyring_by_name(const c
+ if (strcmp(keyring->description, name) != 0)
+ continue;
+
+- if (!skip_perm_check &&
+- key_permission(make_key_ref(keyring, 0),
+- KEY_NEED_SEARCH) < 0)
+- continue;
++ if (uid_keyring) {
++ if (!test_bit(KEY_FLAG_UID_KEYRING,
++ &keyring->flags))
++ continue;
++ } else {
++ if (key_permission(make_key_ref(keyring, 0),
++ KEY_NEED_SEARCH) < 0)
++ continue;
++ }
+
+ /* we've got a match but we might end up racing with
+ * key_cleanup() if the keyring is currently 'dead'
+--- a/security/keys/process_keys.c
++++ b/security/keys/process_keys.c
+@@ -76,7 +76,9 @@ int install_user_keyrings(void)
+ if (IS_ERR(uid_keyring)) {
+ uid_keyring = keyring_alloc(buf, user->uid, INVALID_GID,
+ cred, user_keyring_perm,
+- KEY_ALLOC_IN_QUOTA, NULL);
++ KEY_ALLOC_UID_KEYRING |
++ KEY_ALLOC_IN_QUOTA,
++ NULL);
+ if (IS_ERR(uid_keyring)) {
+ ret = PTR_ERR(uid_keyring);
+ goto error;
+@@ -92,7 +94,9 @@ int install_user_keyrings(void)
+ session_keyring =
+ keyring_alloc(buf, user->uid, INVALID_GID,
+ cred, user_keyring_perm,
+- KEY_ALLOC_IN_QUOTA, NULL);
++ KEY_ALLOC_UID_KEYRING |
++ KEY_ALLOC_IN_QUOTA,
++ NULL);
+ if (IS_ERR(session_keyring)) {
+ ret = PTR_ERR(session_keyring);
+ goto error_release;
--- /dev/null
+From 37863c43b2c6464f252862bf2e9768264e961678 Mon Sep 17 00:00:00 2001
+From: Eric Biggers <ebiggers@google.com>
+Date: Mon, 18 Sep 2017 11:37:23 -0700
+Subject: KEYS: prevent KEYCTL_READ on negative key
+
+From: Eric Biggers <ebiggers@google.com>
+
+commit 37863c43b2c6464f252862bf2e9768264e961678 upstream.
+
+Because keyctl_read_key() looks up the key with no permissions
+requested, it may find a negatively instantiated key. If the key is
+also possessed, we went ahead and called ->read() on the key. But the
+key payload will actually contain the ->reject_error rather than the
+normal payload. Thus, the kernel oopses trying to read the
+user_key_payload from memory address (int)-ENOKEY = 0x00000000ffffff82.
+
+Fortunately the payload data is stored inline, so it shouldn't be
+possible to abuse this as an arbitrary memory read primitive...
+
+Reproducer:
+ keyctl new_session
+ keyctl request2 user desc '' @s
+ keyctl read $(keyctl show | awk '/user: desc/ {print $1}')
+
+It causes a crash like the following:
+ BUG: unable to handle kernel paging request at 00000000ffffff92
+ IP: user_read+0x33/0xa0
+ PGD 36a54067 P4D 36a54067 PUD 0
+ Oops: 0000 [#1] SMP
+ CPU: 0 PID: 211 Comm: keyctl Not tainted 4.14.0-rc1 #337
+ Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-20170228_101828-anatol 04/01/2014
+ task: ffff90aa3b74c3c0 task.stack: ffff9878c0478000
+ RIP: 0010:user_read+0x33/0xa0
+ RSP: 0018:ffff9878c047bee8 EFLAGS: 00010246
+ RAX: 0000000000000001 RBX: ffff90aa3d7da340 RCX: 0000000000000017
+ RDX: 0000000000000000 RSI: 00000000ffffff82 RDI: ffff90aa3d7da340
+ RBP: ffff9878c047bf00 R08: 00000024f95da94f R09: 0000000000000000
+ R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000000
+ R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
+ FS: 00007f58ece69740(0000) GS:ffff90aa3e200000(0000) knlGS:0000000000000000
+ CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
+ CR2: 00000000ffffff92 CR3: 0000000036adc001 CR4: 00000000003606f0
+ Call Trace:
+ keyctl_read_key+0xac/0xe0
+ SyS_keyctl+0x99/0x120
+ entry_SYSCALL_64_fastpath+0x1f/0xbe
+ RIP: 0033:0x7f58ec787bb9
+ RSP: 002b:00007ffc8d401678 EFLAGS: 00000206 ORIG_RAX: 00000000000000fa
+ RAX: ffffffffffffffda RBX: 00007ffc8d402800 RCX: 00007f58ec787bb9
+ RDX: 0000000000000000 RSI: 00000000174a63ac RDI: 000000000000000b
+ RBP: 0000000000000004 R08: 00007ffc8d402809 R09: 0000000000000020
+ R10: 0000000000000000 R11: 0000000000000206 R12: 00007ffc8d402800
+ R13: 00007ffc8d4016e0 R14: 0000000000000000 R15: 0000000000000000
+ Code: e5 41 55 49 89 f5 41 54 49 89 d4 53 48 89 fb e8 a4 b4 ad ff 85 c0 74 09 80 3d b9 4c 96 00 00 74 43 48 8b b3 20 01 00 00 4d 85 ed <0f> b7 5e 10 74 29 4d 85 e4 74 24 4c 39 e3 4c 89 e2 4c 89 ef 48
+ RIP: user_read+0x33/0xa0 RSP: ffff9878c047bee8
+ CR2: 00000000ffffff92
+
+Fixes: 61ea0c0ba904 ("KEYS: Skip key state checks when checking for possession")
+Signed-off-by: Eric Biggers <ebiggers@google.com>
+Signed-off-by: David Howells <dhowells@redhat.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+
+---
+ security/keys/keyctl.c | 5 +++++
+ 1 file changed, 5 insertions(+)
+
+--- a/security/keys/keyctl.c
++++ b/security/keys/keyctl.c
+@@ -744,6 +744,11 @@ long keyctl_read_key(key_serial_t keyid,
+
+ key = key_ref_to_ptr(key_ref);
+
++ if (test_bit(KEY_FLAG_NEGATIVE, &key->flags)) {
++ ret = -ENOKEY;
++ goto error2;
++ }
++
+ /* see if we can read it directly */
+ ret = key_permission(key_ref, KEY_NEED_READ);
+ if (ret == 0)
projects / thirdparty / kernel / stable-queue.git / commitdiff
