From: Greg Kroah-Hartman Date: Mon, 2 Oct 2017 10:15:25 +0000 (+0200) Subject: 3.18-stable patches X-Git-Tag: v3.18.73~25 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=71eb4fd0130a742e41884e422edcc98ece8193ee;p=thirdparty%2Fkernel%2Fstable-queue.git 3.18-stable patches added patches: keys-prevent-creating-a-different-user-s-keyrings.patch keys-prevent-keyctl_read-on-negative-key.patch --- diff --git a/queue-3.18/keys-prevent-creating-a-different-user-s-keyrings.patch b/queue-3.18/keys-prevent-creating-a-different-user-s-keyrings.patch new file mode 100644 index 00000000000..e6cfff9c397 --- /dev/null +++ b/queue-3.18/keys-prevent-creating-a-different-user-s-keyrings.patch @@ -0,0 +1,151 @@ +From 237bbd29f7a049d310d907f4b2716a7feef9abf3 Mon Sep 17 00:00:00 2001 +From: Eric Biggers +Date: Mon, 18 Sep 2017 11:37:03 -0700 +Subject: KEYS: prevent creating a different user's keyrings + +From: Eric Biggers + +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 +Signed-off-by: David Howells +Signed-off-by: Greg Kroah-Hartman + +--- + 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; diff --git a/queue-3.18/keys-prevent-keyctl_read-on-negative-key.patch b/queue-3.18/keys-prevent-keyctl_read-on-negative-key.patch new file mode 100644 index 00000000000..85534fe64ee --- /dev/null +++ b/queue-3.18/keys-prevent-keyctl_read-on-negative-key.patch @@ -0,0 +1,80 @@ +From 37863c43b2c6464f252862bf2e9768264e961678 Mon Sep 17 00:00:00 2001 +From: Eric Biggers +Date: Mon, 18 Sep 2017 11:37:23 -0700 +Subject: KEYS: prevent KEYCTL_READ on negative key + +From: Eric Biggers + +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 +Signed-off-by: David Howells +Signed-off-by: Greg Kroah-Hartman + +--- + 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) diff --git a/queue-3.18/series b/queue-3.18/series index e73a2515521..879606748d4 100644 --- a/queue-3.18/series +++ b/queue-3.18/series @@ -7,3 +7,5 @@ tracing-erase-irqsoff-trace-with-empty-write.patch scsi-scsi_transport_iscsi-fix-the-issue-that-iscsi_if_rx-doesn-t-parse-nlmsg-properly.patch crypto-talitos-fix-sha224.patch keys-fix-writing-past-end-of-user-supplied-buffer-in-keyring_read.patch +keys-prevent-creating-a-different-user-s-keyrings.patch +keys-prevent-keyctl_read-on-negative-key.patch