summaryrefslogtreecommitdiff
path: root/security
AgeCommit message (Collapse)Author
2018-12-01ima: re-initialize iint->atomic_flagsMimi Zohar
commit e2598077dc6a26c9644393e5c21f22a90dbdccdb upstream. Intermittently security.ima is not being written for new files. This patch re-initializes the new slab iint->atomic_flags field before freeing it. Fixes: commit 0d73a55208e9 ("ima: re-introduce own integrity cache lock") Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Signed-off-by: James Morris <jmorris@namei.org> Cc: Aditya Kali <adityakali@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-12-01ima: re-introduce own integrity cache lockDmitry Kasatkin
commit 0d73a55208e94fc9fb6deaeea61438cd3280d4c0 upstream. Before IMA appraisal was introduced, IMA was using own integrity cache lock along with i_mutex. process_measurement and ima_file_free took the iint->mutex first and then the i_mutex, while setxattr, chmod and chown took the locks in reverse order. To resolve the potential deadlock, i_mutex was moved to protect entire IMA functionality and the redundant iint->mutex was eliminated. Solution was based on the assumption that filesystem code does not take i_mutex further. But when file is opened with O_DIRECT flag, direct-io implementation takes i_mutex and produces deadlock. Furthermore, certain other filesystem operations, such as llseek, also take i_mutex. More recently some filesystems have replaced their filesystem specific lock with the global i_rwsem to read a file. As a result, when IMA attempts to calculate the file hash, reading the file attempts to take the i_rwsem again. To resolve O_DIRECT related deadlock problem, this patch re-introduces iint->mutex. But to eliminate the original chmod() related deadlock problem, this patch eliminates the requirement for chmod hooks to take the iint->mutex by introducing additional atomic iint->attr_flags to indicate calling of the hooks. The allowed locking order is to take the iint->mutex first and then the i_rwsem. Original flags were cleared in chmod(), setxattr() or removwxattr() hooks and tested when file was closed or opened again. New atomic flags are set or cleared in those hooks and tested to clear iint->flags on close or on open. Atomic flags are following: * IMA_CHANGE_ATTR - indicates that chATTR() was called (chmod, chown, chgrp) and file attributes have changed. On file open, it causes IMA to clear iint->flags to re-evaluate policy and perform IMA functions again. * IMA_CHANGE_XATTR - indicates that setxattr or removexattr was called and extended attributes have changed. On file open, it causes IMA to clear iint->flags IMA_DONE_MASK to re-appraise. * IMA_UPDATE_XATTR - indicates that security.ima needs to be updated. It is cleared if file policy changes and no update is needed. * IMA_DIGSIG - indicates that file security.ima has signature and file security.ima must not update to file has on file close. * IMA_MUST_MEASURE - indicates the file is in the measurement policy. Fixes: Commit 6552321831dc ("xfs: remove i_iolock and use i_rwsem in the VFS inode instead") Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Cc: Aditya Kali <adityakali@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-12-01EVM: Add support for portable signature formatMatthew Garrett
commit 50b977481fce90aa5fbda55e330b9d722733e358 upstream. The EVM signature includes the inode number and (optionally) the filesystem UUID, making it impractical to ship EVM signatures in packages. This patch adds a new portable format intended to allow distributions to include EVM signatures. It is identical to the existing format but hardcodes the inode and generation numbers to 0 and does not include the filesystem UUID even if the kernel is configured to do so. Removing the inode means that the metadata and signature from one file could be copied to another file without invalidating it. This is avoided by ensuring that an IMA xattr is present during EVM validation. Portable signatures are intended to be immutable - ie, they will never be transformed into HMACs. Based on earlier work by Dmitry Kasatkin and Mikhail Kurinnoi. Signed-off-by: Matthew Garrett <mjg59@google.com> Cc: Dmitry Kasatkin <dmitry.kasatkin@huawei.com> Cc: Mikhail Kurinnoi <viewizard@viewizard.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Cc: Aditya Kali <adityakali@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-12-01ima: always measure and audit files in policyMimi Zohar
commit f3cc6b25dcc5616f0d5c720009b2ac66f97df2ff upstream. All files matching a "measure" rule must be included in the IMA measurement list, even when the file hash cannot be calculated. Similarly, all files matching an "audit" rule must be audited, even when the file hash can not be calculated. The file data hash field contained in the IMA measurement list template data will contain 0's instead of the actual file hash digest. Note: In general, adding, deleting or in anyway changing which files are included in the IMA measurement list is not a good idea, as it might result in not being able to unseal trusted keys sealed to a specific TPM PCR value. This patch not only adds file measurements that were not previously measured, but specifies that the file hash value for these files will be 0's. As the IMA measurement list ordering is not consistent from one boot to the next, it is unlikely that anyone is sealing keys based on the IMA measurement list. Remote attestation servers should be able to process these new measurement records, but might complain about these unknown records. Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Reviewed-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com> Cc: Aditya Kali <adityakali@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-12-01Revert "evm: Translate user/group ids relative to s_user_ns when computing HMAC"Eric W. Biederman
commit 19339c251607a3defc7f089511ce8561936fee45 upstream. This reverts commit 0b3c9761d1e405514a551ed24d3ea89aea26ce14. Seth Forshee <seth.forshee@canonical.com> writes: > All right, I think 0b3c9761d1e405514a551ed24d3ea89aea26ce14 should be > reverted then. EVM is a machine-local integrity mechanism, and so it > makes sense that the signature would be based on the kernel's notion of > the uid and not the filesystem's. I added a commment explaining why the EVM hmac needs to be in the kernel's notion of uid and gid, not the filesystems to prevent remounting the filesystem and gaining unwaranted trust in files. Acked-by: Seth Forshee <seth.forshee@canonical.com> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> Cc: Aditya Kali <adityakali@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-12-01selinux: Add __GFP_NOWARN to allocation at str_read()Tetsuo Handa
commit 4458bba09788e70e8fb39ad003f087cd9dfbd6ac upstream. syzbot is hitting warning at str_read() [1] because len parameter can become larger than KMALLOC_MAX_SIZE. We don't need to emit warning for this case. [1] https://syzkaller.appspot.com/bug?id=7f2f5aad79ea8663c296a2eedb81978401a908f0 Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Reported-by: syzbot <syzbot+ac488b9811036cea7ea0@syzkaller.appspotmail.com> Signed-off-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-11-13ima: fix showing large 'violations' or 'runtime_measurements_count'Eric Biggers
commit 1e4c8dafbb6bf72fb5eca035b861e39c5896c2b7 upstream. The 12 character temporary buffer is not necessarily long enough to hold a 'long' value. Increase it. Signed-off-by: Eric Biggers <ebiggers@google.com> Cc: stable@vger.kernel.org Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-09-26evm: Don't deadlock if a crypto algorithm is unavailableMatthew Garrett
[ Upstream commit e2861fa71641c6414831d628a1f4f793b6562580 ] When EVM attempts to appraise a file signed with a crypto algorithm the kernel doesn't have support for, it will cause the kernel to trigger a module load. If the EVM policy includes appraisal of kernel modules this will in turn call back into EVM - since EVM is holding a lock until the crypto initialisation is complete, this triggers a deadlock. Add a CRYPTO_NOLOAD flag and skip module loading if it's set, and add that flag in the EVM case in order to fail gracefully with an error message instead of deadlocking. Signed-off-by: Matthew Garrett <mjg59@google.com> Acked-by: Herbert Xu <herbert@gondor.apana.org.au> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-09-26Smack: Fix handling of IPv4 traffic received by PF_INET6 socketsPiotr Sawicki
[ Upstream commit 129a99890936766f4b69b9da7ed88366313a9210 ] A socket which has sk_family set to PF_INET6 is able to receive not only IPv6 but also IPv4 traffic (IPv4-mapped IPv6 addresses). Prior to this patch, the smk_skb_to_addr_ipv6() could have been called for socket buffers containing IPv4 packets, in result such traffic was allowed. Signed-off-by: Piotr Sawicki <p.sawicki2@partner.samsung.com> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-09-19selinux: use GFP_NOWAIT in the AVC kmem_cachesMichal Hocko
commit 476accbe2f6ef69caeebe99f52a286e12ac35aee upstream. There is a strange __GFP_NOMEMALLOC usage pattern in SELinux, specifically GFP_ATOMIC | __GFP_NOMEMALLOC which doesn't make much sense. GFP_ATOMIC on its own allows to access memory reserves while __GFP_NOMEMALLOC dictates we cannot use memory reserves. Replace this with the much more sane GFP_NOWAIT in the AVC code as we can tolerate memory allocation failures in that code. Signed-off-by: Michal Hocko <mhocko@kernel.org> Acked-by: Mel Gorman <mgorman@suse.de> Signed-off-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Amit Pundir <amit.pundir@linaro.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-08-24Smack: Mark inode instant in smack_task_to_inodeCasey Schaufler
[ Upstream commit 7b4e88434c4e7982fb053c49657e1c8bbb8692d9 ] Smack: Mark inode instant in smack_task_to_inode /proc clean-up in commit 1bbc55131e59bd099fdc568d3aa0b42634dbd188 resulted in smack_task_to_inode() being called before smack_d_instantiate. This resulted in the smk_inode value being ignored, even while present for files in /proc/self. Marking the inode as instant here fixes that. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> Signed-off-by: James Morris <james.morris@microsoft.com> Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-08-03ima: based on policy verify firmware signatures (pre-allocated buffer)Mimi Zohar
[ Upstream commit fd90bc559bfba743ae8de87ff23b92a5e4668062 ] Don't differentiate, for now, between kernel_read_file_id READING_FIRMWARE and READING_FIRMWARE_PREALLOC_BUFFER enumerations. Fixes: a098ecd firmware: support loading into a pre-allocated buffer (since 4.8) Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Cc: Luis R. Rodriguez <mcgrof@suse.com> Cc: David Howells <dhowells@redhat.com> Cc: Kees Cook <keescook@chromium.org> Cc: Serge E. Hallyn <serge@hallyn.com> Cc: Stephen Boyd <stephen.boyd@linaro.org> Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-06-06selinux: KASAN: slab-out-of-bounds in xattr_getsecuritySachin Grover
commit efe3de79e0b52ca281ef6691480c8c68c82a4657 upstream. Call trace: [<ffffff9203a8d7a8>] dump_backtrace+0x0/0x428 [<ffffff9203a8dbf8>] show_stack+0x28/0x38 [<ffffff920409bfb8>] dump_stack+0xd4/0x124 [<ffffff9203d187e8>] print_address_description+0x68/0x258 [<ffffff9203d18c00>] kasan_report.part.2+0x228/0x2f0 [<ffffff9203d1927c>] kasan_report+0x5c/0x70 [<ffffff9203d1776c>] check_memory_region+0x12c/0x1c0 [<ffffff9203d17cdc>] memcpy+0x34/0x68 [<ffffff9203d75348>] xattr_getsecurity+0xe0/0x160 [<ffffff9203d75490>] vfs_getxattr+0xc8/0x120 [<ffffff9203d75d68>] getxattr+0x100/0x2c8 [<ffffff9203d76fb4>] SyS_fgetxattr+0x64/0xa0 [<ffffff9203a83f70>] el0_svc_naked+0x24/0x28 If user get root access and calls security.selinux setxattr() with an embedded NUL on a file and then if some process performs a getxattr() on that file with a length greater than the actual length of the string, it would result in a panic. To fix this, add the actual length of the string to the security context instead of the length passed by the userspace process. Signed-off-by: Sachin Grover <sgrover@codeaurora.org> Cc: stable@vger.kernel.org Signed-off-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-06-06Revert "ima: limit file hash setting by user to fix and log modes"Mimi Zohar
commit f5acb3dcba1ffb7f0b8cbb9dba61500eea5d610b upstream. Userspace applications have been modified to write security xattrs, but they are not context aware. In the case of security.ima, the security xattr can be either a file hash or a file signature. Permitting writing one, but not the other requires the application to be context aware. In addition, userspace applications might write files to a staging area, which might not be in policy, and then change some file metadata (eg. owner) making it in policy. As a result, these files are not labeled properly. This reverts commit c68ed80c97d9720f51ef31fe91560fdd1e121533, which prevents writing file hashes as security.ima xattrs. Requested-by: Patrick Ohly <patrick.ohly@intel.com> Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-05-30ima: Fallback to the builtin hash algorithmPetr Vorel
[ Upstream commit ab60368ab6a452466885ef4edf0cefd089465132 ] IMA requires having it's hash algorithm be compiled-in due to it's early use. The default IMA algorithm is protected by Kconfig to be compiled-in. The ima_hash kernel parameter allows to choose the hash algorithm. When the specified algorithm is not available or available as a module, IMA initialization fails, which leads to a kernel panic (mknodat syscall calls ima_post_path_mknod()). Therefore as fallback we force IMA to use the default builtin Kconfig hash algorithm. Fixed crash: $ grep CONFIG_CRYPTO_MD4 .config CONFIG_CRYPTO_MD4=m [ 0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-4.12.14-2.3-default root=UUID=74ae8202-9ca7-4e39-813b-22287ec52f7a video=1024x768-16 plymouth.ignore-serial-consoles console=ttyS0 console=tty resume=/dev/disk/by-path/pci-0000:00:07.0-part3 splash=silent showopts ima_hash=md4 ... [ 1.545190] ima: Can not allocate md4 (reason: -2) ... [ 2.610120] BUG: unable to handle kernel NULL pointer dereference at (null) [ 2.611903] IP: ima_match_policy+0x23/0x390 [ 2.612967] PGD 0 P4D 0 [ 2.613080] Oops: 0000 [#1] SMP [ 2.613080] Modules linked in: autofs4 [ 2.613080] Supported: Yes [ 2.613080] CPU: 0 PID: 1 Comm: systemd Not tainted 4.12.14-2.3-default #1 [ 2.613080] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.0.0-prebuilt.qemu-project.org 04/01/2014 [ 2.613080] task: ffff88003e2d0040 task.stack: ffffc90000190000 [ 2.613080] RIP: 0010:ima_match_policy+0x23/0x390 [ 2.613080] RSP: 0018:ffffc90000193e88 EFLAGS: 00010296 [ 2.613080] RAX: 0000000000000000 RBX: 000000000000000c RCX: 0000000000000004 [ 2.613080] RDX: 0000000000000010 RSI: 0000000000000001 RDI: ffff880037071728 [ 2.613080] RBP: 0000000000008000 R08: 0000000000000000 R09: 0000000000000000 [ 2.613080] R10: 0000000000000008 R11: 61c8864680b583eb R12: 00005580ff10086f [ 2.613080] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000008000 [ 2.613080] FS: 00007f5c1da08940(0000) GS:ffff88003fc00000(0000) knlGS:0000000000000000 [ 2.613080] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 2.613080] CR2: 0000000000000000 CR3: 0000000037002000 CR4: 00000000003406f0 [ 2.613080] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 2.613080] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 2.613080] Call Trace: [ 2.613080] ? shmem_mknod+0xbf/0xd0 [ 2.613080] ima_post_path_mknod+0x1c/0x40 [ 2.613080] SyS_mknod+0x210/0x220 [ 2.613080] entry_SYSCALL_64_fastpath+0x1a/0xa5 [ 2.613080] RIP: 0033:0x7f5c1bfde570 [ 2.613080] RSP: 002b:00007ffde1c90dc8 EFLAGS: 00000246 ORIG_RAX: 0000000000000085 [ 2.613080] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f5c1bfde570 [ 2.613080] RDX: 0000000000000000 RSI: 0000000000008000 RDI: 00005580ff10086f [ 2.613080] RBP: 00007ffde1c91040 R08: 00005580ff10086f R09: 0000000000000000 [ 2.613080] R10: 0000000000104000 R11: 0000000000000246 R12: 00005580ffb99660 [ 2.613080] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000002 [ 2.613080] Code: 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 41 57 41 56 44 8d 14 09 41 55 41 54 55 53 44 89 d3 09 cb 48 83 ec 38 48 8b 05 c5 03 29 01 <4c> 8b 20 4c 39 e0 0f 84 d7 01 00 00 4c 89 44 24 08 89 54 24 20 [ 2.613080] RIP: ima_match_policy+0x23/0x390 RSP: ffffc90000193e88 [ 2.613080] CR2: 0000000000000000 [ 2.613080] ---[ end trace 9a9f0a8a73079f6a ]--- [ 2.673052] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009 [ 2.673052] [ 2.675337] Kernel Offset: disabled [ 2.676405] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009 Signed-off-by: Petr Vorel <pvorel@suse.cz> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-05-30integrity/security: fix digsig.c build error with header fileRandy Dunlap
[ Upstream commit 120f3b11ef88fc38ce1d0ff9c9a4b37860ad3140 ] security/integrity/digsig.c has build errors on some $ARCH due to a missing header file, so add it. security/integrity/digsig.c:146:2: error: implicit declaration of function 'vfree' [-Werror=implicit-function-declaration] Reported-by: Michael Ellerman <mpe@ellerman.id.au> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> Cc: Mimi Zohar <zohar@linux.vnet.ibm.com> Cc: linux-integrity@vger.kernel.org Link: http://kisskb.ellerman.id.au/kisskb/head/13396/ Signed-off-by: James Morris <james.morris@microsoft.com> Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-04-13selinux: do not check open permission on socketsStephen Smalley
[ Upstream commit ccb544781d34afdb73a9a73ae53035d824d193bf ] open permission is currently only defined for files in the kernel (COMMON_FILE_PERMS rather than COMMON_FILE_SOCK_PERMS). Construction of an artificial test case that tries to open a socket via /proc/pid/fd will generate a recvfrom avc denial because recvfrom and open happen to map to the same permission bit in socket vs file classes. open of a socket via /proc/pid/fd is not supported by the kernel regardless and will ultimately return ENXIO. But we hit the permission check first and can thus produce these odd/misleading denials. Omit the open check when operating on a socket. Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov> Signed-off-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-04-08selinux: Remove redundant check for unknown labeling behaviorMatthias Kaehlcke
commit 270e8573145a26de924e2dc644596332d400445b upstream. The check is already performed in ocontext_read() when the policy is loaded. Removing the array also fixes the following warning when building with clang: security/selinux/hooks.c:338:20: error: variable 'labeling_behaviors' is not needed and will not be emitted [-Werror,-Wunneeded-internal-declaration] Signed-off-by: Matthias Kaehlcke <mka@chromium.org> Acked-by: Stephen Smalley <sds@tycho.nsa.gov> Signed-off-by: Paul Moore <paul@paul-moore.com> Cc: Nathan Chancellor <natechancellor@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-04-08selinux: Remove unnecessary check of array base in selinux_set_mapping()Matthias Kaehlcke
commit 342e91578eb6909529bc7095964cd44b9c057c4e upstream. 'perms' will never be NULL since it isn't a plain pointer but an array of u32 values. This fixes the following warning when building with clang: security/selinux/ss/services.c:158:16: error: address of array 'p_in->perms' will always evaluate to 'true' [-Werror,-Wpointer-bool-conversion] while (p_in->perms && p_in->perms[k]) { Signed-off-by: Matthias Kaehlcke <mka@chromium.org> Signed-off-by: Paul Moore <paul@paul-moore.com> Cc: Nathan Chancellor <natechancellor@gmail.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-03-22ima: relax requiring a file signature for new files with zero lengthMimi Zohar
[ Upstream commit b7e27bc1d42e8e0cc58b602b529c25cd0071b336 ] Custom policies can require file signatures based on LSM labels. These files are normally created and only afterwards labeled, requiring them to be signed. Instead of requiring file signatures based on LSM labels, entire filesystems could require file signatures. In this case, we need the ability of writing new files without requiring file signatures. The definition of a "new" file was originally defined as any file with a length of zero. Subsequent patches redefined a "new" file to be based on the FILE_CREATE open flag. By combining the open flag with a file size of zero, this patch relaxes the file signature requirement. Fixes: 1ac202e978e1 ima: accept previously set IMA_NEW_FILE Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-03-22apparmor: Make path_max parameter readonlyJohn Johansen
[ Upstream commit 622f6e3265707ebf02ba776ac6e68003bcc31213 ] The path_max parameter determines the max size of buffers allocated but it should not be setable at run time. If can be used to cause an oops root@ubuntu:~# echo 16777216 > /sys/module/apparmor/parameters/path_max root@ubuntu:~# cat /sys/module/apparmor/parameters/path_max Killed [ 122.141911] BUG: unable to handle kernel paging request at ffff880080945fff [ 122.143497] IP: [<ffffffff81228844>] d_absolute_path+0x44/0xa0 [ 122.144742] PGD 220c067 PUD 0 [ 122.145453] Oops: 0002 [#1] SMP [ 122.146204] Modules linked in: vmw_vsock_vmci_transport vsock ppdev vmw_balloon snd_ens1371 btusb snd_ac97_codec gameport snd_rawmidi btrtl snd_seq_device ac97_bus btbcm btintel snd_pcm input_leds bluetooth snd_timer snd joydev soundcore serio_raw coretemp shpchp nfit parport_pc i2c_piix4 8250_fintek vmw_vmci parport mac_hid ib_iser rdma_cm iw_cm ib_cm ib_sa ib_mad ib_core ib_addr iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi autofs4 btrfs raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq libcrc32c raid1 raid0 multipath linear hid_generic usbhid hid crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd vmwgfx psmouse mptspi ttm mptscsih drm_kms_helper mptbase syscopyarea scsi_transport_spi sysfillrect [ 122.163365] ahci sysimgblt e1000 fb_sys_fops libahci drm pata_acpi fjes [ 122.164747] CPU: 3 PID: 1501 Comm: bash Not tainted 4.4.0-59-generic #80-Ubuntu [ 122.166250] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/02/2015 [ 122.168611] task: ffff88003496aa00 ti: ffff880076474000 task.ti: ffff880076474000 [ 122.170018] RIP: 0010:[<ffffffff81228844>] [<ffffffff81228844>] d_absolute_path+0x44/0xa0 [ 122.171525] RSP: 0018:ffff880076477b90 EFLAGS: 00010206 [ 122.172462] RAX: ffff880080945fff RBX: 0000000000000000 RCX: 0000000001000000 [ 122.173709] RDX: 0000000000ffffff RSI: ffff880080946000 RDI: ffff8800348a1010 [ 122.174978] RBP: ffff880076477bb8 R08: ffff880076477c80 R09: 0000000000000000 [ 122.176227] R10: 00007ffffffff000 R11: ffff88007f946000 R12: ffff88007f946000 [ 122.177496] R13: ffff880076477c80 R14: ffff8800348a1010 R15: ffff8800348a2400 [ 122.178745] FS: 00007fd459eb4700(0000) GS:ffff88007b6c0000(0000) knlGS:0000000000000000 [ 122.180176] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 122.181186] CR2: ffff880080945fff CR3: 0000000073422000 CR4: 00000000001406e0 [ 122.182469] Stack: [ 122.182843] 00ffffff00000001 ffff880080946000 0000000000000000 0000000000000000 [ 122.184409] 00000000570f789c ffff880076477c30 ffffffff81385671 ffff88007a2e7a58 [ 122.185810] 0000000000000000 ffff880076477c88 01000000008a1000 0000000000000000 [ 122.187231] Call Trace: [ 122.187680] [<ffffffff81385671>] aa_path_name+0x81/0x370 [ 122.188637] [<ffffffff813875dd>] profile_transition+0xbd/0xb80 [ 122.190181] [<ffffffff811af9bc>] ? zone_statistics+0x7c/0xa0 [ 122.191674] [<ffffffff81389b20>] apparmor_bprm_set_creds+0x9b0/0xac0 [ 122.193288] [<ffffffff812e1971>] ? ext4_xattr_get+0x81/0x220 [ 122.194793] [<ffffffff812e800c>] ? ext4_xattr_security_get+0x1c/0x30 [ 122.196392] [<ffffffff813449b9>] ? get_vfs_caps_from_disk+0x69/0x110 [ 122.198004] [<ffffffff81232d4f>] ? mnt_may_suid+0x3f/0x50 [ 122.199737] [<ffffffff81344b03>] ? cap_bprm_set_creds+0xa3/0x600 [ 122.201377] [<ffffffff81346e53>] security_bprm_set_creds+0x33/0x50 [ 122.203024] [<ffffffff81214ce5>] prepare_binprm+0x85/0x190 [ 122.204515] [<ffffffff81216545>] do_execveat_common.isra.33+0x485/0x710 [ 122.206200] [<ffffffff81216a6a>] SyS_execve+0x3a/0x50 [ 122.207615] [<ffffffff81838795>] stub_execve+0x5/0x5 [ 122.208978] [<ffffffff818384f2>] ? entry_SYSCALL_64_fastpath+0x16/0x71 [ 122.210615] Code: f8 31 c0 48 63 c2 83 ea 01 48 c7 45 e8 00 00 00 00 48 01 c6 85 d2 48 c7 45 f0 00 00 00 00 48 89 75 e0 89 55 dc 78 0c 48 8d 46 ff <c6> 46 ff 00 48 89 45 e0 48 8d 55 e0 48 8d 4d dc 48 8d 75 e8 e8 [ 122.217320] RIP [<ffffffff81228844>] d_absolute_path+0x44/0xa0 [ 122.218860] RSP <ffff880076477b90> [ 122.219919] CR2: ffff880080945fff [ 122.220936] ---[ end trace 506cdbd85eb6c55e ]--- Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Signed-off-by: John Johansen <john.johansen@canonical.com> Signed-off-by: James Morris <james.l.morris@oracle.com> Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-03-22selinux: check for address length in selinux_socket_bind()Alexander Potapenko
[ Upstream commit e2f586bd83177d22072b275edd4b8b872daba924 ] KMSAN (KernelMemorySanitizer, a new error detection tool) reports use of uninitialized memory in selinux_socket_bind(): ================================================================== BUG: KMSAN: use of unitialized memory inter: 0 CPU: 3 PID: 1074 Comm: packet2 Tainted: G B 4.8.0-rc6+ #1916 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 0000000000000000 ffff8800882ffb08 ffffffff825759c8 ffff8800882ffa48 ffffffff818bf551 ffffffff85bab870 0000000000000092 ffffffff85bab550 0000000000000000 0000000000000092 00000000bb0009bb 0000000000000002 Call Trace: [< inline >] __dump_stack lib/dump_stack.c:15 [<ffffffff825759c8>] dump_stack+0x238/0x290 lib/dump_stack.c:51 [<ffffffff818bdee6>] kmsan_report+0x276/0x2e0 mm/kmsan/kmsan.c:1008 [<ffffffff818bf0fb>] __msan_warning+0x5b/0xb0 mm/kmsan/kmsan_instr.c:424 [<ffffffff822dae71>] selinux_socket_bind+0xf41/0x1080 security/selinux/hooks.c:4288 [<ffffffff8229357c>] security_socket_bind+0x1ec/0x240 security/security.c:1240 [<ffffffff84265d98>] SYSC_bind+0x358/0x5f0 net/socket.c:1366 [<ffffffff84265a22>] SyS_bind+0x82/0xa0 net/socket.c:1356 [<ffffffff81005678>] do_syscall_64+0x58/0x70 arch/x86/entry/common.c:292 [<ffffffff8518217c>] entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.o:? chained origin: 00000000ba6009bb [<ffffffff810bb7a7>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67 [< inline >] kmsan_save_stack_with_flags mm/kmsan/kmsan.c:322 [< inline >] kmsan_save_stack mm/kmsan/kmsan.c:337 [<ffffffff818bd2b8>] kmsan_internal_chain_origin+0x118/0x1e0 mm/kmsan/kmsan.c:530 [<ffffffff818bf033>] __msan_set_alloca_origin4+0xc3/0x130 mm/kmsan/kmsan_instr.c:380 [<ffffffff84265b69>] SYSC_bind+0x129/0x5f0 net/socket.c:1356 [<ffffffff84265a22>] SyS_bind+0x82/0xa0 net/socket.c:1356 [<ffffffff81005678>] do_syscall_64+0x58/0x70 arch/x86/entry/common.c:292 [<ffffffff8518217c>] return_from_SYSCALL_64+0x0/0x6a arch/x86/entry/entry_64.o:? origin description: ----address@SYSC_bind (origin=00000000b8c00900) ================================================================== (the line numbers are relative to 4.8-rc6, but the bug persists upstream) , when I run the following program as root: ======================================================= #include <string.h> #include <sys/socket.h> #include <netinet/in.h> int main(int argc, char *argv[]) { struct sockaddr addr; int size = 0; if (argc > 1) { size = atoi(argv[1]); } memset(&addr, 0, sizeof(addr)); int fd = socket(PF_INET6, SOCK_DGRAM, IPPROTO_IP); bind(fd, &addr, size); return 0; } ======================================================= (for different values of |size| other error reports are printed). This happens because bind() unconditionally copies |size| bytes of |addr| to the kernel, leaving the rest uninitialized. Then security_socket_bind() reads the IP address bytes, including the uninitialized ones, to determine the port, or e.g. pass them further to sel_netnode_find(), which uses them to calculate a hash. Signed-off-by: Alexander Potapenko <glider@google.com> Acked-by: Eric Dumazet <edumazet@google.com> [PM: fixed some whitespace damage] Signed-off-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Sasha Levin <alexander.levin@microsoft.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-02-25security/keys: BIG_KEY requires CONFIG_CRYPTOArnd Bergmann
commit 3cd18d1981731d5f74b8e437009124ac99905d14 upstream. The recent rework introduced a possible randconfig build failure when CONFIG_CRYPTO configured to only allow modules: security/keys/big_key.o: In function `big_key_crypt': big_key.c:(.text+0x29f): undefined reference to `crypto_aead_setkey' security/keys/big_key.o: In function `big_key_init': big_key.c:(.init.text+0x1a): undefined reference to `crypto_alloc_aead' big_key.c:(.init.text+0x45): undefined reference to `crypto_aead_setauthsize' big_key.c:(.init.text+0x77): undefined reference to `crypto_destroy_tfm' crypto/gcm.o: In function `gcm_hash_crypt_remain_continue': gcm.c:(.text+0x167): undefined reference to `crypto_ahash_finup' crypto/gcm.o: In function `crypto_gcm_exit_tfm': gcm.c:(.text+0x847): undefined reference to `crypto_destroy_tfm' When we 'select CRYPTO' like the other users, we always get a configuration that builds. Fixes: 428490e38b2e ("security/keys: rewrite all of big_key crypto") Signed-off-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-02-25selinux: skip bounded transition processing if the policy isn't loadedPaul Moore
commit 4b14752ec4e0d87126e636384cf37c8dd9df157c upstream. We can't do anything reasonable in security_bounded_transition() if we don't have a policy loaded, and in fact we could run into problems with some of the code inside expecting a policy. Fix these problems like we do many others in security/selinux/ss/services.c by checking to see if the policy is loaded (ss_initialized) and returning quickly if it isn't. Reported-by: syzbot <syzkaller-bugs@googlegroups.com> Signed-off-by: Paul Moore <paul@paul-moore.com> Acked-by: Stephen Smalley <sds@tycho.nsa.gov> Reviewed-by: James Morris <james.l.morris@oracle.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-02-25selinux: ensure the context is NUL terminated in security_context_to_sid_core()Paul Moore
commit ef28df55ac27e1e5cd122e19fa311d886d47a756 upstream. The syzbot/syzkaller automated tests found a problem in security_context_to_sid_core() during early boot (before we load the SELinux policy) where we could potentially feed context strings without NUL terminators into the strcmp() function. We already guard against this during normal operation (after the SELinux policy has been loaded) by making a copy of the context strings and explicitly adding a NUL terminator to the end. The patch extends this protection to the early boot case (no loaded policy) by moving the context copy earlier in security_context_to_sid_core(). Reported-by: syzbot <syzkaller@googlegroups.com> Signed-off-by: Paul Moore <paul@paul-moore.com> Reviewed-By: William Roberts <william.c.roberts@intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-02-13KEYS: encrypted: fix buffer overread in valid_master_desc()Eric Biggers
commit 794b4bc292f5d31739d89c0202c54e7dc9bc3add upstream. With the 'encrypted' key type it was possible for userspace to provide a data blob ending with a master key description shorter than expected, e.g. 'keyctl add encrypted desc "new x" @s'. When validating such a master key description, validate_master_desc() could read beyond the end of the buffer. Fix this by using strncmp() instead of memcmp(). [Also clean up the code to deduplicate some logic.] Cc: Mimi Zohar <zohar@linux.vnet.ibm.com> Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: James Morris <james.l.morris@oracle.com> Signed-off-by: Jin Qian <jinqian@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-01-05KPTI: Rename to PAGE_TABLE_ISOLATIONKees Cook
This renames CONFIG_KAISER to CONFIG_PAGE_TABLE_ISOLATION. Signed-off-by: Kees Cook <keescook@chromium.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-01-05x86/kaiser: Reenable PARAVIRTBorislav Petkov
Now that the required bits have been addressed, reenable PARAVIRT. Signed-off-by: Borislav Petkov <bp@suse.de> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-01-05kaiser: delete KAISER_REAL_SWITCH optionHugh Dickins
We fail to see what CONFIG_KAISER_REAL_SWITCH is for: it seems to be left over from early development, and now just obscures tricky parts of the code. Delete it before adding PCIDs, or nokaiser boot option. (Or if there is some good reason to keep the option, then it needs a help text - and a "depends on KAISER", so that all those without KAISER are not asked the question. But we'd much rather delete it.) Signed-off-by: Hugh Dickins <hughd@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-01-05kaiser: KAISER depends on SMPHugh Dickins
It is absurd that KAISER should depend on SMP, but apparently nobody has tried a UP build before: which breaks on implicit declaration of function 'per_cpu_offset' in arch/x86/mm/kaiser.c. Now, you would expect that to be trivially fixed up; but looking at the System.map when that block is #ifdef'ed out of kaiser_init(), I see that in a UP build __per_cpu_user_mapped_end is precisely at __per_cpu_user_mapped_start, and the items carefully gathered into that section for user-mapping on SMP, dispersed elsewhere on UP. So, some other kind of section assignment will be needed on UP, but implementing that is not a priority: just make KAISER depend on SMP for now. Also inserted a blank line before the option, tidied up the brief Kconfig help message, and added an "If unsure, Y". Signed-off-by: Hugh Dickins <hughd@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-01-05kaiser: merged updateDave Hansen
Merged fixes and cleanups, rebased to 4.9.51 tree (no 5-level paging). Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com> Signed-off-by: Hugh Dickins <hughd@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2018-01-05KAISER: Kernel Address IsolationRichard Fellner
This patch introduces our implementation of KAISER (Kernel Address Isolation to have Side-channels Efficiently Removed), a kernel isolation technique to close hardware side channels on kernel address information. More information about the patch can be found on: https://github.com/IAIK/KAISER From: Richard Fellner <richard.fellner@student.tugraz.at> From: Daniel Gruss <daniel.gruss@iaik.tugraz.at> Subject: [RFC, PATCH] x86_64: KAISER - do not map kernel in user mode Date: Thu, 4 May 2017 14:26:50 +0200 Link: http://marc.info/?l=linux-kernel&m=149390087310405&w=2 Kaiser-4.10-SHA1: c4b1831d44c6144d3762ccc72f0c4e71a0c713e5 To: <linux-kernel@vger.kernel.org> To: <kernel-hardening@lists.openwall.com> Cc: <clementine.maurice@iaik.tugraz.at> Cc: <moritz.lipp@iaik.tugraz.at> Cc: Michael Schwarz <michael.schwarz@iaik.tugraz.at> Cc: Richard Fellner <richard.fellner@student.tugraz.at> Cc: Ingo Molnar <mingo@kernel.org> Cc: <kirill.shutemov@linux.intel.com> Cc: <anders.fogh@gdata-adan.de> After several recent works [1,2,3] KASLR on x86_64 was basically considered dead by many researchers. We have been working on an efficient but effective fix for this problem and found that not mapping the kernel space when running in user mode is the solution to this problem [4] (the corresponding paper [5] will be presented at ESSoS17). With this RFC patch we allow anybody to configure their kernel with the flag CONFIG_KAISER to add our defense mechanism. If there are any questions we would love to answer them. We also appreciate any comments! Cheers, Daniel (+ the KAISER team from Graz University of Technology) [1] http://www.ieee-security.org/TC/SP2013/papers/4977a191.pdf [2] https://www.blackhat.com/docs/us-16/materials/us-16-Fogh-Using-Undocumented-CPU-Behaviour-To-See-Into-Kernel-Mode-And-Break-KASLR-In-The-Process.pdf [3] https://www.blackhat.com/docs/us-16/materials/us-16-Jang-Breaking-Kernel-Address-Space-Layout-Randomization-KASLR-With-Intel-TSX.pdf [4] https://github.com/IAIK/KAISER [5] https://gruss.cc/files/kaiser.pdf [patch based also on https://raw.githubusercontent.com/IAIK/KAISER/master/KAISER/0001-KAISER-Kernel-Address-Isolation.patch] Signed-off-by: Richard Fellner <richard.fellner@student.tugraz.at> Signed-off-by: Moritz Lipp <moritz.lipp@iaik.tugraz.at> Signed-off-by: Daniel Gruss <daniel.gruss@iaik.tugraz.at> Signed-off-by: Michael Schwarz <michael.schwarz@iaik.tugraz.at> Acked-by: Jiri Kosina <jkosina@suse.cz> Signed-off-by: Hugh Dickins <hughd@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-12-14KEYS: add missing permission check for request_key() destinationEric Biggers
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 <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-12-09ima: fix hash algorithm initializationBoshi Wang
[ Upstream commit ebe7c0a7be92bbd34c6ff5b55810546a0ee05bee ] The hash_setup function always sets the hash_setup_done flag, even when the hash algorithm is invalid. This prevents the default hash algorithm defined as CONFIG_IMA_DEFAULT_HASH from being used. This patch sets hash_setup_done flag only for valid hash algorithms. Fixes: e7a2ad7eb6f4 "ima: enable support for larger default filedata hash algorithms" Signed-off-by: Boshi Wang <wangboshi@huawei.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Signed-off-by: Sasha Levin <alexander.levin@verizon.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-11-24ima: do not update security.ima if appraisal status is not INTEGRITY_PASSRoberto Sassu
commit 020aae3ee58c1af0e7ffc4e2cc9fe4dc630338cb upstream. Commit b65a9cfc2c38 ("Untangling ima mess, part 2: deal with counters") moved the call of ima_file_check() from may_open() to do_filp_open() at a point where the file descriptor is already opened. This breaks the assumption made by IMA that file descriptors being closed belong to files whose access was granted by ima_file_check(). The consequence is that security.ima and security.evm are updated with good values, regardless of the current appraisal status. For example, if a file does not have security.ima, IMA will create it after opening the file for writing, even if access is denied. Access to the file will be allowed afterwards. Avoid this issue by checking the appraisal status before updating security.ima. Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Signed-off-by: James Morris <james.l.morris@oracle.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-11-18security/keys: add CONFIG_KEYS_COMPAT to KconfigBilal Amarni
commit 47b2c3fff4932e6fc17ce13d51a43c6969714e20 upstream. CONFIG_KEYS_COMPAT is defined in arch-specific Kconfigs and is missing for several 64-bit architectures : mips, parisc, tile. At the moment and for those architectures, calling in 32-bit userspace the keyctl syscall would return an ENOSYS error. This patch moves the CONFIG_KEYS_COMPAT option to security/keys/Kconfig, to make sure the compatibility wrapper is registered by default for any 64-bit architecture as long as it is configured with CONFIG_COMPAT. [DH: Modified to remove arm64 compat enablement also as requested by Eric Biggers] Signed-off-by: Bilal Amarni <bilal.amarni@gmail.com> Signed-off-by: David Howells <dhowells@redhat.com> Reviewed-by: Arnd Bergmann <arnd@arndb.de> cc: Eric Biggers <ebiggers3@gmail.com> Signed-off-by: James Morris <james.l.morris@oracle.com> Cc: James Cowgill <james.cowgill@mips.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-11-15KEYS: trusted: fix writing past end of buffer in trusted_read()Eric Biggers
commit a3c812f7cfd80cf51e8f5b7034f7418f6beb56c1 upstream. When calling keyctl_read() on a key of type "trusted", if the user-supplied buffer was too small, the kernel ignored the buffer length and just wrote past the end of the buffer, potentially corrupting userspace memory. Fix it by instead returning the size required, as per the documentation for keyctl_read(). We also don't even fill the buffer at all in this case, as this is slightly easier to implement than doing a short read, and either behavior appears to be permitted. It also makes it match the behavior of the "encrypted" key type. Fixes: d00a1c72f7f4 ("keys: add new trusted key-type") Reported-by: Ben Hutchings <ben@decadent.org.uk> Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com> Reviewed-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Reviewed-by: James Morris <james.l.morris@oracle.com> Signed-off-by: James Morris <james.l.morris@oracle.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-11-15KEYS: trusted: sanitize all key materialEric Biggers
commit ee618b4619b72527aaed765f0f0b74072b281159 upstream. As the previous patch did for encrypted-keys, zero sensitive any potentially sensitive data related to the "trusted" key type before it is freed. Notably, we were not zeroing the tpm_buf structures in which the actual key is stored for TPM seal and unseal, nor were we zeroing the trusted_key_payload in certain error paths. Cc: Mimi Zohar <zohar@linux.vnet.ibm.com> Cc: David Safford <safford@us.ibm.com> Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: James Morris <james.l.morris@oracle.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-11-15apparmor: fix undefined reference to `aa_g_hash_policy'John Johansen
[ Upstream commit 3ccb76c5dfe0d25c1d0168d5b726d0b43d19a485 ] The kernel build bot turned up a bad config combination when CONFIG_SECURITY_APPARMOR is y and CONFIG_SECURITY_APPARMOR_HASH is n, resulting in the build error security/built-in.o: In function `aa_unpack': (.text+0x841e2): undefined reference to `aa_g_hash_policy' Signed-off-by: John Johansen <john.johansen@canonical.com> Signed-off-by: Sasha Levin <alexander.levin@verizon.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-11-08KEYS: return full count in keyring_read() if buffer is too smallEric Biggers
commit 3239b6f29bdfb4b0a2ba59df995fc9e6f4df7f1f upstream. Commit e645016abc80 ("KEYS: fix writing past end of user-supplied buffer in keyring_read()") made keyring_read() stop corrupting userspace memory when the user-supplied buffer is too small. However it also made the return value in that case be the short buffer size rather than the size required, yet keyctl_read() is actually documented to return the size required. Therefore, switch it over to the documented behavior. Note that for now we continue to have it fill the short buffer, since it did that before (pre-v3.13) and dump_key_tree_aux() in keyutils arguably relies on it. Fixes: e645016abc80 ("KEYS: fix writing past end of user-supplied buffer in keyring_read()") Reported-by: Ben Hutchings <ben@decadent.org.uk> Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com> Reviewed-by: James Morris <james.l.morris@oracle.com> Signed-off-by: James Morris <james.l.morris@oracle.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-10-27KEYS: Fix race between updating and finding a negative keyDavid Howells
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 <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com> Reviewed-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-10-27KEYS: don't let add_key() update an uninstantiated keyDavid Howells
commit 60ff5b2f547af3828aebafd54daded44cfb0807a upstream. Currently, when passed a key that already exists, add_key() will call the key's ->update() method if such exists. But this is heavily broken in the case where the key is uninstantiated because it doesn't call __key_instantiate_and_link(). Consequently, it doesn't do most of the things that are supposed to happen when the key is instantiated, such as setting the instantiation state, clearing KEY_FLAG_USER_CONSTRUCT and awakening tasks waiting on it, and incrementing key->user->nikeys. It also never takes key_construction_mutex, which means that ->instantiate() can run concurrently with ->update() on the same key. In the case of the "user" and "logon" key types this causes a memory leak, at best. Maybe even worse, the ->update() methods of the "encrypted" and "trusted" key types actually just dereference a NULL pointer when passed an uninstantiated key. Change key_create_or_update() to wait interruptibly for the key to finish construction before continuing. This patch only affects *uninstantiated* keys. For now we still allow a negatively instantiated key to be updated (thereby positively instantiating it), although that's broken too (the next patch fixes it) and I'm not sure that anyone actually uses that functionality either. Here is a simple reproducer for the bug using the "encrypted" key type (requires CONFIG_ENCRYPTED_KEYS=y), though as noted above the bug pertained to more than just the "encrypted" key type: #include <stdlib.h> #include <unistd.h> #include <keyutils.h> int main(void) { int ringid = keyctl_join_session_keyring(NULL); if (fork()) { for (;;) { const char payload[] = "update user:foo 32"; usleep(rand() % 10000); add_key("encrypted", "desc", payload, sizeof(payload), ringid); keyctl_clear(ringid); } } else { for (;;) request_key("encrypted", "desc", "callout_info", ringid); } } It causes: BUG: unable to handle kernel NULL pointer dereference at 0000000000000018 IP: encrypted_update+0xb0/0x170 PGD 7a178067 P4D 7a178067 PUD 77269067 PMD 0 PREEMPT SMP CPU: 0 PID: 340 Comm: reproduce Tainted: G D 4.14.0-rc1-00025-g428490e38b2e #796 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 task: ffff8a467a39a340 task.stack: ffffb15c40770000 RIP: 0010:encrypted_update+0xb0/0x170 RSP: 0018:ffffb15c40773de8 EFLAGS: 00010246 RAX: 0000000000000000 RBX: ffff8a467a275b00 RCX: 0000000000000000 RDX: 0000000000000005 RSI: ffff8a467a275b14 RDI: ffffffffb742f303 RBP: ffffb15c40773e20 R08: 0000000000000000 R09: ffff8a467a275b17 R10: 0000000000000020 R11: 0000000000000000 R12: 0000000000000000 R13: 0000000000000000 R14: ffff8a4677057180 R15: ffff8a467a275b0f FS: 00007f5d7fb08700(0000) GS:ffff8a467f200000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000018 CR3: 0000000077262005 CR4: 00000000001606f0 Call Trace: key_create_or_update+0x2bc/0x460 SyS_add_key+0x10c/0x1d0 entry_SYSCALL_64_fastpath+0x1f/0xbe RIP: 0033:0x7f5d7f211259 RSP: 002b:00007ffed03904c8 EFLAGS: 00000246 ORIG_RAX: 00000000000000f8 RAX: ffffffffffffffda RBX: 000000003b2a7955 RCX: 00007f5d7f211259 RDX: 00000000004009e4 RSI: 00000000004009ff RDI: 0000000000400a04 RBP: 0000000068db8bad R08: 000000003b2a7955 R09: 0000000000000004 R10: 000000000000001a R11: 0000000000000246 R12: 0000000000400868 R13: 00007ffed03905d0 R14: 0000000000000000 R15: 0000000000000000 Code: 77 28 e8 64 34 1f 00 45 31 c0 31 c9 48 8d 55 c8 48 89 df 48 8d 75 d0 e8 ff f9 ff ff 85 c0 41 89 c4 0f 88 84 00 00 00 4c 8b 7d c8 <49> 8b 75 18 4c 89 ff e8 24 f8 ff ff 85 c0 41 89 c4 78 6d 49 8b RIP: encrypted_update+0xb0/0x170 RSP: ffffb15c40773de8 CR2: 0000000000000018 Reported-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com> cc: Eric Biggers <ebiggers@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-10-27KEYS: encrypted: fix dereference of NULL user_key_payloadEric Biggers
commit 13923d0865ca96312197962522e88bc0aedccd74 upstream. A key of type "encrypted" references a "master key" which is used to encrypt and decrypt the encrypted key's payload. However, when we accessed the master key's payload, we failed to handle the case where the master key has been revoked, which sets the payload pointer to NULL. Note that request_key() *does* skip revoked keys, but there is still a window where the key can be revoked before we acquire its semaphore. Fix it by checking for a NULL payload, treating it like a key which was already revoked at the time it was requested. This was an issue for master keys of type "user" only. Master keys can also be of type "trusted", but those cannot be revoked. Fixes: 7e70cb497850 ("keys: add new key-type encrypted") Reviewed-by: James Morris <james.l.morris@oracle.com> Cc: Mimi Zohar <zohar@linux.vnet.ibm.com> Cc: David Safford <safford@us.ibm.com> Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-10-12lsm: fix smack_inode_removexattr and xattr_getsecurity memleakCasey Schaufler
commit 57e7ba04d422c3d41c8426380303ec9b7533ded9 upstream. security_inode_getsecurity() provides the text string value of a security attribute. It does not provide a "secctx". The code in xattr_getsecurity() that calls security_inode_getsecurity() and then calls security_release_secctx() happened to work because SElinux and Smack treat the attribute and the secctx the same way. It fails for cap_inode_getsecurity(), because that module has no secctx that ever needs releasing. It turns out that Smack is the one that's doing things wrong by not allocating memory when instructed to do so by the "alloc" parameter. The fix is simple enough. Change the security_release_secctx() to kfree() because it isn't a secctx being returned by security_inode_getsecurity(). Change Smack to allocate the string when told to do so. Note: this also fixes memory leaks for LSMs which implement inode_getsecurity but not release_secctx, such as capabilities. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> Reported-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> Signed-off-by: James Morris <james.l.morris@oracle.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-10-05KEYS: prevent KEYCTL_READ on negative keyEric Biggers
commit 37863c43b2c6464f252862bf2e9768264e961678 upstream. Because keyctl_read_key() looks up the key with no permissions requested, it may find a negatively instantiated key. If the key is also possessed, we went ahead and called ->read() on the key. But the key payload will actually contain the ->reject_error rather than the normal payload. Thus, the kernel oopses trying to read the user_key_payload from memory address (int)-ENOKEY = 0x00000000ffffff82. Fortunately the payload data is stored inline, so it shouldn't be possible to abuse this as an arbitrary memory read primitive... Reproducer: keyctl new_session keyctl request2 user desc '' @s keyctl read $(keyctl show | awk '/user: desc/ {print $1}') It causes a crash like the following: BUG: unable to handle kernel paging request at 00000000ffffff92 IP: user_read+0x33/0xa0 PGD 36a54067 P4D 36a54067 PUD 0 Oops: 0000 [#1] SMP CPU: 0 PID: 211 Comm: keyctl Not tainted 4.14.0-rc1 #337 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-20170228_101828-anatol 04/01/2014 task: ffff90aa3b74c3c0 task.stack: ffff9878c0478000 RIP: 0010:user_read+0x33/0xa0 RSP: 0018:ffff9878c047bee8 EFLAGS: 00010246 RAX: 0000000000000001 RBX: ffff90aa3d7da340 RCX: 0000000000000017 RDX: 0000000000000000 RSI: 00000000ffffff82 RDI: ffff90aa3d7da340 RBP: ffff9878c047bf00 R08: 00000024f95da94f R09: 0000000000000000 R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000000 R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 FS: 00007f58ece69740(0000) GS:ffff90aa3e200000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00000000ffffff92 CR3: 0000000036adc001 CR4: 00000000003606f0 Call Trace: keyctl_read_key+0xac/0xe0 SyS_keyctl+0x99/0x120 entry_SYSCALL_64_fastpath+0x1f/0xbe RIP: 0033:0x7f58ec787bb9 RSP: 002b:00007ffc8d401678 EFLAGS: 00000206 ORIG_RAX: 00000000000000fa RAX: ffffffffffffffda RBX: 00007ffc8d402800 RCX: 00007f58ec787bb9 RDX: 0000000000000000 RSI: 00000000174a63ac RDI: 000000000000000b RBP: 0000000000000004 R08: 00007ffc8d402809 R09: 0000000000000020 R10: 0000000000000000 R11: 0000000000000206 R12: 00007ffc8d402800 R13: 00007ffc8d4016e0 R14: 0000000000000000 R15: 0000000000000000 Code: e5 41 55 49 89 f5 41 54 49 89 d4 53 48 89 fb e8 a4 b4 ad ff 85 c0 74 09 80 3d b9 4c 96 00 00 74 43 48 8b b3 20 01 00 00 4d 85 ed <0f> b7 5e 10 74 29 4d 85 e4 74 24 4c 39 e3 4c 89 e2 4c 89 ef 48 RIP: user_read+0x33/0xa0 RSP: ffff9878c047bee8 CR2: 00000000ffffff92 Fixes: 61ea0c0ba904 ("KEYS: Skip key state checks when checking for possession") Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-10-05KEYS: prevent creating a different user's keyringsEric Biggers
commit 237bbd29f7a049d310d907f4b2716a7feef9abf3 upstream. It was possible for an unprivileged user to create the user and user session keyrings for another user. For example: sudo -u '#3000' sh -c 'keyctl add keyring _uid.4000 "" @u keyctl add keyring _uid_ses.4000 "" @u sleep 15' & sleep 1 sudo -u '#4000' keyctl describe @u sudo -u '#4000' keyctl describe @us This is problematic because these "fake" keyrings won't have the right permissions. In particular, the user who created them first will own them and will have full access to them via the possessor permissions, which can be used to compromise the security of a user's keys: -4: alswrv-----v------------ 3000 0 keyring: _uid.4000 -5: alswrv-----v------------ 3000 0 keyring: _uid_ses.4000 Fix it by marking user and user session keyrings with a flag KEY_FLAG_UID_KEYRING. Then, when searching for a user or user session keyring by name, skip all keyrings that don't have the flag set. Fixes: 69664cf16af4 ("keys: don't generate user and user session keyrings unless they're accessed") Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-10-05KEYS: fix writing past end of user-supplied buffer in keyring_read()Eric Biggers
commit e645016abc803dafc75e4b8f6e4118f088900ffb upstream. Userspace can call keyctl_read() on a keyring to get the list of IDs of keys in the keyring. But if the user-supplied buffer is too small, the kernel would write the full list anyway --- which will corrupt whatever userspace memory happened to be past the end of the buffer. Fix it by only filling the space that is available. Fixes: b2a4df200d57 ("KEYS: Expand the capacity of a keyring") Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-10-05security/keys: rewrite all of big_key cryptoJason A. Donenfeld
commit 428490e38b2e352812e0b765d8bceafab0ec441d upstream. This started out as just replacing the use of crypto/rng with get_random_bytes_wait, so that we wouldn't use bad randomness at boot time. But, upon looking further, it appears that there were even deeper underlying cryptographic problems, and that this seems to have been committed with very little crypto review. So, I rewrote the whole thing, trying to keep to the conventions introduced by the previous author, to fix these cryptographic flaws. It makes no sense to seed crypto/rng at boot time and then keep using it like this, when in fact there's already get_random_bytes_wait, which can ensure there's enough entropy and be a much more standard way of generating keys. Since this sensitive material is being stored untrusted, using ECB and no authentication is simply not okay at all. I find it surprising and a bit horrifying that this code even made it past basic crypto review, which perhaps points to some larger issues. This patch moves from using AES-ECB to using AES-GCM. Since keys are uniquely generated each time, we can set the nonce to zero. There was also a race condition in which the same key would be reused at the same time in different threads. A mutex fixes this issue now. So, to summarize, this commit fixes the following vulnerabilities: * Low entropy key generation, allowing an attacker to potentially guess or predict keys. * Unauthenticated encryption, allowing an attacker to modify the cipher text in particular ways in order to manipulate the plaintext, which is is even more frightening considering the next point. * Use of ECB mode, allowing an attacker to trivially swap blocks or compare identical plaintext blocks. * Key re-use. * Faulty memory zeroing. [Note that in backporting this commit to 4.9, get_random_bytes_wait was replaced with get_random_bytes, since 4.9 does not have the former function. This might result in slightly worse entropy in key generation, but common use cases of big_keys makes that likely not a huge deal. And, this is the best we can do with this old kernel. Alas.] Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> Reviewed-by: Eric Biggers <ebiggers3@gmail.com> Signed-off-by: David Howells <dhowells@redhat.com> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: Kirill Marinushkin <k.marinushkin@gmail.com> Cc: security@kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-10-05security/keys: properly zero out sensitive key material in big_keyJason A. Donenfeld
commit 910801809b2e40a4baedd080ef5d80b4a180e70e upstream. Error paths forgot to zero out sensitive material, so this patch changes some kfrees into a kzfrees. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> Signed-off-by: David Howells <dhowells@redhat.com> Reviewed-by: Eric Biggers <ebiggers3@gmail.com> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: Kirill Marinushkin <k.marinushkin@gmail.com> Cc: security@kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2017-07-12KEYS: Fix an error code in request_master_key()Dan Carpenter
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 <dan.carpenter@oracle.com> Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Signed-off-by: James Morris <james.l.morris@oracle.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>