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) {
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)
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);
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;
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,
hulk inclusion category: bugfix
---------------------------
If files with portable signatures are copied from one location to another or are extracted from an archive, verification can temporarily fail until all xattrs/attrs are set in the destination. Portable signatures are the only ones that can be moved to different files, as they don't depend on system-specific information such as the inode generation.
Unlike other security.evm types, portable signatures can never be replaced even if an xattr/attr operation is granted, as once evm_update_evmxattr() detects this type, it returns without updating the HMAC. Thus, it wouldn't be a problem to allow those operations so that verification passes on the destination after all xattrs/attrs are copied.
This patch first introduces a new integrity status called INTEGRITY_FAIL_IMMUTABLE, that allows callers of evm_verify_current_integrity() to detect that a portable signature didn't pass verification and then adds an exception in evm_protect_xattr() and evm_inode_setattr() for this status and returns 0 instead of -EPERM.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- include/linux/integrity.h | 1 + security/integrity/evm/evm_main.c | 25 ++++++++++++++++++++----- security/integrity/ima/ima_appraise.c | 1 + 3 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/include/linux/integrity.h b/include/linux/integrity.h index 54c853ec2fd1..6e46dc7930b5 100644 --- a/include/linux/integrity.h +++ b/include/linux/integrity.h @@ -16,6 +16,7 @@ enum integrity_status { INTEGRITY_PASS = 0, INTEGRITY_PASS_IMMUTABLE, INTEGRITY_FAIL, + INTEGRITY_FAIL_IMMUTABLE, INTEGRITY_NOLABEL, INTEGRITY_NOXATTRS, INTEGRITY_UNKNOWN, diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 84ae070e95e2..5d2fab131bc9 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -32,7 +32,8 @@ int evm_initialized;
static const char * const integrity_status_msg[] = { - "pass", "pass_immutable", "fail", "no_label", "no_xattrs", "unknown" + "pass", "pass_immutable", "fail", "fail_immutable", "no_label", + "no_xattrs", "unknown" }; int evm_hmac_attrs;
@@ -139,7 +140,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, enum integrity_status evm_status = INTEGRITY_PASS; struct evm_digest digest; struct inode *inode; - int rc, xattr_len; + int rc, xattr_len, evm_immutable = 0;
if (iint && (iint->evm_status == INTEGRITY_PASS || iint->evm_status == INTEGRITY_PASS_IMMUTABLE)) @@ -188,8 +189,10 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, if (rc) rc = -EINVAL; break; - case EVM_IMA_XATTR_DIGSIG: case EVM_XATTR_PORTABLE_DIGSIG: + evm_immutable = 1; + /* fall through */ + case EVM_IMA_XATTR_DIGSIG: /* accept xattr with non-empty signature field */ if (xattr_len <= sizeof(struct signature_v2_hdr)) { evm_status = INTEGRITY_FAIL; @@ -228,7 +231,8 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
if (rc) evm_status = (rc == -ENODATA) ? - INTEGRITY_NOXATTRS : INTEGRITY_FAIL; + INTEGRITY_NOXATTRS : evm_immutable ? + INTEGRITY_FAIL_IMMUTABLE : INTEGRITY_FAIL; out: if (iint) iint->evm_status = evm_status; @@ -360,6 +364,12 @@ static int evm_protect_xattr(struct dentry *dentry, const char *xattr_name, -EPERM, 0); } out: + /* It is safe to allow fail_immutable, portable signatures can never be + * updated + */ + if (evm_status == INTEGRITY_FAIL_IMMUTABLE) + return 0; + if (evm_status != INTEGRITY_PASS) integrity_audit_msg(AUDIT_INTEGRITY_METADATA, d_backing_inode(dentry), dentry->d_name.name, "appraise_metadata", @@ -497,9 +507,14 @@ int evm_inode_setattr(struct dentry *dentry, struct iattr *attr) if (!(ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID))) return 0; evm_status = evm_verify_current_integrity(dentry); + /* It is safe to allow fail_immutable, portable signatures can never + * be updated + */ if ((evm_status == INTEGRITY_PASS) || - (evm_status == INTEGRITY_NOXATTRS)) + (evm_status == INTEGRITY_NOXATTRS) || + (evm_status == INTEGRITY_FAIL_IMMUTABLE)) return 0; + integrity_audit_msg(AUDIT_INTEGRITY_METADATA, d_backing_inode(dentry), dentry->d_name.name, "appraise_metadata", integrity_status_msg[evm_status], -EPERM, 0); diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 08aabf763f52..aa847982eb1a 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -250,6 +250,7 @@ int ima_appraise_measurement(enum ima_hooks func, case INTEGRITY_NOLABEL: /* No security.evm xattr. */ cause = "missing-HMAC"; goto out; + case INTEGRITY_FAIL_IMMUTABLE: case INTEGRITY_FAIL: /* Invalid HMAC/signature. */ cause = "invalid-HMAC"; goto out;
hulk inclusion category: bugfix
---------------------------
If metadata are immutable, they cannot be changed. If metadata are already set to the final value before cp and tar restore the value from the source, those applications display an error even if the operation is legitimate (they don't change the value).
This patch determines whether setxattr()/setattr() change metadata and, if not, allows the operations even if metadata are immutable.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/evm/evm_main.c | 72 +++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+)
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 5d2fab131bc9..093cb7881427 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -23,6 +23,7 @@ #include <linux/integrity.h> #include <linux/evm.h> #include <linux/magic.h> +#include <linux/posix_acl_xattr.h>
#include <crypto/hash.h> #include <crypto/hash_info.h> @@ -314,6 +315,56 @@ static enum integrity_status evm_verify_current_integrity(struct dentry *dentry) return evm_verify_hmac(dentry, NULL, NULL, 0, NULL); }
+static int evm_xattr_acl_change(struct dentry *dentry, const char *xattr_name, + const void *xattr_value, size_t xattr_value_len) +{ + umode_t mode; + struct posix_acl *acl = NULL, *acl_res; + struct inode *inode = d_backing_inode(dentry); + int rc; + + /* UID/GID in ACL have been already converted from user to init ns */ + acl = posix_acl_from_xattr(&init_user_ns, xattr_value, xattr_value_len); + if (!acl) + return 1; + + acl_res = acl; + rc = posix_acl_update_mode(inode, &mode, &acl_res); + + posix_acl_release(acl); + + if (rc) + return 1; + + if (acl_res && inode->i_mode != mode) + return 1; + + return 0; +} + +static int evm_xattr_change(struct dentry *dentry, const char *xattr_name, + const void *xattr_value, size_t xattr_value_len) +{ + char *xattr_data = NULL; + int rc = 0; + + if (posix_xattr_acl(xattr_name)) + return evm_xattr_acl_change(dentry, xattr_name, xattr_value, + xattr_value_len); + + rc = vfs_getxattr_alloc(dentry, xattr_name, &xattr_data, 0, GFP_NOFS); + if (rc < 0) + return 1; + + if (rc == xattr_value_len) + rc = memcmp(xattr_value, xattr_data, rc); + else + rc = 1; + + kfree(xattr_data); + return rc; +} + /* * evm_protect_xattr - protect the EVM extended attribute * @@ -370,6 +421,10 @@ static int evm_protect_xattr(struct dentry *dentry, const char *xattr_name, if (evm_status == INTEGRITY_FAIL_IMMUTABLE) return 0;
+ if (evm_status == INTEGRITY_PASS_IMMUTABLE && + !evm_xattr_change(dentry, xattr_name, xattr_value, xattr_value_len)) + return 0; + if (evm_status != INTEGRITY_PASS) integrity_audit_msg(AUDIT_INTEGRITY_METADATA, d_backing_inode(dentry), dentry->d_name.name, "appraise_metadata", @@ -486,6 +541,19 @@ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name) evm_update_evmxattr(dentry, xattr_name, NULL, 0); }
+static int evm_attr_change(struct dentry *dentry, struct iattr *attr) +{ + struct inode *inode = d_backing_inode(dentry); + unsigned int ia_valid = attr->ia_valid; + + if ((!(ia_valid & ATTR_UID) || uid_eq(attr->ia_uid, inode->i_uid)) && + (!(ia_valid & ATTR_GID) || gid_eq(attr->ia_gid, inode->i_gid)) && + (!(ia_valid & ATTR_MODE) || attr->ia_mode == inode->i_mode)) + return 0; + + return 1; +} + /** * evm_inode_setattr - prevent updating an invalid EVM extended attribute * @dentry: pointer to the affected dentry @@ -515,6 +583,10 @@ int evm_inode_setattr(struct dentry *dentry, struct iattr *attr) (evm_status == INTEGRITY_FAIL_IMMUTABLE)) return 0;
+ if (evm_status == INTEGRITY_PASS_IMMUTABLE && + !evm_attr_change(dentry, attr)) + return 0; + integrity_audit_msg(AUDIT_INTEGRITY_METADATA, d_backing_inode(dentry), dentry->d_name.name, "appraise_metadata", integrity_status_msg[evm_status], -EPERM, 0);
hulk inclusion category: bugfix
---------------------------
When EVM_ALLOW_METADATA_WRITES is set, EVM allows any operation on metadata. Its main purpose is to allow users to freely set metadata when they are protected by a portable signature, until the HMAC key is loaded.
However, IMA is not notified about metadata changes and, after the first appraisal, always allows access to the files without checking metadata again.
This patch checks in evm_reset_status() if EVM_ALLOW_METADATA WRITES is enabled and if it is, sets the IMA_CHANGE_XATTR/ATTR bits depending on the operation performed. At the next appraisal, metadata are revalidated.
This patch also adds a call to evm_reset_status() in evm_inode_post_setattr() so that EVM won't return the cached status the next time appraisal is performed.
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_main.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 093cb7881427..2b55286edc49 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -487,13 +487,17 @@ int evm_inode_removexattr(struct dentry *dentry, const char *xattr_name) return evm_protect_xattr(dentry, xattr_name, NULL, 0); }
-static void evm_reset_status(struct inode *inode) +static void evm_reset_status(struct inode *inode, int bit) { struct integrity_iint_cache *iint;
iint = integrity_iint_find(inode); - if (iint) + if (iint) { + if (evm_initialized & EVM_ALLOW_METADATA_WRITES) + set_bit(bit, &iint->atomic_flags); + iint->evm_status = INTEGRITY_UNKNOWN; + } }
/** @@ -516,7 +520,7 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name, && !posix_xattr_acl(xattr_name))) return;
- evm_reset_status(dentry->d_inode); + evm_reset_status(dentry->d_inode, IMA_CHANGE_XATTR);
evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len); } @@ -536,7 +540,7 @@ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name) if (!evm_key_loaded() || !evm_protected_xattr(xattr_name)) return;
- evm_reset_status(dentry->d_inode); + evm_reset_status(dentry->d_inode, IMA_CHANGE_XATTR);
evm_update_evmxattr(dentry, xattr_name, NULL, 0); } @@ -609,6 +613,8 @@ void evm_inode_post_setattr(struct dentry *dentry, int ia_valid) if (!evm_key_loaded()) return;
+ evm_reset_status(dentry->d_inode, IMA_CHANGE_ATTR); + if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)) evm_update_evmxattr(dentry, NULL, NULL, 0); }
hulk inclusion category: bugfix
---------------------------
System administrators can require that all accessed files have a signature by specifying appraise_type=imasig in a policy rule.
Currently, only IMA signatures satisfy this requirement. IMA signatures ensure data source authentication for file content and prevent any change. EVM signatures instead ensure data source authentication for file metadata. Given that the digest or signature of the file content must be included in the metadata, EVM signatures provide at least the same guarantees of IMA signatures.
This patch lets systems protected with EVM signatures pass appraisal verification if the appraise_type=imasig requirement is specified in the policy. This facilitates deployment in the scenarios where only EVM signatures are available.
The patch makes the following changes:
file xattr types: security.ima: IMA_XATTR_DIGEST/IMA_XATTR_DIGEST_NG security.evm: EVM_XATTR_PORTABLE_DIGSIG
execve(), mmap(), open() behavior (with appraise_type=imasig): before: denied (file without IMA signature, imasig requirement not met) after: allowed (file with EVM portable signature, imasig requirement met)
open(O_WRONLY) behavior (without appraise_type=imasig): before: allowed (file without IMA signature, not immutable) after: denied (file with EVM portable signature, immutable)
In addition, similarly to IMA signatures, this patch temporarily allows new files without or with incomplete metadata to be opened so that content can be written.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/ima/ima_appraise.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index aa847982eb1a..8d61d2266d4d 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -251,6 +251,8 @@ int ima_appraise_measurement(enum ima_hooks func, cause = "missing-HMAC"; goto out; case INTEGRITY_FAIL_IMMUTABLE: + set_bit(IMA_DIGSIG, &iint->atomic_flags); + /* fall through */ case INTEGRITY_FAIL: /* Invalid HMAC/signature. */ cause = "invalid-HMAC"; goto out; @@ -269,12 +271,16 @@ int ima_appraise_measurement(enum ima_hooks func, status = INTEGRITY_FAIL; break; } - if (iint->flags & IMA_DIGSIG_REQUIRED) { - cause = "IMA-signature-required"; - status = INTEGRITY_FAIL; - break; + if (status != INTEGRITY_PASS_IMMUTABLE) { + if (iint->flags & IMA_DIGSIG_REQUIRED) { + cause = "IMA-signature-required"; + status = INTEGRITY_FAIL; + break; + } + clear_bit(IMA_DIGSIG, &iint->atomic_flags); + } else { + set_bit(IMA_DIGSIG, &iint->atomic_flags); } - clear_bit(IMA_DIGSIG, &iint->atomic_flags); if (xattr_len - sizeof(xattr_value->type) - hash_start >= iint->ima_hash->length) /* xattr length may be longer. md5 hash in previous @@ -336,9 +342,9 @@ int ima_appraise_measurement(enum ima_hooks func, status = INTEGRITY_PASS; }
- /* Permit new files with file signatures, but without data. */ + /* Permit new files marked as immutable, but without data. */ if (inode->i_size == 0 && iint->flags & IMA_NEW_FILE && - xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG) { + test_bit(IMA_DIGSIG, &iint->atomic_flags)) { status = INTEGRITY_PASS; }
hulk inclusion category: bugfix
---------------------------
Files might come from a remote source and might have xattrs, including security.ima. It should not be IMA task to decide whether security.ima should be kept or not. This patch removes the removexattr() system call in ima_inode_post_setattr().
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/ima/ima_appraise.c | 2 -- 1 file changed, 2 deletions(-)
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 8d61d2266d4d..d5e62385d891 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -403,8 +403,6 @@ void ima_inode_post_setattr(struct dentry *dentry) return;
action = ima_must_appraise(inode, MAY_ACCESS, POST_SETATTR); - if (!action) - __vfs_removexattr(dentry, XATTR_NAME_IMA); iint = integrity_iint_find(inode); if (iint) { set_bit(IMA_CHANGE_ATTR, &iint->atomic_flags);
hulk inclusion category: bugfix
---------------------------
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 --- security/integrity/ima/ima_crypto.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index 829ca1ac8ec4..4fa27b1e962a 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -683,6 +683,8 @@ static int ima_calc_boot_aggregate_tfm(char *digest, ima_pcrread(i, pcr_i); /* now accumulate with current aggregate */ rc = crypto_shash_update(shash, pcr_i, TPM_DIGEST_SIZE); + if (rc != 0) + return rc; } if (!rc) crypto_shash_final(shash, digest);
hulk inclusion category: bugfix
---------------------------
This patch removes the unnecessary semicolon at the end of ima_get_binary_runtime_size().
Cc: stable@vger.kernel.org Fixes: d158847ae89a2 ("ima: maintain memory size needed for serializing the measurement list") Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/ima/ima_queue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c index b186819bd5aa..d6e0502a80e9 100644 --- a/security/integrity/ima/ima_queue.c +++ b/security/integrity/ima/ima_queue.c @@ -136,7 +136,7 @@ unsigned long ima_get_binary_runtime_size(void) return ULONG_MAX; else return binary_runtime_size + sizeof(struct ima_kexec_hdr); -}; +}
static int ima_pcr_extend(const u8 *hash, int pcr) {
From: Mimi Zohar zohar@linux.vnet.ibm.com
hulk inclusion category: feature feature: initramfs-xattrs
---------------------------
This patch adds metadata to a file from a supplied buffer. The buffer might contains multiple metadata records. The format of each record is:
<metadata len (ASCII, 8 chars)><version><type><metadata>
For now, only the TYPE_XATTR metadata type is supported. The specific format of this metadata type is:
<xattr #N name>\0<xattr #N value>
[kamensky: fixed restoring of xattrs for symbolic links by using sys_lsetxattr() instead of sys_setxattr()]
[sassu: removed state management, kept only do_setxattrs(), added support for generic file metadata, replaced sys_lsetxattr() with vfs_setxattr(), added check for entry_size, added check for hdr->c_size, replaced strlen() with strnlen(); moved do_setxattrs() before do_name()]
Signed-off-by: Mimi Zohar zohar@linux.vnet.ibm.com Signed-off-by: Victor Kamensky kamensky@cisco.com Signed-off-by: Taras Kondratiuk takondra@cisco.com Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- include/linux/initramfs.h | 21 +++++++++ init/initramfs.c | 89 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 include/linux/initramfs.h
diff --git a/include/linux/initramfs.h b/include/linux/initramfs.h new file mode 100644 index 000000000000..2f8cee441236 --- /dev/null +++ b/include/linux/initramfs.h @@ -0,0 +1,21 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * include/linux/initramfs.h + * + * Include file for file metadata in the initial ram disk. + */ +#ifndef _LINUX_INITRAMFS_H +#define _LINUX_INITRAMFS_H + +#define METADATA_FILENAME "METADATA!!!" + +enum metadata_types { TYPE_NONE, TYPE_XATTR, TYPE__LAST }; + +struct metadata_hdr { + char c_size[8]; /* total size including c_size field */ + char c_version; /* header version */ + char c_type; /* metadata type */ + char c_metadata[]; /* metadata */ +} __packed; + +#endif /*LINUX_INITRAMFS_H*/ diff --git a/init/initramfs.c b/init/initramfs.c index dab8d63459f6..c337f7a50446 100644 --- a/init/initramfs.c +++ b/init/initramfs.c @@ -10,6 +10,9 @@ #include <linux/syscalls.h> #include <linux/utime.h> #include <linux/file.h> +#include <linux/namei.h> +#include <linux/xattr.h> +#include <linux/initramfs.h>
static ssize_t __init xwrite(int fd, const char *p, size_t count) { @@ -146,7 +149,7 @@ static __initdata time64_t mtime;
static __initdata unsigned long ino, major, minor, nlink; static __initdata umode_t mode; -static __initdata unsigned long body_len, name_len; +static __initdata unsigned long body_len, name_len, metadata_len; static __initdata uid_t uid; static __initdata gid_t gid; static __initdata unsigned rdev; @@ -218,7 +221,7 @@ static void __init read_into(char *buf, unsigned size, enum state next) } }
-static __initdata char *header_buf, *symlink_buf, *name_buf; +static __initdata char *header_buf, *symlink_buf, *name_buf, *metadata_buf;
static int __init do_start(void) { @@ -315,6 +318,88 @@ static int __init maybe_link(void) return 0; }
+static int __init do_setxattrs(char *pathname, char *buf, size_t size) +{ + struct path path; + char *xattr_name, *xattr_value; + uint32_t xattr_name_size, xattr_value_size; + int ret; + + xattr_name = buf; + xattr_name_size = strnlen(xattr_name, size); + if (xattr_name_size == size) { + error("malformed xattrs"); + return -EINVAL; + } + + xattr_value = xattr_name + xattr_name_size + 1; + xattr_value_size = buf + size - xattr_value; + + ret = kern_path(pathname, 0, &path); + if (!ret) { + ret = vfs_setxattr(path.dentry, xattr_name, xattr_value, + xattr_value_size, 0); + + path_put(&path); + } + + pr_debug("%s: %s size: %u val: %s (ret: %d)\n", pathname, + xattr_name, xattr_value_size, xattr_value, ret); + + return ret; +} + +static int __init __maybe_unused do_parse_metadata(char *pathname) +{ + char *buf = metadata_buf; + char *bufend = metadata_buf + metadata_len; + struct metadata_hdr *hdr; + char str[sizeof(hdr->c_size) + 1]; + uint32_t entry_size; + + if (!metadata_len) + return 0; + + str[sizeof(hdr->c_size)] = 0; + + while (buf < bufend) { + int ret; + + if (buf + sizeof(*hdr) > bufend) { + error("malformed metadata"); + break; + } + + hdr = (struct metadata_hdr *)buf; + if (hdr->c_version != 1) { + pr_debug("Unsupported header version\n"); + break; + } + + memcpy(str, hdr->c_size, sizeof(hdr->c_size)); + ret = kstrtou32(str, 16, &entry_size); + if (ret || buf + entry_size > bufend || + entry_size < sizeof(*hdr)) { + error("malformed xattrs"); + break; + } + + switch (hdr->c_type) { + case TYPE_XATTR: + do_setxattrs(pathname, buf + sizeof(*hdr), + entry_size - sizeof(*hdr)); + break; + default: + pr_debug("Unsupported metadata type\n"); + break; + } + + buf += entry_size; + } + + return 0; +} + static __initdata int wfd;
static int __init do_name(void)
hulk inclusion category: feature feature: initramfs-xattrs
---------------------------
Instead of changing the CPIO format, metadata are parsed from regular files with special name 'METADATA!!!'. This file immediately follows the file metadata are added to.
This patch checks if the file being extracted has the special name and, if yes, creates a buffer with the content of that file and calls do_parse_metadata() to parse metadata from the buffer.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com Reported-by: kbuild test robot lkp@intel.com --- init/initramfs.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/init/initramfs.c b/init/initramfs.c index c337f7a50446..430605b8d41d 100644 --- a/init/initramfs.c +++ b/init/initramfs.c @@ -222,6 +222,7 @@ static void __init read_into(char *buf, unsigned size, enum state next) }
static __initdata char *header_buf, *symlink_buf, *name_buf, *metadata_buf; +static __initdata char *metadata_buf_ptr, *previous_name_buf;
static int __init do_start(void) { @@ -401,6 +402,7 @@ static int __init __maybe_unused do_parse_metadata(char *pathname) }
static __initdata int wfd; +static int metadata __initdata;
static int __init do_name(void) { @@ -409,6 +411,10 @@ static int __init do_name(void) if (strcmp(collected, "TRAILER!!!") == 0) { free_hash(); return 0; + } else if (strcmp(collected, METADATA_FILENAME) == 0) { + metadata = 1; + } else { + memcpy(previous_name_buf, collected, strlen(collected) + 1); } clean_path(collected, mode); if (S_ISREG(mode)) { @@ -445,11 +451,47 @@ static int __init do_name(void) return 0; }
+static int __init do_process_metadata(char *buf, int len, bool last) +{ + int ret = 0; + + if (!metadata_buf) { + metadata_buf_ptr = metadata_buf = kmalloc(body_len, GFP_KERNEL); + if (!metadata_buf_ptr) { + ret = -ENOMEM; + goto out; + } + + metadata_len = body_len; + } + + if (metadata_buf_ptr + len > metadata_buf + metadata_len) { + ret = -EINVAL; + goto out; + } + + memcpy(metadata_buf_ptr, buf, len); + metadata_buf_ptr += len; + + if (last) + do_parse_metadata(previous_name_buf); +out: + if (ret < 0 || last) { + kfree(metadata_buf); + metadata_buf = NULL; + metadata = 0; + } + + return ret; +} + static int __init do_copy(void) { if (byte_count >= body_len) { if (xwrite(wfd, victim, body_len) != body_len) error("write error"); + if (metadata) + do_process_metadata(victim, body_len, true); ksys_close(wfd); do_utime(vcollected, mtime); kfree(vcollected); @@ -459,6 +501,8 @@ static int __init do_copy(void) } else { if (xwrite(wfd, victim, byte_count) != byte_count) error("write error"); + if (metadata) + do_process_metadata(victim, byte_count, false); body_len -= byte_count; eat(byte_count); return 1; @@ -468,6 +512,7 @@ static int __init do_copy(void) static int __init do_symlink(void) { collected[N_ALIGN(name_len) + body_len] = '\0'; + memcpy(previous_name_buf, collected, strlen(collected) + 1); clean_path(collected, 0); ksys_symlink(collected + N_ALIGN(name_len), collected); ksys_lchown(collected, uid, gid); @@ -535,8 +580,10 @@ static char * __init unpack_to_rootfs(char *buf, unsigned long len) header_buf = kmalloc(110, GFP_KERNEL); symlink_buf = kmalloc(PATH_MAX + N_ALIGN(PATH_MAX) + 1, GFP_KERNEL); name_buf = kmalloc(N_ALIGN(PATH_MAX), GFP_KERNEL); + previous_name_buf = kmalloc(PATH_MAX + N_ALIGN(PATH_MAX) + 1, + GFP_KERNEL);
- if (!header_buf || !symlink_buf || !name_buf) + if (!header_buf || !symlink_buf || !name_buf || !previous_name_buf) panic("can't allocate buffers");
state = Start; @@ -581,6 +628,7 @@ static char * __init unpack_to_rootfs(char *buf, unsigned long len) len -= my_inptr; } dir_utime(); + kfree(previous_name_buf); kfree(name_buf); kfree(symlink_buf); kfree(header_buf);
hulk inclusion category: feature feature: initramfs-xattrs
---------------------------
This patch adds support for file metadata (only TYPE_XATTR metadata type). gen_init_cpio has been modified to read xattrs from files that will be added to the image and to include file metadata as separate files with the special name 'METADATA!!!'.
This behavior can be selected by setting the desired file metadata type as value for CONFIG_INITRAMFS_FILE_METADATA.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- usr/Kconfig | 8 +++ usr/Makefile | 4 +- usr/gen_init_cpio.c | 137 ++++++++++++++++++++++++++++++++++++-- usr/gen_initramfs_list.sh | 10 ++- 4 files changed, 150 insertions(+), 9 deletions(-)
diff --git a/usr/Kconfig b/usr/Kconfig index 8b4826de1189..d5eeaecab214 100644 --- a/usr/Kconfig +++ b/usr/Kconfig @@ -233,3 +233,11 @@ config INITRAMFS_COMPRESSION default ".lzma" if RD_LZMA default ".bz2" if RD_BZIP2 default "" + +config INITRAMFS_FILE_METADATA + string "File metadata type" + default "" + help + Specify xattr to include xattrs in the image. + + If you are not sure, leave it blank. diff --git a/usr/Makefile b/usr/Makefile index 138c18cefb52..93b1d3376621 100644 --- a/usr/Makefile +++ b/usr/Makefile @@ -32,7 +32,9 @@ ramfs-input := $(if $(filter-out "",$(CONFIG_INITRAMFS_SOURCE)), \ $(shell echo $(CONFIG_INITRAMFS_SOURCE)),-d) ramfs-args := \ $(if $(CONFIG_INITRAMFS_ROOT_UID), -u $(CONFIG_INITRAMFS_ROOT_UID)) \ - $(if $(CONFIG_INITRAMFS_ROOT_GID), -g $(CONFIG_INITRAMFS_ROOT_GID)) + $(if $(CONFIG_INITRAMFS_ROOT_GID), -g $(CONFIG_INITRAMFS_ROOT_GID)) \ + $(if $(filter-out "",$(CONFIG_INITRAMFS_FILE_METADATA)), \ + -e $(CONFIG_INITRAMFS_FILE_METADATA))
# $(datafile_d_y) is used to identify all files included # in initramfs and to detect if any files are added/removed. diff --git a/usr/gen_init_cpio.c b/usr/gen_init_cpio.c index 03b21189d58b..6539b82ae075 100644 --- a/usr/gen_init_cpio.c +++ b/usr/gen_init_cpio.c @@ -3,6 +3,7 @@ #include <stdlib.h> #include <sys/types.h> #include <sys/stat.h> +#include <sys/xattr.h> #include <string.h> #include <unistd.h> #include <time.h> @@ -10,6 +11,7 @@ #include <errno.h> #include <ctype.h> #include <limits.h> +#include "../include/linux/initramfs.h"
/* * Original work by Jeff Garzik @@ -24,6 +26,115 @@ static unsigned int offset; static unsigned int ino = 721; static time_t default_mtime; +static char metadata_path[] = "/tmp/cpio-metadata-XXXXXX"; +static int metadata_fd = -1; + +static enum metadata_types parse_metadata_type(char *arg) +{ + static char *metadata_type_str[TYPE__LAST] = { + [TYPE_NONE] = "none", + [TYPE_XATTR] = "xattr", + }; + int i; + + for (i = 0; i < TYPE__LAST; i++) + if (!strcmp(metadata_type_str[i], arg)) + return i; + + return TYPE_NONE; +} + +static int cpio_mkfile(const char *name, const char *location, + unsigned int mode, uid_t uid, gid_t gid, + unsigned int nlinks); + +static int write_xattrs(const char *path) +{ + struct metadata_hdr hdr = { .c_version = 1, .c_type = TYPE_XATTR }; + char str[sizeof(hdr.c_size) + 1]; + char *xattr_list, *list_ptr, *xattr_value; + ssize_t list_len, name_len, value_len, len; + int ret = -EINVAL; + + if (metadata_fd < 0) + return 0; + + if (path == metadata_path) + return 0; + + list_len = listxattr(path, NULL, 0); + if (list_len <= 0) + return 0; + + list_ptr = xattr_list = malloc(list_len); + if (!list_ptr) { + fprintf(stderr, "out of memory\n"); + return ret; + } + + len = listxattr(path, xattr_list, list_len); + if (len != list_len) + goto out; + + if (ftruncate(metadata_fd, 0)) + goto out; + + lseek(metadata_fd, 0, SEEK_SET); + + while (list_ptr < xattr_list + list_len) { + name_len = strlen(list_ptr); + + value_len = getxattr(path, list_ptr, NULL, 0); + if (value_len < 0) { + fprintf(stderr, "cannot get xattrs\n"); + break; + } + + if (value_len) { + xattr_value = malloc(value_len); + if (!xattr_value) { + fprintf(stderr, "out of memory\n"); + break; + } + } else { + xattr_value = NULL; + } + + len = getxattr(path, list_ptr, xattr_value, value_len); + if (len != value_len) + break; + + snprintf(str, sizeof(str), "%.8lx", + sizeof(hdr) + name_len + 1 + value_len); + + memcpy(hdr.c_size, str, sizeof(hdr.c_size)); + + if (write(metadata_fd, &hdr, sizeof(hdr)) != sizeof(hdr)) + break; + + if (write(metadata_fd, list_ptr, name_len + 1) != name_len + 1) + break; + + if (write(metadata_fd, xattr_value, value_len) != value_len) + break; + + if (fsync(metadata_fd)) + break; + + list_ptr += name_len + 1; + free(xattr_value); + xattr_value = NULL; + } + + free(xattr_value); +out: + if (list_ptr != xattr_list + list_len) + return ret; + + free(xattr_list); + + return cpio_mkfile(METADATA_FILENAME, metadata_path, S_IFREG, 0, 0, 1); +}
struct file_handler { const char *type; @@ -128,7 +239,7 @@ static int cpio_mkslink(const char *name, const char *target, push_pad(); push_string(target); push_pad(); - return 0; + return write_xattrs(name); }
static int cpio_mkslink_line(const char *line) @@ -174,7 +285,7 @@ static int cpio_mkgeneric(const char *name, unsigned int mode, 0); /* chksum */ push_hdr(s); push_rest(name); - return 0; + return write_xattrs(name); }
enum generic_types { @@ -268,7 +379,7 @@ static int cpio_mknod(const char *name, unsigned int mode, 0); /* chksum */ push_hdr(s); push_rest(name); - return 0; + return write_xattrs(name); }
static int cpio_mknod_line(const char *line) @@ -372,8 +483,7 @@ static int cpio_mkfile(const char *name, const char *location, name += namesize; } ino++; - rc = 0; - + rc = write_xattrs(location); error: if (filebuf) free(filebuf); if (file >= 0) close(file); @@ -526,10 +636,11 @@ int main (int argc, char *argv[]) int ec = 0; int line_nr = 0; const char *filename; + enum metadata_types metadata_type = TYPE_NONE;
default_mtime = time(NULL); while (1) { - int opt = getopt(argc, argv, "t:h"); + int opt = getopt(argc, argv, "t:e:h"); char *invalid;
if (opt == -1) @@ -544,6 +655,9 @@ int main (int argc, char *argv[]) exit(1); } break; + case 'e': + metadata_type = parse_metadata_type(optarg); + break; case 'h': case '?': usage(argv[0]); @@ -565,6 +679,14 @@ int main (int argc, char *argv[]) exit(1); }
+ if (metadata_type != TYPE_NONE) { + metadata_fd = mkstemp(metadata_path); + if (metadata_fd < 0) { + fprintf(stderr, "cannot create temporary file\n"); + exit(1); + } + } + while (fgets(line, LINE_SIZE, cpio_list)) { int type_idx; size_t slen = strlen(line); @@ -620,5 +742,8 @@ int main (int argc, char *argv[]) if (ec == 0) cpio_trailer();
+ if (metadata_type != TYPE_NONE) + close(metadata_fd); + exit(ec); } diff --git a/usr/gen_initramfs_list.sh b/usr/gen_initramfs_list.sh index 0aad760fcd8c..0907a4043da9 100755 --- a/usr/gen_initramfs_list.sh +++ b/usr/gen_initramfs_list.sh @@ -15,7 +15,7 @@ set -e usage() { cat << EOF Usage: -$0 [-o <file>] [-u <uid>] [-g <gid>] {-d | <cpio_source>} ... +$0 [-o <file>] [-u <uid>] [-g <gid>] {-d | <cpio_source>} [-e <type>] ... -o <file> Create compressed initramfs file named <file> using gen_init_cpio and compressor depending on the extension -u <uid> User ID to map to user ID 0 (root). @@ -28,6 +28,7 @@ $0 [-o <file>] [-u <uid>] [-g <gid>] {-d | <cpio_source>} ... If <cpio_source> is a .cpio file it will be used as direct input to initramfs. -d Output the default cpio list. + -e <type> File metadata type to include in the cpio archive.
All options except -o and -l may be repeated and are interpreted sequentially and immediately. -u and -g states are preserved across @@ -283,6 +284,10 @@ while [ $# -gt 0 ]; do default_list="$arg" ${dep_list}default_initramfs ;; + "-e") # file metadata type + metadata_arg="-e $1" + shift + ;; "-h") usage exit 0 @@ -312,7 +317,8 @@ if [ ! -z ${output_file} ]; then fi fi cpio_tfile="$(mktemp ${TMPDIR:-/tmp}/cpiofile.XXXXXX)" - usr/gen_init_cpio $timestamp ${cpio_list} > ${cpio_tfile} + usr/gen_init_cpio $metadata_arg $timestamp \ + ${cpio_list} > ${cpio_tfile} else cpio_tfile=${cpio_file} fi
hulk inclusion category: feature feature: initramfs-xattrs
---------------------------
This patch adds the new option initramtmpfs for the kernel command line, to force usage of tmpfs instead of ramfs as filesystem for rootfs.
This option should be used when the initial ram disk contains xattrs, as only tmpfs supports them.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- init/do_mounts.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/init/do_mounts.c b/init/do_mounts.c index e1c9afa9d8c9..ebe388ec1861 100644 --- a/init/do_mounts.c +++ b/init/do_mounts.c @@ -31,6 +31,7 @@ int root_mountflags = MS_RDONLY | MS_SILENT; static char * __initdata root_device_name; static char __initdata saved_root_name[64]; static int root_wait; +static int initramtmpfs;
dev_t ROOT_DEV;
@@ -319,9 +320,16 @@ static int __init root_delay_setup(char *str) return 1; }
+static int __init initramtmpfs_setup(char *str) +{ + initramtmpfs = 1; + return 1; +} + __setup("rootflags=", root_data_setup); __setup("rootfstype=", fs_names_setup); __setup("rootdelay=", root_delay_setup); +__setup("initramtmpfs", initramtmpfs_setup);
static void __init get_fs_names(char *page) { @@ -622,7 +630,8 @@ int __init init_rootfs(void) if (err) return err;
- if (IS_ENABLED(CONFIG_TMPFS) && !saved_root_name[0] && + if (IS_ENABLED(CONFIG_TMPFS) && + (!saved_root_name[0] || initramtmpfs) && (!root_fs_names || strstr(root_fs_names, "tmpfs"))) { err = shmem_init(); is_tmpfs = true;
hulk inclusion category: bugfix
---------------------------
IMA and EVM have been designed as two independent subsystems: the first for checking the integrity of file data; the second for checking file metadata. Making them independent allows users to adopt them incrementally.
The point of intersection is in IMA-Appraise, which calls evm_verifyxattr() to ensure that security.ima wasn't modified during an offline attack. The design choice, to ensure incremental adoption, was to continue appraisal verification if evm_verifyxattr() returns INTEGRITY_UNKNOWN. This value is returned when EVM is not enabled in the kernel configuration, or if the HMAC key has not been loaded yet.
Although this choice appears legitimate, it might not be suitable for hardened systems, where the administrator expects that access is denied if there is any error. An attacker could intentionally delete the EVM keys from the system and set the file digest in security.ima to the actual file digest so that the final appraisal status is INTEGRITY_PASS.
This patch allows such hardened systems to strictly enforce an access control policy based on the validity of signatures/HMACs, by introducing two new values for the ima_appraise= kernel option: enforce-evm and log-evm.
Cc: stable@vger.kernel.org Fixes: 2fe5d6def1672 ("ima: integrity appraisal extension") Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- Documentation/admin-guide/kernel-parameters.txt | 3 ++- security/integrity/ima/ima_appraise.c | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index e67621c68dc2..7b4487f08ea1 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -1553,7 +1553,8 @@ Set number of hash buckets for inode cache.
ima_appraise= [IMA] appraise integrity measurements - Format: { "off" | "enforce" | "fix" | "log" } + Format: { "off" | "enforce" | "fix" | "log" | + "enforce-evm" | "log-evm" } default: "enforce"
ima_appraise_tcb [IMA] diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index d5e62385d891..fb9f7c8deda6 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -18,6 +18,7 @@
#include "ima.h"
+static bool ima_appraise_req_evm __ro_after_init; static int __init default_appraise_setup(char *str) { #ifdef CONFIG_IMA_APPRAISE_BOOTPARAM @@ -28,6 +29,9 @@ static int __init default_appraise_setup(char *str) else if (strncmp(str, "fix", 3) == 0) ima_appraise = IMA_APPRAISE_FIX; #endif + if (strcmp(str, "enforce-evm") == 0 || + strcmp(str, "log-evm") == 0) + ima_appraise_req_evm = true; return 1; }
@@ -244,7 +248,11 @@ int ima_appraise_measurement(enum ima_hooks func, switch (status) { case INTEGRITY_PASS: case INTEGRITY_PASS_IMMUTABLE: + break; case INTEGRITY_UNKNOWN: + if (ima_appraise_req_evm && + xattr_value->type != EVM_IMA_XATTR_DIGSIG) + goto out; break; case INTEGRITY_NOXATTRS: /* No EVM protected xattrs. */ case INTEGRITY_NOLABEL: /* No security.evm xattr. */
hulk inclusion category: bugfix
---------------------------
IMA reads the hash algorithm from security.ima, if exists, so that a signature can be verified with the correct file digest.
This patch moves ima_read_xattr() and ima_get_hash_algo() to ima_main.c, so that the file digest in the measurement list or in the audit logs can be compared with a reference value calculated with a specific hash algorithm.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/ima/ima.h | 17 --------- security/integrity/ima/ima_appraise.c | 51 --------------------------- security/integrity/ima/ima_main.c | 51 +++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 68 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 6c81fd922af8..eb4ebd2cfa56 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -246,11 +246,6 @@ int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func); void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file); enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint, enum ima_hooks func); -enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, - int xattr_len); -int ima_read_xattr(struct dentry *dentry, - struct evm_ima_xattr_data **xattr_value); - #else static inline int ima_appraise_measurement(enum ima_hooks func, struct integrity_iint_cache *iint, @@ -280,18 +275,6 @@ static inline enum integrity_status ima_get_cache_status(struct integrity_iint_c return INTEGRITY_UNKNOWN; }
-static inline enum hash_algo -ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len) -{ - return ima_hash_algo; -} - -static inline int ima_read_xattr(struct dentry *dentry, - struct evm_ima_xattr_data **xattr_value) -{ - return 0; -} - #endif /* CONFIG_IMA_APPRAISE */
/* LSM based policy rules require audit */ diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index fb9f7c8deda6..f51ab67f7ab5 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -153,57 +153,6 @@ static void ima_cache_flags(struct integrity_iint_cache *iint, } }
-enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, - int xattr_len) -{ - struct signature_v2_hdr *sig; - enum hash_algo ret; - - if (!xattr_value || xattr_len < 2) - /* return default hash algo */ - return ima_hash_algo; - - switch (xattr_value->type) { - case EVM_IMA_XATTR_DIGSIG: - sig = (typeof(sig))xattr_value; - if (sig->version != 2 || xattr_len <= sizeof(*sig)) - return ima_hash_algo; - return sig->hash_algo; - break; - case IMA_XATTR_DIGEST_NG: - ret = xattr_value->digest[0]; - if (ret < HASH_ALGO__LAST) - return ret; - break; - case IMA_XATTR_DIGEST: - /* this is for backward compatibility */ - if (xattr_len == 21) { - unsigned int zero = 0; - if (!memcmp(&xattr_value->digest[16], &zero, 4)) - return HASH_ALGO_MD5; - else - return HASH_ALGO_SHA1; - } else if (xattr_len == 17) - return HASH_ALGO_MD5; - break; - } - - /* return default hash algo */ - return ima_hash_algo; -} - -int ima_read_xattr(struct dentry *dentry, - struct evm_ima_xattr_data **xattr_value) -{ - ssize_t ret; - - ret = vfs_getxattr_alloc(dentry, XATTR_NAME_IMA, (char **)xattr_value, - 0, GFP_NOFS); - if (ret == -EOPNOTSUPP) - ret = 0; - return ret; -} - /* * ima_appraise_measurement - appraise file measurement * diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index c08dbc55e5f9..703f65dcedde 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -120,6 +120,57 @@ static void ima_rdwr_violation_check(struct file *file, "invalid_pcr", "open_writers"); }
+static enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, + int xattr_len) +{ + struct signature_v2_hdr *sig; + enum hash_algo ret; + + if (!xattr_value || xattr_len < 2) + /* return default hash algo */ + return ima_hash_algo; + + switch (xattr_value->type) { + case EVM_IMA_XATTR_DIGSIG: + sig = (typeof(sig))xattr_value; + if (sig->version != 2 || xattr_len <= sizeof(*sig)) + return ima_hash_algo; + return sig->hash_algo; + break; + case IMA_XATTR_DIGEST_NG: + ret = xattr_value->digest[0]; + if (ret < HASH_ALGO__LAST) + return ret; + break; + case IMA_XATTR_DIGEST: + /* this is for backward compatibility */ + if (xattr_len == 21) { + unsigned int zero = 0; + if (!memcmp(&xattr_value->digest[16], &zero, 4)) + return HASH_ALGO_MD5; + else + return HASH_ALGO_SHA1; + } else if (xattr_len == 17) + return HASH_ALGO_MD5; + break; + } + + /* return default hash algo */ + return ima_hash_algo; +} + +static int ima_read_xattr(struct dentry *dentry, + struct evm_ima_xattr_data **xattr_value) +{ + ssize_t ret; + + ret = vfs_getxattr_alloc(dentry, XATTR_NAME_IMA, (char **)xattr_value, + 0, GFP_NOFS); + if (ret == -EOPNOTSUPP) + ret = 0; + return ret; +} + static void ima_check_last_writer(struct integrity_iint_cache *iint, struct inode *inode, struct file *file) {
On 2020/7/6 23:41, Roberto Sassu wrote:
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index c08dbc55e5f9..703f65dcedde 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -120,6 +120,57 @@ static void ima_rdwr_violation_check(struct file *file, "invalid_pcr", "open_writers"); }
+static enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
int xattr_len)
+{
- struct signature_v2_hdr *sig;
- enum hash_algo ret;
- if (!xattr_value || xattr_len < 2)
/* return default hash algo */
return ima_hash_algo;
- switch (xattr_value->type) {
- case EVM_IMA_XATTR_DIGSIG:
sig = (typeof(sig))xattr_value;
if (sig->version != 2 || xattr_len <= sizeof(*sig))
return ima_hash_algo;
return sig->hash_algo;
break;
This break after return is not needed.
- case IMA_XATTR_DIGEST_NG:
ret = xattr_value->digest[0];
if (ret < HASH_ALGO__LAST)
return ret;
break;
- case IMA_XATTR_DIGEST:
/* this is for backward compatibility */
if (xattr_len == 21) {
unsigned int zero = 0;
if (!memcmp(&xattr_value->digest[16], &zero, 4))
return HASH_ALGO_MD5;
else
return HASH_ALGO_SHA1;
} else if (xattr_len == 17)
return HASH_ALGO_MD5;
break;
- }
-----Original Message----- From: Guohanjun (Hanjun Guo) Sent: Tuesday, July 7, 2020 5:44 AM To: Roberto Sassu roberto.sassu@huawei.com; kernel@openeuler.org Cc: Silviu Vlasceanu Silviu.Vlasceanu@huawei.com Subject: Re: [PATCH 18/35] ima: Allow choice of file hash algorithm for measurement and audit
On 2020/7/6 23:41, Roberto Sassu wrote:
diff --git a/security/integrity/ima/ima_main.c
b/security/integrity/ima/ima_main.c
index c08dbc55e5f9..703f65dcedde 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -120,6 +120,57 @@ static void ima_rdwr_violation_check(struct file
*file,
"invalid_pcr", "open_writers");
}
+static enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data
*xattr_value,
int xattr_len)
+{
- struct signature_v2_hdr *sig;
- enum hash_algo ret;
- if (!xattr_value || xattr_len < 2)
/* return default hash algo */
return ima_hash_algo;
- switch (xattr_value->type) {
- case EVM_IMA_XATTR_DIGSIG:
sig = (typeof(sig))xattr_value;
if (sig->version != 2 || xattr_len <= sizeof(*sig))
return ima_hash_algo;
return sig->hash_algo;
break;
This break after return is not needed.
Correct, I just wanted to preserve the original code.
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli
- case IMA_XATTR_DIGEST_NG:
ret = xattr_value->digest[0];
if (ret < HASH_ALGO__LAST)
return ret;
break;
- case IMA_XATTR_DIGEST:
/* this is for backward compatibility */
if (xattr_len == 21) {
unsigned int zero = 0;
if (!memcmp(&xattr_value->digest[16], &zero, 4))
return HASH_ALGO_MD5;
else
return HASH_ALGO_SHA1;
} else if (xattr_len == 17)
return HASH_ALGO_MD5;
break;
- }
hulk inclusion category: feature feature: digest-lists
---------------------------
This patch renames ima_read_policy() to ima_read_file() so that the function can be used to read files for different purposes. It also adds the opened file in securityfs as parameter so that the function can determine which action it should do with the passed data.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/ima/ima_fs.c | 48 ++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 15 deletions(-)
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index 604cdac63d84..848ce94aab38 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -26,11 +26,20 @@ #include <linux/rcupdate.h> #include <linux/parser.h> #include <linux/vmalloc.h> +#include <linux/file.h>
#include "ima.h"
static DEFINE_MUTEX(ima_write_mutex);
+static struct dentry *ima_dir; +static struct dentry *ima_symlink; +static struct dentry *binary_runtime_measurements; +static struct dentry *ascii_runtime_measurements; +static struct dentry *runtime_measurements_count; +static struct dentry *violations; +static struct dentry *ima_policy; + bool ima_canonical_fmt; static int __init default_canonical_fmt_setup(char *str) { @@ -275,11 +284,13 @@ static const struct file_operations ima_ascii_measurements_ops = { .release = seq_release, };
-static ssize_t ima_read_policy(char *path) +static ssize_t ima_read_file(char *path, struct dentry *dentry) { void *data; char *datap; loff_t size; + struct file *file; + enum kernel_read_file_id file_id = READING_POLICY; int rc, pathlen = strlen(path);
char *p; @@ -288,21 +299,36 @@ static ssize_t ima_read_policy(char *path) datap = path; strsep(&datap, "\n");
- rc = kernel_read_file_from_path(path, &data, &size, 0, READING_POLICY); + file = filp_open(path, O_RDONLY, 0); + if (IS_ERR(file)) { + pr_err("Unable to open file: %s (%ld)", path, PTR_ERR(file)); + return PTR_ERR(file); + } + + rc = kernel_read_file(file, &data, &size, 0, file_id); if (rc < 0) { - pr_err("Unable to open file: %s (%d)", path, rc); + pr_err("Unable to read file: %s (%d)", path, rc); + fput(file); return rc; }
datap = data; - while (size > 0 && (p = strsep(&datap, "\n"))) { - pr_debug("rule: %s\n", p); - rc = ima_parse_add_rule(p); + while (size > 0) { + if (dentry == ima_policy) { + p = strsep(&datap, "\n"); + if (p == NULL) + break; + + pr_debug("rule: %s\n", p); + rc = ima_parse_add_rule(p); + } + if (rc < 0) break; size -= rc; }
+ fput(file); vfree(data); if (rc < 0) return rc; @@ -337,7 +363,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, goto out_free;
if (data[0] == '/') { - result = ima_read_policy(data); + result = ima_read_file(data, file_dentry(file)); } else if (ima_appraise & IMA_APPRAISE_POLICY) { pr_err("signed policy file (specified as an absolute pathname) required\n"); integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL, @@ -357,14 +383,6 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, return result; }
-static struct dentry *ima_dir; -static struct dentry *ima_symlink; -static struct dentry *binary_runtime_measurements; -static struct dentry *ascii_runtime_measurements; -static struct dentry *runtime_measurements_count; -static struct dentry *violations; -static struct dentry *ima_policy; - enum ima_fs_flags { IMA_FS_BUSY, };
hulk inclusion category: feature feature: digest-lists
---------------------------
ima_write_policy() is being used to load a new policy from user space. This function can be reused to load different types of data.
This patch renames ima_write_policy() to ima_write_data() and executes the appropriate actions depending on the opened file in securityfs.
Also, this patch raises the uploaded data size limit to 64M, to accept files (e.g. digest lists) larger than a policy. The same limit is used for the SELinux policy.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/ima/ima_fs.c | 51 +++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 19 deletions(-)
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index 848ce94aab38..1630b0ec1af6 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -338,25 +338,32 @@ static ssize_t ima_read_file(char *path, struct dentry *dentry) return pathlen; }
-static ssize_t ima_write_policy(struct file *file, const char __user *buf, - size_t datalen, loff_t *ppos) +static ssize_t ima_write_data(struct file *file, const char __user *buf, + size_t datalen, loff_t *ppos) { char *data; ssize_t result; - - if (datalen >= PAGE_SIZE) - datalen = PAGE_SIZE - 1; + struct dentry *dentry = file_dentry(file);
/* No partial writes. */ result = -EINVAL; if (*ppos != 0) goto out;
- data = memdup_user_nul(buf, datalen); - if (IS_ERR(data)) { - result = PTR_ERR(data); + result = -EFBIG; + if (datalen + 1 > 64 * 1024 * 1024) goto out; - } + + result = -ENOMEM; + data = vmalloc(datalen + 1); + if (!data) + goto out; + + result = -EFAULT; + if (copy_from_user(data, buf, datalen) != 0) + goto out_free; + + data[datalen] = '\0';
result = mutex_lock_interruptible(&ima_write_mutex); if (result < 0) @@ -364,20 +371,26 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
if (data[0] == '/') { result = ima_read_file(data, file_dentry(file)); - } else if (ima_appraise & IMA_APPRAISE_POLICY) { - pr_err("signed policy file (specified as an absolute pathname) required\n"); - integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL, - "policy_update", "signed policy required", - 1, 0); - result = -EACCES; + } else if (dentry == ima_policy) { + if (ima_appraise & IMA_APPRAISE_POLICY) { + pr_err("signed policy file (specified " + "as an absolute pathname) required\n"); + integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL, + "policy_update", + "signed policy required", 1, 0); + result = -EACCES; + } else { + result = ima_parse_add_rule(data); + } } else { - result = ima_parse_add_rule(data); + pr_err("Unknown data type\n"); + result = -EINVAL; } mutex_unlock(&ima_write_mutex); out_free: - kfree(data); + vfree(data); out: - if (result < 0) + if (dentry == ima_policy && result < 0) valid_policy = 0;
return result; @@ -463,7 +476,7 @@ static int ima_release_policy(struct inode *inode, struct file *file)
static const struct file_operations ima_measure_policy_ops = { .open = ima_open_policy, - .write = ima_write_policy, + .write = ima_write_data, .read = seq_read, .release = ima_release_policy, .llseek = generic_file_llseek,
hulk inclusion category: feature feature: digest-lists
---------------------------
This patch renames ima_open_policy() and ima_release_policy() respectively to ima_open_data_upload() and ima_release_data_upload(). They will be used to implement file operations for interfaces allowing to load data from user space.
A new flag (IMA_POLICY_BUSY) has been defined to prevent concurrent policy upload.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/ima/ima_fs.c | 60 ++++++++++++++++++++++++--------- 1 file changed, 44 insertions(+), 16 deletions(-)
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index 1630b0ec1af6..c27f8d3ad22c 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -397,9 +397,20 @@ static ssize_t ima_write_data(struct file *file, const char __user *buf, }
enum ima_fs_flags { + IMA_POLICY_BUSY, IMA_FS_BUSY, };
+static enum ima_fs_flags ima_get_dentry_flag(struct dentry *dentry) +{ + enum ima_fs_flags flag = IMA_FS_BUSY; + + if (dentry == ima_policy) + flag = IMA_POLICY_BUSY; + + return flag; +} + static unsigned long ima_fs_flags;
#ifdef CONFIG_IMA_READ_POLICY @@ -412,40 +423,57 @@ static const struct seq_operations ima_policy_seqops = { #endif
/* - * ima_open_policy: sequentialize access to the policy file + * ima_open_data_upload: sequentialize access to the data upload interface */ -static int ima_open_policy(struct inode *inode, struct file *filp) +static int ima_open_data_upload(struct inode *inode, struct file *filp) { + struct dentry *dentry = file_dentry(filp); + const struct seq_operations *seq_ops = NULL; + enum ima_fs_flags flag = ima_get_dentry_flag(dentry); + bool read_allowed = false; + + if (dentry == ima_policy) { +#ifdef CONFIG_IMA_READ_POLICY + read_allowed = true; + seq_ops = &ima_policy_seqops; +#endif + } + if (!(filp->f_flags & O_WRONLY)) { -#ifndef CONFIG_IMA_READ_POLICY - return -EACCES; -#else + if (!read_allowed) + return -EACCES; if ((filp->f_flags & O_ACCMODE) != O_RDONLY) return -EACCES; if (!capable(CAP_SYS_ADMIN)) return -EPERM; - return seq_open(filp, &ima_policy_seqops); -#endif + return seq_open(filp, seq_ops); } - if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags)) + if (test_and_set_bit(flag, &ima_fs_flags)) return -EBUSY; return 0; }
/* - * ima_release_policy - start using the new measure policy rules. + * ima_release_data_upload - start using the new measure policy rules. * * Initially, ima_measure points to the default policy rules, now * point to the new policy rules, and remove the securityfs policy file, * assuming a valid policy. */ -static int ima_release_policy(struct inode *inode, struct file *file) +static int ima_release_data_upload(struct inode *inode, struct file *file) { + struct dentry *dentry = file_dentry(file); const char *cause = valid_policy ? "completed" : "failed"; + enum ima_fs_flags flag = ima_get_dentry_flag(dentry);
if ((file->f_flags & O_ACCMODE) == O_RDONLY) return seq_release(inode, file);
+ if (dentry != ima_policy) { + clear_bit(flag, &ima_fs_flags); + return 0; + } + if (valid_policy && ima_check_policy() < 0) { cause = "failed"; valid_policy = 0; @@ -458,7 +486,7 @@ static int ima_release_policy(struct inode *inode, struct file *file) if (!valid_policy) { ima_delete_rules(); valid_policy = 1; - clear_bit(IMA_FS_BUSY, &ima_fs_flags); + clear_bit(flag, &ima_fs_flags); return 0; }
@@ -467,18 +495,18 @@ static int ima_release_policy(struct inode *inode, struct file *file) securityfs_remove(ima_policy); ima_policy = NULL; #elif defined(CONFIG_IMA_WRITE_POLICY) - clear_bit(IMA_FS_BUSY, &ima_fs_flags); + clear_bit(flag, &ima_fs_flags); #elif defined(CONFIG_IMA_READ_POLICY) inode->i_mode &= ~S_IWUSR; #endif return 0; }
-static const struct file_operations ima_measure_policy_ops = { - .open = ima_open_policy, +static const struct file_operations ima_data_upload_ops = { + .open = ima_open_data_upload, .write = ima_write_data, .read = seq_read, - .release = ima_release_policy, + .release = ima_release_data_upload, .llseek = generic_file_llseek, };
@@ -522,7 +550,7 @@ int __init ima_fs_init(void)
ima_policy = securityfs_create_file("policy", POLICY_FILE_FLAGS, ima_dir, NULL, - &ima_measure_policy_ops); + &ima_data_upload_ops); if (IS_ERR(ima_policy)) goto out;
hulk inclusion category: feature feature: digest-lists
---------------------------
ima_show_htable_violations() and ima_show_measurements_count() both call ima_show_htable_value() to copy the value of an atomic_long_t variable to a buffer.
This patch modifies the definition of ima_show_htable_value(), so that this function can be used in any file_operations structure. The atomic_long_t variable used as source is chosen depending on the opened file in the securityfs filesystem.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/ima/ima_fs.c | 38 +++++++++++---------------------- 1 file changed, 12 insertions(+), 26 deletions(-)
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index c27f8d3ad22c..d0cc9d849dae 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -52,38 +52,24 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
static int valid_policy = 1;
-static ssize_t ima_show_htable_value(char __user *buf, size_t count, - loff_t *ppos, atomic_long_t *val) +static ssize_t ima_show_htable_value(struct file *filp, char __user *buf, + size_t count, loff_t *ppos) { + atomic_long_t *val = NULL; char tmpbuf[32]; /* greater than largest 'long' string value */ ssize_t len;
+ if (filp->f_path.dentry == violations) + val = &ima_htable.violations; + else if (filp->f_path.dentry == runtime_measurements_count) + val = &ima_htable.len; + len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", atomic_long_read(val)); return simple_read_from_buffer(buf, count, ppos, tmpbuf, len); }
-static ssize_t ima_show_htable_violations(struct file *filp, - char __user *buf, - size_t count, loff_t *ppos) -{ - return ima_show_htable_value(buf, count, ppos, &ima_htable.violations); -} - -static const struct file_operations ima_htable_violations_ops = { - .read = ima_show_htable_violations, - .llseek = generic_file_llseek, -}; - -static ssize_t ima_show_measurements_count(struct file *filp, - char __user *buf, - size_t count, loff_t *ppos) -{ - return ima_show_htable_value(buf, count, ppos, &ima_htable.len); - -} - -static const struct file_operations ima_measurements_count_ops = { - .read = ima_show_measurements_count, +static const struct file_operations ima_htable_value_ops = { + .read = ima_show_htable_value, .llseek = generic_file_llseek, };
@@ -538,13 +524,13 @@ int __init ima_fs_init(void) runtime_measurements_count = securityfs_create_file("runtime_measurements_count", S_IRUSR | S_IRGRP, ima_dir, NULL, - &ima_measurements_count_ops); + &ima_htable_value_ops); if (IS_ERR(runtime_measurements_count)) goto out;
violations = securityfs_create_file("violations", S_IRUSR | S_IRGRP, - ima_dir, NULL, &ima_htable_violations_ops); + ima_dir, NULL, &ima_htable_value_ops); if (IS_ERR(violations)) goto out;
hulk inclusion category: feature feature: digest-lists
---------------------------
This patch introduces the parser of the compact digest list. The format is optimized to store a large quantity of data with the same type. It is the only format supported by the kernel. Digest lists can be uploaded by writing the path to securityfs, as the same as for IMA policies.
A compact list is a set of consecutive data blocks, each consisting of a header and a payload. The header indicates the version of the header, the type of data, type modifiers, the hash algorithm, how many elements and the length of the payload.
COMPACT_KEY identifies public keys used for signature verification of the digest lists; COMPACT_PARSER identifies digests of user space parsers allowed to directly upload parsed digest lists to the kernel; COMPACT_FILE identifies digests of regular files; COMPACT_METADATA identifies digest of file metadata.
Type modifiers indicate attributes of the elements included in the payload. The COMPACT_MOD_IMMUTABLE modifier indicates that a file or metadata are immutable.
This patch also introduces ima_lookup_loaded_digest() and ima_add_digest_data_entry() to search and add digests in the new hash table (ima_digests_htable).
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/ima/Kconfig | 10 ++ security/integrity/ima/Makefile | 1 + security/integrity/ima/ima_digest_list.c | 176 +++++++++++++++++++++++ security/integrity/ima/ima_digest_list.h | 32 +++++ security/integrity/integrity.h | 37 +++++ 5 files changed, 256 insertions(+) create mode 100644 security/integrity/ima/ima_digest_list.c create mode 100644 security/integrity/ima/ima_digest_list.h
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 13b446328dda..e063e14d8740 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -287,3 +287,13 @@ config IMA_APPRAISE_SIGNED_INIT default n help This option requires user-space init to be signed. + +config IMA_DIGEST_LIST + bool "Measure and appraise files with digest lists" + depends on IMA + default n + help + This option allows users to load digest lists. If calculated digests + of accessed files are found in one of those lists, no new entries are + added to the measurement list, and access to the file is granted if + appraisal is in enforcing mode. diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile index d921dc4f9eb0..29b49c7aacd0 100644 --- a/security/integrity/ima/Makefile +++ b/security/integrity/ima/Makefile @@ -10,4 +10,5 @@ ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \ ima_policy.o ima_template.o ima_template_lib.o ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o +ima-$(CONFIG_IMA_DIGEST_LIST) += ima_digest_list.o obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o diff --git a/security/integrity/ima/ima_digest_list.c b/security/integrity/ima/ima_digest_list.c new file mode 100644 index 000000000000..0dcc69954887 --- /dev/null +++ b/security/integrity/ima/ima_digest_list.c @@ -0,0 +1,176 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2017-2019 Huawei Technologies Duesseldorf GmbH + * + * Author: Roberto Sassu roberto.sassu@huawei.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation, version 2 of the + * License. + * + * File: ima_digest_list.c + * Functions to manage digest lists. + */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/vmalloc.h> +#include <linux/module.h> + +#include "ima.h" +#include "ima_digest_list.h" + +struct ima_h_table ima_digests_htable = { + .len = ATOMIC_LONG_INIT(0), + .queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT +}; + +/************************* + * Get/add/del functions * + *************************/ +struct ima_digest *ima_lookup_digest(u8 *digest, enum hash_algo algo, + enum compact_types type) +{ + struct ima_digest *d = NULL; + int digest_len = hash_digest_size[algo]; + unsigned int key = ima_hash_key(digest); + + rcu_read_lock(); + hlist_for_each_entry_rcu(d, &ima_digests_htable.queue[key], hnext) + if (d->algo == algo && d->type == type && + !memcmp(d->digest, digest, digest_len)) + break; + + rcu_read_unlock(); + return d; +} + +static int ima_add_digest_data_entry(u8 *digest, enum hash_algo algo, + enum compact_types type, u16 modifiers) +{ + struct ima_digest *d; + int digest_len = hash_digest_size[algo]; + unsigned int key = ima_hash_key(digest); + + d = ima_lookup_digest(digest, algo, type); + if (d) { + d->modifiers |= modifiers; + if (d->count < (u16)(~((u16)0))) + d->count++; + return -EEXIST; + } + + d = kmalloc(sizeof(*d) + digest_len, GFP_KERNEL); + if (d == NULL) + return -ENOMEM; + + d->algo = algo; + d->type = type; + d->modifiers = modifiers; + d->count = 1; + + memcpy(d->digest, digest, digest_len); + hlist_add_head_rcu(&d->hnext, &ima_digests_htable.queue[key]); + atomic_long_inc(&ima_digests_htable.len); + return 0; +} + +static void ima_del_digest_data_entry(u8 *digest, enum hash_algo algo, + enum compact_types type) +{ + struct ima_digest *d; + + d = ima_lookup_digest(digest, algo, type); + if (!d) + return; + + if (--d->count > 0) + return; + + hlist_del_rcu(&d->hnext); + atomic_long_dec(&ima_digests_htable.len); +} + +/*********************** + * Compact list parser * + ***********************/ +struct compact_list_hdr { + u8 version; + u8 _reserved; + u16 type; + u16 modifiers; + u16 algo; + u32 count; + u32 datalen; +} __packed; + +int ima_parse_compact_list(loff_t size, void *buf, int op) +{ + u8 *digest; + void *bufp = buf, *bufendp = buf + size; + struct compact_list_hdr *hdr; + size_t digest_len; + int ret = 0, i; + + while (bufp < bufendp) { + if (bufp + sizeof(*hdr) > bufendp) { + pr_err("compact list, invalid data\n"); + return -EINVAL; + } + + hdr = bufp; + + if (hdr->version != 1) { + pr_err("compact list, unsupported version\n"); + return -EINVAL; + } + + if (ima_canonical_fmt) { + hdr->type = le16_to_cpu(hdr->type); + hdr->modifiers = le16_to_cpu(hdr->modifiers); + hdr->algo = le16_to_cpu(hdr->algo); + hdr->count = le32_to_cpu(hdr->count); + hdr->datalen = le32_to_cpu(hdr->datalen); + } + + if (hdr->algo >= HASH_ALGO__LAST) + return -EINVAL; + + digest_len = hash_digest_size[hdr->algo]; + + if (hdr->type >= COMPACT__LAST) { + pr_err("compact list, invalid type %d\n", hdr->type); + return -EINVAL; + } + + bufp += sizeof(*hdr); + + for (i = 0; i < hdr->count; i++) { + if (bufp + digest_len > bufendp) { + pr_err("compact list, invalid data\n"); + return -EINVAL; + } + + digest = bufp; + bufp += digest_len; + + if (op == DIGEST_LIST_OP_ADD) + ret = ima_add_digest_data_entry(digest, + hdr->algo, hdr->type, hdr->modifiers); + else if (op == DIGEST_LIST_OP_DEL) + ima_del_digest_data_entry(digest, hdr->algo, + hdr->type); + if (ret < 0 && ret != -EEXIST) + return ret; + } + + if (i != hdr->count || + bufp != (void *)hdr + sizeof(*hdr) + hdr->datalen) { + pr_err("compact list, invalid data\n"); + return -EINVAL; + } + } + + return bufp - buf; +} diff --git a/security/integrity/ima/ima_digest_list.h b/security/integrity/ima/ima_digest_list.h new file mode 100644 index 000000000000..e9c6473c1829 --- /dev/null +++ b/security/integrity/ima/ima_digest_list.h @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2017-2019 Huawei Technologies Duesseldorf GmbH + * + * Author: Roberto Sassu roberto.sassu@huawei.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation, version 2 of the + * License. + * + * File: ima_digest_list.h + * Header of ima_digest_list.c + */ + +#ifndef __LINUX_IMA_DIGEST_LIST_H +#define __LINUX_IMA_DIGEST_LIST_H + +#define DIGEST_LIST_OP_ADD 0 +#define DIGEST_LIST_OP_DEL 1 + +#ifdef CONFIG_IMA_DIGEST_LIST +extern struct ima_h_table ima_digests_htable; + +int ima_parse_compact_list(loff_t size, void *buf, int op); +#else +static inline int ima_parse_compact_list(loff_t size, void *buf, int op) +{ + return -ENOTSUPP; +} +#endif /*CONFIG_IMA_DIGEST_LIST*/ +#endif /*LINUX_IMA_DIGEST_LIST_H*/ diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 0ec55122363d..6f5d1e6cdba2 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -16,6 +16,7 @@ #include <crypto/sha.h> #include <linux/key.h> #include <linux/audit.h> +#include <linux/hash_info.h>
/* iint action cache flags */ #define IMA_MEASURE 0x00000001 @@ -131,6 +132,42 @@ struct integrity_iint_cache { struct ima_digest_data *ima_hash; };
+enum compact_types { COMPACT_KEY, COMPACT_PARSER, COMPACT_FILE, + COMPACT_METADATA, COMPACT__LAST }; +enum compact_modifiers { COMPACT_MOD_IMMUTABLE, COMPACT_MOD__LAST }; + +struct ima_digest { + struct hlist_node hnext; + enum hash_algo algo; + enum compact_types type; + u16 modifiers; + u16 count; + u8 digest[0]; +}; + +static inline bool ima_digest_is_immutable(struct ima_digest *digest) +{ + return (digest->modifiers & (1 << COMPACT_MOD_IMMUTABLE)); +} + +#ifdef CONFIG_IMA_DIGEST_LIST +struct ima_digest *ima_lookup_digest(u8 *digest, enum hash_algo algo, + enum compact_types type); +struct ima_digest *ima_digest_allow(struct ima_digest *digest, int action); +#else +static inline struct ima_digest *ima_lookup_digest(u8 *digest, + enum hash_algo algo, + enum compact_types type) +{ + return NULL; +} +static inline struct ima_digest *ima_digest_allow(struct ima_digest *digest, + int action) +{ + return NULL; +} +#endif + /* rbtree tree calls to lookup, insert, delete * integrity data associated with an inode. */
hulk inclusion category: feature feature: digest-lists
---------------------------
Loading a digest list affects the behavior of IMA for subsequent operations. For example, if the digest of a file is found in a loaded digest list, the file won't be added to the measurement list (with PCR 11). If an administrator loaded the digest list before the IMA policy, he could hide from verifiers the fact that files in that digest list were accessed.
To avoid this situation, this patch prevents usage of digest lists for an IMA submodule if that submodule didn't process it. If a digest list wasn't measured, the digest of measured files will not be searched in the digest list and regular measurement will be performed. The same mechanism applies for appraisal.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/ima/ima.h | 1 + security/integrity/ima/ima_digest_list.c | 45 ++++++++++++++++++++++++ security/integrity/ima/ima_digest_list.h | 4 +++ security/integrity/ima/ima_main.c | 4 +++ 4 files changed, 54 insertions(+)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index eb4ebd2cfa56..1cf20bbb106c 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -57,6 +57,7 @@ extern int ima_hash_algo; extern int ima_appraise; extern struct tpm_chip *ima_tpm_chip; extern const char boot_aggregate_name[]; +extern int ima_digest_list_actions;
/* IMA event related data */ struct ima_event_data { diff --git a/security/integrity/ima/ima_digest_list.c b/security/integrity/ima/ima_digest_list.c index 0dcc69954887..58cdbd82840d 100644 --- a/security/integrity/ima/ima_digest_list.c +++ b/security/integrity/ima/ima_digest_list.c @@ -113,6 +113,9 @@ int ima_parse_compact_list(loff_t size, void *buf, int op) size_t digest_len; int ret = 0, i;
+ if (!(ima_digest_list_actions & ima_policy_flag)) + return -EACCES; + while (bufp < bufendp) { if (bufp + sizeof(*hdr) > bufendp) { pr_err("compact list, invalid data\n"); @@ -174,3 +177,45 @@ int ima_parse_compact_list(loff_t size, void *buf, int op)
return bufp - buf; } + +/*************************** + * Digest list usage check * + ***************************/ +void ima_check_measured_appraised(struct file *file) +{ + struct integrity_iint_cache *iint; + + if (!ima_digest_list_actions) + return; + + iint = integrity_iint_find(file_inode(file)); + if (!iint) { + pr_err("disabling digest lists lookup\n"); + ima_digest_list_actions = 0; + return; + } + + mutex_lock(&iint->mutex); + if ((ima_digest_list_actions & IMA_MEASURE) && + !(iint->flags & IMA_MEASURED)) { + pr_err("disabling digest lists lookup for measurement\n"); + ima_digest_list_actions &= ~IMA_MEASURE; + } + + if ((ima_digest_list_actions & IMA_APPRAISE) && + (!(iint->flags & IMA_APPRAISED) || + !test_bit(IMA_DIGSIG, &iint->atomic_flags))) { + pr_err("disabling digest lists lookup for appraisal\n"); + ima_digest_list_actions &= ~IMA_APPRAISE; + } + + mutex_unlock(&iint->mutex); +} + +struct ima_digest *ima_digest_allow(struct ima_digest *digest, int action) +{ + if (!(ima_digest_list_actions & action)) + return NULL; + + return digest; +} diff --git a/security/integrity/ima/ima_digest_list.h b/security/integrity/ima/ima_digest_list.h index e9c6473c1829..f587e9ab8f75 100644 --- a/security/integrity/ima/ima_digest_list.h +++ b/security/integrity/ima/ima_digest_list.h @@ -23,10 +23,14 @@ extern struct ima_h_table ima_digests_htable;
int ima_parse_compact_list(loff_t size, void *buf, int op); +void ima_check_measured_appraised(struct file *file); #else static inline int ima_parse_compact_list(loff_t size, void *buf, int op) { return -ENOTSUPP; } +static inline void ima_check_measured_appraised(struct file *file) +{ +} #endif /*CONFIG_IMA_DIGEST_LIST*/ #endif /*LINUX_IMA_DIGEST_LIST_H*/ diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 703f65dcedde..78a904525711 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -39,6 +39,10 @@ int ima_appraise; #endif
int ima_hash_algo = HASH_ALGO_SHA256; + +/* Actions (measure/appraisal) for which digest lists can be used */ +int ima_digest_list_actions; + static int hash_setup_done;
static int __init hash_setup(char *str)
hulk inclusion category: feature feature: digest-lists
---------------------------
This patch introduces three new files in the securityfs filesystem. digest_list_data: loads a digest list from the specified path and adds the digests to the hash table; digest_list_data_del: does the same but removes the digests from the hash table; digests_count: shows the current number of digests stored in the hash table.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- include/linux/fs.h | 1 + security/integrity/ima/ima_fs.c | 49 +++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+)
diff --git a/include/linux/fs.h b/include/linux/fs.h index 2af6e5a9528f..0a25421c1b02 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2909,6 +2909,7 @@ extern int do_pipe_flags(int *, int); id(KEXEC_INITRAMFS, kexec-initramfs) \ id(POLICY, security-policy) \ id(X509_CERTIFICATE, x509-certificate) \ + id(DIGEST_LIST, digest-list) \ id(MAX_ID, )
#define __fid_enumify(ENUM, dummy) READING_ ## ENUM, diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index d0cc9d849dae..895e242066ac 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -29,6 +29,7 @@ #include <linux/file.h>
#include "ima.h" +#include "ima_digest_list.h"
static DEFINE_MUTEX(ima_write_mutex);
@@ -39,6 +40,9 @@ static struct dentry *ascii_runtime_measurements; static struct dentry *runtime_measurements_count; static struct dentry *violations; static struct dentry *ima_policy; +static struct dentry *digests_count; +static struct dentry *digest_list_data; +static struct dentry *digest_list_data_del;
bool ima_canonical_fmt; static int __init default_canonical_fmt_setup(char *str) @@ -63,6 +67,10 @@ static ssize_t ima_show_htable_value(struct file *filp, char __user *buf, val = &ima_htable.violations; else if (filp->f_path.dentry == runtime_measurements_count) val = &ima_htable.len; +#ifdef CONFIG_IMA_DIGEST_LIST + else if (filp->f_path.dentry == digests_count) + val = &ima_digests_htable.len; +#endif
len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", atomic_long_read(val)); return simple_read_from_buffer(buf, count, ppos, tmpbuf, len); @@ -277,6 +285,7 @@ static ssize_t ima_read_file(char *path, struct dentry *dentry) loff_t size; struct file *file; enum kernel_read_file_id file_id = READING_POLICY; + int op = DIGEST_LIST_OP_ADD; int rc, pathlen = strlen(path);
char *p; @@ -291,6 +300,9 @@ static ssize_t ima_read_file(char *path, struct dentry *dentry) return PTR_ERR(file); }
+ if (dentry == digest_list_data || dentry == digest_list_data_del) + file_id = READING_DIGEST_LIST; + rc = kernel_read_file(file, &data, &size, 0, file_id); if (rc < 0) { pr_err("Unable to read file: %s (%d)", path, rc); @@ -307,6 +319,18 @@ static ssize_t ima_read_file(char *path, struct dentry *dentry)
pr_debug("rule: %s\n", p); rc = ima_parse_add_rule(p); + } else if (dentry == digest_list_data || + dentry == digest_list_data_del) { + /* + * Disable usage of digest lists if not measured + * or appraised. + */ + ima_check_measured_appraised(file); + + if (dentry == digest_list_data_del) + op = DIGEST_LIST_OP_DEL; + + rc = ima_parse_compact_list(size, data, op); }
if (rc < 0) @@ -384,6 +408,7 @@ static ssize_t ima_write_data(struct file *file, const char __user *buf,
enum ima_fs_flags { IMA_POLICY_BUSY, + IMA_DIGEST_LIST_DATA_BUSY, IMA_FS_BUSY, };
@@ -393,6 +418,8 @@ static enum ima_fs_flags ima_get_dentry_flag(struct dentry *dentry)
if (dentry == ima_policy) flag = IMA_POLICY_BUSY; + else if (dentry == digest_list_data || dentry == digest_list_data_del) + flag = IMA_DIGEST_LIST_DATA_BUSY;
return flag; } @@ -540,8 +567,30 @@ int __init ima_fs_init(void) if (IS_ERR(ima_policy)) goto out;
+#ifdef CONFIG_IMA_DIGEST_LIST + digests_count = securityfs_create_file("digests_count", + S_IRUSR | S_IRGRP, ima_dir, + NULL, &ima_htable_value_ops); + if (IS_ERR(digests_count)) + goto out; + + digest_list_data = securityfs_create_file("digest_list_data", S_IWUSR, + ima_dir, NULL, + &ima_data_upload_ops); + if (IS_ERR(digest_list_data)) + goto out; + + digest_list_data_del = securityfs_create_file("digest_list_data_del", + S_IWUSR, ima_dir, NULL, + &ima_data_upload_ops); + if (IS_ERR(digest_list_data_del)) + goto out; +#endif return 0; out: + securityfs_remove(digest_list_data_del); + securityfs_remove(digest_list_data); + securityfs_remove(digests_count); securityfs_remove(violations); securityfs_remove(runtime_measurements_count); securityfs_remove(ascii_runtime_measurements);
hulk inclusion category: feature feature: digest-lists
---------------------------
This patch introduces a new hook called DIGEST_LIST_CHECK to measure and appraise digest lists in addition to executables and shared libraries, without including the FILE_CHECK hook in the IMA policy.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/ima/ima.h | 2 ++ security/integrity/ima/ima_main.c | 3 ++- security/integrity/ima/ima_policy.c | 7 +++++++ 3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 1cf20bbb106c..c8cd23bb8b8c 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -186,6 +186,7 @@ static inline unsigned int ima_hash_key(u8 *digest) hook(KEXEC_KERNEL_CHECK) \ hook(KEXEC_INITRAMFS_CHECK) \ hook(POLICY_CHECK) \ + hook(DIGEST_LIST_CHECK) \ hook(MAX_CHECK) #define __ima_hook_enumify(ENUM) ENUM,
@@ -236,6 +237,7 @@ int ima_policy_show(struct seq_file *m, void *v); #define IMA_APPRAISE_FIRMWARE 0x10 #define IMA_APPRAISE_POLICY 0x20 #define IMA_APPRAISE_KEXEC 0x40 +#define IMA_APPRAISE_DIGEST_LIST 0x80
#ifdef CONFIG_IMA_APPRAISE int ima_appraise_measurement(enum ima_hooks func, diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 78a904525711..5e30bf38c7de 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -504,7 +504,8 @@ static int read_idmap[READING_MAX_ID] = { [READING_MODULE] = MODULE_CHECK, [READING_KEXEC_IMAGE] = KEXEC_KERNEL_CHECK, [READING_KEXEC_INITRAMFS] = KEXEC_INITRAMFS_CHECK, - [READING_POLICY] = POLICY_CHECK + [READING_POLICY] = POLICY_CHECK, + [READING_DIGEST_LIST] = DIGEST_LIST_CHECK };
/** diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 2d5a3daa02f9..ac49f07e8997 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -133,6 +133,7 @@ static struct ima_rule_entry default_measurement_rules[] __ro_after_init = { {.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC}, {.action = MEASURE, .func = FIRMWARE_CHECK, .flags = IMA_FUNC}, {.action = MEASURE, .func = POLICY_CHECK, .flags = IMA_FUNC}, + {.action = MEASURE, .func = DIGEST_LIST_CHECK, .flags = IMA_FUNC}, };
static struct ima_rule_entry default_appraise_rules[] __ro_after_init = { @@ -191,6 +192,8 @@ static struct ima_rule_entry secure_boot_rules[] __ro_after_init = { .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED}, {.action = APPRAISE, .func = POLICY_CHECK, .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED}, + {.action = APPRAISE, .func = DIGEST_LIST_CHECK, + .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED}, };
static LIST_HEAD(ima_default_rules); @@ -470,6 +473,8 @@ static int ima_appraise_flag(enum ima_hooks func) return IMA_APPRAISE_POLICY; else if (func == KEXEC_KERNEL_CHECK) return IMA_APPRAISE_KEXEC; + else if (func == DIGEST_LIST_CHECK) + return IMA_APPRAISE_DIGEST_LIST; return 0; }
@@ -785,6 +790,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) entry->func = KEXEC_INITRAMFS_CHECK; else if (strcmp(args[0].from, "POLICY_CHECK") == 0) entry->func = POLICY_CHECK; + else if (strcmp(args[0].from, "DIGEST_LIST_CHECK") == 0) + entry->func = DIGEST_LIST_CHECK; else result = -EINVAL; if (!result)
hulk inclusion category: feature feature: digest-lists
---------------------------
Digest lists should be uploaded to IMA as soon as possible, otherwise file digests would appear in the measurement list or access would be denied if appraisal is in enforcing mode.
This patch adds a call to ima_load_digest_lists() in integrity_load_keys(), so that the function is executed when rootfs becomes available, before files are accessed.
ima_load_digest_lists() iterates in the directory specified as value of CONFIG_IMA_DIGEST_LISTS_DIR and uploads all digest lists to the kernel.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/iint.c | 1 + security/integrity/ima/Kconfig | 8 +++ security/integrity/ima/ima_digest_list.c | 75 ++++++++++++++++++++++++ security/integrity/integrity.h | 4 ++ 4 files changed, 88 insertions(+)
diff --git a/security/integrity/iint.c b/security/integrity/iint.c index 5a6810041e5c..1d5222ca4fb1 100644 --- a/security/integrity/iint.c +++ b/security/integrity/iint.c @@ -213,6 +213,7 @@ void __init integrity_load_keys(void) { ima_load_x509(); evm_load_x509(); + ima_load_digest_lists(); }
static int __init integrity_fs_init(void) diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index e063e14d8740..5fe5f67b2e18 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -297,3 +297,11 @@ config IMA_DIGEST_LIST of accessed files are found in one of those lists, no new entries are added to the measurement list, and access to the file is granted if appraisal is in enforcing mode. + +config IMA_DIGEST_LISTS_DIR + string "Path of the directory containing digest lists" + depends on IMA_DIGEST_LIST + default "/etc/ima/digest_lists" + help + This option defines the path of the directory containing digest + lists. diff --git a/security/integrity/ima/ima_digest_list.c b/security/integrity/ima/ima_digest_list.c index 58cdbd82840d..607a33ba52ee 100644 --- a/security/integrity/ima/ima_digest_list.c +++ b/security/integrity/ima/ima_digest_list.c @@ -17,6 +17,8 @@
#include <linux/vmalloc.h> #include <linux/module.h> +#include <linux/file.h> +#include <linux/namei.h>
#include "ima.h" #include "ima_digest_list.h" @@ -219,3 +221,76 @@ struct ima_digest *ima_digest_allow(struct ima_digest *digest, int action)
return digest; } + +/************************************** + * Digest list loading at kernel init * + **************************************/ +struct readdir_callback { + struct dir_context ctx; + struct path *path; +}; + +static int __init load_digest_list(struct dir_context *__ctx, const char *name, + int namelen, loff_t offset, u64 ino, + unsigned int d_type) +{ + struct readdir_callback *ctx = container_of(__ctx, typeof(*ctx), ctx); + struct path *dir = ctx->path; + struct file *file; + void *datap; + loff_t size; + int ret; + + if (!strcmp(name, ".") || !strcmp(name, "..")) + return 0; + + file = file_open_root(dir->dentry, dir->mnt, name, O_RDONLY, 0); + if (IS_ERR(file)) { + pr_err("Unable to open file: %s (%ld)", name, PTR_ERR(file)); + return 0; + } + + ret = kernel_read_file(file, &datap, &size, 0, READING_DIGEST_LIST); + if (ret < 0) { + pr_err("Unable to read file: %s (%d)", name, ret); + goto out; + } + + ima_check_measured_appraised(file); + + ret = ima_parse_compact_list(size, datap, DIGEST_LIST_OP_ADD); + if (ret < 0) + pr_err("Unable to parse file: %s (%d)", name, ret); + + vfree(datap); +out: + fput(file); + return 0; +} + +void __init ima_load_digest_lists(void) +{ + struct path path; + struct file *file; + int ret; + struct readdir_callback buf = { + .ctx.actor = load_digest_list, + }; + + if (!(ima_digest_list_actions & ima_policy_flag)) + return; + + ret = kern_path(CONFIG_IMA_DIGEST_LISTS_DIR, 0, &path); + if (ret) + return; + + file = dentry_open(&path, O_RDONLY, current_cred()); + if (IS_ERR(file)) + goto out; + + buf.path = &path; + iterate_dir(file, &buf.ctx); + fput(file); +out: + path_put(&path); +} diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 6f5d1e6cdba2..73550ebe9372 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -154,6 +154,7 @@ static inline bool ima_digest_is_immutable(struct ima_digest *digest) struct ima_digest *ima_lookup_digest(u8 *digest, enum hash_algo algo, enum compact_types type); struct ima_digest *ima_digest_allow(struct ima_digest *digest, int action); +void __init ima_load_digest_lists(void); #else static inline struct ima_digest *ima_lookup_digest(u8 *digest, enum hash_algo algo, @@ -166,6 +167,9 @@ static inline struct ima_digest *ima_digest_allow(struct ima_digest *digest, { return NULL; } +static inline void ima_load_digest_lists(void) +{ +} #endif
/* rbtree tree calls to lookup, insert, delete
hulk inclusion category: feature feature: digest-lists
---------------------------
IMA-Measure creates a new measurement entry every time a file is measured, unless the same entry is already in the measurement list.
This patch introduces a new type of measurement list, recognizable by the PCR number specified with the new ima_digest_list_pcr= kernel option. This type of measurement list includes measurements of digest lists and files not found in those lists.
The benefit of this patch is the availability of a predictable PCR that can be used to seal data or TPM keys to the OS software. Unlike standard measurements, digest list measurements only indicate that files with a digest in those lists could have been accessed, but not if and when. With standard measurements, however, the chosen PCR is unlikely predictable.
Both standard and digest list measurements can be generated at the same time by adding '+' as a prefix to the value of ima_digest_list_pcr= (example: with ima_digest_list_pcr=+11, IMA generates standard measurements with PCR 10 and digest list measurements with PCR 11).
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- .../admin-guide/kernel-parameters.txt | 9 ++++ security/integrity/ima/ima.h | 9 ++-- security/integrity/ima/ima_api.c | 43 ++++++++++++++++--- security/integrity/ima/ima_digest_list.c | 25 +++++++++++ security/integrity/ima/ima_init.c | 2 +- security/integrity/ima/ima_main.c | 16 ++++++- security/integrity/ima/ima_policy.c | 3 +- security/integrity/integrity.h | 1 + 8 files changed, 95 insertions(+), 13 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 7b4487f08ea1..d6423042d787 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -1565,6 +1565,15 @@ Use the canonical format for the binary runtime measurements, instead of host native format.
+ ima_digest_list_pcr= + [IMA] + Specify which PCR is extended when file digests are + not found in the loaded digest lists. If '+' is not + specified, no measurement entry is created if the + digest is found. Otherwise, IMA creates also entries + with PCR 10, according to the existing behavior. + Format: { [+]<unsigned int> } + ima_hash= [IMA] Format: { md5 | sha1 | rmd160 | sha256 | sha384 | sha512 | ... } diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index c8cd23bb8b8c..dbdbd9c84262 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -56,6 +56,8 @@ extern int ima_policy_flag; extern int ima_hash_algo; extern int ima_appraise; extern struct tpm_chip *ima_tpm_chip; +extern int ima_digest_list_pcr; +extern bool ima_plus_standard_pcr; extern const char boot_aggregate_name[]; extern int ima_digest_list_actions;
@@ -204,14 +206,15 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, - int xattr_len, int pcr); + int xattr_len, int pcr, + struct ima_digest *digest); void ima_audit_measurement(struct integrity_iint_cache *iint, const unsigned char *filename); int ima_alloc_init_template(struct ima_event_data *event_data, struct ima_template_entry **entry); int ima_store_template(struct ima_template_entry *entry, int violation, - struct inode *inode, - const unsigned char *filename, int pcr); + struct inode *inode, const unsigned char *filename, + int pcr, struct ima_digest *digest); void ima_free_template_entry(struct ima_template_entry *entry); const char *ima_d_path(const struct path *path, char **pathbuf, char *filename);
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index a02c5acfd403..3c0eb503e3ab 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -88,11 +88,13 @@ int ima_alloc_init_template(struct ima_event_data *event_data, */ int ima_store_template(struct ima_template_entry *entry, int violation, struct inode *inode, - const unsigned char *filename, int pcr) + const unsigned char *filename, int pcr, + struct ima_digest *digest) { static const char op[] = "add_template_measure"; static const char audit_cause[] = "hashing_error"; char *template_name = entry->template_desc->name; + struct ima_template_entry *duplicated_entry = NULL; int result; struct { struct ima_digest_data hdr; @@ -115,8 +117,27 @@ int ima_store_template(struct ima_template_entry *entry, } memcpy(entry->digest, hash.hdr.digest, hash.hdr.length); } + + if (ima_plus_standard_pcr && !digest) { + duplicated_entry = kmemdup(entry, + sizeof(*entry) + entry->template_desc->num_fields * + sizeof(struct ima_field_data), GFP_KERNEL); + if (duplicated_entry) + duplicated_entry->pcr = ima_digest_list_pcr; + } else if (!ima_plus_standard_pcr && ima_digest_list_pcr >= 0) { + pcr = ima_digest_list_pcr; + } + entry->pcr = pcr; + result = ima_add_template_entry(entry, violation, op, inode, filename); + if (!result && duplicated_entry) { + result = ima_add_template_entry(duplicated_entry, violation, op, + inode, filename); + if (result < 0) + kfree(duplicated_entry); + } + return result; }
@@ -146,8 +167,8 @@ void ima_add_violation(struct file *file, const unsigned char *filename, result = -ENOMEM; goto err_out; } - result = ima_store_template(entry, violation, inode, - filename, CONFIG_IMA_MEASURE_PCR_IDX); + result = ima_store_template(entry, violation, inode, filename, + CONFIG_IMA_MEASURE_PCR_IDX, NULL); if (result < 0) ima_free_template_entry(entry); err_out: @@ -277,13 +298,14 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, void ima_store_measurement(struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, - int xattr_len, int pcr) + int xattr_len, int pcr, + struct ima_digest *digest) { static const char op[] = "add_template_measure"; static const char audit_cause[] = "ENOMEM"; int result = -ENOMEM; struct inode *inode = file_inode(file); - struct ima_template_entry *entry; + struct ima_template_entry *entry = NULL; struct ima_event_data event_data = {iint, file, filename, xattr_value, xattr_len, NULL}; int violation = 0; @@ -291,6 +313,11 @@ void ima_store_measurement(struct integrity_iint_cache *iint, if (iint->measured_pcrs & (0x1 << pcr)) return;
+ if (digest && !ima_plus_standard_pcr && ima_digest_list_pcr >= 0) { + result = -EEXIST; + goto out; + } + result = ima_alloc_init_template(&event_data, &entry); if (result < 0) { integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode, filename, @@ -298,12 +325,14 @@ void ima_store_measurement(struct integrity_iint_cache *iint, return; }
- result = ima_store_template(entry, violation, inode, filename, pcr); + result = ima_store_template(entry, violation, inode, filename, pcr, + digest); +out: if ((!result || result == -EEXIST) && !(file->f_flags & O_DIRECT)) { iint->flags |= IMA_MEASURED; iint->measured_pcrs |= (0x1 << pcr); } - if (result < 0) + if (result < 0 && entry) ima_free_template_entry(entry); }
diff --git a/security/integrity/ima/ima_digest_list.c b/security/integrity/ima/ima_digest_list.c index 607a33ba52ee..7e901bdb4340 100644 --- a/security/integrity/ima/ima_digest_list.c +++ b/security/integrity/ima/ima_digest_list.c @@ -28,6 +28,31 @@ struct ima_h_table ima_digests_htable = { .queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT };
+static int __init digest_list_pcr_setup(char *str) +{ + int pcr, ret; + + ret = kstrtouint(str, 10, &pcr); + if (ret) { + pr_err("Invalid PCR number %s\n", str); + return 1; + } + + if (pcr == CONFIG_IMA_MEASURE_PCR_IDX) { + pr_err("Default PCR cannot be used for digest lists\n"); + return 1; + } + + ima_digest_list_pcr = pcr; + ima_digest_list_actions |= IMA_MEASURE; + + if (*str == '+') + ima_plus_standard_pcr = true; + + return 1; +} +__setup("ima_digest_list_pcr=", digest_list_pcr_setup); + /************************* * Get/add/del functions * *************************/ diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c index a2bc4cb4482a..af19454788cb 100644 --- a/security/integrity/ima/ima_init.c +++ b/security/integrity/ima/ima_init.c @@ -80,7 +80,7 @@ static int __init ima_add_boot_aggregate(void)
result = ima_store_template(entry, violation, NULL, boot_aggregate_name, - CONFIG_IMA_MEASURE_PCR_IDX); + CONFIG_IMA_MEASURE_PCR_IDX, NULL); if (result < 0) { ima_free_template_entry(entry); audit_cause = "store_entry"; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 5e30bf38c7de..198b2b4c3654 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -31,6 +31,7 @@ #include <linux/fs.h>
#include "ima.h" +#include "ima_digest_list.h"
#ifdef CONFIG_IMA_APPRAISE int ima_appraise = IMA_APPRAISE_ENFORCE; @@ -42,6 +43,10 @@ int ima_hash_algo = HASH_ALGO_SHA256;
/* Actions (measure/appraisal) for which digest lists can be used */ int ima_digest_list_actions; +/* PCR used for digest list measurements */ +int ima_digest_list_pcr = -1; +/* Flag to include standard measurement if digest list PCR is specified */ +bool ima_plus_standard_pcr;
static int hash_setup_done;
@@ -141,6 +146,8 @@ static enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, return ima_hash_algo; return sig->hash_algo; break; + case EVM_IMA_XATTR_DIGEST_LIST: + /* fall through */ case IMA_XATTR_DIGEST_NG: ret = xattr_value->digest[0]; if (ret < HASH_ALGO__LAST) @@ -233,6 +240,7 @@ static int process_measurement(struct file *file, const struct cred *cred, const char *pathname = NULL; int rc = 0, action, must_appraise = 0; int pcr = CONFIG_IMA_MEASURE_PCR_IDX; + struct ima_digest *found_digest; struct evm_ima_xattr_data *xattr_value = NULL; int xattr_len = 0; bool violation_check; @@ -346,9 +354,15 @@ static int process_measurement(struct file *file, const struct cred *cred, if (!pathname || strlen(pathname) > IMA_EVENT_NAME_LEN_MAX) pathname = file->f_path.dentry->d_name.name;
+ found_digest = ima_lookup_digest(iint->ima_hash->digest, hash_algo, + COMPACT_FILE); + if (action & IMA_MEASURE) ima_store_measurement(iint, file, pathname, - xattr_value, xattr_len, pcr); + xattr_value, xattr_len, pcr, + ima_digest_allow(found_digest, + IMA_MEASURE)); + if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) { inode_lock(inode); rc = ima_appraise_measurement(func, iint, file, pathname, diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index ac49f07e8997..8ffd5958ae3e 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -971,7 +971,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) ima_log_string(ab, "pcr", args[0].from);
result = kstrtoint(args[0].from, 10, &entry->pcr); - if (result || INVALID_PCR(entry->pcr)) + if (result || INVALID_PCR(entry->pcr) || + entry->pcr == ima_digest_list_pcr) result = -EINVAL; else entry->flags |= IMA_PCR; diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 73550ebe9372..02d40e5f401c 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -75,6 +75,7 @@ enum evm_ima_xattr_type { EVM_IMA_XATTR_DIGSIG, IMA_XATTR_DIGEST_NG, EVM_XATTR_PORTABLE_DIGSIG, + EVM_IMA_XATTR_DIGEST_LIST, IMA_XATTR_LAST };
hulk inclusion category: feature feature: digest-lists
---------------------------
IMA-Appraise grants access to files with a valid signature or with actual file digest equal to the digest included in security.ima.
This patch adds support for appraisal based on digest lists. Instead of using the reference value from security.ima, this patch checks if the calculated file digest is included in the uploaded digest lists.
This functionality must be explicitly enabled by providing one of the following values for the ima_appraise_digest_list= kernel option:
- digest: this mode enables appraisal verification with digest lists until EVM is initialized; after that, EVM verification must be successful even if the file digest is found in a digest list;
- digest-nometadata: this mode enables appraisal verification with digest lists even after EVM has been initialized; files without security.evm are allowed if the digest of the content is found in the digest list, and security.evm is created with current values of xattrs (trust at first use); all files created in this way will have the new security.ima type EVM_IMA_XATTR_DIGEST_LIST; they can be accessed later only if this mode has been selected.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- .../admin-guide/kernel-parameters.txt | 9 +++ security/integrity/ima/ima.h | 5 +- security/integrity/ima/ima_appraise.c | 70 ++++++++++++++++++- security/integrity/ima/ima_main.c | 4 +- security/integrity/integrity.h | 1 + 5 files changed, 83 insertions(+), 6 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index d6423042d787..65efa24e3dd0 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -1557,6 +1557,15 @@ "enforce-evm" | "log-evm" } default: "enforce"
+ ima_appraise_digest_list= [IMA] + Format: { "digest" | "digest-nometadata" } + + digest: enables appraisal of files with digest lists + until EVM is initialized. + + digest-nometadata: enables appraisal of files with + digest lists even after EVM is initialized. + ima_appraise_tcb [IMA] The builtin appraise policy appraises all files owned by uid=0. diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index dbdbd9c84262..9fea7431b0cc 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -247,7 +247,7 @@ int ima_appraise_measurement(enum ima_hooks func, struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, - int xattr_len); + int xattr_len, struct ima_digest *found_digest); int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func); void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file); enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint, @@ -258,7 +258,8 @@ static inline int ima_appraise_measurement(enum ima_hooks func, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, - int xattr_len) + int xattr_len, + struct ima_digest *found_digest) { return INTEGRITY_UNKNOWN; } diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index f51ab67f7ab5..b7f234292054 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -17,6 +17,7 @@ #include <linux/evm.h>
#include "ima.h" +#include "ima_digest_list.h"
static bool ima_appraise_req_evm __ro_after_init; static int __init default_appraise_setup(char *str) @@ -37,6 +38,22 @@ static int __init default_appraise_setup(char *str)
__setup("ima_appraise=", default_appraise_setup);
+static bool ima_appraise_no_metadata __ro_after_init; +#ifdef CONFIG_IMA_DIGEST_LIST +static int __init appraise_digest_list_setup(char *str) +{ + if (!strncmp(str, "digest", 6)) { + ima_digest_list_actions |= IMA_APPRAISE; + + if (!strcmp(str + 6, "-nometadata")) + ima_appraise_no_metadata = true; + } + + return 1; +} +__setup("ima_appraise_digest_list=", appraise_digest_list_setup); +#endif + /* * is_ima_appraise_enabled - return appraise status * @@ -70,7 +87,11 @@ static int ima_fix_xattr(struct dentry *dentry, int rc, offset; u8 algo = iint->ima_hash->algo;
- if (algo <= HASH_ALGO_SHA1) { + if (test_bit(IMA_DIGEST_LIST, &iint->atomic_flags)) { + offset = 0; + iint->ima_hash->xattr.ng.type = EVM_IMA_XATTR_DIGEST_LIST; + iint->ima_hash->xattr.ng.algo = algo; + } else if (algo <= HASH_ALGO_SHA1) { offset = 1; iint->ima_hash->xattr.sha1.type = IMA_XATTR_DIGEST; } else { @@ -165,18 +186,29 @@ int ima_appraise_measurement(enum ima_hooks func, struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, - int xattr_len) + int xattr_len, struct ima_digest *found_digest) { static const char op[] = "appraise_data"; const char *cause = "unknown"; struct dentry *dentry = file_dentry(file); struct inode *inode = d_backing_inode(dentry); enum integrity_status status = INTEGRITY_UNKNOWN; + struct evm_ima_xattr_data digest_list_value; int rc = xattr_len, hash_start = 0;
if (!(inode->i_opflags & IOP_XATTR)) return INTEGRITY_UNKNOWN;
+ if (rc == -ENODATA && found_digest && + !(file->f_mode && FMODE_CREATED)) { + digest_list_value.type = EVM_IMA_XATTR_DIGEST_LIST; + digest_list_value.digest[0] = found_digest->algo; + memcpy(&digest_list_value.digest[1], found_digest->digest, + hash_digest_size[found_digest->algo]); + xattr_value = &digest_list_value; + rc = hash_digest_size[found_digest->algo] + 2; + } + if (rc <= 0) { if (rc && rc != -ENODATA) goto out; @@ -200,11 +232,18 @@ int ima_appraise_measurement(enum ima_hooks func, break; case INTEGRITY_UNKNOWN: if (ima_appraise_req_evm && - xattr_value->type != EVM_IMA_XATTR_DIGSIG) + xattr_value->type != EVM_IMA_XATTR_DIGSIG && !found_digest) goto out; break; case INTEGRITY_NOXATTRS: /* No EVM protected xattrs. */ case INTEGRITY_NOLABEL: /* No security.evm xattr. */ + /* + * If the digest-nometadata mode is selected, allow access + * without metadata check. EVM will eventually create an HMAC + * based on current xattr values. + */ + if (ima_appraise_no_metadata && found_digest) + break; cause = "missing-HMAC"; goto out; case INTEGRITY_FAIL_IMMUTABLE: @@ -218,6 +257,31 @@ int ima_appraise_measurement(enum ima_hooks func, }
switch (xattr_value->type) { + case EVM_IMA_XATTR_DIGEST_LIST: + set_bit(IMA_DIGEST_LIST, &iint->atomic_flags); + + if (found_digest) { + if (!ima_digest_is_immutable(found_digest)) { + if (iint->flags & IMA_DIGSIG_REQUIRED) { + cause = "IMA-signature-required"; + status = INTEGRITY_FAIL; + break; + } + clear_bit(IMA_DIGSIG, &iint->atomic_flags); + } else { + set_bit(IMA_DIGSIG, &iint->atomic_flags); + } + + status = INTEGRITY_PASS; + break; + } + + if (!ima_appraise_no_metadata) { + cause = "IMA-xattr-untrusted"; + status = INTEGRITY_FAIL; + break; + } + /* fall through */ case IMA_XATTR_DIGEST_NG: /* first byte contains algorithm id */ hash_start = 1; diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 198b2b4c3654..a386a5bf1ed9 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -366,7 +366,9 @@ static int process_measurement(struct file *file, const struct cred *cred, if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) { inode_lock(inode); rc = ima_appraise_measurement(func, iint, file, pathname, - xattr_value, xattr_len); + xattr_value, xattr_len, + ima_digest_allow(found_digest, + IMA_APPRAISE)); inode_unlock(inode); } if (action & IMA_AUDIT) diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 02d40e5f401c..49132ab9ef0a 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -68,6 +68,7 @@ #define IMA_CHANGE_ATTR 2 #define IMA_DIGSIG 3 #define IMA_MUST_MEASURE 4 +#define IMA_DIGEST_LIST 5
enum evm_ima_xattr_type { IMA_XATTR_DIGEST = 0x01,
-----Original Message----- From: Roberto Sassu [mailto:roberto.sassu@huawei.com] Sent: Monday, July 6, 2020 5:41 PM To: kernel@openeuler.org Cc: Silviu Vlasceanu Silviu.Vlasceanu@huawei.com Subject: [PATCH 29/35] ima: Add support for appraisal with digest lists
hulk inclusion category: feature feature: digest-lists
IMA-Appraise grants access to files with a valid signature or with actual file digest equal to the digest included in security.ima.
This patch adds support for appraisal based on digest lists. Instead of using the reference value from security.ima, this patch checks if the calculated file digest is included in the uploaded digest lists.
This functionality must be explicitly enabled by providing one of the following values for the ima_appraise_digest_list= kernel option:
digest: this mode enables appraisal verification with digest lists until EVM is initialized; after that, EVM verification must be successful even if the file digest is found in a digest list;
digest-nometadata: this mode enables appraisal verification with digest lists even after EVM has been initialized; files without security.evm are allowed if the digest of the content is found in the digest list, and security.evm is created with current values of xattrs (trust at first use); all files created in this way will have the new security.ima type EVM_IMA_XATTR_DIGEST_LIST; they can be accessed later only if this
mode has been selected.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com
.../admin-guide/kernel-parameters.txt | 9 +++ security/integrity/ima/ima.h | 5 +- security/integrity/ima/ima_appraise.c | 70 ++++++++++++++++++- security/integrity/ima/ima_main.c | 4 +- security/integrity/integrity.h | 1 + 5 files changed, 83 insertions(+), 6 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index d6423042d787..65efa24e3dd0 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -1557,6 +1557,15 @@ "enforce-evm" | "log-evm" } default: "enforce"
- ima_appraise_digest_list= [IMA]
Format: { "digest" | "digest-nometadata" }
digest: enables appraisal of files with digest lists
until EVM is initialized.
digest-nometadata: enables appraisal of files with
digest lists even after EVM is initialized.
- ima_appraise_tcb [IMA] The builtin appraise policy appraises all files owned by uid=0.
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index dbdbd9c84262..9fea7431b0cc 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -247,7 +247,7 @@ int ima_appraise_measurement(enum ima_hooks func, struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value,
int xattr_len);
int xattr_len, struct ima_digest *found_digest);
int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func); void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file); enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint, @@ -258,7 +258,8 @@ static inline int ima_appraise_measurement(enum ima_hooks func, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value,
int xattr_len)
int xattr_len,
struct ima_digest *found_digest)
{ return INTEGRITY_UNKNOWN; } diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index f51ab67f7ab5..b7f234292054 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -17,6 +17,7 @@ #include <linux/evm.h>
#include "ima.h" +#include "ima_digest_list.h"
static bool ima_appraise_req_evm __ro_after_init; static int __init default_appraise_setup(char *str) @@ -37,6 +38,22 @@ static int __init default_appraise_setup(char *str)
__setup("ima_appraise=", default_appraise_setup);
+static bool ima_appraise_no_metadata __ro_after_init; +#ifdef CONFIG_IMA_DIGEST_LIST +static int __init appraise_digest_list_setup(char *str) +{
- if (!strncmp(str, "digest", 6)) {
ima_digest_list_actions |= IMA_APPRAISE;
if (!strcmp(str + 6, "-nometadata"))
ima_appraise_no_metadata = true;
- }
- return 1;
+} +__setup("ima_appraise_digest_list=", appraise_digest_list_setup); +#endif
/*
- is_ima_appraise_enabled - return appraise status
@@ -70,7 +87,11 @@ static int ima_fix_xattr(struct dentry *dentry, int rc, offset; u8 algo = iint->ima_hash->algo;
- if (algo <= HASH_ALGO_SHA1) {
- if (test_bit(IMA_DIGEST_LIST, &iint->atomic_flags)) {
offset = 0;
iint->ima_hash->xattr.ng.type =
EVM_IMA_XATTR_DIGEST_LIST;
iint->ima_hash->xattr.ng.algo = algo;
- } else if (algo <= HASH_ALGO_SHA1) { offset = 1; iint->ima_hash->xattr.sha1.type = IMA_XATTR_DIGEST; } else {
@@ -165,18 +186,29 @@ int ima_appraise_measurement(enum ima_hooks func, struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value,
int xattr_len)
int xattr_len, struct ima_digest *found_digest)
{ static const char op[] = "appraise_data"; const char *cause = "unknown"; struct dentry *dentry = file_dentry(file); struct inode *inode = d_backing_inode(dentry); enum integrity_status status = INTEGRITY_UNKNOWN;
- struct evm_ima_xattr_data digest_list_value;
I will fix this. Maximum size of the digest is SHA1_DIGEST_SIZE. It is not enough for SHA256 digests.
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Li Peng, Li Jian, Shi Yanli
int rc = xattr_len, hash_start = 0;
if (!(inode->i_opflags & IOP_XATTR)) return INTEGRITY_UNKNOWN;
- if (rc == -ENODATA && found_digest &&
!(file->f_mode && FMODE_CREATED)) {
digest_list_value.type = EVM_IMA_XATTR_DIGEST_LIST;
digest_list_value.digest[0] = found_digest->algo;
memcpy(&digest_list_value.digest[1], found_digest->digest,
hash_digest_size[found_digest->algo]);
xattr_value = &digest_list_value;
rc = hash_digest_size[found_digest->algo] + 2;
- }
- if (rc <= 0) { if (rc && rc != -ENODATA) goto out;
@@ -200,11 +232,18 @@ int ima_appraise_measurement(enum ima_hooks func, break; case INTEGRITY_UNKNOWN: if (ima_appraise_req_evm &&
xattr_value->type != EVM_IMA_XATTR_DIGSIG)
xattr_value->type != EVM_IMA_XATTR_DIGSIG
&& !found_digest) goto out; break; case INTEGRITY_NOXATTRS: /* No EVM protected xattrs. */ case INTEGRITY_NOLABEL: /* No security.evm xattr. */
/*
* If the digest-nometadata mode is selected, allow access
* without metadata check. EVM will eventually create an
HMAC
* based on current xattr values.
*/
if (ima_appraise_no_metadata && found_digest)
cause = "missing-HMAC"; goto out; case INTEGRITY_FAIL_IMMUTABLE:break;
@@ -218,6 +257,31 @@ int ima_appraise_measurement(enum ima_hooks func, }
switch (xattr_value->type) {
- case EVM_IMA_XATTR_DIGEST_LIST:
set_bit(IMA_DIGEST_LIST, &iint->atomic_flags);
if (found_digest) {
if (!ima_digest_is_immutable(found_digest)) {
if (iint->flags & IMA_DIGSIG_REQUIRED) {
cause = "IMA-signature-required";
status = INTEGRITY_FAIL;
break;
}
clear_bit(IMA_DIGSIG, &iint->atomic_flags);
} else {
set_bit(IMA_DIGSIG, &iint->atomic_flags);
}
status = INTEGRITY_PASS;
break;
}
if (!ima_appraise_no_metadata) {
cause = "IMA-xattr-untrusted";
status = INTEGRITY_FAIL;
break;
}
case IMA_XATTR_DIGEST_NG: /* first byte contains algorithm id */ hash_start = 1;/* fall through */
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 198b2b4c3654..a386a5bf1ed9 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -366,7 +366,9 @@ static int process_measurement(struct file *file, const struct cred *cred, if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) { inode_lock(inode); rc = ima_appraise_measurement(func, iint, file, pathname,
xattr_value, xattr_len);
xattr_value, xattr_len,
ima_digest_allow(found_digest,
inode_unlock(inode); } if (action & IMA_AUDIT)IMA_APPRAISE));
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 02d40e5f401c..49132ab9ef0a 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -68,6 +68,7 @@ #define IMA_CHANGE_ATTR 2 #define IMA_DIGSIG 3 #define IMA_MUST_MEASURE 4 +#define IMA_DIGEST_LIST 5
enum evm_ima_xattr_type { IMA_XATTR_DIGEST = 0x01, -- 2.27.GIT _______________________________________________ Kernel mailing list -- kernel@openeuler.org To unsubscribe send an email to kernel-leave@openeuler.org
On 2020/7/15 17:11, Roberto Sassu wrote:
-----Original Message----- From: Roberto Sassu [mailto:roberto.sassu@huawei.com] Sent: Monday, July 6, 2020 5:41 PM To: kernel@openeuler.org Cc: Silviu Vlasceanu Silviu.Vlasceanu@huawei.com Subject: [PATCH 29/35] ima: Add support for appraisal with digest lists
hulk inclusion category: feature feature: digest-lists
IMA-Appraise grants access to files with a valid signature or with actual file digest equal to the digest included in security.ima.
This patch adds support for appraisal based on digest lists. Instead of using the reference value from security.ima, this patch checks if the calculated file digest is included in the uploaded digest lists.
This functionality must be explicitly enabled by providing one of the following values for the ima_appraise_digest_list= kernel option:
digest: this mode enables appraisal verification with digest lists until EVM is initialized; after that, EVM verification must be successful even if the file digest is found in a digest list;
digest-nometadata: this mode enables appraisal verification with digest lists even after EVM has been initialized; files without security.evm are allowed if the digest of the content is found in the digest list, and security.evm is created with current values of xattrs (trust at first use); all files created in this way will have the new security.ima type EVM_IMA_XATTR_DIGEST_LIST; they can be accessed later only if this
mode has been selected.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com
.../admin-guide/kernel-parameters.txt | 9 +++ security/integrity/ima/ima.h | 5 +- security/integrity/ima/ima_appraise.c | 70 ++++++++++++++++++- security/integrity/ima/ima_main.c | 4 +- security/integrity/integrity.h | 1 + 5 files changed, 83 insertions(+), 6 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index d6423042d787..65efa24e3dd0 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -1557,6 +1557,15 @@ "enforce-evm" | "log-evm" } default: "enforce"
- ima_appraise_digest_list= [IMA]
Format: { "digest" | "digest-nometadata" }
digest: enables appraisal of files with digest lists
until EVM is initialized.
digest-nometadata: enables appraisal of files with
digest lists even after EVM is initialized.
- ima_appraise_tcb [IMA] The builtin appraise policy appraises all files owned by uid=0.
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index dbdbd9c84262..9fea7431b0cc 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -247,7 +247,7 @@ int ima_appraise_measurement(enum ima_hooks func, struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value,
int xattr_len);
int ima_must_appraise(struct inode *inode, int mask, enum ima_hooksint xattr_len, struct ima_digest *found_digest);
func); void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file); enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint, @@ -258,7 +258,8 @@ static inline int ima_appraise_measurement(enum ima_hooks func, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value,
int xattr_len)
int xattr_len,
{ return INTEGRITY_UNKNOWN; }struct ima_digest *found_digest)
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index f51ab67f7ab5..b7f234292054 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -17,6 +17,7 @@ #include <linux/evm.h>
#include "ima.h" +#include "ima_digest_list.h"
static bool ima_appraise_req_evm __ro_after_init; static int __init default_appraise_setup(char *str) @@ -37,6 +38,22 @@ static int __init default_appraise_setup(char *str)
__setup("ima_appraise=", default_appraise_setup);
+static bool ima_appraise_no_metadata __ro_after_init; +#ifdef CONFIG_IMA_DIGEST_LIST +static int __init appraise_digest_list_setup(char *str) +{
- if (!strncmp(str, "digest", 6)) {
ima_digest_list_actions |= IMA_APPRAISE;
if (!strcmp(str + 6, "-nometadata"))
ima_appraise_no_metadata = true;
- }
- return 1;
+} +__setup("ima_appraise_digest_list=", appraise_digest_list_setup); +#endif
- /*
- is_ima_appraise_enabled - return appraise status
@@ -70,7 +87,11 @@ static int ima_fix_xattr(struct dentry *dentry, int rc, offset; u8 algo = iint->ima_hash->algo;
- if (algo <= HASH_ALGO_SHA1) {
- if (test_bit(IMA_DIGEST_LIST, &iint->atomic_flags)) {
offset = 0;
iint->ima_hash->xattr.ng.type =
EVM_IMA_XATTR_DIGEST_LIST;
iint->ima_hash->xattr.ng.algo = algo;
- } else if (algo <= HASH_ALGO_SHA1) { offset = 1; iint->ima_hash->xattr.sha1.type = IMA_XATTR_DIGEST; } else {
@@ -165,18 +186,29 @@ int ima_appraise_measurement(enum ima_hooks func, struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value,
int xattr_len)
{ static const char op[] = "appraise_data"; const char *cause = "unknown"; struct dentry *dentry = file_dentry(file); struct inode *inode = d_backing_inode(dentry); enum integrity_status status = INTEGRITY_UNKNOWN;int xattr_len, struct ima_digest *found_digest)
- struct evm_ima_xattr_data digest_list_value;
I will fix this. Maximum size of the digest is SHA1_DIGEST_SIZE. It is not enough for SHA256 digests.
I think add a patch on top will be good.
Thanks Hanjun
hulk inclusion category: feature feature: digest-lists
---------------------------
This patch adds support in EVM to verify file metadata digest with digest lists. Metadata digest, calculated in the same way as for portable signatures, is searched in the digest lists only if the file has the security.evm xattr with type EVM_IMA_XATTR_DIGEST_LIST.
If found digest is marked as immutable, content and xattr/attr updates are not allowed. Otherwise, after update, an HMAC will be written to security.evm.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/evm/evm_crypto.c | 9 ++++-- security/integrity/evm/evm_main.c | 48 ++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 4 deletions(-)
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 157efcc3ad6e..61082bff1645 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -158,7 +158,8 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode, /* Don't include the inode or generation number in portable * signatures */ - if (type != EVM_XATTR_PORTABLE_DIGSIG) { + if (type != EVM_XATTR_PORTABLE_DIGSIG && + type != EVM_IMA_XATTR_DIGEST_LIST) { hmac_misc.ino = inode->i_ino; hmac_misc.generation = inode->i_generation; } @@ -175,7 +176,8 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode, hmac_misc.mode = inode->i_mode; crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc)); if ((evm_hmac_attrs & EVM_ATTR_FSUUID) && - type != EVM_XATTR_PORTABLE_DIGSIG) + type != EVM_XATTR_PORTABLE_DIGSIG && + type != EVM_IMA_XATTR_DIGEST_LIST) crypto_shash_update(desc, &inode->i_sb->s_uuid.b[0], sizeof(inode->i_sb->s_uuid)); crypto_shash_final(desc, digest); @@ -289,7 +291,8 @@ static int evm_is_immutable(struct dentry *dentry, struct inode *inode) return 0; return rc; } - if (xattr_data->type == EVM_XATTR_PORTABLE_DIGSIG) + if (xattr_data->type == EVM_XATTR_PORTABLE_DIGSIG || + xattr_data->type == EVM_IMA_XATTR_DIGEST_LIST) rc = 1; else rc = 0; diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 2b55286edc49..ece39b80ca9c 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -140,6 +140,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, struct signature_v2_hdr *hdr; enum integrity_status evm_status = INTEGRITY_PASS; struct evm_digest digest; + struct ima_digest *found_digest; struct inode *inode; int rc, xattr_len, evm_immutable = 0;
@@ -225,6 +226,50 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, } } break; + case EVM_IMA_XATTR_DIGEST_LIST: + if (xattr_len < offsetof(struct signature_v2_hdr, keyid)) { + 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, + xattr_value_len, xattr_data->type, &digest); + if (rc) + break; + + found_digest = ima_lookup_digest(digest.digest, hdr->hash_algo, + COMPACT_METADATA); + if (!found_digest) { + rc = -ENOENT; + break; + } + + if (!ima_digest_allow(found_digest, IMA_APPRAISE)) { + rc = -EACCES; + break; + } + + if (ima_digest_is_immutable(found_digest)) { + evm_immutable = 1; + + if (iint) + iint->flags |= EVM_IMMUTABLE_DIGSIG; + evm_status = INTEGRITY_PASS_IMMUTABLE; + } else { + inode = d_backing_inode(dentry); + if (!IS_RDONLY(inode) && + !(inode->i_sb->s_readonly_remount) && + !IS_IMMUTABLE(inode)) { + rc = __vfs_removexattr(dentry, XATTR_NAME_EVM); + if (!rc) + evm_update_evmxattr(dentry, xattr_name, + xattr_value, + xattr_value_len); + } + } + break; default: rc = -EINVAL; break; @@ -461,7 +506,8 @@ int evm_inode_setxattr(struct dentry *dentry, const char *xattr_name, if (!xattr_value_len) return -EINVAL; if (xattr_data->type != EVM_IMA_XATTR_DIGSIG && - xattr_data->type != EVM_XATTR_PORTABLE_DIGSIG) + xattr_data->type != EVM_XATTR_PORTABLE_DIGSIG && + xattr_data->type != EVM_IMA_XATTR_DIGEST_LIST) return -EPERM; } return evm_protect_xattr(dentry, xattr_name, xattr_value,
hulk inclusion category: feature feature: digest-lists
---------------------------
Currently, IMA supports the appraise_type=imasig option in the policy to require file signatures. This patch introduces the new option appraise_type=meta_immutable to require that file metadata are signed and immutable. This requirement can be satisfied by portable signatures and by digest lists if they are marked as immutable.
The main purpose of this option is to ensure that file metadata are correct at the time of access, so that policies relying on labels can be correctly enforced. For example, requiring immutable metadata would prevent an administrator from altering the label assigned to a process during execve() by changing the label of the executable.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/ima/ima_appraise.c | 8 ++++++++ security/integrity/ima/ima_policy.c | 4 ++++ security/integrity/integrity.h | 1 + 3 files changed, 13 insertions(+)
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index b7f234292054..a11577147022 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -229,6 +229,14 @@ int ima_appraise_measurement(enum ima_hooks func, switch (status) { case INTEGRITY_PASS: case INTEGRITY_PASS_IMMUTABLE: + if (iint->flags & IMA_META_IMMUTABLE_REQUIRED && + status != INTEGRITY_PASS_IMMUTABLE) { + status = INTEGRITY_FAIL; + cause = "metadata-modifiable"; + integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, + filename, op, cause, rc, 0); + goto out; + } break; case INTEGRITY_UNKNOWN: if (ima_appraise_req_evm && diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 8ffd5958ae3e..a4014eef4fb4 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -957,6 +957,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) ima_log_string(ab, "appraise_type", args[0].from); if ((strcmp(args[0].from, "imasig")) == 0) entry->flags |= IMA_DIGSIG_REQUIRED; + else if (strcmp(args[0].from, "meta_immutable") == 0) + entry->flags |= IMA_META_IMMUTABLE_REQUIRED; else result = -EINVAL; break; @@ -1255,6 +1257,8 @@ int ima_policy_show(struct seq_file *m, void *v) } if (entry->flags & IMA_DIGSIG_REQUIRED) seq_puts(m, "appraise_type=imasig "); + if (entry->flags & IMA_META_IMMUTABLE_REQUIRED) + seq_puts(m, "appraise_type=meta_immutable "); if (entry->flags & IMA_PERMIT_DIRECTIO) seq_puts(m, "permit_directio "); rcu_read_unlock(); diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 49132ab9ef0a..2828661c6097 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -37,6 +37,7 @@ #define IMA_NEW_FILE 0x04000000 #define EVM_IMMUTABLE_DIGSIG 0x08000000 #define IMA_FAIL_UNVERIFIABLE_SIGS 0x10000000 +#define IMA_META_IMMUTABLE_REQUIRED 0x20000000
#define IMA_DO_MASK (IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \ IMA_HASH | IMA_APPRAISE_SUBMASK)
hulk inclusion category: feature feature: digest-lists
---------------------------
This patch introduces a new hard-coded policy to measure executable code:
dont_measure fsmagic=0x9fa0 dont_measure fsmagic=0x62656572 dont_measure fsmagic=0x64626720 dont_measure fsmagic=0x1cd1 dont_measure fsmagic=0x42494e4d dont_measure fsmagic=0x73636673 dont_measure fsmagic=0xf97cff8c dont_measure fsmagic=0x43415d53 dont_measure fsmagic=0x27e0eb dont_measure fsmagic=0x63677270 dont_measure fsmagic=0x6e736673 measure func=MMAP_CHECK mask=MAY_EXEC measure func=BPRM_CHECK mask=MAY_EXEC measure func=MODULE_CHECK measure func=FIRMWARE_CHECK measure func=POLICY_CHECK measure func=DIGEST_LIST_CHECK
It can be selected by specifying ima_policy=exec_tcb in the kernel command line. Files in tmpfs are not excluded from measurement.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- .../admin-guide/kernel-parameters.txt | 5 ++++ security/integrity/ima/ima_policy.c | 23 ++++++++++++++++--- 2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 65efa24e3dd0..e0e8d054782d 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -1601,6 +1601,11 @@ mode bit set by either the effective uid (euid=0) or uid=0.
+ The "exec_tcb" policy is similar to the "tcb" policy + except for file open, which is not considered. Files + in the tmpfs filesystem are not excluded from + measurement. + The "appraise_tcb" policy appraises the integrity of all files owned by root. (This is the equivalent of ima_appraise_tcb.) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index a4014eef4fb4..3de081671725 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -56,7 +56,7 @@ enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE, LSM_SUBJ_USER, LSM_SUBJ_ROLE, LSM_SUBJ_TYPE };
-enum policy_types { ORIGINAL_TCB = 1, DEFAULT_TCB }; +enum policy_types { ORIGINAL_TCB = 1, DEFAULT_TCB, EXEC_TCB };
struct ima_rule_entry { struct list_head list; @@ -225,6 +225,8 @@ static int __init policy_setup(char *str) continue; if ((strcmp(p, "tcb") == 0) && !ima_policy) ima_policy = DEFAULT_TCB; + else if ((strcmp(p, "exec_tcb") == 0) && !ima_policy) + ima_policy = EXEC_TCB; else if (strcmp(p, "appraise_tcb") == 0) ima_use_appraise_tcb = true; else if (strcmp(p, "secure_boot") == 0) @@ -495,8 +497,14 @@ void __init ima_init_policy(void) secure_boot_entries = ima_use_secure_boot ? ARRAY_SIZE(secure_boot_rules) : 0;
- for (i = 0; i < measure_entries; i++) + for (i = 0; i < measure_entries; i++) { + if (ima_policy == EXEC_TCB && + (dont_measure_rules[i].flags & IMA_FSMAGIC) && + dont_measure_rules[i].fsmagic == TMPFS_MAGIC) + continue; + list_add_tail(&dont_measure_rules[i].list, &ima_default_rules); + }
switch (ima_policy) { case ORIGINAL_TCB: @@ -505,9 +513,18 @@ void __init ima_init_policy(void) &ima_default_rules); break; case DEFAULT_TCB: - for (i = 0; i < ARRAY_SIZE(default_measurement_rules); i++) + /* fall through */ + case EXEC_TCB: + for (i = 0; i < ARRAY_SIZE(default_measurement_rules); i++) { + if (ima_policy == EXEC_TCB && + (default_measurement_rules[i].flags & IMA_FUNC) && + default_measurement_rules[i].func == FILE_CHECK) + continue; + list_add_tail(&default_measurement_rules[i].list, &ima_default_rules); + } + break; default: break; }
hulk inclusion category: feature feature: digest-lists
---------------------------
This patch introduces a new hard-coded policy to appraise executable code:
appraise func=MODULE_CHECK appraise_type=imasig appraise_type=meta_immutable appraise func=FIRMWARE_CHECK appraise_type=imasig appraise_type=meta_immutable appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig appraise_type=meta_immutable appraise func=POLICY_CHECK appraise_type=imasig appraise_type=meta_immutable appraise func=DIGEST_LIST_CHECK appraise_type=imasig appraise_type=meta_immutable dont_appraise fsmagic=0x9fa0 dont_appraise fsmagic=0x62656572 dont_appraise fsmagic=0x64626720 dont_appraise fsmagic=0x858458f6 dont_appraise fsmagic=0x1cd1 dont_appraise fsmagic=0x42494e4d dont_appraise fsmagic=0x73636673 dont_appraise fsmagic=0xf97cff8c dont_appraise fsmagic=0x43415d53 dont_appraise fsmagic=0x6e736673 dont_appraise fsmagic=0x27e0eb dont_appraise fsmagic=0x63677270 appraise func=BPRM_CHECK appraise_type=imasig appraise_type=meta_immutable appraise func=MMAP_CHECK appraise_type=meta_immutable
The imasig requirement cannot be requested for MMAP_CHECK hook, as there are libraries mapped in memory with PROT_EXEC flag files in the tmpfs filesystem.
The new policy can be selected by specifying ima_policy=appraise_exec_tcb in the kernel command line.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- .../admin-guide/kernel-parameters.txt | 6 ++++ security/integrity/ima/ima_policy.c | 29 +++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index e0e8d054782d..8904da12caf9 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -1614,6 +1614,12 @@ of files (eg. kexec kernel image, kernel modules, firmware, policy, etc) based on file signatures.
+ The "appraise_exec_tcb" includes the "secure_boot" + policy and additionally includes all programs exec'd and + files mmap'd for exec (no digsig requirement for the + latter, as it would break processes accessing files on + tmpfs, e.g. firewalld). + The "fail_securely" policy forces file signature verification failure also on privileged mounted filesystems with the SB_I_UNVERIFIABLE_SIGNATURE diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 3de081671725..cd76088878ee 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -164,6 +164,13 @@ static struct ima_rule_entry default_appraise_rules[] __ro_after_init = { #endif };
+static struct ima_rule_entry appraise_exec_rules[] __ro_after_init = { + {.action = APPRAISE, .func = BPRM_CHECK, + .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED}, + {.action = APPRAISE, .func = MMAP_CHECK, + .flags = IMA_FUNC}, +}; + static struct ima_rule_entry build_appraise_rules[] __ro_after_init = { #ifdef CONFIG_IMA_APPRAISE_REQUIRE_MODULE_SIGS {.action = APPRAISE, .func = MODULE_CHECK, @@ -214,6 +221,7 @@ static int __init default_measure_policy_setup(char *str) __setup("ima_tcb", default_measure_policy_setup);
static bool ima_use_appraise_tcb __initdata; +static bool ima_use_appraise_exec_tcb __initdata; static bool ima_use_secure_boot __initdata; static bool ima_fail_unverifiable_sigs __ro_after_init; static int __init policy_setup(char *str) @@ -229,6 +237,8 @@ static int __init policy_setup(char *str) ima_policy = EXEC_TCB; else if (strcmp(p, "appraise_tcb") == 0) ima_use_appraise_tcb = true; + else if (strcmp(p, "appraise_exec_tcb") == 0) + ima_use_appraise_exec_tcb = true; else if (strcmp(p, "secure_boot") == 0) ima_use_secure_boot = true; else if (strcmp(p, "fail_securely") == 0) @@ -489,12 +499,16 @@ static int ima_appraise_flag(enum ima_hooks func) void __init ima_init_policy(void) { int i, measure_entries, appraise_entries, secure_boot_entries; + int appraise_exec_entries;
/* if !ima_policy set entries = 0 so we load NO default rules */ measure_entries = ima_policy ? ARRAY_SIZE(dont_measure_rules) : 0; - appraise_entries = ima_use_appraise_tcb ? + appraise_entries = (ima_use_appraise_tcb || ima_use_appraise_exec_tcb) ? ARRAY_SIZE(default_appraise_rules) : 0; - secure_boot_entries = ima_use_secure_boot ? + appraise_exec_entries = ima_use_appraise_exec_tcb ? + ARRAY_SIZE(appraise_exec_rules) : 0; + secure_boot_entries = (ima_use_secure_boot || + ima_use_appraise_exec_tcb) ? ARRAY_SIZE(secure_boot_rules) : 0;
for (i = 0; i < measure_entries; i++) { @@ -560,12 +574,23 @@ void __init ima_init_policy(void) }
for (i = 0; i < appraise_entries; i++) { + if (ima_use_appraise_exec_tcb && + default_appraise_rules[i].action != DONT_APPRAISE) + continue; + if (ima_use_appraise_exec_tcb && + (default_appraise_rules[i].flags & IMA_FSMAGIC) && + default_appraise_rules[i].fsmagic == TMPFS_MAGIC) + continue; list_add_tail(&default_appraise_rules[i].list, &ima_default_rules); if (default_appraise_rules[i].func == POLICY_CHECK) temp_ima_appraise |= IMA_APPRAISE_POLICY; }
+ for (i = 0; i < appraise_exec_entries; i++) + list_add_tail(&appraise_exec_rules[i].list, + &ima_default_rules); + ima_update_policy_flag(); }
hulk inclusion category: feature feature: digest-lists
---------------------------
This patch modifies the existing "secure_boot" and "appraise_exec_tcb" policies, by adding the appraise_type=meta_immutable requirement for all appraise rules:
appraise func=MODULE_CHECK appraise_type=imasig appraise_type=meta_immutable appraise func=FIRMWARE_CHECK appraise_type=imasig appraise_type=meta_immutable appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig appraise_type=meta_immutable appraise func=POLICY_CHECK appraise_type=imasig appraise_type=meta_immutable appraise func=DIGEST_LIST_CHECK appraise_type=imasig appraise_type=meta_immutable dont_appraise fsmagic=0x9fa0 dont_appraise fsmagic=0x62656572 dont_appraise fsmagic=0x64626720 dont_appraise fsmagic=0x858458f6 dont_appraise fsmagic=0x1cd1 dont_appraise fsmagic=0x42494e4d dont_appraise fsmagic=0x73636673 dont_appraise fsmagic=0xf97cff8c dont_appraise fsmagic=0x43415d53 dont_appraise fsmagic=0x6e736673 dont_appraise fsmagic=0x27e0eb dont_appraise fsmagic=0x63677270 appraise func=BPRM_CHECK appraise_type=imasig appraise_type=meta_immutable appraise func=MMAP_CHECK appraise_type=meta_immutable
This policy can be selected by specifying ima_policy="appraise_exec_tcb|appraise_exec_immutable" in the kernel command line.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- Documentation/admin-guide/kernel-parameters.txt | 4 ++++ security/integrity/ima/ima_policy.c | 12 +++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 8904da12caf9..14fd02ce8367 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -1620,6 +1620,10 @@ latter, as it would break processes accessing files on tmpfs, e.g. firewalld).
+ The "appraise_exec_immutable" policy requires immutable + metadata for all files appraised by the "secure_boot" + and "appraise_exec_tcb" policies. + The "fail_securely" policy forces file signature verification failure also on privileged mounted filesystems with the SB_I_UNVERIFIABLE_SIGNATURE diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index cd76088878ee..94ba751a8191 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -222,6 +222,7 @@ __setup("ima_tcb", default_measure_policy_setup);
static bool ima_use_appraise_tcb __initdata; static bool ima_use_appraise_exec_tcb __initdata; +static bool ima_use_appraise_exec_immutable __initdata; static bool ima_use_secure_boot __initdata; static bool ima_fail_unverifiable_sigs __ro_after_init; static int __init policy_setup(char *str) @@ -239,6 +240,8 @@ static int __init policy_setup(char *str) ima_use_appraise_tcb = true; else if (strcmp(p, "appraise_exec_tcb") == 0) ima_use_appraise_exec_tcb = true; + else if (strcmp(p, "appraise_exec_immutable") == 0) + ima_use_appraise_exec_immutable = true; else if (strcmp(p, "secure_boot") == 0) ima_use_secure_boot = true; else if (strcmp(p, "fail_securely") == 0) @@ -548,6 +551,9 @@ void __init ima_init_policy(void) * signatures, prior to any other appraise rules. */ for (i = 0; i < secure_boot_entries; i++) { + if (ima_use_appraise_exec_immutable) + secure_boot_rules[i].flags |= + IMA_META_IMMUTABLE_REQUIRED; list_add_tail(&secure_boot_rules[i].list, &ima_default_rules); temp_ima_appraise |= ima_appraise_flag(secure_boot_rules[i].func); @@ -587,9 +593,13 @@ void __init ima_init_policy(void) temp_ima_appraise |= IMA_APPRAISE_POLICY; }
- for (i = 0; i < appraise_exec_entries; i++) + for (i = 0; i < appraise_exec_entries; i++) { + if (ima_use_appraise_exec_immutable) + appraise_exec_rules[i].flags |= + IMA_META_IMMUTABLE_REQUIRED; list_add_tail(&appraise_exec_rules[i].list, &ima_default_rules); + }
ima_update_policy_flag(); }
hulk inclusion category: feature feature: digest-lists
---------------------------
This patch adds the documentation of the IMA Digest Lists extension.
Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- Documentation/security/IMA-digest-lists.txt | 255 ++++++++++++++++++++ 1 file changed, 255 insertions(+) create mode 100644 Documentation/security/IMA-digest-lists.txt
diff --git a/Documentation/security/IMA-digest-lists.txt b/Documentation/security/IMA-digest-lists.txt new file mode 100644 index 000000000000..53dafd9278aa --- /dev/null +++ b/Documentation/security/IMA-digest-lists.txt @@ -0,0 +1,255 @@ +========================== +IMA Digest Lists Extension +========================== + +INTRODUCTION +============ + +Integrity Measurement Architecture (IMA) is a security module that performs +measurement of files accessed with the execve(), mmap() and open() system +calls. File measurements can be used for different purposes. They can be +added in a measurement list and sent to a remote verifier for an integrity +evaluation. They can be compared to reference values provided by the +software vendor and access can be denied if there is a mismatch. File +measurements can also be included in system logs for auditing purposes. + +IMA Digest Lists is an extension providing additional functionality for +the IMA submodules and EVM. Its main task is to load reference values for +file content and metadata in the kernel memory, and to communicate to IMA +submodules (measurement and appraisal) or EVM whether the digest of a file +being accessed is found in the uploaded digest lists. + +The IMA-Measure submodule uses the IMA Digest Lists extension to create +a new measurement list (with a different PCR, for example 11) which +contains the measurement of uploaded digest lists and unknown files. Since +the loading of digest lists is sequential, the chosen PCR will have a +predictable value for the whole boot cycle and can be used for sealing +policies based on the OS software integrity. + +Both the standard and the new measurement list can be generated at the same +time, allowing for implicit attestation based on the usage of a TPM key +sealed to the OS, and for explicit attestation when a more precise +integrity evaluation is necessary. + +The IMA-Appraise submodule uses the IMA Digest Lists extension to +grant/deny access to the files depending on whether the file digest is +found in the uploaded digest lists, instead of checking security.ima. + +EVM uses the IMA Digest Lists extension to check file metadata integrity. + +The main advantage of the extension is that reference measurements used for +the digest comparison can be extracted from already existing sources (for +example RPM headers). Another benefit is that the overhead compared to file +signatures is lower as only one signature is verified for all files +included in the digest list. A disadvantage is that file metadata are not +verified. + + + +WORKFLOW +======== + +The IMA workflow is modified as follows. The added steps are marked as +[NEW]. + + +------------+ + | IMA hook | + +------------+ + | + +---------+ + | collect | + +---------+ + | +---------------------+ ++---------------+ ------------| don't measure [NEW] | +| digest lookup | | yes +---------------------+ +| [NEW] | -------------- no +---------------------+ ++---------------+ -----/ digest ---| add to measurement | + | | \ found? [NEW] / | list (PCR 11) [NEW] | + | | -------------- +---------------------+ + +----------+ +-------------+ +--------------------+ + | switch |-------| IMA-Measure |----------------| add to measurement | + | (action) | +-------------+ | list (PCR 10) | + +----------+ +--------------------+ + | + | + | no xattr ++--------------+ --------------- yes +| IMA-Appraise |----/ filec created ------------------------------- ++--------------+ \ created [NEW] / | + xattr | --------------- | + | | no | + | ------------------ no -------------- yes +--------+ + | / EVM initialized? ----/ digest ------| grant | + | \ [NEW] / \ found? [NEW] / | access | + | ------------------ -------------- +--------+ + | | yes | no + | | | +--------+ + | ------------------- no ---------------| deny | + | / digest-nometadata -------------------------| access | + | \ mode? [NEW] / +--------+ + | ------------------- + | ^ | yes +--------+ + | | -------------------------------------| grant | + | | | access | + | | +--------+ + | | yes + | ------------------------- no +--------+ + |----------/ security.ima (new type) -------------------| deny | + | \ present and valid? / | access | + | ------------------------- +--------+ + | | + | -------------------------- no | + -----------/ security.ima (cur types) ----------------------- + \ present and valid? / + -------------------------- + | yes +--------+ + ------------------------------------| grant | + | access | + +--------+ + + ++-----+ -------------------------- yes +--------+ +| EVM |---------/ security.evm (cur types) ---------------------| grant | ++-----+ \ present and valid? / | access | + | -------------------------- +--------+ + | | + | | no +--------+ + | ------------------------------------| deny | + | | access | + | +--------+ + | | + | +-----------+ | + | ------------------------- | calculate | | + -------------/ security.evm (new type) ---| metadata | | + \ present? [NEW] / | digest | | + ------------------------- +-----------+ | + | | + -------------- no | + / digest --------- + \ found? [NEW] / + -------------- +--------+ + | yes | grant | + | | access | + | +--------+ + | | + ------------------ yes | + / digest -----| + \ immutable? [NEW] / | + ------------------ | + | no | + +---------+ | + | convert |------------ + | to HMAC | + +---------+ + + +After the file digest is calculated, it is searched in the hash table +containing all digests extracted from the uploaded digest lists. Then, if +the digest is found, a structure is returned to IMA with information +associated to that digest. The structure is returned only to the IMA +submodules that processed the digest lists (i.e. the action returned by +ima_get_action() was 'measure' or 'appraise'). + +IMA-Measure behavior depends on whether the digest list PCR has been +specified in the kernel command line. If the PCR was not specified, the +submodule behaves as before. If the PCR was specified, IMA-Measure creates +a new measurement with that PCR, only if the file digest is not found in +the digest lists. It additionally creates a measurement with the default +PCR if '+' is added as a prefix to the PCR. + +IMA-Appraise behavior depends on whether either the 'digest' or +'digest-nometadata' appraisal modes have been specified in the kernel +command line. If they were not specified, IMA-Appraise relies solely on the +security.ima xattr for verification. If the 'digest' mode was specified, +verification succeeds if the file digest is found in the digest lists and +EVM is not initialized, as there is no other way to verify file metadata. + +If the 'digest-nometadata' mode was specified, verification succeeds +regardless of the fact that EVM is initialized or not. However, after a +write, files for which access was granted without verifying metadata will +have a new security.ima type, so that they can be identified also after +reboot. Specifying 'digest-nometadata' is required also to access files +with the new security.ima type. + +EVM determines whether metadata digest should be searched in the digest +lists depending on the security.evm type. If the new type is set, EVM +calculates metadata digest and searches it in the digest lists. A structure +is returned to EVM if the digest is found and the digest lists were +appraised. + + + +ARCHITECTURE +============ + ++-----+ 6) digest lookup +| EVM |------------------------------------------------ ++-----+ | + | ++--------------+ 5) digest lookup | +| IMA-Appraise |--------------------------------------| ++--------------+ || + || ++-------------+ 4) digest lookup || +| IMA-Measure | -------------------------------------|| ++-------------+ ||| + ||| ++-------------+ 2) parse compact list +-------------------+ +| IMA (secfs) | ------------------------> | IMA Digests Lists | ++-------------+ +-------------------+ + ^ | 3) add digests + | +------------+ + | | hash table | + | +------------+ + | + | kernel space +---|------------------------------------------------------------------------- + | user space + | + | + | + | + | + | + | +1) echo <digest list path> + +The main addition to IMA is a new hash table (similar to that used to check +for duplicate measurement entries), which contains file content and metadata +digests extracted from the digest lists. + +Digest lists can be uploaded to the kernel by writing their path to +digest_list_data in the securityfs filesystem. After digest lists are +uploaded, they are parsed by the kernel and extracted digests are added to +the hash table. + +IMA submodules, Measure and Appraise, search the digest of an accessed file +in the hash table and perform actions depending on whether the digest was +found or not. IMA submodules can search digests in the hash table only if +they also processed the digest lists. + +EVM searches the metadata digest of an accessed file in the hash table and +returns the result to IMA, which perform actions depending on the result. +EVM can search digests in the hash table if IMA-Appraise processed the +digest lists. + + + +CONFIGURATION +============= + +The first step consists in generating digest lists with the +gen_digest_lists tool included in the digest-list-tools package. +digest-list-tools can be retrieved at the URL: + +https://github.com/euleros/digest-list-tools + +gen_digest_lists can generate digest lists from different sources (for +example: RPM package DB). By default, it saves generated digest lists in +the /etc/ima/digest_lists directory. digest-list-tools includes also two +bash scripts setup_ima_digest_lists and setup_digest_lists_demo to simplify +the digest list generation for the users. + +To use digest lists during early boot, it is necessary to regenerate the +initial ram disk. Digest lists will be added to the ram disk by the new +dracut/initramfs-tools modules, included in the digest-list-tools package.
On 2020/7/6 23:40, Roberto Sassu wrote:
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:
- allow `setxattr` on mounts without `SBLABEL_MNT` (which is all of them if no policy is loaded yet)
- 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
For this patch set,
Acked-by: Hanjun Guo guohanjun@huawei.com
Applied.
On 2020/7/6 23:40, Roberto Sassu wrote:
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:
- allow `setxattr` on mounts without `SBLABEL_MNT` (which is all of them if no policy is loaded yet)
- 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) {