[PATCH 01/35] selinux: allow labeling before policy is loaded
From: Jonathan Lebon <jlebon@redhat.com> mainline inclusion from mainline-v5.5-rc1 commit 3e3e24b42043eceb97ed834102c2d094dfd7aaa6 category: bugfix --------------------------- Currently, the SELinux LSM prevents one from setting the `security.selinux` xattr on an inode without a policy first being loaded. However, this restriction is problematic: it makes it impossible to have newly created files with the correct label before actually loading the policy. This is relevant in distributions like Fedora, where the policy is loaded by systemd shortly after pivoting out of the initrd. In such instances, all files created prior to pivoting will be unlabeled. One then has to relabel them after pivoting, an operation which inherently races with other processes trying to access those same files. Going further, there are use cases for creating the entire root filesystem on first boot from the initrd (e.g. Container Linux supports this today[1], and we'd like to support it in Fedora CoreOS as well[2]). One can imagine doing this in two ways: at the block device level (e.g. laying down a disk image), or at the filesystem level. In the former, labeling can simply be part of the image. But even in the latter scenario, one still really wants to be able to set the right labels when populating the new filesystem. This patch enables this by changing behaviour in the following two ways: 1. allow `setxattr` on mounts without `SBLABEL_MNT` (which is all of them if no policy is loaded yet) 2. don't try to set the in-core inode SID if we're not initialized; instead leave it as `LABEL_INVALID` so that revalidation may be attempted at a later time Note the first hunk of this patch is functionally the same as a previously discussed one[3], though it was part of a larger series which wasn't accepted. Co-developed-by: Victor Kamensky <kamensky@cisco.com> Signed-off-by: Victor Kamensky <kamensky@cisco.com> Signed-off-by: Jonathan Lebon <jlebon@redhat.com> [1] https://coreos.com/os/docs/latest/root-filesystem-placement.html [2] https://github.com/coreos/fedora-coreos-tracker/issues/94 [3] https://www.spinics.net/lists/linux-initramfs/msg04593.html --- security/selinux/hooks.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 452254fd89f8..931546d80211 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3305,7 +3305,7 @@ static int selinux_inode_setxattr(struct dentry *dentry, const char *name, } sbsec = inode->i_sb->s_security; - if (!(sbsec->flags & SBLABEL_MNT)) + if (!(sbsec->flags & SBLABEL_MNT) && selinux_state.initialized) return -EOPNOTSUPP; if (!inode_owner_or_capable(inode)) @@ -3387,6 +3387,15 @@ static void selinux_inode_post_setxattr(struct dentry *dentry, const char *name, return; } + if (!selinux_state.initialized) { + /* If we haven't even been initialized, then we can't validate + * against a policy, so leave the label as invalid. It may + * resolve to a valid label on the next revalidation try if + * we've since initialized. + */ + return; + } + rc = security_context_to_sid_force(&selinux_state, value, size, &newsid); if (rc) { -- 2.27.GIT
From: Dan Carpenter <dan.carpenter@oracle.com> mainline inclusion from mainline-v5.7-rc7 commit 8433856947217ebb5697a8ff9c4c9cad4639a2cf category: bugfix --------------------------- The IS_ERR_OR_NULL() function has two conditions and if we got really unlucky we could hit a race where "ptr" started as an error pointer and then was set to NULL. Both conditions would be false even though the pointer at the end was NULL. This patch fixes the problem by ensuring that "*tfm" can only be NULL or valid. I have introduced a "tmp_tfm" variable to make that work. I also reversed a condition and pulled the code in one tab. Note: CRYPTO_ALG_ASYNC has been preserved since commit 3d234b3313cd1 ("crypto: drop mask=CRYPTO_ALG_ASYNC from 'shash' tfm allocations") is not applied to stable kernels. Reported-by: Roberto Sassu <roberto.sassu@huawei.com> Fixes: 53de3b080d5e ("evm: Check also if *tfm is an error pointer in init_desc()") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Acked-by: Roberto Sassu <roberto.sassu@huawei.com> Acked-by: Krzysztof Struczynski <krzysztof.struczynski@huawei.com> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com> --- security/integrity/evm/evm_crypto.c | 45 ++++++++++++++--------------- 1 file changed, 22 insertions(+), 23 deletions(-) diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index d6b65b237840..157efcc3ad6e 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -78,7 +78,7 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo) { long rc; const char *algo; - struct crypto_shash **tfm; + struct crypto_shash **tfm, *tmp_tfm; struct shash_desc *desc; if (type == EVM_XATTR_HMAC) { @@ -96,32 +96,31 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo) algo = hash_algo_name[hash_algo]; } - if (IS_ERR_OR_NULL(*tfm)) { - mutex_lock(&mutex); - if (*tfm) - goto out; - *tfm = crypto_alloc_shash(algo, 0, - CRYPTO_ALG_ASYNC | CRYPTO_NOLOAD); - if (IS_ERR(*tfm)) { - rc = PTR_ERR(*tfm); - pr_err("Can not allocate %s (reason: %ld)\n", algo, rc); - *tfm = NULL; + if (*tfm) + goto alloc; + mutex_lock(&mutex); + if (*tfm) + goto unlock; + + tmp_tfm = crypto_alloc_shash(algo, 0, CRYPTO_ALG_ASYNC | CRYPTO_NOLOAD); + if (IS_ERR(tmp_tfm)) { + pr_err("Can not allocate %s (reason: %ld)\n", algo, + PTR_ERR(tmp_tfm)); + mutex_unlock(&mutex); + return ERR_CAST(tmp_tfm); + } + if (type == EVM_XATTR_HMAC) { + rc = crypto_shash_setkey(tmp_tfm, evmkey, evmkey_len); + if (rc) { + crypto_free_shash(tmp_tfm); mutex_unlock(&mutex); return ERR_PTR(rc); } - if (type == EVM_XATTR_HMAC) { - rc = crypto_shash_setkey(*tfm, evmkey, evmkey_len); - if (rc) { - crypto_free_shash(*tfm); - *tfm = NULL; - mutex_unlock(&mutex); - return ERR_PTR(rc); - } - } -out: - mutex_unlock(&mutex); } - + *tfm = tmp_tfm; +unlock: + mutex_unlock(&mutex); +alloc: desc = kmalloc(sizeof(*desc) + crypto_shash_descsize(*tfm), GFP_KERNEL); if (!desc) -- 2.27.GIT
hulk inclusion category: bugfix --------------------------- evm_inode_init_security() requires the HMAC key to calculate the HMAC on initial xattrs provided by LSMs. Unfortunately, with the evm_key_loaded() check, the function continues even if the HMAC key is not loaded (evm_key_loaded() returns true also if EVM has been initialized only with a public key). If the HMAC key is not loaded, evm_inode_init_security() returns an error later when it calls evm_init_hmac(). Thus, this patch replaces the evm_key_loaded() check with a check of the EVM_INIT_HMAC flag in evm_initialized, so that evm_inode_init_security() returns 0 instead of an error. Cc: stable@vger.kernel.org # 4.5.x Fixes: 26ddabfe96b ("evm: enable EVM when X509 certificate is loaded") Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> --- security/integrity/evm/evm_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 3bb8d5f02364..496dbf69b0c7 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -530,7 +530,8 @@ int evm_inode_init_security(struct inode *inode, struct evm_ima_xattr_data *xattr_data; int rc; - if (!evm_key_loaded() || !evm_protected_xattr(lsm_xattr->name)) + if (!(evm_initialized & EVM_INIT_HMAC) || + !evm_protected_xattr(lsm_xattr->name)) return 0; xattr_data = kzalloc(sizeof(*xattr_data), GFP_NOFS); -- 2.27.GIT
hulk inclusion category: bugfix --------------------------- Granting metadata write is safe if the HMAC key is not loaded, as it won't let an attacker obtain a valid HMAC from corrupted xattrs. evm_write_key() however does not allow it if any key is loaded, including a public key, which should not be a problem. This patch allows setting EVM_ALLOW_METADATA_WRITES if the EVM_INIT_HMAC flag is not set. Cc: stable@vger.kernel.org # 4.16.x Fixes: ae1ba1676b88e ("EVM: Allow userland to permit modification of EVM-protected metadata") Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> --- security/integrity/evm/evm_secfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c index f112ca593adc..6a6e293d4e41 100644 --- a/security/integrity/evm/evm_secfs.c +++ b/security/integrity/evm/evm_secfs.c @@ -89,7 +89,7 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf, * keys are loaded. */ if ((i & EVM_ALLOW_METADATA_WRITES) && - ((evm_initialized & EVM_KEY_MASK) != 0) && + ((evm_initialized & EVM_INIT_HMAC) != 0) && !(evm_initialized & EVM_ALLOW_METADATA_WRITES)) return -EPERM; -- 2.27.GIT
hulk inclusion category: bugfix --------------------------- 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> --- security/integrity/evm/evm_main.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 496dbf69b0c7..84ae070e95e2 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -190,6 +190,12 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, break; case EVM_IMA_XATTR_DIGSIG: case EVM_XATTR_PORTABLE_DIGSIG: + /* accept xattr with non-empty signature field */ + if (xattr_len <= sizeof(struct signature_v2_hdr)) { + evm_status = INTEGRITY_FAIL; + goto out; + } + hdr = (struct signature_v2_hdr *)xattr_data; digest.hdr.algo = hdr->hash_algo; rc = evm_calc_hash(dentry, xattr_name, xattr_value, -- 2.27.GIT
participants (1)
-
Roberto Sassu