summaryrefslogtreecommitdiffstats
path: root/security/integrity
Commit message (Collapse)AuthorAgeFilesLines
* ima: Allow template selection with ima_template[_fmt]= after ima_hash=Roberto Sassu2022-02-161-3/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | commit bb8e52e4906f148c2faf6656b5106cf7233e9301 upstream. Commit c2426d2ad5027 ("ima: added support for new kernel cmdline parameter ima_template_fmt") introduced an additional check on the ima_template variable to avoid multiple template selection. Unfortunately, ima_template could be also set by the setup function of the ima_hash= parameter, when it calls ima_template_desc_current(). This causes attempts to choose a new template with ima_template= or with ima_template_fmt=, after ima_hash=, to be ignored. Achieve the goal of the commit mentioned with the new static variable template_setup_done, so that template selection requests after ima_hash= are not ignored. Finally, call ima_init_template_list(), if not already done, to initialize the list of templates before lookup_template_desc() is called. Reported-by: Guo Zihua <guozihua@huawei.com> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Cc: stable@vger.kernel.org Fixes: c2426d2ad5027 ("ima: added support for new kernel cmdline parameter ima_template_fmt") Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* ima: Remove ima_policy file before directoryStefan Berger2022-02-161-1/+1
| | | | | | | | | | | | | | commit f7333b9572d0559e00352a926c92f29f061b4569 upstream. The removal of ima_dir currently fails since ima_policy still exists, so remove the ima_policy file before removing the directory. Fixes: 4af4662fa4a9 ("integrity: IMA policy") Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Cc: <stable@vger.kernel.org> Acked-by: Christian Brauner <brauner@kernel.org> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* integrity: check the return value of audit_log_start()Xiaoke Wang2022-02-161-0/+2
| | | | | | | | | | | | | | commit 83230351c523b04ff8a029a4bdf97d881ecb96fc upstream. audit_log_start() returns audit_buffer pointer on success or NULL on error, so it is better to check the return value of it. Fixes: 3323eec921ef ("integrity: IMA as an integrity service provider") Signed-off-by: Xiaoke Wang <xkernel.wang@foxmail.com> Cc: <stable@vger.kernel.org> Reviewed-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* evm: mark evm_fixmode as __ro_after_initAustin Kim2021-11-261-1/+1
| | | | | | | | | | | | commit 32ba540f3c2a7ef61ed5a577ce25069a3d714fc9 upstream. The evm_fixmode is only configurable by command-line option and it is never modified outside initcalls, so declaring it with __ro_after_init is better. Signed-off-by: Austin Kim <austin.kim@lge.com> Cc: stable@vger.kernel.org Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* IMA: remove the dependency on CRYPTO_MD5THOBY Simon2021-09-221-1/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | commit 8510505d55e194d3f6c9644c9f9d12c4f6b0395a upstream. MD5 is a weak digest algorithm that shouldn't be used for cryptographic operation. It hinders the efficiency of a patch set that aims to limit the digests allowed for the extended file attribute namely security.ima. MD5 is no longer a requirement for IMA, nor should it be used there. The sole place where we still use the MD5 algorithm inside IMA is setting the ima_hash algorithm to MD5, if the user supplies 'ima_hash=md5' parameter on the command line. With commit ab60368ab6a4 ("ima: Fallback to the builtin hash algorithm"), setting "ima_hash=md5" fails gracefully when CRYPTO_MD5 is not set: ima: Can not allocate md5 (reason: -2) ima: Allocating md5 failed, going to use default hash algorithm sha256 Remove the CRYPTO_MD5 dependency for IMA. Signed-off-by: THOBY Simon <Simon.THOBY@viveris.fr> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> [zohar@linux.ibm.com: include commit number in patch description for stable.] Cc: stable@vger.kernel.org # 4.17 Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* IMA: remove -Wmissing-prototypes warningAustin Kim2021-09-221-1/+1
| | | | | | | | | | | | | | | | | | commit a32ad90426a9c8eb3915eed26e08ce133bd9e0da upstream. With W=1 build, the compiler throws warning message as below: security/integrity/ima/ima_mok.c:24:12: warning: no previous prototype for ‘ima_mok_init’ [-Wmissing-prototypes] __init int ima_mok_init(void) Silence the warning by adding static keyword to ima_mok_init(). Signed-off-by: Austin Kim <austin.kim@lge.com> Fixes: 41c89b64d718 ("IMA: create machine owner and blacklist keyrings") Cc: stable@vger.kernel.org Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* evm: fix writing <securityfs>/evm overflowMimi Zohar2021-07-201-2/+3
| | | | | | | | | | | | | [ Upstream commit 49219d9b8785ba712575c40e48ce0f7461254626 ] EVM_SETUP_COMPLETE is defined as 0x80000000, which is larger than INT_MAX. The "-fno-strict-overflow" compiler option properly prevents signaling EVM that the EVM policy setup is complete. Define and read an unsigned int. Fixes: f00d79750712 ("EVM: Allow userspace to signal an RSA key has been loaded") Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
* evm: Refuse EVM_ALLOW_METADATA_WRITES only if an HMAC key is loadedRoberto Sassu2021-07-201-4/+4
| | | | | | | | | | | | | | | | | | | | | commit 9acc89d31f0c94c8e573ed61f3e4340bbd526d0c upstream. EVM_ALLOW_METADATA_WRITES is an EVM initialization flag that can be set to temporarily disable metadata verification until all xattrs/attrs necessary to verify an EVM portable signature are copied to the file. This flag is cleared when EVM is initialized with an HMAC key, to avoid that the HMAC is calculated on unverified xattrs/attrs. Currently EVM unnecessarily denies setting this flag if EVM is initialized with a public key, which is not a concern as it cannot be used to trust xattrs/attrs updates. This patch removes this limitation. Fixes: ae1ba1676b88e ("EVM: Allow userland to permit modification of EVM-protected metadata") Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Cc: stable@vger.kernel.org # 4.16.x Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* evm: Execute evm_inode_init_security() only when an HMAC key is loadedRoberto Sassu2021-07-201-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | commit 9eea2904292c2d8fa98df141d3bf7c41ec9dc1b5 upstream. evm_inode_init_security() requires an HMAC key to calculate the HMAC on initial xattrs provided by LSMs. However, it checks generically whether a key has been loaded, including also public keys, which is not correct as public keys are not suitable to calculate the HMAC. Originally, support for signature verification was introduced to verify a possibly immutable initial ram disk, when no new files are created, and to switch to HMAC for the root filesystem. By that time, an HMAC key should have been loaded and usable to calculate HMACs for new files. More recently support for requiring an HMAC key was removed from the kernel, so that signature verification can be used alone. Since this is a legitimate use case, evm_inode_init_security() should not return an error when no HMAC key has been loaded. This patch fixes this problem by replacing the evm_key_loaded() check with a check of the EVM_INIT_HMAC flag in evm_initialized. Fixes: 26ddabfe96b ("evm: enable EVM when X509 certificate is loaded") Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Cc: stable@vger.kernel.org # 4.5.x Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* certs: Fix blacklist flag type confusionDavid Howells2021-03-041-3/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | [ Upstream commit 4993e1f9479a4161fd7d93e2b8b30b438f00cb0f ] KEY_FLAG_KEEP is not meant to be passed to keyring_alloc() or key_alloc(), as these only take KEY_ALLOC_* flags. KEY_FLAG_KEEP has the same value as KEY_ALLOC_BYPASS_RESTRICTION, but fortunately only key_create_or_update() uses it. LSMs using the key_alloc hook don't check that flag. KEY_FLAG_KEEP is then ignored but fortunately (again) the root user cannot write to the blacklist keyring, so it is not possible to remove a key/hash from it. Fix this by adding a KEY_ALLOC_SET_KEEP flag that tells key_alloc() to set KEY_FLAG_KEEP on the new key. blacklist_init() can then, correctly, pass this to keyring_alloc(). We can also use this in ima_mok_init() rather than setting the flag manually. Note that this doesn't fix an observable bug with the current implementation but it is required to allow addition of new hashes to the blacklist in the future without making it possible for them to be removed. Fixes: 734114f8782f ("KEYS: Add a system blacklist keyring") Reported-by: Mickaël Salaün <mic@linux.microsoft.com> Signed-off-by: David Howells <dhowells@redhat.com> cc: Mickaël Salaün <mic@linux.microsoft.com> cc: Mimi Zohar <zohar@linux.vnet.ibm.com> Cc: David Woodhouse <dwmw2@infradead.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
* ima: Free IMA measurement buffer after kexec syscallLakshmi Ramasubramanian2021-03-041-0/+2
| | | | | | | | | | | | | | | | | | | | | | [ Upstream commit f31e3386a4e92ba6eda7328cb508462956c94c64 ] IMA allocates kernel virtual memory to carry forward the measurement list, from the current kernel to the next kernel on kexec system call, in ima_add_kexec_buffer() function. This buffer is not freed before completing the kexec system call resulting in memory leak. Add ima_buffer field in "struct kimage" to store the virtual address of the buffer allocated for the IMA measurement list. Free the memory allocated for the IMA measurement list in kimage_file_post_load_cleanup() function. Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> Suggested-by: Tyler Hicks <tyhicks@linux.microsoft.com> Reviewed-by: Thiago Jung Bauermann <bauerman@linux.ibm.com> Reviewed-by: Tyler Hicks <tyhicks@linux.microsoft.com> Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list") Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
* ima: Free IMA measurement buffer on errorLakshmi Ramasubramanian2021-03-041-0/+1
| | | | | | | | | | | | | | | | | | [ Upstream commit 6d14c6517885fa68524238787420511b87d671df ] IMA allocates kernel virtual memory to carry forward the measurement list, from the current kernel to the next kernel on kexec system call, in ima_add_kexec_buffer() function. In error code paths this memory is not freed resulting in memory leak. Free the memory allocated for the IMA measurement list in the error code paths in ima_add_kexec_buffer() function. Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> Suggested-by: Tyler Hicks <tyhicks@linux.microsoft.com> Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list") Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
* ima: Remove __init annotation from ima_pcrread()Roberto Sassu2021-01-191-1/+1
| | | | | | | | | | | | | | | | | | | | | | commit 8b8c704d913b0fe490af370631a4200e26334ec0 upstream. Commit 6cc7c266e5b4 ("ima: Call ima_calc_boot_aggregate() in ima_eventdigest_init()") added a call to ima_calc_boot_aggregate() so that the digest can be recalculated for the boot_aggregate measurement entry if the 'd' template field has been requested. For the 'd' field, only SHA1 and MD5 digests are accepted. Given that ima_eventdigest_init() does not have the __init annotation, all functions called should not have it. This patch removes __init from ima_pcrread(). Cc: stable@vger.kernel.org Fixes: 6cc7c266e5b4 ("ima: Call ima_calc_boot_aggregate() in ima_eventdigest_init()") Reported-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* ima: Don't modify file descriptor mode on the flyRoberto Sassu2020-12-301-15/+5
| | | | | | | | | | | | | | | | | | | | | | | | commit 207cdd565dfc95a0a5185263a567817b7ebf5467 upstream. Commit a408e4a86b36b ("ima: open a new file instance if no read permissions") already introduced a second open to measure a file when the original file descriptor does not allow it. However, it didn't remove the existing method of changing the mode of the original file descriptor, which is still necessary if the current process does not have enough privileges to open a new one. Changing the mode isn't really an option, as the filesystem might need to do preliminary steps to make the read possible. Thus, this patch removes the code and keeps the second open as the only option to measure a file when it is unreadable with the original file descriptor. Cc: <stable@vger.kernel.org> # 4.20.x: 0014cc04e8ec0 ima: Set file->f_mode Fixes: 2fe5d6def1672 ("ima: integrity appraisal extension") Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* evm: Check size of security.evm before using itRoberto Sassu2020-11-051-0/+6
| | | | | | | | | | | | | | | commit 455b6c9112eff8d249e32ba165742085678a80a4 upstream. This patch checks the size for the EVM_IMA_XATTR_DIGSIG and EVM_XATTR_PORTABLE_DIGSIG types to ensure that the algorithm is read from the buffer returned by vfs_getxattr_alloc(). Cc: stable@vger.kernel.org # 4.19.x Fixes: 5feeb61183dde ("evm: Allow non-SHA1 digital signatures") Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* ima: Don't ignore errors from crypto_shash_update()Roberto Sassu2020-10-291-0/+2
| | | | | | | | | | | | | | | | commit 60386b854008adc951c470067f90a2d85b5d520f upstream. Errors returned by crypto_shash_update() are not checked in ima_calc_boot_aggregate_tfm() and thus can be overwritten at the next iteration of the loop. This patch adds a check after calling crypto_shash_update() and returns immediately if the result is not zero. Cc: stable@vger.kernel.org Fixes: 3323eec921efd ("integrity: IMA as an integrity service provider") Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* ima: Call ima_calc_boot_aggregate() in ima_eventdigest_init()Roberto Sassu2020-06-224-5/+24
| | | | | | | | | | | | | | | | | | | | [ Upstream commit 6cc7c266e5b47d3cd2b5bb7fd3aac4e6bb2dd1d2 ] If the template field 'd' is chosen and the digest to be added to the measurement entry was not calculated with SHA1 or MD5, it is recalculated with SHA1, by using the passed file descriptor. However, this cannot be done for boot_aggregate, because there is no file descriptor. This patch adds a call to ima_calc_boot_aggregate() in ima_eventdigest_init(), so that the digest can be recalculated also for the boot_aggregate entry. Cc: stable@vger.kernel.org # 3.13.x Fixes: 3ce1217d6cd5d ("ima: define template fields library and new helpers") Reported-by: Takashi Iwai <tiwai@suse.de> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
* evm: Fix possible memory leak in evm_calc_hmac_or_hash()Roberto Sassu2020-06-221-1/+1
| | | | | | | | | | | | | | | commit 0c4395fb2aa77341269ea619c5419ea48171883f upstream. Don't immediately return if the signature is portable and security.ima is not present. Just set error so that memory allocated is freed before returning from evm_calc_hmac_or_hash(). Fixes: 50b977481fce9 ("EVM: Add support for portable signature format") Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Cc: stable@vger.kernel.org Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* ima: Directly assign the ima_default_policy pointer to ima_rulesRoberto Sassu2020-06-221-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | commit 067a436b1b0aafa593344fddd711a755a58afb3b upstream. This patch prevents the following oops: [ 10.771813] BUG: kernel NULL pointer dereference, address: 0000000000000 [...] [ 10.779790] RIP: 0010:ima_match_policy+0xf7/0xb80 [...] [ 10.798576] Call Trace: [ 10.798993] ? ima_lsm_policy_change+0x2b0/0x2b0 [ 10.799753] ? inode_init_owner+0x1a0/0x1a0 [ 10.800484] ? _raw_spin_lock+0x7a/0xd0 [ 10.801592] ima_must_appraise.part.0+0xb6/0xf0 [ 10.802313] ? ima_fix_xattr.isra.0+0xd0/0xd0 [ 10.803167] ima_must_appraise+0x4f/0x70 [ 10.804004] ima_post_path_mknod+0x2e/0x80 [ 10.804800] do_mknodat+0x396/0x3c0 It occurs when there is a failure during IMA initialization, and ima_init_policy() is not called. IMA hooks still call ima_match_policy() but ima_rules is NULL. This patch prevents the crash by directly assigning the ima_default_policy pointer to ima_rules when ima_rules is defined. This wouldn't alter the existing behavior, as ima_rules is always set at the end of ima_init_policy(). Cc: stable@vger.kernel.org # 3.7.x Fixes: 07f6a79415d7d ("ima: add appraise action keywords and default rules") Reported-by: Takashi Iwai <tiwai@suse.de> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* ima: Fix ima digest hash table key calculationKrzysztof Struczynski2020-06-221-3/+4
| | | | | | | | | | | | | | | | | | | | | | commit 1129d31b55d509f15e72dc68e4b5c3a4d7b4da8d upstream. Function hash_long() accepts unsigned long, while currently only one byte is passed from ima_hash_key(), which calculates a key for ima_htable. Given that hashing the digest does not give clear benefits compared to using the digest itself, remove hash_long() and return the modulus calculated on the first two bytes of the digest with the number of slots. Also reduce the depth of the hash table by doubling the number of slots. Cc: stable@vger.kernel.org Fixes: 3323eec921ef ("integrity: IMA as an integrity service provider") Co-developed-by: Roberto Sassu <roberto.sassu@huawei.com> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Signed-off-by: Krzysztof Struczynski <krzysztof.struczynski@huawei.com> Acked-by: David.Laight@aculab.com (big endian system concerns) Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* evm: Fix RCU list related warningsMadhuparna Bhowmik2020-06-073-4/+11
| | | | | | | | | | | | | | | | | | | | | | | | | [ Upstream commit 770f60586d2af0590be263f55fd079226313922c ] This patch fixes the following warning and few other instances of traversal of evm_config_xattrnames list: [ 32.848432] ============================= [ 32.848707] WARNING: suspicious RCU usage [ 32.848966] 5.7.0-rc1-00006-ga8d5875ce5f0b #1 Not tainted [ 32.849308] ----------------------------- [ 32.849567] security/integrity/evm/evm_main.c:231 RCU-list traversed in non-reader section!! Since entries are only added to the list and never deleted, use list_for_each_entry_lockless() instead of list_for_each_entry_rcu for traversing the list. Also, add a relevant comment in evm_secfs.c to indicate this fact. Reported-by: kernel test robot <lkp@intel.com> Suggested-by: Paul E. McKenney <paulmck@kernel.org> Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik10@gmail.com> Acked-by: Paul E. McKenney <paulmck@kernel.org> (RCU viewpoint) Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
* ima: Fix return value of ima_write_policy()Roberto Sassu2020-05-271-2/+1
| | | | | | | | | | | | | | | | | | [ Upstream commit 2e3a34e9f409ebe83d1af7cd2f49fca7af97dfac ] This patch fixes the return value of ima_write_policy() when a new policy is directly passed to IMA and the current policy requires appraisal of the file containing the policy. Currently, if appraisal is not in ENFORCE mode, ima_write_policy() returns 0 and leads user space applications to an endless loop. Fix this issue by denying the operation regardless of the appraisal mode. Cc: stable@vger.kernel.org # 4.10.x Fixes: 19f8a84713edc ("ima: measure and appraise the IMA policy itself") Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Reviewed-by: Krzysztof Struczynski <krzysztof.struczynski@huawei.com> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
* evm: Check also if *tfm is an error pointer in init_desc()Roberto Sassu2020-05-271-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | [ Upstream commit 53de3b080d5eae31d0de219617155dcc34e7d698 ] This patch avoids a kernel panic due to accessing an error pointer set by crypto_alloc_shash(). It occurs especially when there are many files that require an unsupported algorithm, as it would increase the likelihood of the following race condition: Task A: *tfm = crypto_alloc_shash() <= error pointer Task B: if (*tfm == NULL) <= *tfm is not NULL, use it Task B: rc = crypto_shash_init(desc) <= panic Task A: *tfm = NULL This patch uses the IS_ERR_OR_NULL macro to determine whether or not a new crypto context must be created. Cc: stable@vger.kernel.org Fixes: d46eb3699502b ("evm: crypto hash replaced by shash") Co-developed-by: Krzysztof Struczynski <krzysztof.struczynski@huawei.com> Signed-off-by: Krzysztof Struczynski <krzysztof.struczynski@huawei.com> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
* ima: Set file->f_mode instead of file->f_flags in ima_calc_file_hash()Roberto Sassu2020-05-271-6/+6
| | | | | | | | | | | | | | | | | | | | [ Upstream commit 0014cc04e8ec077dc482f00c87dfd949cfe2b98f ] Commit a408e4a86b36 ("ima: open a new file instance if no read permissions") tries to create a new file descriptor to calculate a file digest if the file has not been opened with O_RDONLY flag. However, if a new file descriptor cannot be obtained, it sets the FMODE_READ flag to file->f_flags instead of file->f_mode. This patch fixes this issue by replacing f_flags with f_mode as it was before that commit. Cc: stable@vger.kernel.org # 4.20.x Fixes: a408e4a86b36 ("ima: open a new file instance if no read permissions") Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Reviewed-by: Goldwyn Rodrigues <rgoldwyn@suse.com> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
* ima: fix freeing ongoing ahash_requestSascha Hauer2019-10-111-0/+5
| | | | | | | | | | | | | | | [ Upstream commit 4ece3125f21b1d42b84896c5646dbf0e878464e1 ] integrity_kernel_read() can fail in which case we forward to call ahash_request_free() on a currently running request. We have to wait for its completion before we can free the request. This was observed by interrupting a "find / -type f -xdev -print0 | xargs -0 cat 1>/dev/null" with ctrl-c on an IMA enabled filesystem. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
* ima: always return negative code for errorSascha Hauer2019-10-111-1/+4
| | | | | | | | | | | | | | | | [ Upstream commit f5e1040196dbfe14c77ce3dfe3b7b08d2d961e88 ] integrity_kernel_read() returns the number of bytes read. If this is a short read then this positive value is returned from ima_calc_file_hash_atfm(). Currently this is only indirectly called from ima_calc_file_hash() and this function only tests for the return value being zero or nonzero and also doesn't forward the return value. Nevertheless there's no point in returning a positive value as an error, so translate a short read into -EINVAL. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
* evm: check hash algorithm passed to init_desc()Roberto Sassu2019-06-091-0/+3
| | | | | | | | | | | | | | | | commit 221be106d75c1b511973301542f47d6000d0b63e upstream. This patch prevents memory access beyond the evm_tfm array by checking the validity of the index (hash algorithm) passed to init_desc(). The hash algorithm can be arbitrarily set if the security.ima xattr type is not EVM_XATTR_HMAC. Fixes: 5feeb61183dde ("evm: Allow non-SHA1 digital signatures") Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Cc: stable@vger.kernel.org Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* ima: show rules with IMA_INMASK correctlyRoberto Sassu2019-06-091-9/+12
| | | | | | | | | | | | | commit 8cdc23a3d9ec0944000ad43bad588e36afdc38cd upstream. Show the '^' character when a policy rule has flag IMA_INMASK. Fixes: 80eae209d63ac ("IMA: allow reading back the current IMA policy") Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> Cc: stable@vger.kernel.org Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* ima: open a new file instance if no read permissionsGoldwyn Rodrigues2018-11-131-20/+34
| | | | | | | | | | | | | | | | | commit a408e4a86b36bf98ad15b9ada531cf0e5118ac67 upstream. Open a new file instance as opposed to changing file->f_mode when the file is not readable. This is done to accomodate overlayfs stacked file operations change. The real struct file is hidden behind the overlays struct file. So, any file->f_mode manipulations are not reflected on the real struct file. Open the file again in read mode if original file cannot be read, read and calculate the hash. Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> Cc: stable@vger.kernel.org (linux-4.19) Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
* ima: fix showing large 'violations' or 'runtime_measurements_count'Eric Biggers2018-11-131-3/+3
| | | | | | | | | | | | | 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>
* Merge branch 'next-integrity' of ↵Linus Torvalds2018-08-1511-45/+102
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security Pull integrity updates from James Morris: "This adds support for EVM signatures based on larger digests, contains a new audit record AUDIT_INTEGRITY_POLICY_RULE to differentiate the IMA policy rules from the IMA-audit messages, addresses two deadlocks due to either loading or searching for crypto algorithms, and cleans up the audit messages" * 'next-integrity' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security: EVM: fix return value check in evm_write_xattrs() integrity: prevent deadlock during digsig verification. evm: Allow non-SHA1 digital signatures evm: Don't deadlock if a crypto algorithm is unavailable integrity: silence warning when CONFIG_SECURITYFS is not enabled ima: Differentiate auditing policy rules from "audit" actions ima: Do not audit if CONFIG_INTEGRITY_AUDIT is not set ima: Use audit_log_format() rather than audit_log_string() ima: Call audit_log_string() rather than logging it untrusted
| * EVM: fix return value check in evm_write_xattrs()Wei Yongjun2018-07-221-2/+2
| | | | | | | | | | | | | | | | | | | | | | In case of error, the function audit_log_start() returns NULL pointer not ERR_PTR(). The IS_ERR() test in the return value check should be replaced with NULL test. Fixes: fa516b66a1bf ("EVM: Allow runtime modification of the set of verified xattrs") Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com> Acked-by: Serge Hallyn <serge@hallyn.com> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
| * integrity: prevent deadlock during digsig verification.Mikhail Kurinnoi2018-07-181-0/+23
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch aimed to prevent deadlock during digsig verification.The point of issue - user space utility modprobe and/or it's dependencies (ld-*.so, libz.so.*, libc-*.so and /lib/modules/ files) that could be used for kernel modules load during digsig verification and could be signed by digsig in the same time. First at all, look at crypto_alloc_tfm() work algorithm: crypto_alloc_tfm() will first attempt to locate an already loaded algorithm. If that fails and the kernel supports dynamically loadable modules, it will then attempt to load a module of the same name or alias. If that fails it will send a query to any loaded crypto manager to construct an algorithm on the fly. We have situation, when public_key_verify_signature() in case of RSA algorithm use alg_name to store internal information in order to construct an algorithm on the fly, but crypto_larval_lookup() will try to use alg_name in order to load kernel module with same name. 1) we can't do anything with crypto module work, since it designed to work exactly in this way; 2) we can't globally filter module requests for modprobe, since it designed to work with any requests. In this patch, I propose add an exception for "crypto-pkcs1pad(rsa,*)" module requests only in case of enabled integrity asymmetric keys support. Since we don't have any real "crypto-pkcs1pad(rsa,*)" kernel modules for sure, we are safe to fail such module request from crypto_larval_lookup(). In this way we prevent modprobe execution during digsig verification and avoid possible deadlock if modprobe and/or it's dependencies also signed with digsig. Requested "crypto-pkcs1pad(rsa,*)" kernel module name formed by: 1) "pkcs1pad(rsa,%s)" in public_key_verify_signature(); 2) "crypto-%s" / "crypto-%s-all" in crypto_larval_lookup(). "crypto-pkcs1pad(rsa," part of request is a constant and unique and could be used as filter. Signed-off-by: Mikhail Kurinnoi <viewizard@viewizard.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> include/linux/integrity.h | 13 +++++++++++++ security/integrity/digsig_asymmetric.c | 23 +++++++++++++++++++++++ security/security.c | 7 ++++++- 3 files changed, 42 insertions(+), 1 deletion(-)
| * evm: Allow non-SHA1 digital signaturesMatthew Garrett2018-07-184-31/+46
| | | | | | | | | | | | | | | | | | | | | | SHA1 is reasonable in HMAC constructs, but it's desirable to be able to use stronger hashes in digital signatures. Modify the EVM crypto code so the hash type is imported from the digital signature and passed down to the hash calculation code, and return the digest size to higher layers for validation. Signed-off-by: Matthew Garrett <mjg59@google.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
| * evm: Don't deadlock if a crypto algorithm is unavailableMatthew Garrett2018-07-181-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * integrity: silence warning when CONFIG_SECURITYFS is not enabledSudeep Holla2018-07-181-3/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When CONFIG_SECURITYFS is not enabled, securityfs_create_dir returns -ENODEV which throws the following error: "Unable to create integrity sysfs dir: -19" However, if the feature is disabled, it can't be warning and hence we need to silence the error. This patch checks for the error -ENODEV which is returned when CONFIG_SECURITYFS is disabled to stop the error being thrown. Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> Acked-by: Matthew Garrett <mjg59@google.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
| * ima: Differentiate auditing policy rules from "audit" actionsStefan Berger2018-07-181-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The AUDIT_INTEGRITY_RULE is used for auditing IMA policy rules and the IMA "audit" policy action. This patch defines AUDIT_INTEGRITY_POLICY_RULE to reflect the IMA policy rules. Since we defined a new message type we can now also pass the audit_context and get an associated SYSCALL record. This now produces the following records when parsing IMA policy's rules: type=UNKNOWN[1807] msg=audit(1527888965.738:320): action=audit \ func=MMAP_CHECK mask=MAY_EXEC res=1 type=UNKNOWN[1807] msg=audit(1527888965.738:320): action=audit \ func=FILE_CHECK mask=MAY_READ res=1 type=SYSCALL msg=audit(1527888965.738:320): arch=c000003e syscall=1 \ success=yes exit=17 a0=1 a1=55bcfcca9030 a2=11 a3=7fcc1b55fb38 \ items=0 ppid=1567 pid=1601 auid=0 uid=0 gid=0 euid=0 suid=0 \ fsuid=0 egid=0 sgid=0 fsgid=0 tty=tty2 ses=2 comm="echo" \ exe="/usr/bin/echo" \ subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null) Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Acked-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
| * ima: Do not audit if CONFIG_INTEGRITY_AUDIT is not setStefan Berger2018-07-183-1/+21
| | | | | | | | | | | | | | | | If Integrity is not auditing, IMA shouldn't audit, either. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Acked-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
| * ima: Use audit_log_format() rather than audit_log_string()Stefan Berger2018-07-182-7/+2
| | | | | | | | | | | | | | | | | | | | Remove the usage of audit_log_string() and replace it with audit_log_format(). Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Suggested-by: Steve Grubb <sgrubb@redhat.com> Acked-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
| * ima: Call audit_log_string() rather than logging it untrustedStefan Berger2018-07-181-1/+1
| | | | | | | | | | | | | | | | | | | | | | The parameters passed to this logging function are all provided by a privileged user and therefore we can call audit_log_string() rather than audit_log_untrustedstring(). Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Suggested-by: Steve Grubb <sgrubb@redhat.com> Acked-by: Paul Moore <paul@paul-moore.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
* | Merge branch 'next-tpm' of ↵Linus Torvalds2018-08-154-16/+10
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security Pull TPM updates from James Morris: - Migrate away from PM runtime as explicit cmdReady/goIdle transactions for every command is a spec requirement. PM runtime adds only a layer of complexity on our case. - tpm_tis drivers can now specify the hwrng quality. - TPM 2.0 code uses now tpm_buf for constructing messages. Jarkko thinks Tomas Winkler has done the same for TPM 1.2, and will start digging those changes from the patchwork in the near future. - Bug fixes and clean ups * 'next-tpm' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security: ima: Get rid of ima_used_chip and use ima_tpm_chip != NULL instead ima: Use tpm_default_chip() and call TPM functions with a tpm_chip tpm: replace TPM_TRANSMIT_RAW with TPM_TRANSMIT_NESTED tpm: Convert tpm_find_get_ops() to use tpm_default_chip() tpm: Implement tpm_default_chip() to find a TPM chip tpm: rename tpm_chip_find_get() to tpm_find_get_ops() tpm: Allow tpm_tis drivers to set hwrng quality. tpm: Return the actual size when receiving an unsupported command tpm: separate cmd_ready/go_idle from runtime_pm tpm/tpm_i2c_infineon: switch to i2c_lock_bus(..., I2C_LOCK_SEGMENT) tpm_tis_spi: Pass the SPI IRQ down to the driver tpm: migrate tpm2_get_random() to use struct tpm_buf tpm: migrate tpm2_get_tpm_pt() to use struct tpm_buf tpm: migrate tpm2_probe() to use struct tpm_buf tpm: migrate tpm2_shutdown() to use struct tpm_buf
| * | ima: Get rid of ima_used_chip and use ima_tpm_chip != NULL insteadStefan Berger2018-07-284-8/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Get rid of ima_used_chip and use ima_tpm_chip variable instead for determining whether to use the TPM chip. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
| * | ima: Use tpm_default_chip() and call TPM functions with a tpm_chipStefan Berger2018-07-284-9/+7
| |/ | | | | | | | | | | | | | | | | | | | | Rather than accessing the TPM functions by passing a NULL pointer for the tpm_chip, which causes a lookup for a suitable chip every time, get a hold of a tpm_chip and access the TPM functions using it. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
* | Merge branch 'next-general' of ↵Linus Torvalds2018-08-154-17/+158
|\| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security Pull security subsystem updates from James Morris: - kstrdup() return value fix from Eric Biggers - Add new security_load_data hook to differentiate security checking of kernel-loaded binaries in the case of there being no associated file descriptor, from Mimi Zohar. - Add ability to IMA to specify a policy at build-time, rather than just via command line params or by loading a custom policy, from Mimi. - Allow IMA and LSMs to prevent sysfs firmware load fallback (e.g. if using signed firmware), from Mimi. - Allow IMA to deny loading of kexec kernel images, as they cannot be measured by IMA, from Mimi. * 'next-general' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security: security: check for kstrdup() failure in lsm_append() security: export security_kernel_load_data function ima: based on policy warn about loading firmware (pre-allocated buffer) module: replace the existing LSM hook in init_module ima: add build time policy ima: based on policy require signed firmware (sysfs fallback) firmware: add call to LSM hook before firmware sysfs fallback ima: based on policy require signed kexec kernel images kexec: add call to LSM hook in original kexec_load syscall security: define new LSM hook named security_kernel_load_data MAINTAINERS: remove the outdated "LINUX SECURITY MODULE (LSM) FRAMEWORK" entry
| * ima: based on policy warn about loading firmware (pre-allocated buffer)Mimi Zohar2018-07-161-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some systems are memory constrained but they need to load very large firmwares. The firmware subsystem allows drivers to request this firmware be loaded from the filesystem, but this requires that the entire firmware be loaded into kernel memory first before it's provided to the driver. This can lead to a situation where we map the firmware twice, once to load the firmware into kernel memory and once to copy the firmware into the final resting place. To resolve this problem, commit a098ecd2fa7d ("firmware: support loading into a pre-allocated buffer") introduced request_firmware_into_buf() API that allows drivers to request firmware be loaded directly into a pre-allocated buffer. Do devices using pre-allocated memory run the risk of the firmware being accessible to the device prior to the completion of IMA's signature verification any more than when using two buffers? (Refer to mailing list discussion[1]). Only on systems with an IOMMU can the access be prevented. As long as the signature verification completes prior to the DMA map is performed, the device can not access the buffer. This implies that the same buffer can not be re-used. Can we ensure the buffer has not been DMA mapped before using the pre-allocated buffer? [1] https://lkml.org/lkml/2018/7/10/56 Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Cc: Luis R. Rodriguez <mcgrof@suse.com> Cc: Stephen Boyd <sboyd@kernel.org> Cc: Bjorn Andersson <bjorn.andersson@linaro.org> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Kees Cook <keescook@chromium.org> Signed-off-by: James Morris <james.morris@microsoft.com>
| * module: replace the existing LSM hook in init_moduleMimi Zohar2018-07-161-13/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Both the init_module and finit_module syscalls call either directly or indirectly the security_kernel_read_file LSM hook. This patch replaces the direct call in init_module with a call to the new security_kernel_load_data hook and makes the corresponding changes in SELinux, LoadPin, and IMA. Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Cc: Jeff Vander Stoep <jeffv@google.com> Cc: Casey Schaufler <casey@schaufler-ca.com> Cc: Kees Cook <keescook@chromium.org> Acked-by: Jessica Yu <jeyu@kernel.org> Acked-by: Paul Moore <paul@paul-moore.com> Acked-by: Kees Cook <keescook@chromium.org> Signed-off-by: James Morris <james.morris@microsoft.com>
| * ima: add build time policyMimi Zohar2018-07-162-3/+101
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | IMA by default does not measure, appraise or audit files, but can be enabled at runtime by specifying a builtin policy on the boot command line or by loading a custom policy. This patch defines a build time policy, which verifies kernel modules, firmware, kexec image, and/or the IMA policy signatures. This build time policy is automatically enabled at runtime and persists after loading a custom policy. Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Reviewed-by: Kees Cook <keescook@chromium.org> Signed-off-by: James Morris <james.morris@microsoft.com>
| * ima: based on policy require signed firmware (sysfs fallback)Mimi Zohar2018-07-161-1/+9
| | | | | | | | | | | | | | | | | | | | | | With an IMA policy requiring signed firmware, this patch prevents the sysfs fallback method of loading firmware. Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Reviewed-by: Kees Cook <keescook@chromium.org> Cc: Luis R. Rodriguez <mcgrof@suse.com> Cc: Matthew Garrett <mjg59@google.com> Signed-off-by: James Morris <james.morris@microsoft.com>
| * ima: based on policy require signed kexec kernel imagesMimi Zohar2018-07-163-0/+30
| | | | | | | | | | | | | | | | | | | | | | | | The original kexec_load syscall can not verify file signatures, nor can the kexec image be measured. Based on policy, deny the kexec_load syscall. Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com> Cc: Eric Biederman <ebiederm@xmission.com> Cc: Kees Cook <keescook@chromium.org> Reviewed-by: Kees Cook <keescook@chromium.org> Signed-off-by: James Morris <james.morris@microsoft.com>
* | IMA: don't propagate opened through the entire thingAl Viro2018-07-123-12/+12
|/ | | | | | | just check ->f_mode in ima_appraise_measurement() Acked-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>