[PATCH 0/2][T] CVE-2015-8539, CVE-2017-15299 - Multiple issues in the kernel keyring

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2][T] CVE-2015-8539, CVE-2017-15299 - Multiple issues in the kernel keyring

Tyler Hicks-2
https://people.canonical.com/~ubuntu-security/cve/2015/CVE-2015-8539.html

 The KEYS subsystem in the Linux kernel before 4.4 allows local users to
 gain privileges or cause a denial of service (BUG) via crafted keyctl
 commands that negatively instantiate a key, related to
 security/keys/encrypted-keys/encrypted.c, security/keys/trusted.c, and
 security/keys/user_defined.c.

https://people.canonical.com/~ubuntu-security/cve/?cve=CVE-2017-15299

 The KEYS subsystem in the Linux kernel through 4.13.7 mishandles use of
 add_key for a key that already exists but is uninstantiated, which allows
 local users to cause a denial of service (NULL pointer dereference and
 system crash) or possibly have unspecified other impact via a crafted
 system call.

These patches have been tested with the reproducers for both CVEs as
well as the test-ecryptfs-utils.py QRT test which makes use of the
kernel keyring when setting up and decrypting user's home directories.

Tyler


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] KEYS: Fix handling of stored error in a negatively instantiated user key

Tyler Hicks-2
From: David Howells <[hidden email]>

If a user key gets negatively instantiated, an error code is cached in the
payload area.  A negatively instantiated key may be then be positively
instantiated by updating it with valid data.  However, the ->update key
type method must be aware that the error code may be there.

The following may be used to trigger the bug in the user key type:

    keyctl request2 user user "" @u
    keyctl add user user "a" @u

