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.
