From: Mimi Zohar [mailto:zohar@linux.ibm.com]
Sent: Monday, May 3, 2021 2:13 AM
Hi Roberto,

On Wed, 2021-04-07 at 12:52 +0200, Roberto Sassu wrote:

quoted

When a file is being created, LSMs can set the initial label with the
inode_init_security hook. If no HMAC key is loaded, the new file will have
LSM xattrs but not the HMAC. It is also possible that the file remains
without protected xattrs after creation if no active LSM provided it.

Unfortunately, EVM will deny any further metadata operation on new files,
as evm_protect_xattr() will always return the INTEGRITY_NOLABEL error, or
INTEGRITY_NOXATTRS if no protected xattrs exist. This would limit the
usability of EVM when only a public key is loaded, as commands such as cp
or tar with the option to preserve xattrs won't work.

This patch ignores these errors when they won't be an issue, if no HMAC

key

quoted

is loaded and cannot be loaded in the future (which can be enforced by
setting the EVM_SETUP_COMPLETE initialization flag).

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/evm/evm_main.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/security/integrity/evm/evm_main.c

b/security/integrity/evm/evm_main.c

quoted

index 998818283fda..6556e8c22da9 100644

--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c

@@ -90,6 +90,24 @@ static bool evm_key_loaded(void)
 	return (bool)(evm_initialized & EVM_KEY_MASK);
 }

+/*
+ * Ignoring INTEGRITY_NOLABEL/INTEGRITY_NOXATTRS is safe if no HMAC

key

quoted

+ * is loaded and the EVM_SETUP_COMPLETE initialization flag is set.
+ */
+static bool evm_ignore_error_safe(enum integrity_status evm_status)
+{
+	if (evm_initialized & EVM_INIT_HMAC)
+		return false;
+
+	if (!(evm_initialized & EVM_SETUP_COMPLETE))
+		return false;
+
+	if (evm_status != INTEGRITY_NOLABEL && evm_status !=

INTEGRITY_NOXATTRS)

quoted

+		return false;
+
+	return true;
+}
+
 static int evm_find_protected_xattrs(struct dentry *dentry)
 {
 	struct inode *inode = d_backing_inode(dentry);

@@ -354,6 +372,8 @@ static int evm_protect_xattr(struct dentry *dentry,

const char *xattr_name,

quoted

 				    -EPERM, 0);
 	}
 out:
+	if (evm_ignore_error_safe(evm_status))
+		return 0;

I agree with the concept, but the function name doesn't provide enough
context.  Perhaps defining a function more along the lines of
"evm_hmac_disabled()" would be more appropriate and at the same time
self documenting.

Since the function checks if the passed error can be ignored,
would evm_ignore_error_hmac_disabled() also be ok?

quoted

 	if (evm_status != INTEGRITY_PASS)
 		integrity_audit_msg(AUDIT_INTEGRITY_METADATA,

d_backing_inode(dentry),

quoted

 				    dentry->d_name.name,

"appraise_metadata",

quoted

@@ -515,7 +535,8 @@ int evm_inode_setattr(struct dentry *dentry, struct

iattr *attr)

quoted

 		return 0;
 	evm_status = evm_verify_current_integrity(dentry);
 	if ((evm_status == INTEGRITY_PASS) ||
-	    (evm_status == INTEGRITY_NOXATTRS))
+	    (evm_status == INTEGRITY_NOXATTRS) ||
+	    (evm_ignore_error_safe(evm_status)))

It would also remove the INTEGRITY_NOXATTRS test duplication here.

Ok.

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli

thanks,

Mimi

quoted

 		return 0;
 	integrity_audit_msg(AUDIT_INTEGRITY_METADATA,

d_backing_inode(dentry),

quoted

 			    dentry->d_name.name, "appraise_metadata",