* [PATCH] ima: setting security.ima to fix security.evm for a file with IMA signature @ 2025-09-09 4:19 Coiby Xu 2025-09-09 15:31 ` Mimi Zohar 0 siblings, 1 reply; 9+ messages in thread From: Coiby Xu @ 2025-09-09 4:19 UTC (permalink / raw) To: linux-integrity Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list 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> --- 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; + } } /* base-commit: b320789d6883cc00ac78ce83bccbfe7ed58afcf0 -- 2.51.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] ima: setting security.ima to fix security.evm for a file with IMA signature 2025-09-09 4:19 [PATCH] ima: setting security.ima to fix security.evm for a file with IMA signature Coiby Xu @ 2025-09-09 15:31 ` Mimi Zohar 2025-09-10 1:20 ` Coiby Xu 0 siblings, 1 reply; 9+ messages in thread From: Mimi Zohar @ 2025-09-09 15:31 UTC (permalink / raw) To: Coiby Xu, linux-integrity Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list 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? thanks, Mimi > --- > 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; > + } > } > > /* > > base-commit: b320789d6883cc00ac78ce83bccbfe7ed58afcf0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ima: setting security.ima to fix security.evm for a file with IMA signature 2025-09-09 15:31 ` Mimi Zohar @ 2025-09-10 1:20 ` Coiby Xu 2025-09-10 11:15 ` Mimi Zohar 2025-09-15 21:05 ` Mimi Zohar 0 siblings, 2 replies; 9+ messages in thread From: Coiby Xu @ 2025-09-10 1:20 UTC (permalink / raw) To: Mimi Zohar Cc: linux-integrity, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list 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. > >thanks, > >Mimi > >> --- >> 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; >> + } >> } >> >> /* >> >> base-commit: b320789d6883cc00ac78ce83bccbfe7ed58afcf0 > -- Best regards, Coiby ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ima: setting security.ima to fix security.evm for a file with IMA signature 2025-09-10 1:20 ` Coiby Xu @ 2025-09-10 11:15 ` Mimi Zohar 2025-09-30 2:31 ` Coiby Xu 2025-09-15 21:05 ` Mimi Zohar 1 sibling, 1 reply; 9+ messages in thread From: Mimi Zohar @ 2025-09-10 11:15 UTC (permalink / raw) To: Coiby Xu Cc: linux-integrity, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list 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. Right, unlike file hashes, new or the same file signature can be written, but cannot be "fixed" in the literal sense, as that would require calculating the file hash and signing it with a private key. This patch triggers "fixing" the EVM HMAC by re-writing the existing IMA file signature. I assume the same result could be achieved by simply re-installing the file signature. In both cases, the EVM HMAC key needs to exist and be loaded. > > > > > > --- > > > 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; > > > + } > > > } > > > > > > /* > > > > > > base-commit: b320789d6883cc00ac78ce83bccbfe7ed58afcf0 > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ima: setting security.ima to fix security.evm for a file with IMA signature 2025-09-10 11:15 ` Mimi Zohar @ 2025-09-30 2:31 ` Coiby Xu 0 siblings, 0 replies; 9+ messages in thread From: Coiby Xu @ 2025-09-30 2:31 UTC (permalink / raw) To: Mimi Zohar Cc: linux-integrity, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list On Wed, Sep 10, 2025 at 07:15:19AM -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. >> > > >> > > 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. > >Right, unlike file hashes, new or the same file signature can be written, but >cannot be "fixed" in the literal sense, as that would require calculating the >file hash and signing it with a private key. Thanks for the confirmation! I also added some code comments to explain the IMA iint cache atomic_flags including IMA_DIGSIG in v2. > >This patch triggers "fixing" the EVM HMAC by re-writing the existing IMA file >signature. I assume the same result could be achieved by simply re-installing >the file signature. In both cases, the EVM HMAC key needs to exist and be >loaded. -- Best regards, Coiby ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ima: setting security.ima to fix security.evm for a file with IMA signature 2025-09-10 1:20 ` Coiby Xu 2025-09-10 11:15 ` Mimi Zohar @ 2025-09-15 21:05 ` Mimi Zohar 2025-09-24 8:03 ` Coiby Xu 1 sibling, 1 reply; 9+ messages in thread From: Mimi Zohar @ 2025-09-15 21:05 UTC (permalink / raw) To: Coiby Xu Cc: linux-integrity, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list 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 > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ima: setting security.ima to fix security.evm for a file with IMA signature 2025-09-15 21:05 ` Mimi Zohar @ 2025-09-24 8:03 ` Coiby Xu 2025-09-25 3:27 ` Mimi Zohar 0 siblings, 1 reply; 9+ messages in thread From: Coiby Xu @ 2025-09-24 8:03 UTC (permalink / raw) To: Mimi Zohar Cc: linux-integrity, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ima: setting security.ima to fix security.evm for a file with IMA signature 2025-09-24 8:03 ` Coiby Xu @ 2025-09-25 3:27 ` Mimi Zohar 2025-09-30 2:28 ` Coiby Xu 0 siblings, 1 reply; 9+ messages in thread From: Mimi Zohar @ 2025-09-25 3:27 UTC (permalink / raw) To: Coiby Xu Cc: linux-integrity, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ima: setting security.ima to fix security.evm for a file with IMA signature 2025-09-25 3:27 ` Mimi Zohar @ 2025-09-30 2:28 ` Coiby Xu 0 siblings, 0 replies; 9+ messages in thread From: Coiby Xu @ 2025-09-30 2:28 UTC (permalink / raw) To: Mimi Zohar Cc: linux-integrity, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn, open list:SECURITY SUBSYSTEM, open list 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-09-30 2:35 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-09 4:19 [PATCH] ima: setting security.ima to fix security.evm for a file with IMA signature Coiby Xu 2025-09-09 15:31 ` Mimi Zohar 2025-09-10 1:20 ` Coiby Xu 2025-09-10 11:15 ` Mimi Zohar 2025-09-30 2:31 ` Coiby Xu 2025-09-15 21:05 ` Mimi Zohar 2025-09-24 8:03 ` Coiby Xu 2025-09-25 3:27 ` Mimi Zohar 2025-09-30 2:28 ` Coiby Xu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox