linux-stable.git/security, branch v3.2.89

KEYS: fix keyctl_set_reqkey_keyring() to not leak thread keyrings

2017-06-05T20:13:54+00:00

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 
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings

KEYS: Disallow keyrings beginning with '.' to be joined as session keyrings

2017-06-05T20:13:53+00:00

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: Ben Hutchings

KEYS: Reinstate EPERM for a key type name beginning with a '.'

2017-06-05T20:13:52+00:00

commit 54e2c2c1a9d6cbb270b0999a38545fa9a69bee43 upstream.

Reinstate the generation of EPERM for a key type name beginning with a '.' in
a userspace call.  Types whose name begins with a '.' are internal only.

The test was removed by:

	commit a4e3b8d79a5c6d40f4a9703abf7fe3abcc6c3b8d
	Author: Mimi Zohar 
	Date:   Thu May 22 14:02:23 2014 -0400
	Subject: KEYS: special dot prefixed keyring name bug fix

I think we want to keep the restriction on type name so that userspace can't
add keys of a special internal type.

Note that removal of the test causes several of the tests in the keyutils
testsuite to fail.

Signed-off-by: David Howells 
Acked-by: Vivek Goyal 
cc: Mimi Zohar 
Signed-off-by: Ben Hutchings

KEYS: special dot prefixed keyring name bug fix

2017-06-05T20:13:52+00:00

commit a4e3b8d79a5c6d40f4a9703abf7fe3abcc6c3b8d upstream.

Dot prefixed keyring names are supposed to be reserved for the
kernel, but add_key() calls key_get_type_from_user(), which
incorrectly verifies the 'type' field, not the 'description' field.
This patch verifies the 'description' field isn't dot prefixed,
when creating a new keyring, and removes the dot prefix test in
key_get_type_from_user().

Changelog v6:
- whitespace and other cleanup

Changelog v5:
- Only prevent userspace from creating a dot prefixed keyring, not
  regular keys  - Dmitry

Reported-by: Dmitry Kasatkin 
Signed-off-by: Mimi Zohar 
Acked-by: David Howells 
[bwh: Backported to 3.2: adjust context, indentation]
Signed-off-by: Ben Hutchings

KEYS: Fix an error code in request_master_key()

2017-06-05T20:13:48+00:00

commit 57cb17e764ba0aaa169d07796acce54ccfbc6cae upstream.

This function has two callers and neither are able to handle a NULL
return.  Really, -EINVAL is the correct thing return here anyway.  This
fixes some static checker warnings like:

	security/keys/encrypted-keys/encrypted.c:709 encrypted_key_decrypt()
	error: uninitialized symbol 'master_key'.

Fixes: 7e70cb497850 ("keys: add new key-type encrypted")
Signed-off-by: Dan Carpenter 
Acked-by: Mimi Zohar 
Signed-off-by: James Morris 
Signed-off-by: Ben Hutchings

keys: Guard against null match function in keyring_search_aux()

2017-04-04T21:18:33+00:00

The "dead" key type has no match operation, and a search for keys of
this type can cause a null dereference in keyring_search_aux().
keyring_search() has a check for this, but request_keyring_and_link()
does not.  Move the check into keyring_search_aux(), covering both of
them.

