linux-security-module.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/2] evm: fix security.evm for a file with IMA signature
@ 2025-09-30  2:26 Coiby Xu
  2025-09-30  2:26 ` [PATCH v2 2/2] ima: Add code comments to explain IMA iint cache atomic_flags Coiby Xu
  2025-12-01  3:15 ` [PATCH v2 1/2] evm: fix security.evm for a file with IMA signature Coiby Xu
  0 siblings, 2 replies; 4+ messages in thread
From: Coiby Xu @ 2025-09-30  2:26 UTC (permalink / raw)
  To: linux-integrity, Mimi Zohar
  Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
	James Morris, Serge E. Hallyn, open list,
	open list:SECURITY SUBSYSTEM

When both IMA and EVM fix modes are enabled, accessing a file with IMA
signature but missing EVM HMAC won't cause security.evm to be fixed.

Add a function evm_fix_hmac which will be explicitly called to fix EVM
HMAC for this case.

Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 include/linux/evm.h                   |  8 ++++++++
 security/integrity/evm/evm_main.c     | 28 +++++++++++++++++++++++++++
 security/integrity/ima/ima_appraise.c |  5 +++++
 3 files changed, 41 insertions(+)

diff --git a/include/linux/evm.h b/include/linux/evm.h
index ddece4a6b25d..913f4573b203 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -18,6 +18,8 @@ extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
 					     const char *xattr_name,
 					     void *xattr_value,
 					     size_t xattr_value_len);
+int evm_fix_hmac(struct dentry *dentry, const char *xattr_name,
+		 const char *xattr_value, size_t xattr_value_len);
 int evm_inode_init_security(struct inode *inode, struct inode *dir,
 			    const struct qstr *qstr, struct xattr *xattrs,
 			    int *xattr_count);
@@ -51,6 +53,12 @@ static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
 {
 	return INTEGRITY_UNKNOWN;
 }
+
+static inline int evm_fix_hmac(struct dentry *dentry, const char *xattr_name,
+			       const char *xattr_value, size_t xattr_value_len)
+{
+	return -EOPNOTSUPP;
+}
 #endif
 
 static inline int evm_inode_init_security(struct inode *inode, struct inode *dir,
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 0add782e73ba..1b3edc6d26e9 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -787,6 +787,34 @@ bool evm_revalidate_status(const char *xattr_name)
 	return true;
 }
 
