linux-stable.git/security/keys/request_key.c, branch v3.16.78

KEYS: always initialize keyring_index_key::desc_len

2019-05-02T20:41:58+00:00

commit ede0fa98a900e657d1fcd80b50920efc896c1a4c upstream.

syzbot hit the 'BUG_ON(index_key->desc_len == 0);' in __key_link_begin()
called from construct_alloc_key() during sys_request_key(), because the
length of the key description was never calculated.

The problem is that we rely on ->desc_len being initialized by
search_process_keyrings(), specifically by search_nested_keyrings().
But, if the process isn't subscribed to any keyrings that never happens.

Fix it by always initializing keyring_index_key::desc_len as soon as the
description is set, like we already do in some places.

The following program reproduces the BUG_ON() when it's run as root and
no session keyring has been installed.  If it doesn't work, try removing
pam_keyinit.so from /etc/pam.d/login and rebooting.

    #include 
    #include 
    #include 

    int main(void)
    {
            int id = add_key("keyring", "syz", NULL, 0, KEY_SPEC_USER_KEYRING);

            keyctl_setperm(id, KEY_OTH_WRITE);
            setreuid(5000, 5000);
            request_key("user", "desc", "", id);
    }

Reported-by: syzbot+ec24e95ea483de0a24da@syzkaller.appspotmail.com
Fixes: b2a4df200d57 ("KEYS: Expand the capacity of a keyring")
Signed-off-by: Eric Biggers 
Signed-off-by: David Howells 
Signed-off-by: James Morris 
Signed-off-by: Ben Hutchings

KEYS: add missing permission check for request_key() destination

2018-01-01T20:52:12+00:00

commit 4dca6ea1d9432052afb06baf2e3ae78188a4410b upstream.

When the request_key() syscall is not passed a destination keyring, it
links the requested key (if constructed) into the "default" request-key
keyring.  This should require Write permission to the keyring.  However,
there is actually no permission check.

This can be abused to add keys to any keyring to which only Search
permission is granted.  This is because Search permission allows joining
the keyring.  keyctl_set_reqkey_keyring(KEY_REQKEY_DEFL_SESSION_KEYRING)
then will set the default request-key keyring to the session keyring.
Then, request_key() can be used to add keys to the keyring.

Both negatively and positively instantiated keys can be added using this
method.  Adding negative keys is trivial.  Adding a positive key is a
bit trickier.  It requires that either /sbin/request-key positively
instantiates the key, or that another thread adds the key to the process
keyring at just the right time, such that request_key() misses it
initially but then finds it in construct_alloc_key().

Fix this bug by checking for Write permission to the keyring in
construct_get_dest_keyring() when the default keyring is being used.

We don't do the permission check for non-default keyrings because that
was already done by the earlier call to lookup_user_key().  Also,
request_key_and_link() is currently passed a 'struct key *' rather than
a key_ref_t, so the "possessed" bit is unavailable.

We also don't do the permission check for the "requestor keyring", to
continue to support the use case described by commit 8bbf4976b59f
("KEYS: Alter use of key instantiation link-to-keyring argument") where
/sbin/request-key recursively calls request_key() to add keys to the
original requestor's destination keyring.  (I don't know of any users
who actually do that, though...)

Fixes: 3e30148c3d52 ("[PATCH] Keys: Make request-key create an authorisation key")
Signed-off-by: Eric Biggers 
Signed-off-by: David Howells 
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings

KEYS: Don't permit request_key() to construct a new keyring

2015-10-28T10:33:23+00:00

commit 911b79cde95c7da0ec02f48105358a36636b7a71 upstream.

If request_key() is used to find a keyring, only do the search part - don't
do the construction part if the keyring was not found by the search.  We
don't really want keyrings in the negative instantiated state since the
rejected/negative instantiation error value in the payload is unioned with
keyring metadata.

Now the kernel gives an error:

	request_key("keyring", "#selinux,bdekeyring", "keyring", KEY_SPEC_USER_SESSION_KEYRING) = -1 EPERM (Operation not permitted)

Signed-off-by: David Howells 
Cc: Kamal Mostafa 
Signed-off-by: Luis Henriques

KEYS: Fix a race between negating a key and reading the error set

2013-10-30T11:15:24+00:00

key_reject_and_link() marking a key as negative and setting the error with
which it was negated races with keyring searches and other things that read
that error.

The fix is to switch the order in which the assignments are done in
key_reject_and_link() and to use memory barriers.

Kudos to Dave Wysochanski  and Scott Mayhew
 for tracking this down.

This may be the cause of:

BUG: unable to handle kernel NULL pointer dereference at 0000000000000070
IP: [] wait_for_key_construction+0x31/0x80
PGD c6b2c3067 PUD c59879067 PMD 0
Oops: 0000 [#1] SMP
last sysfs file: /sys/devices/system/cpu/cpu3/cache/index2/shared_cpu_map
CPU 0
Modules linked in: ...

Pid: 13359, comm: amqzxma0 Not tainted 2.6.32-358.20.1.el6.x86_64 #1 IBM System x3650 M3 -[7945PSJ]-/00J6159
RIP: 0010:[] wait_for_key_construction+0x31/0x80
RSP: 0018:ffff880c6ab33758  EFLAGS: 00010246
RAX: ffffffff81219080 RBX: 0000000000000000 RCX: 0000000000000002
RDX: ffffffff81219060 RSI: 0000000000000000 RDI: 0000000000000000
RBP: ffff880c6ab33768 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000000 R12: ffff880adfcbce40
R13: ffffffffa03afb84 R14: ffff880adfcbce40 R15: ffff880adfcbce43
FS:  00007f29b8042700(0000) GS:ffff880028200000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000070 CR3: 0000000c613dc000 CR4: 00000000000007f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process amqzxma0 (pid: 13359, threadinfo ffff880c6ab32000, task ffff880c610deae0)
Stack:
 ffff880adfcbce40 0000000000000000 ffff880c6ab337b8 ffffffff81219695
 0000000000000000 ffff880a000000d0 ffff880c6ab337a8 000000000000000f
 ffffffffa03afb93 000000000000000f ffff88186c7882c0 0000000000000014
Call Trace:
 [] request_key+0x65/0xa0
 [] nfs_idmap_request_key+0xc5/0x170 [nfs]
 [] nfs_idmap_lookup_id+0x34/0x80 [nfs]
 [] nfs_map_group_to_gid+0x75/0xa0 [nfs]
 [] decode_getfattr_attrs+0xbdd/0xfb0 [nfs]
 [] ? __dequeue_entity+0x30/0x50
 [] ? __switch_to+0x26e/0x320
 [] decode_getfattr+0x83/0xe0 [nfs]
 [] ? nfs4_xdr_dec_getattr+0x0/0xa0 [nfs]
 [] nfs4_xdr_dec_getattr+0x8f/0xa0 [nfs]
 [] rpcauth_unwrap_resp+0x84/0xb0 [sunrpc]
 [] ? nfs4_xdr_dec_getattr+0x0/0xa0 [nfs]
 [] call_decode+0x1b3/0x800 [sunrpc]
 [] ? wake_bit_function+0x0/0x50
 [] ? call_decode+0x0/0x800 [sunrpc]
 [] __rpc_execute+0x77/0x350 [sunrpc]
 [] ? bit_waitqueue+0x17/0xd0
 [] rpc_execute+0x61/0xa0 [sunrpc]
 [] rpc_run_task+0x75/0x90 [sunrpc]
 [] rpc_call_sync+0x42/0x70 [sunrpc]
 [] _nfs4_call_sync+0x30/0x40 [nfs]
 [] _nfs4_proc_getattr+0xac/0xc0 [nfs]
 [] ? futex_wait+0x227/0x380
 [] nfs4_proc_getattr+0x56/0x80 [nfs]
 [] __nfs_revalidate_inode+0xe3/0x220 [nfs]
 [] nfs_revalidate_mapping+0x4e/0x170 [nfs]
 [] nfs_file_read+0x77/0x130 [nfs]
 [] do_sync_read+0xfa/0x140
 [] ? autoremove_wake_function+0x0/0x40
 [] ? apic_timer_interrupt+0xe/0x20
 [] ? common_interrupt+0xe/0x13
 [] ? selinux_file_permission+0xfb/0x150
 [] ? security_file_permission+0x16/0x20
 [] vfs_read+0xb5/0x1a0
 [] sys_read+0x51/0x90
 [] ? __audit_syscall_exit+0x265/0x290
 [] system_call_fastpath+0x16/0x1b

Signed-off-by: David Howells 
cc: Dave Wysochanski 
cc: Scott Mayhew

KEYS: Expand the capacity of a keyring

2013-09-24T09:35:18+00:00

Expand the capacity of a keyring to be able to hold a lot more keys by using
the previously added associative array implementation.  Currently the maximum
capacity is:

	(PAGE_SIZE - sizeof(header)) / sizeof(struct key *)

which, on a 64-bit system, is a little more 500.  However, since this is being
used for the NFS uid mapper, we need more than that.  The new implementation
gives us effectively unlimited capacity.

With some alterations, the keyutils testsuite runs successfully to completion
after this patch is applied.  The alterations are because (a) keyrings that
are simply added to no longer appear ordered and (b) some of the errors have
changed a bit.

Signed-off-by: David Howells

KEYS: Introduce a search context structure

2013-09-24T09:35:15+00:00

Search functions pass around a bunch of arguments, each of which gets copied
with each call.  Introduce a search context structure to hold these.

Whilst we're at it, create a search flag that indicates whether the search
should be directly to the description or whether it should iterate through all
keys looking for a non-description match.

This will be useful when keyrings use a generic data struct with generic
routines to manage their content as the search terms can just be passed
through to the iterator callback function.

Also, for future use, the data to be supplied to the match function is
separated from the description pointer in the search context.  This makes it
clear which is being supplied.

Signed-off-by: David Howells

KEYS: Consolidate the concept of an 'index key' for key access

2013-09-24T09:35:15+00:00

Consolidate the concept of an 'index key' for accessing keys.  The index key
is the search term needed to find a key directly - basically the key type and
the key description.  We can add to that the description length.

This will be useful when turning a keyring into an associative array rather
than just a pointer block.

Signed-off-by: David Howells

KEYS: Skip key state checks when checking for possession

2013-09-24T09:35:13+00:00

Skip key state checks (invalidation, revocation and expiration) when checking
for possession.  Without this, keys that have been marked invalid, revoked
keys and expired keys are not given a possession attribute - which means the
possessor is not granted any possession permits and cannot do anything with
them unless they also have one a user, group or other permit.

This causes failures in the keyutils test suite's revocation and expiration
tests now that commit 96b5c8fea6c0861621051290d705ec2e971963f1 reduced the
initial permissions granted to a key.

The failures are due to accesses to revoked and expired keys being given
EACCES instead of EKEYREVOKED or EKEYEXPIRED.

Signed-off-by: David Howells

KEYS: split call to call_usermodehelper_fns()

2013-05-01T00:04:06+00:00

Use call_usermodehelper_setup() + call_usermodehelper_exec() instead of
calling call_usermodehelper_fns().  In case there's an OOM in this last
function the cleanup function may not be called - in this case we would
miss a call to key_put().

Signed-off-by: Lucas De Marchi 
Cc: Oleg Nesterov 
Acked-by: David Howells 
Acked-by: James Morris 
Cc: Al Viro 
Cc: Tejun Heo 
Cc: "Rafael J. Wysocki" 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds

Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security

2012-12-16T23:40:50+00:00

Pull security subsystem updates from James Morris:
 "A quiet cycle for the security subsystem with just a few maintenance
  updates."

* 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security:
  Smack: create a sysfs mount point for smackfs
  Smack: use select not depends in Kconfig
  Yama: remove locking from delete path
  Yama: add RCU to drop read locking
  drivers/char/tpm: remove tasklet and cleanup
  KEYS: Use keyring_alloc() to create special keyrings
  KEYS: Reduce initial permissions on keys
  KEYS: Make the session and process keyrings per-thread
  seccomp: Make syscall skipping and nr changes more consistent
  key: Fix resource leak
  keys: Fix unreachable code
  KEYS: Add payload preparsing opportunity prior to key instantiate or update