From b5737b92560efcb956d2def4dcd3f4b6d4118e58 Mon Sep 17 00:00:00 2001 From: David Howells Date: Tue, 18 Apr 2017 15:31:07 +0100 Subject: KEYS: Disallow keyrings beginning with '.' to be joined as session keyrings commit ee8f844e3c5a73b999edf733df1c529d6503ec2f upstream. This fixes CVE-2016-9604. Keyrings whose name begin with a '.' are special internal keyrings and so userspace isn't allowed to create keyrings by this name to prevent shadowing. However, the patch that added the guard didn't fix KEYCTL_JOIN_SESSION_KEYRING. Not only can that create dot-named keyrings, it can also subscribe to them as a session keyring if they grant SEARCH permission to the user. This, for example, allows a root process to set .builtin_trusted_keys as its session keyring, at which point it has full access because now the possessor permissions are added. This permits root to add extra public keys, thereby bypassing module verification. This also affects kexec and IMA. This can be tested by (as root): keyctl session .builtin_trusted_keys keyctl add user a a @s keyctl list @s which on my test box gives me: 2 keys in keyring: 180010936: ---lswrv 0 0 asymmetric: Build time autogenerated kernel key: ae3d4a31b82daa8e1a75b49dc2bba949fd992a05 801382539: --alswrv 0 0 user: a Fix this by rejecting names beginning with a '.' in the keyctl. Signed-off-by: David Howells Acked-by: Mimi Zohar cc: linux-ima-devel@lists.sourceforge.net Signed-off-by: Greg Kroah-Hartman --- security/keys/keyctl.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'security/keys/keyctl.c') diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index 1c3872aeed14..4ffb51ff0a61 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -271,7 +271,8 @@ error: * Create and join an anonymous session keyring or join a named session * keyring, creating it if necessary. A named session keyring must have Search * permission for it to be joined. Session keyrings without this permit will - * be skipped over. + * be skipped over. It is not permitted for userspace to create or join + * keyrings whose name begin with a dot. * * If successful, the ID of the joined session keyring will be returned. */ @@ -288,12 +289,16 @@ long keyctl_join_session_keyring(const char __user *_name) ret = PTR_ERR(name); goto error; } + + ret = -EPERM; + if (name[0] == '.') + goto error_name; } /* join the session */ ret = join_session_keyring(name); +error_name: kfree(name); - error: return ret; } -- cgit v1.2.3 From c9460fbceb2f3efa1d20050cdbffa51ec025745a Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 18 Apr 2017 15:31:09 +0100 Subject: KEYS: fix keyctl_set_reqkey_keyring() to not leak thread keyrings commit c9f838d104fed6f2f61d68164712e3204bf5271b upstream. This fixes CVE-2017-7472. Running the following program as an unprivileged user exhausts kernel memory by leaking thread keyrings: #include int main() { for (;;) keyctl_set_reqkey_keyring(KEY_REQKEY_DEFL_THREAD_KEYRING); } Fix it by only creating a new thread keyring if there wasn't one before. To make things more consistent, make install_thread_keyring_to_cred() and install_process_keyring_to_cred() both return 0 if the corresponding keyring is already present. Fixes: d84f4f992cbd ("CRED: Inaugurate COW credentials") Signed-off-by: Eric Biggers Signed-off-by: David Howells Signed-off-by: Greg Kroah-Hartman --- security/keys/keyctl.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) (limited to 'security/keys/keyctl.c') diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index 4ffb51ff0a61..442e350c209d 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -1228,8 +1228,8 @@ error: * Read or set the default keyring in which request_key() will cache keys and * return the old setting. * - * If a process keyring is specified then this will be created if it doesn't - * yet exist. The old setting will be returned if successful. + * If a thread or process keyring is specified then it will be created if it + * doesn't yet exist. The old setting will be returned if successful. */ long keyctl_set_reqkey_keyring(int reqkey_defl) { @@ -1254,11 +1254,8 @@ long keyctl_set_reqkey_keyring(int reqkey_defl) case KEY_REQKEY_DEFL_PROCESS_KEYRING: ret = install_process_keyring_to_cred(new); - if (ret < 0) { - if (ret != -EEXIST) - goto error; - ret = 0; - } + if (ret < 0) + goto error; goto set; case KEY_REQKEY_DEFL_DEFAULT: -- cgit v1.2.3 From bc6be3433e694d1ab1d0012b6053ae4e9a3b189e Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Thu, 8 Jun 2017 14:48:40 +0100 Subject: KEYS: fix dereferencing NULL payload with nonzero length commit 5649645d725c73df4302428ee4e02c869248b4c5 upstream. sys_add_key() and the KEYCTL_UPDATE operation of sys_keyctl() allowed a NULL payload with nonzero length to be passed to the key type's ->preparse(), ->instantiate(), and/or ->update() methods. Various key types including asymmetric, cifs.idmap, cifs.spnego, and pkcs7_test did not handle this case, allowing an unprivileged user to trivially cause a NULL pointer dereference (kernel oops) if one of these key types was present. Fix it by doing the copy_from_user() when 'plen' is nonzero rather than when '_payload' is non-NULL, causing the syscall to fail with EFAULT as expected when an invalid buffer is specified. Signed-off-by: Eric Biggers Signed-off-by: David Howells Signed-off-by: James Morris Signed-off-by: Greg Kroah-Hartman --- security/keys/keyctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'security/keys/keyctl.c') diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index 442e350c209d..671709d8610d 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -97,7 +97,7 @@ SYSCALL_DEFINE5(add_key, const char __user *, _type, /* pull the payload in if one was supplied */ payload = NULL; - if (_payload) { + if (plen) { ret = -ENOMEM; payload = kmalloc(plen, GFP_KERNEL | __GFP_NOWARN); if (!payload) { @@ -327,7 +327,7 @@ long keyctl_update_key(key_serial_t id, /* pull the payload in if one was supplied */ payload = NULL; - if (_payload) { + if (plen) { ret = -ENOMEM; payload = kmalloc(plen, GFP_KERNEL); if (!payload) -- cgit v1.2.3