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.
--
Best regards,
Coiby
