linux-kernel - Re: [PATCH] ima: setting security.ima to fix security.evm for a file with IMA signature

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <r3himk4z2aiyqsjstlpnda4wafeo7i4oum3n2dbvnasmtep5ex@zqodcpjmyx5b>
Date: Wed, 10 Sep 2025 09:20:43 +0800
From: Coiby Xu <coxu@...hat.com>
To: Mimi Zohar <zohar@...ux.ibm.com>
Cc: linux-integrity@...r.kernel.org, 
	Roberto Sassu <roberto.sassu@...wei.com>, Dmitry Kasatkin <dmitry.kasatkin@...il.com>, 
	Eric Snowberg <eric.snowberg@...cle.com>, Paul Moore <paul@...l-moore.com>, 
	James Morris <jmorris@...ei.org>, "Serge E. Hallyn" <serge@...lyn.com>, 
	"open list:SECURITY SUBSYSTEM" <linux-security-module@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ima: setting security.ima to fix security.evm for a file
 with IMA signature

On Tue, Sep 09, 2025 at 11:31:20AM -0400, Mimi Zohar wrote:
>On Tue, 2025-09-09 at 12:19 +0800, Coiby Xu wrote:
>> When both IMA and EVM fix modes are enabled, accessing a file with IMA
>> signature won't cause security.evm to be fixed. But this doesn't happen
>> to a file with correct IMA hash already set because accessing it will
>> cause setting security.ima again which triggers fixing security.evm
>> thanks to security_inode_post_setxattr->evm_update_evmxattr.
>>
>> Let's use the same mechanism to fix security.evm for a file with IMA
>> signature.
>>
>> Signed-off-by: Coiby Xu <coxu@...hat.com>
>
>Agreed, re-writing the file signature stored as security.ima would force
>security.evm to be updated.
>
>Unfortunately, I'm missing something. ima_appraise_measurement() first verifies
>the existing security.evm xattr, before verifying the security.ima xattr.  If
>the EVM HMAC fails to verify, it immediately exits ima_appraise_measurement().
>security.ima in this case is never verified.
>
>This patch seems to address the case where the existing security.evm is valid,
>but the file signature stored in security.ima is invalid.  (To get to the new
>code, the "status" flag is not INTEGRITY_PASS.)  Re-writing the same invalid
>file signature would solve an invalid security.evm, but not an invalid IMA file
>signature.  What am I missing?

Hi, Mimi,

Thanks for raising the question! This patch is to address the case where
IMA signature is already added but security.evm doesn't yet exist. So
EVM HMAC fails to verify but there is no exiting
ima_appraise_measurement immediately.

And you are right that re-writing an invalid IMA file won't fix an
invalid IMA file signature. And even when IMA signature is valid, the
verification may fail because the key is missing from .ima keyring. This
happens because we need to turn off secure boot to enable fix mode. As a
result, CA keys won't be loaded into .machine keyring. Btw, if I'm not
mistaken, current IMA code assumes we are not supposed to fix IMA file
signature.

>
>thanks,
>
>Mimi
>
>> ---
>>  security/integrity/ima/ima_appraise.c | 27 +++++++++++++++++++++------
>>  1 file changed, 21 insertions(+), 6 deletions(-)
>>
>> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
>> index f435eff4667f..18c3907c5e44 100644
>> --- a/security/integrity/ima/ima_appraise.c
>> +++ b/security/integrity/ima/ima_appraise.c
>> @@ -595,12 +595,27 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
>>  		integrity_audit_msg(audit_msgno, inode, filename,
>>  				    op, cause, rc, 0);
>>  	} else if (status != INTEGRITY_PASS) {
>> -		/* Fix mode, but don't replace file signatures. */
>> -		if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig &&
>> -		    (!xattr_value ||
>> -		     xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
>> -			if (!ima_fix_xattr(dentry, iint))
>> -				status = INTEGRITY_PASS;
>> +		/*
>> +		 * Fix mode, but don't replace file signatures.
>> +		 *
>> +		 * When EVM fix mode is also enabled, security.evm will be
>> +		 * fixed automatically when security.ima is set because of
>> +		 * security_inode_post_setxattr->evm_update_evmxattr.
>> +		 */
>> +		if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig) {
>> +			if (!xattr_value ||
>> +			    xattr_value->type != EVM_IMA_XATTR_DIGSIG) {
>> +				if (ima_fix_xattr(dentry, iint))
>> +					status = INTEGRITY_PASS;
>> +			} else if (xattr_value->type == EVM_IMA_XATTR_DIGSIG &&
>> +				   evm_revalidate_status(XATTR_NAME_IMA)) {
>> +				if (!__vfs_setxattr_noperm(&nop_mnt_idmap,
>> +							   dentry,
>> +							   XATTR_NAME_IMA,
>> +							   xattr_value,
>> +							   xattr_len, 0))
>> +					status = INTEGRITY_PASS;
>> +			}
>>  		}
>>
>>  		/*
>>
>> base-commit: b320789d6883cc00ac78ce83bccbfe7ed58afcf0
>

-- 
Best regards,
Coiby