<feed xmlns='http://www.w3.org/2005/Atom'>
<title>user/sven/linux.git/security/keys/proc.c, branch v4.9.296</title>
<subtitle>Linux Kernel
</subtitle>
<id>https://git.stealer.net/cgit.cgi/user/sven/linux.git/atom?h=v4.9.296</id>
<link rel='self' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/atom?h=v4.9.296'/>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/'/>
<updated>2019-03-19T12:14:08Z</updated>
<entry>
<title>KEYS: restrict /proc/keys by credentials at open time</title>
<updated>2019-03-19T12:14:08Z</updated>
<author>
<name>Eric Biggers</name>
<email>ebiggers@google.com</email>
</author>
<published>2017-09-18T18:38:29Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=ccc2aae5c4df7c6830b79280bf6fd1bf57dd5a72'/>
<id>urn:sha1:ccc2aae5c4df7c6830b79280bf6fd1bf57dd5a72</id>
<content type='text'>
commit 4aa68e07d845562561f5e73c04aa521376e95252 upstream.

When checking for permission to view keys whilst reading from
/proc/keys, we should use the credentials with which the /proc/keys file
was opened.  This is because, in a classic type of exploit, it can be
possible to bypass checks for the *current* credentials by passing the
file descriptor to a suid program.

Following commit 34dbbcdbf633 ("Make file credentials available to the
seqfile interfaces") we can finally fix it.  So let's do it.

Signed-off-by: Eric Biggers &lt;ebiggers@google.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Signed-off-by: Zubin Mithra &lt;zsm@chromium.org&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;

</content>
</entry>
<entry>
<title>KEYS: always initialize keyring_index_key::desc_len</title>
<updated>2019-02-27T09:07:00Z</updated>
<author>
<name>Eric Biggers</name>
<email>ebiggers@google.com</email>
</author>
<published>2019-02-22T15:36:18Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=dc070cdb427e42a6b6b503427a029083e307ed34'/>
<id>urn:sha1:dc070cdb427e42a6b6b503427a029083e307ed34</id>
<content type='text'>
commit ede0fa98a900e657d1fcd80b50920efc896c1a4c upstream.

syzbot hit the 'BUG_ON(index_key-&gt;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 -&gt;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 &lt;stdlib.h&gt;
    #include &lt;unistd.h&gt;
    #include &lt;keyutils.h&gt;

    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 &lt;ebiggers@google.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Cc: stable@vger.kernel.org
Signed-off-by: James Morris &lt;james.morris@microsoft.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;

</content>
</entry>
<entry>
<title>KEYS: Fix race between updating and finding a negative key</title>
<updated>2017-10-27T08:38:11Z</updated>
<author>
<name>David Howells</name>
<email>dhowells@redhat.com</email>
</author>
<published>2017-10-04T15:43:25Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=63c8e452554962f88c0952212c8a4202469d4914'/>
<id>urn:sha1:63c8e452554962f88c0952212c8a4202469d4914</id>
<content type='text'>
commit 363b02dab09b3226f3bd1420dad9c72b79a42a76 upstream.

Consolidate KEY_FLAG_INSTANTIATED, KEY_FLAG_NEGATIVE and the rejection
error into one field such that:

 (1) The instantiation state can be modified/read atomically.

 (2) The error can be accessed atomically with the state.

 (3) The error isn't stored unioned with the payload pointers.

This deals with the problem that the state is spread over three different
objects (two bits and a separate variable) and reading or updating them
atomically isn't practical, given that not only can uninstantiated keys
change into instantiated or rejected keys, but rejected keys can also turn
into instantiated keys - and someone accessing the key might not be using
any locking.

The main side effect of this problem is that what was held in the payload
may change, depending on the state.  For instance, you might observe the
key to be in the rejected state.  You then read the cached error, but if
the key semaphore wasn't locked, the key might've become instantiated
between the two reads - and you might now have something in hand that isn't
actually an error code.

The state is now KEY_IS_UNINSTANTIATED, KEY_IS_POSITIVE or a negative error
code if the key is negatively instantiated.  The key_is_instantiated()
function is replaced with key_is_positive() to avoid confusion as negative
keys are also 'instantiated'.

Additionally, barriering is included:

 (1) Order payload-set before state-set during instantiation.

 (2) Order state-read before payload-read when using the key.

Further separate barriering is necessary if RCU is being used to access the
payload content after reading the payload pointers.

Fixes: 146aa8b1453b ("KEYS: Merge the type-specific data with the payload data")
Reported-by: Eric Biggers &lt;ebiggers@google.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Reviewed-by: Eric Biggers &lt;ebiggers@google.com&gt;
Signed-off-by: Greg Kroah-Hartman &lt;gregkh@linuxfoundation.org&gt;
</content>
</entry>
<entry>
<title>KEYS: Fix short sprintf buffer in /proc/keys show function</title>
<updated>2016-10-27T05:03:24Z</updated>
<author>
<name>David Howells</name>
<email>dhowells@redhat.com</email>
</author>
<published>2016-10-26T14:01:54Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=03dab869b7b239c4e013ec82aea22e181e441cfc'/>
<id>urn:sha1:03dab869b7b239c4e013ec82aea22e181e441cfc</id>
<content type='text'>
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:
 [&lt;ffffffff813d941f&gt;] dump_stack+0x63/0x84
 [&lt;ffffffff811b2cb6&gt;] panic+0xde/0x22a
 [&lt;ffffffff81352ebe&gt;] ? proc_keys_show+0x3ce/0x3d0
 [&lt;ffffffff8109f7f9&gt;] __stack_chk_fail+0x19/0x30
 [&lt;ffffffff81352ebe&gt;] proc_keys_show+0x3ce/0x3d0
 [&lt;ffffffff81350410&gt;] ? key_validate+0x50/0x50
 [&lt;ffffffff8134db30&gt;] ? key_default_cmp+0x20/0x20
 [&lt;ffffffff8126b31c&gt;] seq_read+0x2cc/0x390
 [&lt;ffffffff812b6b12&gt;] proc_reg_read+0x42/0x70
 [&lt;ffffffff81244fc7&gt;] __vfs_read+0x37/0x150
 [&lt;ffffffff81357020&gt;] ? security_file_permission+0xa0/0xc0
 [&lt;ffffffff81246156&gt;] vfs_read+0x96/0x130
 [&lt;ffffffff81247635&gt;] SyS_read+0x55/0xc0
 [&lt;ffffffff817eb872&gt;] entry_SYSCALL_64_fastpath+0x1a/0xa4

Reported-by: Ondrej Kozina &lt;okozina@redhat.com&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Tested-by: Ondrej Kozina &lt;okozina@redhat.com&gt;
cc: stable@vger.kernel.org
Signed-off-by: James Morris &lt;james.l.morris@oracle.com&gt;
</content>
</entry>
<entry>
<title>KEYS: Make /proc/keys unconditional if CONFIG_KEYS=y</title>
<updated>2015-01-22T22:34:32Z</updated>
<author>
<name>David Howells</name>
<email>dhowells@redhat.com</email>
</author>
<published>2015-01-22T22:34:32Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=dabd39cc2fb1b0e97313ebbe7309ea8e05b7cfb5'/>
<id>urn:sha1:dabd39cc2fb1b0e97313ebbe7309ea8e05b7cfb5</id>
<content type='text'>
Now that /proc/keys is used by libkeyutils to look up a key by type and
description, we should make it unconditional and remove
CONFIG_DEBUG_PROC_KEYS.

Reported-by: Jiri Kosina &lt;jkosina@suse.cz&gt;
Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Tested-by: Jiri Kosina &lt;jkosina@suse.cz&gt;
</content>
</entry>
<entry>
<title>KEYS: Preparse match data</title>
<updated>2014-09-16T16:36:02Z</updated>
<author>
<name>David Howells</name>
<email>dhowells@redhat.com</email>
</author>
<published>2014-09-16T16:36:02Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=462919591a1791e76042dc5c1e0148715df59beb'/>
<id>urn:sha1:462919591a1791e76042dc5c1e0148715df59beb</id>
<content type='text'>
Preparse the match data.  This provides several advantages:

 (1) The preparser can reject invalid criteria up front.

 (2) The preparser can convert the criteria to binary data if necessary (the
     asymmetric key type really wants to do binary comparison of the key IDs).

 (3) The preparser can set the type of search to be performed.  This means
     that it's not then a one-off setting in the key type.

 (4) The preparser can set an appropriate comparator function.

Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Acked-by: Vivek Goyal &lt;vgoyal@redhat.com&gt;
</content>
</entry>
<entry>
<title>KEYS: Move the flags representing required permission to linux/key.h</title>
<updated>2014-03-14T17:44:49Z</updated>
<author>
<name>David Howells</name>
<email>dhowells@redhat.com</email>
</author>
<published>2014-03-14T17:44:49Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=f5895943d91b41b0368830cdb6eaffb8eda0f4c8'/>
<id>urn:sha1:f5895943d91b41b0368830cdb6eaffb8eda0f4c8</id>
<content type='text'>
Move the flags representing required permission to linux/key.h as the perm
parameter of security_key_permission() is in terms of them - and not the
permissions mask flags used in key-&gt;perm.

Whilst we're at it:

 (1) Rename them to be KEY_NEED_xxx rather than KEY_xxx to avoid collisions
     with symbols in uapi/linux/input.h.

 (2) Don't use key_perm_t for a mask of required permissions, but rather limit
     it to the permissions mask attached to the key and arguments related
     directly to that.

Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
Tested-by: Dmitry Kasatkin &lt;d.kasatkin@samsung.com&gt;
</content>
</entry>
<entry>
<title>KEYS: Introduce a search context structure</title>
<updated>2013-09-24T09:35:15Z</updated>
<author>
<name>David Howells</name>
<email>dhowells@redhat.com</email>
</author>
<published>2013-09-24T09:35:15Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=4bdf0bc300314141e5475e145acb8b5ad846f00d'/>
<id>urn:sha1:4bdf0bc300314141e5475e145acb8b5ad846f00d</id>
<content type='text'>
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 &lt;dhowells@redhat.com&gt;
</content>
</entry>
<entry>
<title>userns: Convert security/keys to the new userns infrastructure</title>
<updated>2012-09-14T01:28:02Z</updated>
<author>
<name>Eric W. Biederman</name>
<email>ebiederm@xmission.com</email>
</author>
<published>2012-02-08T15:53:04Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=9a56c2db49e7349c7963f0ce66c1ef578d44ebd3'/>
<id>urn:sha1:9a56c2db49e7349c7963f0ce66c1ef578d44ebd3</id>
<content type='text'>
- Replace key_user -&gt;user_ns equality checks with kuid_has_mapping checks.
- Use from_kuid to generate key descriptions
- Use kuid_t and kgid_t and the associated helpers instead of uid_t and gid_t
- Avoid potential problems with file descriptor passing by displaying
  keys in the user namespace of the opener of key status proc files.

Cc: linux-security-module@vger.kernel.org
Cc: keyrings@linux-nfs.org
Cc: David Howells &lt;dhowells@redhat.com&gt;
Signed-off-by: Eric W. Biederman &lt;ebiederm@xmission.com&gt;
</content>
</entry>
<entry>
<title>KEYS: Add invalidation support</title>
<updated>2012-05-11T09:56:56Z</updated>
<author>
<name>David Howells</name>
<email>dhowells@redhat.com</email>
</author>
<published>2012-05-11T09:56:56Z</published>
<link rel='alternate' type='text/html' href='https://git.stealer.net/cgit.cgi/user/sven/linux.git/commit/?id=fd75815f727f157a05f4c96b5294a4617c0557da'/>
<id>urn:sha1:fd75815f727f157a05f4c96b5294a4617c0557da</id>
<content type='text'>
Add support for invalidating a key - which renders it immediately invisible to
further searches and causes the garbage collector to immediately wake up,
remove it from keyrings and then destroy it when it's no longer referenced.

It's better not to do this with keyctl_revoke() as that marks the key to start
returning -EKEYREVOKED to searches when what is actually desired is to have the
key refetched.

To invalidate a key the caller must be granted SEARCH permission by the key.
This may be too strict.  It may be better to also permit invalidation if the
caller has any of READ, WRITE or SETATTR permission.

The primary use for this is to evict keys that are cached in special keyrings,
such as the DNS resolver or an ID mapper.

Signed-off-by: David Howells &lt;dhowells@redhat.com&gt;
</content>
</entry>
</feed>
