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

[Mimi Zohar <zohar@linux.ibm.com>]

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.
> > > 
> > > Signed-off-by: Coiby Xu <coxu@redhat.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.
> > 
> > > ---
> > >  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, ....);    
                        }                                                       
                }                

> > > 
> > >  		/*
> > > 
> > > base-commit: b320789d6883cc00ac78ce83bccbfe7ed58afcf0
> > 
>