On Mon, 2021-05-03 at 15:32 +0000, Roberto Sassu wrote:

quoted

From: Mimi Zohar [mailto:zohar@linux.ibm.com]
Sent: Monday, May 3, 2021 5:26 PM
On Mon, 2021-05-03 at 15:11 +0000, Roberto Sassu wrote:

quoted

quoted

From: Mimi Zohar [mailto:zohar@linux.ibm.com]
Sent: Monday, May 3, 2021 3:00 PM
On Wed, 2021-04-07 at 12:52 +0200, Roberto Sassu wrote:

quoted

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

b/security/integrity/evm/evm_main.c

quoted

@@ -389,6 +473,11 @@ static int evm_protect_xattr(struct

user_namespace *mnt_userns,

quoted

 	if (evm_status == INTEGRITY_FAIL_IMMUTABLE)
 		return 0;

+	if (evm_status == INTEGRITY_PASS_IMMUTABLE &&
+	    !evm_xattr_change(mnt_userns, dentry, xattr_name, xattr_value,
+			      xattr_value_len))
+		return 0;
+

If the purpose of evm_protect_xattr() is to prevent allowing an invalid
security.evm xattr from being re-calculated and updated, making it
valid, INTEGRITY_PASS_IMMUTABLE shouldn't need to be conditional.

Any

quoted

quoted

time there is an attr or xattr change, including setting it to the
existing value, the status flag should be reset.

I'm wondering if making INTEGRITY_PASS_IMMUTABLE conditional would
prevent the file from being resigned.

quoted

 	if (evm_status != INTEGRITY_PASS)
 		integrity_audit_msg(AUDIT_INTEGRITY_METADATA,

d_backing_inode(dentry),

quoted

 				    dentry->d_name.name,

"appraise_metadata",

This would then be updated to if not INTEGRITY_PASS or
INTEGRITY_PASS_IMMUTABLE.  The subsequent "return" would need to

be

quoted

quoted

updated as well.

I agree on the first suggestion, to reduce the number of log messages.
For the second, if you meant that we should return 0 if the status is
INTEGRITY_PASS_IMMUTABLE, I thought we wanted to deny xattr
changes when there is an EVM portable signature.

Why?  I must be missing something.  As long as we're not relying on the
cached status, allowing the file metadata to be updated shouldn't be an
issue.

We may want to prevent accidental changes, for example.

Let's keep it simple, getting the basics working properly first.  Then
we can decide if this is something that we really want/need to defend
against.

thanks,

Mimi