On Mon, 2021-05-03 at 14:48 +0000, Roberto Sassu wrote:

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
time there is an attr or xattr change, including setting it to the
existing value, the status flag should be reset.

The status is always reset if evm_protect_xattr() returns 0. This does not
change.

Not making INTEGRITY_PASS_IMMUTABLE conditional would cause issues.
Suppose that the status is INTEGRITY_FAIL. Writing the same xattr would
cause evm_protect_xattr() to return 0 and the HMAC to be updated.

This example is mixing security.evm types.  Please clarify.

quoted

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

INTEGRITY_FAIL_IMMUTABLE should be enough to continue the
operation.

Agreed.

Mimi