+/**
+ * evm_fix_hmac - Calculate the HMAC and add it to security.evm for fix mode
+ * @dentry: pointer to the affected dentry which doesn't yet have security.evm
+ *          xattr
+ * @xattr_name: pointer to the affected extended attribute name
+ * @xattr_value: pointer to the new extended attribute value
+ * @xattr_value_len: pointer to the new extended attribute value length
+ *
+ * Expects to be called with i_mutex locked.
+ *
+ * Return: 0 on success, -EPERM/-ENOMEM/-EOPNOTSUPP on failure
+ */
+int evm_fix_hmac(struct dentry *dentry, const char *xattr_name,
+		 const char *xattr_value, size_t xattr_value_len)
+
+{
+	if (!evm_fixmode || !evm_revalidate_status((xattr_name)))
+		return -EPERM;
+
+	if (!(evm_initialized & EVM_INIT_HMAC))
+		return -EPERM;
+
+	if (is_unsupported_hmac_fs(dentry))
+		return -EOPNOTSUPP;
+
+	return evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len);
+}
+
 /**
  * evm_inode_post_setxattr - update 'security.evm' to reflect the changes
  * @dentry: pointer to the affected dentry
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index f435eff4667f..f48ef5ec185e 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -601,6 +601,11 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
 		     xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
 			if (!ima_fix_xattr(dentry, iint))
 				status = INTEGRITY_PASS;
+		} else if (status == INTEGRITY_NOLABEL) {
+			if (!evm_fix_hmac(dentry, XATTR_NAME_IMA,
+					  (const char *)xattr_value,
+					  xattr_len))
+				status = INTEGRITY_PASS;
 		}
 
 		/*

base-commit: e129e479f2e444eaccd822717d418119d39d3d5c
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v2 2/2] ima: Add code comments to explain IMA iint cache atomic_flags
  2025-09-30  2:26 [PATCH v2 1/2] evm: fix security.evm for a file with IMA signature Coiby Xu
@ 2025-09-30  2:26 ` Coiby Xu
  2025-12-01  3:15 ` [PATCH v2 1/2] evm: fix security.evm for a file with IMA signature Coiby Xu
  1 sibling, 0 replies; 4+ messages in thread
From: Coiby Xu @ 2025-09-30  2:26 UTC (permalink / raw)
  To: linux-integrity, Mimi Zohar
  Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
	James Morris, Serge E. Hallyn, open list:SECURITY SUBSYSTEM,
	open list

Explain these atomic flags to improve code readability. For example, the
flag IMA_DIGSIG is to indicate we mustn't update a file's security.ima
on close because the file already has IMA signature. The code comments
for the first three flags come from commit 0d73a55208e9 ("ima:
re-introduce own integrity cache lock") with a minor tweak.

Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 security/integrity/ima/ima.h | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index e3d71d8d56e3..2130d3764f6a 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -176,7 +176,32 @@ struct ima_kexec_hdr {
 				 IMA_BPRM_APPRAISED | IMA_READ_APPRAISED | \
 				 IMA_CREDS_APPRAISED)
 
-/* IMA iint cache atomic_flags */
+/*
+ * IMA iint cache atomic_flags
+ *
+ * IMA_CHANGE_ATTR - indicates that chATTR() was called (chmod, chown, chgrp)
+ * and file attributes have changed. On file open, it causes IMA to clear
+ * iint->flags to re-evaluate policy and perform IMA functions again.
+ *
+ * IMA_CHANGE_XATTR - indicates that setxattr or removexattr was called and
+ * extended attributes have changed. On file open, it causes IMA to clear
+ * iint->flags IMA_DONE_MASK to re-appraise.
+ *
+ * IMA_UPDATE_XATTR - indicates that security.ima needs to be updated. It is
+ * cleared if file policy changes and no update is needed.
+ *
+ * IMA_DIGSIG - indicates that file security.ima has signature and file
+ * security.ima must not update on file close.
+ *
+ * IMA_MAY_EMIT_TOMTOU - indicates to add Time-of-Measure-Time-of-Use (ToMToU)
+ * integrity violation (a file, that is already opened for read, is opened for
+ * write) to measurement list and to also emit audit message.
+ *
+ * IMA_EMITTED_OPENWRITERS - indicates to add open-writers integrity violations
+ * integrity violation (a file, that is already opened for write, is opened for
+ * read) to measurement list and to also emit audit message.
+ *
+ */
 #define IMA_CHANGE_XATTR	0
 #define IMA_UPDATE_XATTR	1
 #define IMA_CHANGE_ATTR		2
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/2] evm: fix security.evm for a file with IMA signature
  2025-09-30  2:26 [PATCH v2 1/2] evm: fix security.evm for a file with IMA signature Coiby Xu
  2025-09-30  2:26 ` [PATCH v2 2/2] ima: Add code comments to explain IMA iint cache atomic_flags Coiby Xu
@ 2025-12-01  3:15 ` Coiby Xu
  2025-12-02 17:10   ` Mimi Zohar
  1 sibling, 1 reply; 4+ messages in thread
From: Coiby Xu @ 2025-12-01  3:15 UTC (permalink / raw)
  To: linux-integrity, Mimi Zohar
  Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
	James Morris, Serge E. Hallyn, open list,
	open list:SECURITY SUBSYSTEM


On Tue, Sep 30, 2025 at 10:26:56AM +0800, Coiby Xu wrote:
>When both IMA and EVM fix modes are enabled, accessing a file with IMA
>signature but missing EVM HMAC won't cause security.evm to be fixed.
>
>Add a function evm_fix_hmac which will be explicitly called to fix EVM
>HMAC for this case.
>
>Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
>Signed-off-by: Coiby Xu <coxu@redhat.com>
>---
> include/linux/evm.h                   |  8 ++++++++
> security/integrity/evm/evm_main.c     | 28 +++++++++++++++++++++++++++
> security/integrity/ima/ima_appraise.c |  5 +++++
> 3 files changed, 41 insertions(+)
>
>diff --git a/include/linux/evm.h b/include/linux/evm.h
>index ddece4a6b25d..913f4573b203 100644
>--- a/include/linux/evm.h
>+++ b/include/linux/evm.h
>@@ -18,6 +18,8 @@ extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
> 					     const char *xattr_name,
> 					     void *xattr_value,
> 					     size_t xattr_value_len);
>+int evm_fix_hmac(struct dentry *dentry, const char *xattr_name,
>+		 const char *xattr_value, size_t xattr_value_len);
> int evm_inode_init_security(struct inode *inode, struct inode *dir,
> 			    const struct qstr *qstr, struct xattr *xattrs,
> 			    int *xattr_count);
>@@ -51,6 +53,12 @@ static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
> {
> 	return INTEGRITY_UNKNOWN;
> }
>+
>+static inline int evm_fix_hmac(struct dentry *dentry, const char *xattr_name,
>+			       const char *xattr_value, size_t xattr_value_len)
>+{
>+	return -EOPNOTSUPP;
>+}
> #endif
>
> static inline int evm_inode_init_security(struct inode *inode, struct inode *dir,
>diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
>index 0add782e73ba..1b3edc6d26e9 100644
>--- a/security/integrity/evm/evm_main.c
>+++ b/security/integrity/evm/evm_main.c
>@@ -787,6 +787,34 @@ bool evm_revalidate_status(const char *xattr_name)
> 	return true;
> }
>
>+/**
>+ * evm_fix_hmac - Calculate the HMAC and add it to security.evm for fix mode
>+ * @dentry: pointer to the affected dentry which doesn't yet have security.evm
>+ *          xattr
>+ * @xattr_name: pointer to the affected extended attribute name
>+ * @xattr_value: pointer to the new extended attribute value
>+ * @xattr_value_len: pointer to the new extended attribute value length
>+ *
>+ * Expects to be called with i_mutex locked.
>+ *
>+ * Return: 0 on success, -EPERM/-ENOMEM/-EOPNOTSUPP on failure
>+ */
>+int evm_fix_hmac(struct dentry *dentry, const char *xattr_name,
>+		 const char *xattr_value, size_t xattr_value_len)
>+
>+{
>+	if (!evm_fixmode || !evm_revalidate_status((xattr_name)))
>+		return -EPERM;
>+
>+	if (!(evm_initialized & EVM_INIT_HMAC))
>+		return -EPERM;
>+
>+	if (is_unsupported_hmac_fs(dentry))
>+		return -EOPNOTSUPP;
>+
>+	return evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len);
>+}
>+
> /**
>  * evm_inode_post_setxattr - update 'security.evm' to reflect the changes
>  * @dentry: pointer to the affected dentry
>diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
>index f435eff4667f..f48ef5ec185e 100644
>--- a/security/integrity/ima/ima_appraise.c
>+++ b/security/integrity/ima/ima_appraise.c
>@@ -601,6 +601,11 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
> 		     xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
> 			if (!ima_fix_xattr(dentry, iint))
> 				status = INTEGRITY_PASS;
>+		} else if (status == INTEGRITY_NOLABEL) {
>+			if (!evm_fix_hmac(dentry, XATTR_NAME_IMA,
>+					  (const char *)xattr_value,
>+					  xattr_len))
>+				status = INTEGRITY_PASS;
> 		}
>
> 		/*
>
>base-commit: e129e479f2e444eaccd822717d418119d39d3d5c
>-- 
>2.51.0
>

Hi Mimi,

I think this patch set just fell off the radar. Can you take a look at
it when time permits? Thanks! Btw, the patch set is still applicable to
current next-integrity tree Linus and main tree.


-- 
Best regards,
Coiby


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/2] evm: fix security.evm for a file with IMA signature
  2025-12-01  3:15 ` [PATCH v2 1/2] evm: fix security.evm for a file with IMA signature Coiby Xu
