diff mbox

[RFC,v2,2/9] ima: preserve flags in ima_inode_post_setattr() if file must be appraised

Message ID 20171130105610.15761-3-roberto.sassu@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

Roberto Sassu Nov. 30, 2017, 10:56 a.m. UTC
Before commit d79d72e02485 ("ima: per hook cache integrity appraisal
status"), ima_inode_post_setattr() clears the iint flags only if the file
does not match policy rules after attributes changed. After the commit
above, it clears the flags in any case. This patch restores the original
behavior.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_appraise.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Mimi Zohar Dec. 1, 2017, 4:31 p.m. UTC | #1
On Thu, 2017-11-30 at 11:56 +0100, Roberto Sassu wrote:
> Before commit d79d72e02485 ("ima: per hook cache integrity appraisal
> status"), ima_inode_post_setattr() clears the iint flags only if the file
> does not match policy rules after attributes changed. After the commit
> above, it clears the flags in any case. This patch restores the original
> behavior.
> 
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Prior to this commit, the file was either in/out of policy for all
hooks.  This commit introduced the concept of different hooks
requiring different types of integrity appraisal (eg. hash,
signature).  An attribute change (eg. fowner) could require a
different type of integrity appraisal.  With this patch, knowing just
that a file is in/out of policy isn't fine grained enough.

Mimi

> ---
>  security/integrity/ima/ima_appraise.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 1b56ee949315..a54ad18affb1 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -360,11 +360,13 @@ 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) {
> -		iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED |
> -				 IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK |
> -				 IMA_ACTION_RULE_FLAGS);
>  		if (must_appraise)
>  			iint->flags |= IMA_APPRAISE;
> +		else
> +			iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED |
> +					 IMA_APPRAISE_SUBMASK |
> +					 IMA_APPRAISED_SUBMASK |
> +					 IMA_ACTION_RULE_FLAGS);
>  	}
>  	if (!must_appraise)
>  		__vfs_removexattr(dentry, XATTR_NAME_IMA);
diff mbox

Patch

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 1b56ee949315..a54ad18affb1 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -360,11 +360,13 @@  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) {
-		iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED |
-				 IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK |
-				 IMA_ACTION_RULE_FLAGS);
 		if (must_appraise)
 			iint->flags |= IMA_APPRAISE;
+		else
+			iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED |
+					 IMA_APPRAISE_SUBMASK |
+					 IMA_APPRAISED_SUBMASK |
+					 IMA_ACTION_RULE_FLAGS);
 	}
 	if (!must_appraise)
 		__vfs_removexattr(dentry, XATTR_NAME_IMA);