|
Revision tags: v6.15, v6.15-rc7, v6.15-rc6, v6.15-rc5, v6.15-rc4, v6.15-rc3, v6.15-rc2, v6.15-rc1, v6.14, v6.14-rc7, v6.14-rc6, v6.14-rc5, v6.14-rc4, v6.14-rc3, v6.14-rc2, v6.14-rc1, v6.13, v6.13-rc7, v6.13-rc6, v6.13-rc5, v6.13-rc4, v6.13-rc3, v6.13-rc2, v6.13-rc1, v6.12, v6.12-rc7, v6.12-rc6, v6.12-rc5, v6.12-rc4, v6.12-rc3 |
|
| #
4a74da04 |
| 08-Oct-2024 |
Chen Ridong <[email protected]> |
security/keys: fix slab-out-of-bounds in key_task_permission
KASAN reports an out of bounds read: BUG: KASAN: slab-out-of-bounds in __kuid_val include/linux/uidgid.h:36 BUG: KASAN: slab-out-of-bound
security/keys: fix slab-out-of-bounds in key_task_permission
KASAN reports an out of bounds read: BUG: KASAN: slab-out-of-bounds in __kuid_val include/linux/uidgid.h:36 BUG: KASAN: slab-out-of-bounds in uid_eq include/linux/uidgid.h:63 [inline] BUG: KASAN: slab-out-of-bounds in key_task_permission+0x394/0x410 security/keys/permission.c:54 Read of size 4 at addr ffff88813c3ab618 by task stress-ng/4362
CPU: 2 PID: 4362 Comm: stress-ng Not tainted 5.10.0-14930-gafbffd6c3ede #15 Call Trace: __dump_stack lib/dump_stack.c:82 [inline] dump_stack+0x107/0x167 lib/dump_stack.c:123 print_address_description.constprop.0+0x19/0x170 mm/kasan/report.c:400 __kasan_report.cold+0x6c/0x84 mm/kasan/report.c:560 kasan_report+0x3a/0x50 mm/kasan/report.c:585 __kuid_val include/linux/uidgid.h:36 [inline] uid_eq include/linux/uidgid.h:63 [inline] key_task_permission+0x394/0x410 security/keys/permission.c:54 search_nested_keyrings+0x90e/0xe90 security/keys/keyring.c:793
This issue was also reported by syzbot.
It can be reproduced by following these steps(more details [1]): 1. Obtain more than 32 inputs that have similar hashes, which ends with the pattern '0xxxxxxxe6'. 2. Reboot and add the keys obtained in step 1.
The reproducer demonstrates how this issue happened: 1. In the search_nested_keyrings function, when it iterates through the slots in a node(below tag ascend_to_node), if the slot pointer is meta and node->back_pointer != NULL(it means a root), it will proceed to descend_to_node. However, there is an exception. If node is the root, and one of the slots points to a shortcut, it will be treated as a keyring. 2. Whether the ptr is keyring decided by keyring_ptr_is_keyring function. However, KEYRING_PTR_SUBTYPE is 0x2UL, the same as ASSOC_ARRAY_PTR_SUBTYPE_MASK. 3. When 32 keys with the similar hashes are added to the tree, the ROOT has keys with hashes that are not similar (e.g. slot 0) and it splits NODE A without using a shortcut. When NODE A is filled with keys that all hashes are xxe6, the keys are similar, NODE A will split with a shortcut. Finally, it forms the tree as shown below, where slot 6 points to a shortcut.
NODE A +------>+---+ ROOT | | 0 | xxe6 +---+ | +---+ xxxx | 0 | shortcut : : xxe6 +---+ | +---+ xxe6 : : | | | xxe6 +---+ | +---+ | 6 |---+ : : xxe6 +---+ +---+ xxe6 : : | f | xxe6 +---+ +---+ xxe6 | f | +---+
4. As mentioned above, If a slot(slot 6) of the root points to a shortcut, it may be mistakenly transferred to a key*, leading to a read out-of-bounds read.
To fix this issue, one should jump to descend_to_node if the ptr is a shortcut, regardless of whether the node is root or not.
[1] https://lore.kernel.org/linux-kernel/[email protected]/
[jarkko: tweaked the commit message a bit to have an appropriate closes tag.] Fixes: b2a4df200d57 ("KEYS: Expand the capacity of a keyring") Reported-by: [email protected] Closes: https://lore.kernel.org/all/[email protected]/T/ Signed-off-by: Chen Ridong <[email protected]> Reviewed-by: Jarkko Sakkinen <[email protected]> Signed-off-by: Jarkko Sakkinen <[email protected]>
show more ...
|
|
Revision tags: v6.12-rc2, v6.12-rc1, v6.11, v6.11-rc7, v6.11-rc6, v6.11-rc5, v6.11-rc4, v6.11-rc3, v6.11-rc2, v6.11-rc1, v6.10, v6.10-rc7, v6.10-rc6, v6.10-rc5, v6.10-rc4, v6.10-rc3, v6.10-rc2, v6.10-rc1, v6.9, v6.9-rc7, v6.9-rc6, v6.9-rc5, v6.9-rc4, v6.9-rc3, v6.9-rc2, v6.9-rc1, v6.8, v6.8-rc7, v6.8-rc6, v6.8-rc5, v6.8-rc4, v6.8-rc3, v6.8-rc2, v6.8-rc1, v6.7, v6.7-rc8, v6.7-rc7, v6.7-rc6, v6.7-rc5, v6.7-rc4, v6.7-rc3, v6.7-rc2, v6.7-rc1, v6.6, v6.6-rc7, v6.6-rc6, v6.6-rc5, v6.6-rc4, v6.6-rc3, v6.6-rc2, v6.6-rc1, v6.5, v6.5-rc7, v6.5-rc6, v6.5-rc5, v6.5-rc4, v6.5-rc3, v6.5-rc2, v6.5-rc1, v6.4, v6.4-rc7, v6.4-rc6, v6.4-rc5, v6.4-rc4, v6.4-rc3, v6.4-rc2, v6.4-rc1, v6.3, v6.3-rc7, v6.3-rc6, v6.3-rc5, v6.3-rc4, v6.3-rc3, v6.3-rc2, v6.3-rc1, v6.2, v6.2-rc8, v6.2-rc7, v6.2-rc6, v6.2-rc5, v6.2-rc4, v6.2-rc3, v6.2-rc2, v6.2-rc1, v6.1, v6.1-rc8, v6.1-rc7, v6.1-rc6, v6.1-rc5, v6.1-rc4, v6.1-rc3, v6.1-rc2, v6.1-rc1, v6.0, v6.0-rc7, v6.0-rc6, v6.0-rc5 |
|
| #
72e9be6b |
| 07-Sep-2022 |
Vincenzo Frascino <[email protected]> |
security/keys: Remove inconsistent __user annotation
The declaration of keyring_read does not match the definition (security/keys/keyring.c). In this case the definition is correct because it matche
security/keys: Remove inconsistent __user annotation
The declaration of keyring_read does not match the definition (security/keys/keyring.c). In this case the definition is correct because it matches what defined in "struct key_type::read" (linux/key-type.h).
Fix the declaration removing the inconsistent __user annotation.
Cc: David Howells <[email protected]> Cc: Jarkko Sakkinen <[email protected]> Cc: Paul Moore <[email protected]> Cc: James Morris <[email protected]> Signed-off-by: Vincenzo Frascino <[email protected]> Reviewed-by: Paul Moore <[email protected]> Acked-by: Jarkko Sakkinen <[email protected]> Signed-off-by: Jarkko Sakkinen <[email protected]>
show more ...
|
|
Revision tags: v6.0-rc4, v6.0-rc3, v6.0-rc2, v6.0-rc1, v5.19, v5.19-rc8, v5.19-rc7, v5.19-rc6, v5.19-rc5, v5.19-rc4, v5.19-rc3, v5.19-rc2, v5.19-rc1, v5.18, v5.18-rc7, v5.18-rc6, v5.18-rc5, v5.18-rc4, v5.18-rc3, v5.18-rc2, v5.18-rc1, v5.17, v5.17-rc8, v5.17-rc7, v5.17-rc6, v5.17-rc5, v5.17-rc4, v5.17-rc3, v5.17-rc2, v5.17-rc1, v5.16, v5.16-rc8, v5.16-rc7, v5.16-rc6, v5.16-rc5, v5.16-rc4, v5.16-rc3, v5.16-rc2, v5.16-rc1, v5.15, v5.15-rc7, v5.15-rc6, v5.15-rc5, v5.15-rc4, v5.15-rc3, v5.15-rc2, v5.15-rc1, v5.14, v5.14-rc7, v5.14-rc6, v5.14-rc5, v5.14-rc4, v5.14-rc3, v5.14-rc2, v5.14-rc1, v5.13, v5.13-rc7, v5.13-rc6, v5.13-rc5, v5.13-rc4, v5.13-rc3, v5.13-rc2, v5.13-rc1, v5.12, v5.12-rc8, v5.12-rc7, v5.12-rc6, v5.12-rc5, v5.12-rc4, v5.12-rc3, v5.12-rc2, v5.12-rc1, v5.12-rc1-dontuse, v5.11, v5.11-rc7, v5.11-rc6, v5.11-rc5, v5.11-rc4, v5.11-rc3, v5.11-rc2, v5.11-rc1, v5.10, v5.10-rc7, v5.10-rc6, v5.10-rc5, v5.10-rc4, v5.10-rc3, v5.10-rc2, v5.10-rc1, v5.9, v5.9-rc8, v5.9-rc7, v5.9-rc6, v5.9-rc5, v5.9-rc4, v5.9-rc3, v5.9-rc2, v5.9-rc1 |
|
| #
328c95db |
| 07-Aug-2020 |
Randy Dunlap <[email protected]> |
security: keys: delete repeated words in comments
Drop repeated words in comments. {to, will, the}
Signed-off-by: Randy Dunlap <[email protected]> Signed-off-by: David Howells <dhowells@redhat.
security: keys: delete repeated words in comments
Drop repeated words in comments. {to, will, the}
Signed-off-by: Randy Dunlap <[email protected]> Signed-off-by: David Howells <[email protected]> Reviewed-by: Jarkko Sakkinen <[email protected]> Reviewed-by: Ben Boeckel <[email protected]> Cc: [email protected] Cc: James Morris <[email protected]> Cc: "Serge E. Hallyn" <[email protected]> Cc: [email protected]
show more ...
|
| #
796e46f9 |
| 23-Nov-2020 |
Jann Horn <[email protected]> |
keys: Remove outdated __user annotations
When the semantics of the ->read() handlers were changed such that "buffer" is a kernel pointer, some __user annotations survived. Since they're wrong now, g
keys: Remove outdated __user annotations
When the semantics of the ->read() handlers were changed such that "buffer" is a kernel pointer, some __user annotations survived. Since they're wrong now, get rid of them.
Fixes: d3ec10aa9581 ("KEYS: Don't write out to userspace while holding key semaphore") Signed-off-by: Jann Horn <[email protected]> Signed-off-by: David Howells <[email protected]> Reviewed-by: Ben Boeckel <[email protected]>
show more ...
|
|
Revision tags: v5.8, v5.8-rc7, v5.8-rc6, v5.8-rc5, v5.8-rc4, v5.8-rc3, v5.8-rc2, v5.8-rc1, v5.7, v5.7-rc7, v5.7-rc6, v5.7-rc5, v5.7-rc4, v5.7-rc3, v5.7-rc2, v5.7-rc1, v5.6, v5.6-rc7, v5.6-rc6, v5.6-rc5, v5.6-rc4, v5.6-rc3, v5.6-rc2, v5.6-rc1, v5.5, v5.5-rc7 |
|
| #
f7e47677 |
| 14-Jan-2020 |
David Howells <[email protected]> |
watch_queue: Add a key/keyring notification facility
Add a key/keyring change notification facility whereby notifications about changes in key and keyring content and attributes can be received.
Fi
watch_queue: Add a key/keyring notification facility
Add a key/keyring change notification facility whereby notifications about changes in key and keyring content and attributes can be received.
Firstly, an event queue needs to be created:
pipe2(fds, O_NOTIFICATION_PIPE); ioctl(fds[1], IOC_WATCH_QUEUE_SET_SIZE, 256);
then a notification can be set up to report notifications via that queue:
struct watch_notification_filter filter = { .nr_filters = 1, .filters = { [0] = { .type = WATCH_TYPE_KEY_NOTIFY, .subtype_filter[0] = UINT_MAX, }, }, }; ioctl(fds[1], IOC_WATCH_QUEUE_SET_FILTER, &filter); keyctl_watch_key(KEY_SPEC_SESSION_KEYRING, fds[1], 0x01);
After that, records will be placed into the queue when events occur in which keys are changed in some way. Records are of the following format:
struct key_notification { struct watch_notification watch; __u32 key_id; __u32 aux; } *n;
Where:
n->watch.type will be WATCH_TYPE_KEY_NOTIFY.
n->watch.subtype will indicate the type of event, such as NOTIFY_KEY_REVOKED.
n->watch.info & WATCH_INFO_LENGTH will indicate the length of the record.
n->watch.info & WATCH_INFO_ID will be the second argument to keyctl_watch_key(), shifted.
n->key will be the ID of the affected key.
n->aux will hold subtype-dependent information, such as the key being linked into the keyring specified by n->key in the case of NOTIFY_KEY_LINKED.
Note that it is permissible for event records to be of variable length - or, at least, the length may be dependent on the subtype. Note also that the queue can be shared between multiple notifications of various types.
Signed-off-by: David Howells <[email protected]> Reviewed-by: James Morris <[email protected]>
show more ...
|
| #
d3ec10aa |
| 22-Mar-2020 |
Waiman Long <[email protected]> |
KEYS: Don't write out to userspace while holding key semaphore
A lockdep circular locking dependency report was seen when running a keyutils test:
[12537.027242] ===================================
KEYS: Don't write out to userspace while holding key semaphore
A lockdep circular locking dependency report was seen when running a keyutils test:
[12537.027242] ====================================================== [12537.059309] WARNING: possible circular locking dependency detected [12537.088148] 4.18.0-147.7.1.el8_1.x86_64+debug #1 Tainted: G OE --------- - - [12537.125253] ------------------------------------------------------ [12537.153189] keyctl/25598 is trying to acquire lock: [12537.175087] 000000007c39f96c (&mm->mmap_sem){++++}, at: __might_fault+0xc4/0x1b0 [12537.208365] [12537.208365] but task is already holding lock: [12537.234507] 000000003de5b58d (&type->lock_class){++++}, at: keyctl_read_key+0x15a/0x220 [12537.270476] [12537.270476] which lock already depends on the new lock. [12537.270476] [12537.307209] [12537.307209] the existing dependency chain (in reverse order) is: [12537.340754] [12537.340754] -> #3 (&type->lock_class){++++}: [12537.367434] down_write+0x4d/0x110 [12537.385202] __key_link_begin+0x87/0x280 [12537.405232] request_key_and_link+0x483/0xf70 [12537.427221] request_key+0x3c/0x80 [12537.444839] dns_query+0x1db/0x5a5 [dns_resolver] [12537.468445] dns_resolve_server_name_to_ip+0x1e1/0x4d0 [cifs] [12537.496731] cifs_reconnect+0xe04/0x2500 [cifs] [12537.519418] cifs_readv_from_socket+0x461/0x690 [cifs] [12537.546263] cifs_read_from_socket+0xa0/0xe0 [cifs] [12537.573551] cifs_demultiplex_thread+0x311/0x2db0 [cifs] [12537.601045] kthread+0x30c/0x3d0 [12537.617906] ret_from_fork+0x3a/0x50 [12537.636225] [12537.636225] -> #2 (root_key_user.cons_lock){+.+.}: [12537.664525] __mutex_lock+0x105/0x11f0 [12537.683734] request_key_and_link+0x35a/0xf70 [12537.705640] request_key+0x3c/0x80 [12537.723304] dns_query+0x1db/0x5a5 [dns_resolver] [12537.746773] dns_resolve_server_name_to_ip+0x1e1/0x4d0 [cifs] [12537.775607] cifs_reconnect+0xe04/0x2500 [cifs] [12537.798322] cifs_readv_from_socket+0x461/0x690 [cifs] [12537.823369] cifs_read_from_socket+0xa0/0xe0 [cifs] [12537.847262] cifs_demultiplex_thread+0x311/0x2db0 [cifs] [12537.873477] kthread+0x30c/0x3d0 [12537.890281] ret_from_fork+0x3a/0x50 [12537.908649] [12537.908649] -> #1 (&tcp_ses->srv_mutex){+.+.}: [12537.935225] __mutex_lock+0x105/0x11f0 [12537.954450] cifs_call_async+0x102/0x7f0 [cifs] [12537.977250] smb2_async_readv+0x6c3/0xc90 [cifs] [12538.000659] cifs_readpages+0x120a/0x1e50 [cifs] [12538.023920] read_pages+0xf5/0x560 [12538.041583] __do_page_cache_readahead+0x41d/0x4b0 [12538.067047] ondemand_readahead+0x44c/0xc10 [12538.092069] filemap_fault+0xec1/0x1830 [12538.111637] __do_fault+0x82/0x260 [12538.129216] do_fault+0x419/0xfb0 [12538.146390] __handle_mm_fault+0x862/0xdf0 [12538.167408] handle_mm_fault+0x154/0x550 [12538.187401] __do_page_fault+0x42f/0xa60 [12538.207395] do_page_fault+0x38/0x5e0 [12538.225777] page_fault+0x1e/0x30 [12538.243010] [12538.243010] -> #0 (&mm->mmap_sem){++++}: [12538.267875] lock_acquire+0x14c/0x420 [12538.286848] __might_fault+0x119/0x1b0 [12538.306006] keyring_read_iterator+0x7e/0x170 [12538.327936] assoc_array_subtree_iterate+0x97/0x280 [12538.352154] keyring_read+0xe9/0x110 [12538.370558] keyctl_read_key+0x1b9/0x220 [12538.391470] do_syscall_64+0xa5/0x4b0 [12538.410511] entry_SYSCALL_64_after_hwframe+0x6a/0xdf [12538.435535] [12538.435535] other info that might help us debug this: [12538.435535] [12538.472829] Chain exists of: [12538.472829] &mm->mmap_sem --> root_key_user.cons_lock --> &type->lock_class [12538.472829] [12538.524820] Possible unsafe locking scenario: [12538.524820] [12538.551431] CPU0 CPU1 [12538.572654] ---- ---- [12538.595865] lock(&type->lock_class); [12538.613737] lock(root_key_user.cons_lock); [12538.644234] lock(&type->lock_class); [12538.672410] lock(&mm->mmap_sem); [12538.687758] [12538.687758] *** DEADLOCK *** [12538.687758] [12538.714455] 1 lock held by keyctl/25598: [12538.732097] #0: 000000003de5b58d (&type->lock_class){++++}, at: keyctl_read_key+0x15a/0x220 [12538.770573] [12538.770573] stack backtrace: [12538.790136] CPU: 2 PID: 25598 Comm: keyctl Kdump: loaded Tainted: G [12538.844855] Hardware name: HP ProLiant DL360 Gen9/ProLiant DL360 Gen9, BIOS P89 12/27/2015 [12538.881963] Call Trace: [12538.892897] dump_stack+0x9a/0xf0 [12538.907908] print_circular_bug.isra.25.cold.50+0x1bc/0x279 [12538.932891] ? save_trace+0xd6/0x250 [12538.948979] check_prev_add.constprop.32+0xc36/0x14f0 [12538.971643] ? keyring_compare_object+0x104/0x190 [12538.992738] ? check_usage+0x550/0x550 [12539.009845] ? sched_clock+0x5/0x10 [12539.025484] ? sched_clock_cpu+0x18/0x1e0 [12539.043555] __lock_acquire+0x1f12/0x38d0 [12539.061551] ? trace_hardirqs_on+0x10/0x10 [12539.080554] lock_acquire+0x14c/0x420 [12539.100330] ? __might_fault+0xc4/0x1b0 [12539.119079] __might_fault+0x119/0x1b0 [12539.135869] ? __might_fault+0xc4/0x1b0 [12539.153234] keyring_read_iterator+0x7e/0x170 [12539.172787] ? keyring_read+0x110/0x110 [12539.190059] assoc_array_subtree_iterate+0x97/0x280 [12539.211526] keyring_read+0xe9/0x110 [12539.227561] ? keyring_gc_check_iterator+0xc0/0xc0 [12539.249076] keyctl_read_key+0x1b9/0x220 [12539.266660] do_syscall_64+0xa5/0x4b0 [12539.283091] entry_SYSCALL_64_after_hwframe+0x6a/0xdf
One way to prevent this deadlock scenario from happening is to not allow writing to userspace while holding the key semaphore. Instead, an internal buffer is allocated for getting the keys out from the read method first before copying them out to userspace without holding the lock.
That requires taking out the __user modifier from all the relevant read methods as well as additional changes to not use any userspace write helpers. That is,
1) The put_user() call is replaced by a direct copy. 2) The copy_to_user() call is replaced by memcpy(). 3) All the fault handling code is removed.
Compiling on a x86-64 system, the size of the rxrpc_read() function is reduced from 3795 bytes to 2384 bytes with this patch.
Fixes: ^1da177e4c3f4 ("Linux-2.6.12-rc2") Reviewed-by: Jarkko Sakkinen <[email protected]> Signed-off-by: Waiman Long <[email protected]> Signed-off-by: David Howells <[email protected]>
show more ...
|
|
Revision tags: v5.5-rc6, v5.5-rc5, v5.5-rc4, v5.5-rc3, v5.5-rc2, v5.5-rc1, v5.4, v5.4-rc8, v5.4-rc7, v5.4-rc6, v5.4-rc5, v5.4-rc4, v5.4-rc3, v5.4-rc2, v5.4-rc1, v5.3, v5.3-rc8, v5.3-rc7, v5.3-rc6, v5.3-rc5, v5.3-rc4, v5.3-rc3, v5.3-rc2, v5.3-rc1 |
|
| #
028db3e2 |
| 11-Jul-2019 |
Linus Torvalds <[email protected]> |
Revert "Merge tag 'keys-acl-20190703' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs"
This reverts merge 0f75ef6a9cff49ff612f7ce0578bced9d0b38325 (and thus effectively commits
Revert "Merge tag 'keys-acl-20190703' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs"
This reverts merge 0f75ef6a9cff49ff612f7ce0578bced9d0b38325 (and thus effectively commits
7a1ade847596 ("keys: Provide KEYCTL_GRANT_PERMISSION") 2e12256b9a76 ("keys: Replace uid/gid/perm permissions checking with an ACL")
that the merge brought in).
It turns out that it breaks booting with an encrypted volume, and Eric biggers reports that it also breaks the fscrypt tests [1] and loading of in-kernel X.509 certificates [2].
The root cause of all the breakage is likely the same, but David Howells is off email so rather than try to work it out it's getting reverted in order to not impact the rest of the merge window.
[1] https://lore.kernel.org/lkml/[email protected]/ [2] https://lore.kernel.org/lkml/[email protected]/
Link: https://lore.kernel.org/lkml/CAHk-=wjxoeMJfeBahnWH=9zShKp2bsVy527vo3_y8HfOdhwAAw@mail.gmail.com/ Reported-by: Eric Biggers <[email protected]> Cc: David Howells <[email protected]> Cc: James Morris <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
show more ...
|
|
Revision tags: v5.2, v5.2-rc7 |
|
| #
2e12256b |
| 27-Jun-2019 |
David Howells <[email protected]> |
keys: Replace uid/gid/perm permissions checking with an ACL
Replace the uid/gid/perm permissions checking on a key with an ACL to allow the SETATTR and SEARCH permissions to be split. This will als
keys: Replace uid/gid/perm permissions checking with an ACL
Replace the uid/gid/perm permissions checking on a key with an ACL to allow the SETATTR and SEARCH permissions to be split. This will also allow a greater range of subjects to represented.
============ WHY DO THIS? ============
The problem is that SETATTR and SEARCH cover a slew of actions, not all of which should be grouped together.
For SETATTR, this includes actions that are about controlling access to a key:
(1) Changing a key's ownership.
(2) Changing a key's security information.
(3) Setting a keyring's restriction.
And actions that are about managing a key's lifetime:
(4) Setting an expiry time.
(5) Revoking a key.
and (proposed) managing a key as part of a cache:
(6) Invalidating a key.
Managing a key's lifetime doesn't really have anything to do with controlling access to that key.
Expiry time is awkward since it's more about the lifetime of the content and so, in some ways goes better with WRITE permission. It can, however, be set unconditionally by a process with an appropriate authorisation token for instantiating a key, and can also be set by the key type driver when a key is instantiated, so lumping it with the access-controlling actions is probably okay.
As for SEARCH permission, that currently covers:
(1) Finding keys in a keyring tree during a search.
(2) Permitting keyrings to be joined.
(3) Invalidation.
But these don't really belong together either, since these actions really need to be controlled separately.
Finally, there are number of special cases to do with granting the administrator special rights to invalidate or clear keys that I would like to handle with the ACL rather than key flags and special checks.
=============== WHAT IS CHANGED ===============
The SETATTR permission is split to create two new permissions:
(1) SET_SECURITY - which allows the key's owner, group and ACL to be changed and a restriction to be placed on a keyring.
(2) REVOKE - which allows a key to be revoked.
The SEARCH permission is split to create:
(1) SEARCH - which allows a keyring to be search and a key to be found.
(2) JOIN - which allows a keyring to be joined as a session keyring.
(3) INVAL - which allows a key to be invalidated.
The WRITE permission is also split to create:
(1) WRITE - which allows a key's content to be altered and links to be added, removed and replaced in a keyring.
(2) CLEAR - which allows a keyring to be cleared completely. This is split out to make it possible to give just this to an administrator.
(3) REVOKE - see above.
Keys acquire ACLs which consist of a series of ACEs, and all that apply are unioned together. An ACE specifies a subject, such as:
(*) Possessor - permitted to anyone who 'possesses' a key (*) Owner - permitted to the key owner (*) Group - permitted to the key group (*) Everyone - permitted to everyone
Note that 'Other' has been replaced with 'Everyone' on the assumption that you wouldn't grant a permit to 'Other' that you wouldn't also grant to everyone else.
Further subjects may be made available by later patches.
The ACE also specifies a permissions mask. The set of permissions is now:
VIEW Can view the key metadata READ Can read the key content WRITE Can update/modify the key content SEARCH Can find the key by searching/requesting LINK Can make a link to the key SET_SECURITY Can change owner, ACL, expiry INVAL Can invalidate REVOKE Can revoke JOIN Can join this keyring CLEAR Can clear this keyring
The KEYCTL_SETPERM function is then deprecated.
The KEYCTL_SET_TIMEOUT function then is permitted if SET_SECURITY is set, or if the caller has a valid instantiation auth token.
The KEYCTL_INVALIDATE function then requires INVAL.
The KEYCTL_REVOKE function then requires REVOKE.
The KEYCTL_JOIN_SESSION_KEYRING function then requires JOIN to join an existing keyring.
The JOIN permission is enabled by default for session keyrings and manually created keyrings only.
====================== BACKWARD COMPATIBILITY ======================
To maintain backward compatibility, KEYCTL_SETPERM will translate the permissions mask it is given into a new ACL for a key - unless KEYCTL_SET_ACL has been called on that key, in which case an error will be returned.
It will convert possessor, owner, group and other permissions into separate ACEs, if each portion of the mask is non-zero.
SETATTR permission turns on all of INVAL, REVOKE and SET_SECURITY. WRITE permission turns on WRITE, REVOKE and, if a keyring, CLEAR. JOIN is turned on if a keyring is being altered.
The KEYCTL_DESCRIBE function translates the ACL back into a permissions mask to return depending on possessor, owner, group and everyone ACEs.
It will make the following mappings:
(1) INVAL, JOIN -> SEARCH
(2) SET_SECURITY -> SETATTR
(3) REVOKE -> WRITE if SETATTR isn't already set
(4) CLEAR -> WRITE
Note that the value subsequently returned by KEYCTL_DESCRIBE may not match the value set with KEYCTL_SETATTR.
======= TESTING =======
This passes the keyutils testsuite for all but a couple of tests:
(1) tests/keyctl/dh_compute/badargs: The first wrong-key-type test now returns EOPNOTSUPP rather than ENOKEY as READ permission isn't removed if the type doesn't have ->read(). You still can't actually read the key.
(2) tests/keyctl/permitting/valid: The view-other-permissions test doesn't work as Other has been replaced with Everyone in the ACL.
Signed-off-by: David Howells <[email protected]>
show more ...
|
| #
a58946c1 |
| 26-Jun-2019 |
David Howells <[email protected]> |
keys: Pass the network namespace into request_key mechanism
Create a request_key_net() function and use it to pass the network namespace domain tag into DNS revolver keys and rxrpc/AFS keys so that
keys: Pass the network namespace into request_key mechanism
Create a request_key_net() function and use it to pass the network namespace domain tag into DNS revolver keys and rxrpc/AFS keys so that keys for different domains can coexist in the same keyring.
Signed-off-by: David Howells <[email protected]> cc: [email protected] cc: [email protected] cc: [email protected] cc: [email protected]
show more ...
|
| #
9b242610 |
| 26-Jun-2019 |
David Howells <[email protected]> |
keys: Network namespace domain tag
Create key domain tags for network namespaces and make it possible to automatically tag keys that are used by networked services (e.g. AF_RXRPC, AFS, DNS) with the
keys: Network namespace domain tag
Create key domain tags for network namespaces and make it possible to automatically tag keys that are used by networked services (e.g. AF_RXRPC, AFS, DNS) with the default network namespace if not set by the caller.
This allows keys with the same description but in different namespaces to coexist within a keyring.
Signed-off-by: David Howells <[email protected]> cc: [email protected] cc: [email protected] cc: [email protected] cc: [email protected]
show more ...
|
| #
218e6424 |
| 26-Jun-2019 |
David Howells <[email protected]> |
keys: Garbage collect keys for which the domain has been removed
If a key operation domain (such as a network namespace) has been removed then attempt to garbage collect all the keys that use it.
S
keys: Garbage collect keys for which the domain has been removed
If a key operation domain (such as a network namespace) has been removed then attempt to garbage collect all the keys that use it.
Signed-off-by: David Howells <[email protected]>
show more ...
|
| #
3b6e4de0 |
| 26-Jun-2019 |
David Howells <[email protected]> |
keys: Include target namespace in match criteria
Currently a key has a standard matching criteria of { type, description } and this is used to only allow keys with unique criteria in a keyring. This
keys: Include target namespace in match criteria
Currently a key has a standard matching criteria of { type, description } and this is used to only allow keys with unique criteria in a keyring. This means, however, that you cannot have keys with the same type and description but a different target namespace in the same keyring.
This is a potential problem for a containerised environment where, say, a container is made up of some parts of its mount space involving netfs superblocks from two different network namespaces.
This is also a problem for shared system management keyrings such as the DNS records keyring or the NFS idmapper keyring that might contain keys from different network namespaces.
Fix this by including a namespace component in a key's matching criteria. Keyring types are marked to indicate which, if any, namespace is relevant to keys of that type, and that namespace is set when the key is created from the current task's namespace set.
The capability bit KEYCTL_CAPS1_NS_KEY_TAG is set if the kernel is employing this feature.
Signed-off-by: David Howells <[email protected]>
show more ...
|
| #
0f44e4d9 |
| 26-Jun-2019 |
David Howells <[email protected]> |
keys: Move the user and user-session keyrings to the user_namespace
Move the user and user-session keyrings to the user_namespace struct rather than pinning them from the user_struct struct. This p
keys: Move the user and user-session keyrings to the user_namespace
Move the user and user-session keyrings to the user_namespace struct rather than pinning them from the user_struct struct. This prevents these keyrings from propagating across user-namespaces boundaries with regard to the KEY_SPEC_* flags, thereby making them more useful in a containerised environment.
The issue is that a single user_struct may be represent UIDs in several different namespaces.
The way the patch does this is by attaching a 'register keyring' in each user_namespace and then sticking the user and user-session keyrings into that. It can then be searched to retrieve them.
Signed-off-by: David Howells <[email protected]> cc: Jann Horn <[email protected]>
show more ...
|
| #
b206f281 |
| 26-Jun-2019 |
David Howells <[email protected]> |
keys: Namespace keyring names
Keyring names are held in a single global list that any process can pick from by means of keyctl_join_session_keyring (provided the keyring grants Search permission).
keys: Namespace keyring names
Keyring names are held in a single global list that any process can pick from by means of keyctl_join_session_keyring (provided the keyring grants Search permission). This isn't very container friendly, however.
Make the following changes:
(1) Make default session, process and thread keyring names begin with a '.' instead of '_'.
(2) Keyrings whose names begin with a '.' aren't added to the list. Such keyrings are system specials.
(3) Replace the global list with per-user_namespace lists. A keyring adds its name to the list for the user_namespace that it is currently in.
(4) When a user_namespace is deleted, it just removes itself from the keyring name list.
The global keyring_name_lock is retained for accessing the name lists. This allows (4) to work.
This can be tested by:
# keyctl newring foo @s 995906392 # unshare -U $ keyctl show ... 995906392 --alswrv 65534 65534 \_ keyring: foo ... $ keyctl session foo Joined session keyring: 935622349
As can be seen, a new session keyring was created.
The capability bit KEYCTL_CAPS1_NS_KEYRING_NAME is set if the kernel is employing this feature.
Signed-off-by: David Howells <[email protected]> cc: Eric W. Biederman <[email protected]>
show more ...
|
| #
dcf49dbc |
| 26-Jun-2019 |
David Howells <[email protected]> |
keys: Add a 'recurse' flag for keyring searches
Add a 'recurse' flag for keyring searches so that the flag can be omitted and recursion disabled, thereby allowing just the nominated keyring to be se
keys: Add a 'recurse' flag for keyring searches
Add a 'recurse' flag for keyring searches so that the flag can be omitted and recursion disabled, thereby allowing just the nominated keyring to be searched and none of the children.
Signed-off-by: David Howells <[email protected]>
show more ...
|
| #
355ef8e1 |
| 26-Jun-2019 |
David Howells <[email protected]> |
keys: Cache the hash value to avoid lots of recalculation
Cache the hash of the key's type and description in the index key so that we're not recalculating it every time we look at a key during a se
keys: Cache the hash value to avoid lots of recalculation
Cache the hash of the key's type and description in the index key so that we're not recalculating it every time we look at a key during a search. The hash function does a bunch of multiplications, so evading those is probably worthwhile - especially as this is done for every key examined during a search.
This also allows the methods used by assoc_array to get chunks of index-key to be simplified.
Signed-off-by: David Howells <[email protected]>
show more ...
|
| #
f771fde8 |
| 26-Jun-2019 |
David Howells <[email protected]> |
keys: Simplify key description management
Simplify key description management by cramming the word containing the length with the first few chars of the description also. This simplifies the code t
keys: Simplify key description management
Simplify key description management by cramming the word containing the length with the first few chars of the description also. This simplifies the code that generates the index-key used by assoc_array. It should speed up key searching a bit too.
Signed-off-by: David Howells <[email protected]>
show more ...
|
|
Revision tags: v5.2-rc6 |
|
| #
e59428f7 |
| 19-Jun-2019 |
David Howells <[email protected]> |
keys: Move the RCU locks outwards from the keyring search functions
Move the RCU locks outwards from the keyring search functions so that it will become possible to provide an RCU-capable partial re
keys: Move the RCU locks outwards from the keyring search functions
Move the RCU locks outwards from the keyring search functions so that it will become possible to provide an RCU-capable partial request_key() function in a later commit.
Signed-off-by: David Howells <[email protected]>
show more ...
|
|
Revision tags: v5.2-rc5, v5.2-rc4, v5.2-rc3, v5.2-rc2 |
|
| #
ed0ac5c7 |
| 20-May-2019 |
David Howells <[email protected]> |
keys: Add a keyctl to move a key between keyrings
Add a keyctl to atomically move a link to a key from one keyring to another. The key must exist in "from" keyring and a flag can be given to cause
keys: Add a keyctl to move a key between keyrings
Add a keyctl to atomically move a link to a key from one keyring to another. The key must exist in "from" keyring and a flag can be given to cause the operation to fail if there's a matching key already in the "to" keyring.
This can be done with:
keyctl(KEYCTL_MOVE, key_serial_t key, key_serial_t from_keyring, key_serial_t to_keyring, unsigned int flags);
The key being moved must grant Link permission and both keyrings must grant Write permission.
flags should be 0 or KEYCTL_MOVE_EXCL, with the latter preventing displacement of a matching key from the "to" keyring.
Signed-off-by: David Howells <[email protected]>
show more ...
|
| #
df593ee2 |
| 30-May-2019 |
David Howells <[email protected]> |
keys: Hoist locking out of __key_link_begin()
Hoist the locking of out of __key_link_begin() and into its callers. This is necessary to allow the upcoming key_move() operation to correctly order ta
keys: Hoist locking out of __key_link_begin()
Hoist the locking of out of __key_link_begin() and into its callers. This is necessary to allow the upcoming key_move() operation to correctly order taking of the source keyring semaphore, the destination keyring semaphore and the keyring serialisation lock.
Signed-off-by: David Howells <[email protected]>
show more ...
|
| #
2874c5fd |
| 27-May-2019 |
Thomas Gleixner <[email protected]> |
treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 152
Based on 1 normalized pattern(s):
this program is free software you can redistribute it and or modify it under the terms of th
treewide: Replace GPLv2 boilerplate/reference with SPDX - rule 152
Based on 1 normalized pattern(s):
this program is free software you can redistribute it and or modify it under the terms of the gnu general public license as published by the free software foundation either version 2 of the license or at your option any later version
extracted by the scancode license scanner the SPDX license identifier
GPL-2.0-or-later
has been chosen to replace the boilerplate/reference in 3029 file(s).
Signed-off-by: Thomas Gleixner <[email protected]> Reviewed-by: Allison Randal <[email protected]> Cc: [email protected] Link: https://lkml.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
show more ...
|
| #
eb0f68cb |
| 30-May-2019 |
David Howells <[email protected]> |
keys: Break bits out of key_unlink()
Break bits out of key_unlink() into helper functions so that they can be used in implementing key_move().
Signed-off-by: David Howells <[email protected]>
|
| #
3be59f74 |
| 30-May-2019 |
David Howells <[email protected]> |
keys: Change keyring_serialise_link_sem to a mutex
Change keyring_serialise_link_sem to a mutex as it's only ever write-locked.
Signed-off-by: David Howells <[email protected]>
|
| #
9fd16537 |
| 22-May-2019 |
David Howells <[email protected]> |
keys: sparse: Fix kdoc mismatches
Fix some kdoc argument description mismatches reported by sparse and give keyring_restrict() a description.
Signed-off-by: David Howells <[email protected]> Revi
keys: sparse: Fix kdoc mismatches
Fix some kdoc argument description mismatches reported by sparse and give keyring_restrict() a description.
Signed-off-by: David Howells <[email protected]> Reviewed-by: James Morris <[email protected]> cc: Mat Martineau <[email protected]>
show more ...
|
|
Revision tags: v5.2-rc1, v5.1, v5.1-rc7, v5.1-rc6, v5.1-rc5, v5.1-rc4, v5.1-rc3, v5.1-rc2, v5.1-rc1, v5.0, v5.0-rc8 |
|
| #
ede0fa98 |
| 22-Feb-2019 |
Eric Biggers <[email protected]> |
KEYS: always initialize keyring_index_key::desc_len
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 l
KEYS: always initialize keyring_index_key::desc_len
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 <stdlib.h> #include <unistd.h> #include <keyutils.h>
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: [email protected] Fixes: b2a4df200d57 ("KEYS: Expand the capacity of a keyring") Signed-off-by: Eric Biggers <[email protected]> Signed-off-by: David Howells <[email protected]> Cc: [email protected] Signed-off-by: James Morris <[email protected]>
show more ...
|