@ 2025-12-02 17:10   ` Mimi Zohar
  0 siblings, 0 replies; 4+ messages in thread
From: Mimi Zohar @ 2025-12-02 17:10 UTC (permalink / raw)
  To: Coiby Xu, linux-integrity
  Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
	James Morris, Serge E. Hallyn, open list,
	open list:SECURITY SUBSYSTEM

On Mon, 2025-12-01 at 11:15 +0800, Coiby Xu wrote:
> Hi Mimi,
> 
> I think this patch set just fell off the radar. Can you take a look at
> it when time permits? Thanks! Btw, the patch set is still applicable to
> current next-integrity tree Linus and main tree.

Hi Coiby.  I haven't forgotten about this patch, but would prefer upstreaming it
together with another EVM change.  I'm really sorry about the delay.

-- 
thanks,

Mimi

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-12-02 17:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-30  2:26 [PATCH v2 1/2] evm: fix security.evm for a file with IMA signature Coiby Xu
2025-09-30  2:26 ` [PATCH v2 2/2] ima: Add code comments to explain IMA iint cache atomic_flags Coiby Xu
2025-12-01  3:15 ` [PATCH v2 1/2] evm: fix security.evm for a file with IMA signature Coiby Xu
2025-12-02 17:10   ` Mimi Zohar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).