This was fixed upstream by commit c06cfb08b88d ("KEYS: Remove
key_type::match in favour of overriding default by match_preparse"),
part of a series of large changes that are not suitable for
backporting.

CVE-2017-2647 / CVE-2017-6951

Reported-by: Igor Redko 
Reported-by: Andrey Ryabinin 
References: https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2017-2647
Reported-by: idl3r 
References: https://www.spinics.net/lists/keyrings/msg01845.html
Signed-off-by: Ben Hutchings 
Cc: David Howells

KEYS: Fix short sprintf buffer in /proc/keys show function

2016-11-20T01:01:43+00:00

commit 03dab869b7b239c4e013ec82aea22e181e441cfc upstream.

This fixes CVE-2016-7042.

Fix a short sprintf buffer in proc_keys_show().  If the gcc stack protector
is turned on, this can cause a panic due to stack corruption.

The problem is that xbuf[] is not big enough to hold a 64-bit timeout
rendered as weeks:

	(gdb) p 0xffffffffffffffffULL/(60*60*24*7)
	$2 = 30500568904943

That's 14 chars plus NUL, not 11 chars plus NUL.

Expand the buffer to 16 chars.

I think the unpatched code apparently works if the stack-protector is not
enabled because on a 32-bit machine the buffer won't be overflowed and on a
64-bit machine there's a 64-bit aligned pointer at one side and an int that
isn't checked again on the other side.

The panic incurred looks something like:

Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: ffffffff81352ebe
CPU: 0 PID: 1692 Comm: reproducer Not tainted 4.7.2-201.fc24.x86_64 #1
Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
 0000000000000086 00000000fbbd2679 ffff8800a044bc00 ffffffff813d941f
 ffffffff81a28d58 ffff8800a044bc98 ffff8800a044bc88 ffffffff811b2cb6
 ffff880000000010 ffff8800a044bc98 ffff8800a044bc30 00000000fbbd2679
Call Trace:
 [] dump_stack+0x63/0x84
 [] panic+0xde/0x22a
 [] ? proc_keys_show+0x3ce/0x3d0
 [] __stack_chk_fail+0x19/0x30
 [] proc_keys_show+0x3ce/0x3d0
 [] ? key_validate+0x50/0x50
 [] ? key_default_cmp+0x20/0x20
 [] seq_read+0x2cc/0x390
 [] proc_reg_read+0x42/0x70
 [] __vfs_read+0x37/0x150
 [] ? security_file_permission+0xa0/0xc0
 [] vfs_read+0x96/0x130
 [] SyS_read+0x55/0xc0
 [] entry_SYSCALL_64_fastpath+0x1a/0xa4

Reported-by: Ondrej Kozina 
Signed-off-by: David Howells 
Tested-by: Ondrej Kozina 
Signed-off-by: James Morris 
Signed-off-by: Ben Hutchings

KEYS: potential uninitialized variable

2016-08-22T21:37:14+00:00

commit 38327424b40bcebe2de92d07312c89360ac9229a upstream.

If __key_link_begin() failed then "edit" would be uninitialized.  I've
added a check to fix that.

This allows a random user to crash the kernel, though it's quite
difficult to achieve.  There are three ways it can be done as the user
would have to cause an error to occur in __key_link():

 (1) Cause the kernel to run out of memory.  In practice, this is difficult
     to achieve without ENOMEM cropping up elsewhere and aborting the
     attempt.

 (2) Revoke the destination keyring between the keyring ID being looked up
     and it being tested for revocation.  In practice, this is difficult to
     time correctly because the KEYCTL_REJECT function can only be used
     from the request-key upcall process.  Further, users can only make use
     of what's in /sbin/request-key.conf, though this does including a
     rejection debugging test - which means that the destination keyring
     has to be the caller's session keyring in practice.

 (3) Have just enough key quota available to create a key, a new session
     keyring for the upcall and a link in the session keyring, but not then
     sufficient quota to create a link in the nominated destination keyring
     so that it fails with EDQUOT.

The bug can be triggered using option (3) above using something like the
following:

	echo 80 >/proc/sys/kernel/keys/root_maxbytes
	keyctl request2 user debug:fred negate @t

The above sets the quota to something much lower (80) to make the bug
easier to trigger, but this is dependent on the system.  Note also that
the name of the keyring created contains a random number that may be
between 1 and 10 characters in size, so may throw the test off by
changing the amount of quota used.

Assuming the failure occurs, something like the following will be seen:

	kfree_debugcheck: out of range ptr 6b6b6b6b6b6b6b68h
	------------[ cut here ]------------
	kernel BUG at ../mm/slab.c:2821!
	...
	RIP: 0010:[] kfree_debugcheck+0x20/0x25
	RSP: 0018:ffff8804014a7de8  EFLAGS: 00010092
	RAX: 0000000000000034 RBX: 6b6b6b6b6b6b6b68 RCX: 0000000000000000
	RDX: 0000000000040001 RSI: 00000000000000f6 RDI: 0000000000000300
	RBP: ffff8804014a7df0 R08: 0000000000000001 R09: 0000000000000000
	R10: ffff8804014a7e68 R11: 0000000000000054 R12: 0000000000000202
	R13: ffffffff81318a66 R14: 0000000000000000 R15: 0000000000000001
	...
	Call Trace:
	  kfree+0xde/0x1bc
	  assoc_array_cancel_edit+0x1f/0x36
	  __key_link_end+0x55/0x63
	  key_reject_and_link+0x124/0x155
	  keyctl_reject_key+0xb6/0xe0
	  keyctl_negate_key+0x10/0x12
	  SyS_keyctl+0x9f/0xe7
	  do_syscall_64+0x63/0x13a
	  entry_SYSCALL64_slow_path+0x25/0x25

Fixes: f70e2e06196a ('KEYS: Do preallocation for __key_link()')
Signed-off-by: Dan Carpenter 
Signed-off-by: David Howells 
Signed-off-by: Linus Torvalds 
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings

KEYS: Fix race between read and revoke

2016-01-22T21:40:08+00:00

commit b4a1b4f5047e4f54e194681125c74c0aa64d637d upstream.

This fixes CVE-2015-7550.

There's a race between keyctl_read() and keyctl_revoke().  If the revoke
happens between keyctl_read() checking the validity of a key and the key's
semaphore being taken, then the key type read method will see a revoked key.

This causes a problem for the user-defined key type because it assumes in
its read method that there will always be a payload in a non-revoked key
and doesn't check for a NULL pointer.

Fix this by making keyctl_read() check the validity of a key after taking
semaphore instead of before.

I think the bug was introduced with the original keyrings code.

This was discovered by a multithreaded test program generated by syzkaller
(http://github.com/google/syzkaller).  Here's a cleaned up version:

	#include 
	#include 
	#include 
	void *thr0(void *arg)
	{
		key_serial_t key = (unsigned long)arg;
		keyctl_revoke(key);
		return 0;
	}
	void *thr1(void *arg)
	{
		key_serial_t key = (unsigned long)arg;
		char buffer[16];
		keyctl_read(key, buffer, 16);
		return 0;
	}
	int main()
	{
		key_serial_t key = add_key("user", "%", "foo", 3, KEY_SPEC_USER_KEYRING);
		pthread_t th[5];
		pthread_create(&th[0], 0, thr0, (void *)(unsigned long)key);
		pthread_create(&th[1], 0, thr1, (void *)(unsigned long)key);
		pthread_create(&th[2], 0, thr0, (void *)(unsigned long)key);
		pthread_create(&th[3], 0, thr1, (void *)(unsigned long)key);
		pthread_join(th[0], 0);
		pthread_join(th[1], 0);
		pthread_join(th[2], 0);
		pthread_join(th[3], 0);
		return 0;
	}

Build as:

	cc -o keyctl-race keyctl-race.c -lkeyutils -lpthread

Run as:

	while keyctl-race; do :; done

as it may need several iterations to crash the kernel.  The crash can be
summarised as:

	BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
	IP: [] user_read+0x56/0xa3
	...
	Call Trace:
	 [] keyctl_read_key+0xb6/0xd7
	 [] SyS_keyctl+0x83/0xe0
	 [] entry_SYSCALL_64_fastpath+0x12/0x6f

Reported-by: Dmitry Vyukov 
Signed-off-by: David Howells 
Tested-by: Dmitry Vyukov 
Signed-off-by: James Morris 
Signed-off-by: Ben Hutchings

KEYS: Fix crash when attempt to garbage collect an uninstantiated keyring

2015-11-17T15:54:47+00:00

commit f05819df10d7b09f6d1eb6f8534a8f68e5a4fe61 upstream.

The following sequence of commands:

    i=`keyctl add user a a @s`
    keyctl request2 keyring foo bar @t
    keyctl unlink $i @s

tries to invoke an upcall to instantiate a keyring if one doesn't already
exist by that name within the user's keyring set.  However, if the upcall
fails, the code sets keyring->type_data.reject_error to -ENOKEY or some
other error code.  When the key is garbage collected, the key destroy
function is called unconditionally and keyring_destroy() uses list_empty()
on keyring->type_data.link - which is in a union with reject_error.
Subsequently, the kernel tries to unlink the keyring from the keyring names
list - which oopses like this:

	BUG: unable to handle kernel paging request at 00000000ffffff8a
	IP: [] keyring_destroy+0x3d/0x88
	...
	Workqueue: events key_garbage_collector
	...
	RIP: 0010:[] keyring_destroy+0x3d/0x88
	RSP: 0018:ffff88003e2f3d30  EFLAGS: 00010203
	RAX: 00000000ffffff82 RBX: ffff88003bf1a900 RCX: 0000000000000000
	RDX: 0000000000000000 RSI: 000000003bfc6901 RDI: ffffffff81a73a40
	RBP: ffff88003e2f3d38 R08: 0000000000000152 R09: 0000000000000000
	R10: ffff88003e2f3c18 R11: 000000000000865b R12: ffff88003bf1a900
	R13: 0000000000000000 R14: ffff88003bf1a908 R15: ffff88003e2f4000
	...
	CR2: 00000000ffffff8a CR3: 000000003e3ec000 CR4: 00000000000006f0
	...
	Call Trace:
	 [] key_gc_unused_keys.constprop.1+0x5d/0x10f
	 [] key_garbage_collector+0x1fa/0x351
	 [] process_one_work+0x28e/0x547
	 [] worker_thread+0x26e/0x361
	 [] ? rescuer_thread+0x2a8/0x2a8
	 [] kthread+0xf3/0xfb
	 [] ? kthread_create_on_node+0x1c2/0x1c2
	 [] ret_from_fork+0x3f/0x70
	 [] ? kthread_create_on_node+0x1c2/0x1c2

Note the value in RAX.  This is a 32-bit representation of -ENOKEY.

The solution is to only call ->destroy() if the key was successfully
instantiated.

Reported-by: Dmitry Vyukov 
Signed-off-by: David Howells 
Tested-by: Dmitry Vyukov 
[carnil: Backported for 3.2: adjust context]
Signed-off-by: Ben Hutchings