From 928f2de735ab38802984938618aa051dd55f536c Mon Sep 17 00:00:00 2001 From: Kam Nasim Date: Wed, 4 Oct 2017 14:23:13 -0400 Subject: [PATCH] US103091: IMA: System Configuration When appraise_type="imasig" is set in the IMA policy then don't allow IMA to put a hash value for the security.ima xattr, if the extended attribute is missing. This is a fool's errand, as there is already a check in the driver which will give an appraisal failure if it detects that the security.ima xattr is a Hash and NOT a Signature, so appraisal would fail again next time that file is executed. The advantage of this fix is that it improves driver performance as we are not collecting a measurement on appraisal failure By virtue of the same, we will not remove the security.ima xattr if we detect that imasig is set on that iint --- ima/ima_appraise.c | 33 +++++++++++++++++++++++++++++---- ima/ima_main.c | 2 -- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/ima/ima_appraise.c b/ima/ima_appraise.c index 88b5091..cff2ad2 100644 --- a/ima/ima_appraise.c +++ b/ima/ima_appraise.c @@ -205,7 +208,11 @@ int ima_appraise_measurement(enum ima_hooks func, if (rc && rc != -ENODATA) goto out; - cause = "missing-hash"; + if (iint->flags & IMA_DIGSIG_REQUIRED) + cause = "missing-signature"; + else + cause = "missing-hash"; + status = INTEGRITY_NOLABEL; if (opened & FILE_CREATED) iint->flags |= IMA_NEW_FILE; @@ -352,7 +355,8 @@ void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file) int rc = 0; /* do not collect and update hash for digital signatures */ - if (iint->flags & IMA_DIGSIG) + /* WRS: Don't do it if appraise_type is set to imasig */ + if ((iint->flags & IMA_DIGSIG) || (iint->flags & IMA_DIGSIG_REQUIRED)) return; rc = ima_collect_measurement(iint, file, NULL, 0, ima_hash_algo); @@ -376,6 +380,7 @@ void __ima_inode_post_setattr(struct dentry *dentry) struct inode *inode = d_backing_inode(dentry); struct integrity_iint_cache *iint; int must_appraise, rc; + int imasig = 0; if (!(ima_policy_flag & IMA_APPRAISE) || !S_ISREG(inode->i_mode) || !inode->i_op->removexattr) @@ -384,11 +389,20 @@ void __ima_inode_post_setattr(struct dentry *dentry) must_appraise = ima_must_appraise(inode, MAY_ACCESS, POST_SETATTR); iint = integrity_iint_find(inode); if (iint) { + /* WRS: Before we clear all the ACTION RULE FLAGS, check if + * imasig was set on this iint, which implies that we are + * expecting a signature for the security.ima xattr + */ + if (iint->flags & IMA_DIGSIG_REQUIRED) + imasig = 1; iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED | IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK | IMA_ACTION_RULE_FLAGS); - if (must_appraise) + if (must_appraise) { iint->flags |= IMA_APPRAISE; + if (imasig) + iint->flags |= IMA_DIGSIG_REQUIRED; + } } if (!must_appraise) rc = inode->i_op->removexattr(dentry, XATTR_NAME_IMA); @@ -450,6 +464,17 @@ int __ima_inode_setxattr(struct dentry *dentry, const char *xattr_name, int __ima_inode_removexattr(struct dentry *dentry, const char *xattr_name) { int result; + + /* WRS: If this security.ima xattr is a digital signature + * then we will not allow it to be removed (only if we + * have a cached iint entry for it) + */ + struct inode *inode = d_backing_inode(dentry); + struct integrity_iint_cache *iint = integrity_iint_find(inode); + if (iint) { + if (iint->flags & IMA_DIGSIG_REQUIRED) + return -EPERM; + } result = ima_protect_xattr(dentry, xattr_name, NULL, 0); if (result == 1) { diff --git a/ima/ima_main.c b/ima/ima_main.c index ea3ace3..15ac6a7 100644 --- a/ima/ima_main.c +++ b/ima/ima_main.c @@ -129,7 +129,6 @@ static void ima_check_last_writer(struct integrity_iint_cache *iint, if (!(mode & FMODE_WRITE)) return; - inode_lock(inode); if (atomic_read(&inode->i_writecount) == 1) { if ((iint->version != inode->i_version) || (iint->flags & IMA_NEW_FILE)) { @@ -139,7 +138,6 @@ static void ima_check_last_writer(struct integrity_iint_cache *iint, ima_update_xattr(iint, file); } } - inode_unlock(inode); } /** -- 1.8.3.1