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

[Coiby Xu]

On Wed, Sep 24, 2025 at 11:27:03PM -0400, Mimi Zohar wrote:
>On Wed, 2025-09-24 at 16:03 +0800, Coiby Xu wrote:
>> On Mon, Sep 15, 2025 at 05:05:42PM -0400, Mimi Zohar wrote:
>> > On Wed, 2025-09-10 at 09:20 +0800, Coiby Xu wrote:
>> > > 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.
>> > > > >
>> [...]
>> > > > > ---
>> > > > >  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;
>> > > > > +			}
>> > > > >  		}
>> >
>> > Instead of re-writing the IMA signature without a clear explanation, define a
>> > new EVM function named evm_fix_hmac() and add a call here in IMA. Only in EVM
>> > fix mode would evm_fix_hmac() update the EVM hmac.
>> >
>> >        } else if (status != INTEGRITY_PASS) {
>> >                /*
>> >                 * IMA fix mode updates the IMA file hash, which triggers EVM
>> >                 * to update security.evm.  ....
>> >                 *
>> >                 * Similarly, trigger fixing EVM HMAC for IMA file signatures.
>> >                 */
>> >                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 (status == INTEGRITY_NOLABEL) {
>> >                                evm_fix_hmac(dentry, XATTR_NAME_IMA, ....);
>> >                        }
>> >                }
>>
>> Thanks for the advice! I wonder if we should use existing
>> evm_update_evmxattr instead of defining a new EVM function.
>>
>>      /*
>>       * Calculate the hmac and update security.evm xattr
>>       *
>>       * Expects to be called with i_mutex locked.
>>       */
>>      int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
>>      			const char *xattr_value, size_t xattr_value_len)
>>      {
>>      }
>>
>>
>> I already tried evm_update_evmxattr and can confirm it works.  But later
>> I switched to __vfs_setxattr_noperm because I thought it's consistent
>> with current logic of adding security.evm when there is already correct
>> security.ima and it's a slightly smaller change.
>
>Calling evm_inode_updatexattr() is limited to EVM.  Only after verifying the
>existing EVM value is evm_inode_updatexattr() called. For example, in
>evm_inode_setxattr() the existing EVM value is verified and then updated in
>evm_inode_post_setxattr(), by calling evm_inode_updatexattr().
>
>In this case, the new function evm_fix_hmac() would call evm_update_evmxattr()
>only after verifying the EVM mode is set to fix.

Thanks for the clarification! I've sent v2 to add the new function
evm_fix_hmac.

-- 
Best regards,
Coiby