[PATCH] Revert "ima: limit file hash setting by user to fix and log modes"

[PATCH] Revert "ima: limit file hash setting by user to fix and log modes"

[Mike Rapoport]

Hi,

On a system that has IMA appraisal enabled it is impossible to create
security.ima extended attribute files that contain IMA hash.

For instance, consider the following use case:

1) extract application files to a staging area as non root user
2) verify that installation is correct
3) create IMA extended attributes for the installed files
4) move the files to their destination
5) change the files ownership to root

With the longterm kernels 4.4.x and 4.9.x step 3 will fail.

The issues is fixed in upstream kernels by the commit
f5acb3dcba1ffb7f0b8cbb9dba61500eea5d610b ("Revert "ima: limit file hash
setting by user to fix and log modes"), with the patch also quoted below.

--
Sincerely yours,
Mike.

>From f5acb3dcba1ffb7f0b8cbb9dba61500eea5d610b Mon Sep 17 00:00:00 2001
From: Mimi Zohar <zohar@linux.vnet.ibm.com>
Date: Wed, 2 Nov 2016 09:14:16 -0400
Subject: [PATCH] Revert "ima: limit file hash setting by user to fix and log
 modes"

Userspace applications have been modified to write security xattrs,
but they are not context aware.  In the case of security.ima, the
security xattr can be either a file hash or a file signature.
Permitting writing one, but not the other requires the application to
be context aware.

In addition, userspace applications might write files to a staging
area, which might not be in policy, and then change some file metadata
(eg. owner) making it in policy.  As a result, these files are not
labeled properly.

This reverts commit c68ed80c97d9720f51ef31fe91560fdd1e121533, which
prevents writing file hashes as security.ima xattrs.

Requested-by: Patrick Ohly <patrick.ohly@intel.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/ima_appraise.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 389325ac6067..a705598ced5f 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -384,14 +384,10 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
 	result = ima_protect_xattr(dentry, xattr_name, xattr_value,
 				   xattr_value_len);
 	if (result == 1) {
-		bool digsig;
-
 		if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST))
 			return -EINVAL;
-		digsig = (xvalue->type == EVM_IMA_XATTR_DIGSIG);
-		if (!digsig && (ima_appraise & IMA_APPRAISE_ENFORCE))
-			return -EPERM;
-		ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
+		ima_reset_appraise_flags(d_backing_inode(dentry),
+			 (xvalue->type == EVM_IMA_XATTR_DIGSIG) ? 1 : 0);
 		result = 0;
 	}
 	return result;
-- 
2.7.4

Re: [PATCH] Revert "ima: limit file hash setting by user to fix and log modes"

[Greg KH]

On Thu, May 31, 2018 at 08:42:31PM +0300, Mike Rapoport wrote:
> Hi,
> 
> On a system that has IMA appraisal enabled it is impossible to create
> security.ima extended attribute files that contain IMA hash.
> 
> For instance, consider the following use case:
> 
> 1) extract application files to a staging area as non root user
> 2) verify that installation is correct
> 3) create IMA extended attributes for the installed files
> 4) move the files to their destination
> 5) change the files ownership to root
> 
> With the longterm kernels 4.4.x and 4.9.x step 3 will fail.
> 
> The issues is fixed in upstream kernels by the commit
> f5acb3dcba1ffb7f0b8cbb9dba61500eea5d610b ("Revert "ima: limit file hash
> setting by user to fix and log modes"), with the patch also quoted below.

Now queued up, thanks.

greg k-h