which manifests itself as:

        BUG: unable to handle kernel paging request at 00000000ffffff8a
        IP: [<ffffffff810a376f>] __call_rcu.constprop.76+0x1f/0x280 kernel/rcu/tree.c:3046
        PGD 7cc30067 PUD 0
        Oops: 0002 [#1] SMP
        Modules linked in:
        CPU: 3 PID: 2644 Comm: a.out Not tainted 4.3.0+ #49
        Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
        task: ffff88003ddea700 ti: ffff88003dd88000 task.ti: ffff88003dd88000
        RIP: 0010:[<ffffffff810a376f>]  [<ffffffff810a376f>] __call_rcu.constprop.76+0x1f/0x280
         [<ffffffff810a376f>] __call_rcu.constprop.76+0x1f/0x280 kernel/rcu/tree.c:3046
        RSP: 0018:ffff88003dd8bdb0  EFLAGS: 00010246
        RAX: 00000000ffffff82 RBX: 0000000000000000 RCX: 0000000000000001
        RDX: ffffffff81e3fe40 RSI: 0000000000000000 RDI: 00000000ffffff82
        RBP: ffff88003dd8bde0 R08: ffff88007d2d2da0 R09: 0000000000000000
        R10: 0000000000000000 R11: ffff88003e8073c0 R12: 00000000ffffff82
        R13: ffff88003dd8be68 R14: ffff88007d027600 R15: ffff88003ddea700
        FS:  0000000000b92880(0063) GS:ffff88007fd00000(0000) knlGS:0000000000000000
        CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
        CR2: 00000000ffffff8a CR3: 000000007cc5f000 CR4: 00000000000006e0
        Stack:
         ffff88003dd8bdf0 ffffffff81160a8a 0000000000000000 00000000ffffff82
         ffff88003dd8be68 ffff88007d027600 ffff88003dd8bdf0 ffffffff810a39e5
         ffff88003dd8be20 ffffffff812a31ab ffff88007d027600 ffff88007d027620
        Call Trace:
         [<ffffffff810a39e5>] kfree_call_rcu+0x15/0x20 kernel/rcu/tree.c:3136
         [<ffffffff812a31ab>] user_update+0x8b/0xb0 security/keys/user_defined.c:129
         [<     inline     >] __key_update security/keys/key.c:730
         [<ffffffff8129e5c1>] key_create_or_update+0x291/0x440 security/keys/key.c:908
         [<     inline     >] SYSC_add_key security/keys/keyctl.c:125
         [<ffffffff8129fc21>] SyS_add_key+0x101/0x1e0 security/keys/keyctl.c:60
         [<ffffffff8185f617>] entry_SYSCALL_64_fastpath+0x12/0x6a arch/x86/entry/entry_64.S:185

Note the error code (-ENOKEY) in EDX.

A similar bug can be tripped by:

    keyctl request2 trusted user "" @u
    keyctl add trusted user "a" @u

This should also affect encrypted keys - but that has to be correctly
parameterised or it will fail with EINVAL before getting to the bit that
will crashes.

Reported-by: Dmitry Vyukov <[hidden email]>
Signed-off-by: David Howells <[hidden email]>
Acked-by: Mimi Zohar <[hidden email]>
Signed-off-by: James Morris <[hidden email]>

CVE-2015-8539

(backported from commit 096fe9eaea40a17e125569f9e657e34cdb6d73bd)
Signed-off-by: Tyler Hicks <[hidden email]>
---
 security/keys/encrypted-keys/encrypted.c | 2 ++
 security/keys/trusted.c                  | 5 ++++-
 security/keys/user_defined.c             | 5 ++++-
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 258bd532cbf3..5451a1711461 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -844,6 +844,8 @@ static int encrypted_update(struct key *key, struct key_preparsed_payload *prep)
  size_t datalen = prep->datalen;
  int ret = 0;
 
+ if (test_bit(KEY_FLAG_NEGATIVE, &key->flags))
+ return -ENOKEY;
  if (datalen <= 0 || datalen > 32767 || !prep->data)
  return -EINVAL;
 
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index e13fcf7636f7..d72ff37e3da9 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -984,13 +984,16 @@ static void trusted_rcu_free(struct rcu_head *rcu)
  */
 static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
 {
- struct trusted_key_payload *p = key->payload.data;
+ struct trusted_key_payload *p;
  struct trusted_key_payload *new_p;
  struct trusted_key_options *new_o;
  size_t datalen = prep->datalen;
  char *datablob;
  int ret = 0;
 
+ if (test_bit(KEY_FLAG_NEGATIVE, &key->flags))
+ return -ENOKEY;
+ p = key->payload.data;
  if (!p->migratable)
  return -EPERM;
  if (datalen <= 0 || datalen > 32767 || !prep->data)
diff --git a/security/keys/user_defined.c b/security/keys/user_defined.c
index faa2caeb593f..8383f36f2ae8 100644
--- a/security/keys/user_defined.c
+++ b/security/keys/user_defined.c
@@ -121,7 +121,10 @@ int user_update(struct key *key, struct key_preparsed_payload *prep)
 
  if (ret == 0) {
  /* attach the new data, displacing the old */
- zap = key->payload.data;
+ if (!test_bit(KEY_FLAG_NEGATIVE, &key->flags))
+ zap = key->payload.data;
+ else
+ zap = NULL;
  rcu_assign_keypointer(key, upayload);
  key->expiry = 0;
  }
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] KEYS: don't let add_key() update an uninstantiated key

Tyler Hicks-2
In reply to this post by Tyler Hicks-2
From: David Howells <[hidden email]>

Currently, when passed a key that already exists, add_key() will call the
key's ->update() method if such exists.  But this is heavily broken in the
case where the key is uninstantiated because it doesn't call
__key_instantiate_and_link().  Consequently, it doesn't do most of the
things that are supposed to happen when the key is instantiated, such as
setting the instantiation state, clearing KEY_FLAG_USER_CONSTRUCT and
awakening tasks waiting on it, and incrementing key->user->nikeys.

It also never takes key_construction_mutex, which means that
->instantiate() can run concurrently with ->update() on the same key.  In
the case of the "user" and "logon" key types this causes a memory leak, at
best.  Maybe even worse, the ->update() methods of the "encrypted" and
"trusted" key types actually just dereference a NULL pointer when passed an
uninstantiated key.

Change key_create_or_update() to wait interruptibly for the key to finish
construction before continuing.

This patch only affects *uninstantiated* keys.  For now we still allow a
negatively instantiated key to be updated (thereby positively
instantiating it), although that's broken too (the next patch fixes it)
and I'm not sure that anyone actually uses that functionality either.

Here is a simple reproducer for the bug using the "encrypted" key type
(requires CONFIG_ENCRYPTED_KEYS=y), though as noted above the bug
pertained to more than just the "encrypted" key type:

    #include <stdlib.h>
    #include <unistd.h>
    #include <keyutils.h>

    int main(void)
    {
        int ringid = keyctl_join_session_keyring(NULL);

        if (fork()) {
            for (;;) {
                const char payload[] = "update user:foo 32";

                usleep(rand() % 10000);
                add_key("encrypted", "desc", payload, sizeof(payload), ringid);
                keyctl_clear(ringid);
            }
        } else {
            for (;;)
                request_key("encrypted", "desc", "callout_info", ringid);
        }
    }

It causes:

    BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
    IP: encrypted_update+0xb0/0x170
    PGD 7a178067 P4D 7a178067 PUD 77269067 PMD 0
    PREEMPT SMP
    CPU: 0 PID: 340 Comm: reproduce Tainted: G      D         4.14.0-rc1-00025-g428490e38b2e #796
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
    task: ffff8a467a39a340 task.stack: ffffb15c40770000
    RIP: 0010:encrypted_update+0xb0/0x170
    RSP: 0018:ffffb15c40773de8 EFLAGS: 00010246
    RAX: 0000000000000000 RBX: ffff8a467a275b00 RCX: 0000000000000000
    RDX: 0000000000000005 RSI: ffff8a467a275b14 RDI: ffffffffb742f303
    RBP: ffffb15c40773e20 R08: 0000000000000000 R09: ffff8a467a275b17
    R10: 0000000000000020 R11: 0000000000000000 R12: 0000000000000000
    R13: 0000000000000000 R14: ffff8a4677057180 R15: ffff8a467a275b0f
    FS:  00007f5d7fb08700(0000) GS:ffff8a467f200000(0000) knlGS:0000000000000000
    CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 0000000000000018 CR3: 0000000077262005 CR4: 00000000001606f0
    Call Trace:
     key_create_or_update+0x2bc/0x460
     SyS_add_key+0x10c/0x1d0
     entry_SYSCALL_64_fastpath+0x1f/0xbe
    RIP: 0033:0x7f5d7f211259
    RSP: 002b:00007ffed03904c8 EFLAGS: 00000246 ORIG_RAX: 00000000000000f8
    RAX: ffffffffffffffda RBX: 000000003b2a7955 RCX: 00007f5d7f211259
    RDX: 00000000004009e4 RSI: 00000000004009ff RDI: 0000000000400a04
    RBP: 0000000068db8bad R08: 000000003b2a7955 R09: 0000000000000004
    R10: 000000000000001a R11: 0000000000000246 R12: 0000000000400868
    R13: 00007ffed03905d0 R14: 0000000000000000 R15: 0000000000000000
    Code: 77 28 e8 64 34 1f 00 45 31 c0 31 c9 48 8d 55 c8 48 89 df 48 8d 75 d0 e8 ff f9 ff ff 85 c0 41 89 c4 0f 88 84 00 00 00 4c 8b 7d c8 <49> 8b 75 18 4c 89 ff e8 24 f8 ff ff 85 c0 41 89 c4 78 6d 49 8b
    RIP: encrypted_update+0xb0/0x170 RSP: ffffb15c40773de8
    CR2: 0000000000000018

Cc: <[hidden email]> # v2.6.12+
Reported-by: Eric Biggers <[hidden email]>
Signed-off-by: David Howells <[hidden email]>
cc: Eric Biggers <[hidden email]>

CVE-2017-15299

(cherry picked from commit 60ff5b2f547af3828aebafd54daded44cfb0807a)
Signed-off-by: Tyler Hicks <[hidden email]>
---
 security/keys/key.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/security/keys/key.c b/security/keys/key.c
index 7ee46477082b..d9ed9e493eb1 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -903,6 +903,16 @@ error:
  */
  __key_link_end(keyring, &index_key, edit);
 
+ key = key_ref_to_ptr(key_ref);
+ if (test_bit(KEY_FLAG_USER_CONSTRUCT, &key->flags)) {
+ ret = wait_for_key_construction(key, true);
+ if (ret < 0) {
+ key_ref_put(key_ref);
+ key_ref = ERR_PTR(ret);
+ goto error_free_prep;
+ }
+ }
+
  key_ref = __key_update(key_ref, &prep);
  goto error_free_prep;
 }
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team