| 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
| ||
|
Message-ID: <e74e81465e168a7f43583d5783850cc5fe7ca1c5.camel@linux.ibm.com>
Date: Wed, 24 Sep 2025 23:27:03 -0400
From: Mimi Zohar <zohar@...ux.ibm.com>
To: Coiby Xu <coxu@...hat.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 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.
Powered by blists - more mailing lists