[PATCH v4 0/2] ima: reject modsig when detached data cannot be supplied

[PATCH v4 0/2] ima: reject modsig when detached data cannot be supplied

[Jérémy Jean]

ima_collect_modsig() supplies the file contents to the parsed PKCS#7
message as detached data. If the message already contains embedded data,
pkcs7_supply_detached_data() returns -EINVAL, but IMA ignores the error.
ima_modsig_verify() then verifies the embedded data instead of the file
being appraised. This could bypass the crypto check if one supplies data
with an already known valid signature.

Patch 1 fixes the verification path by discarding a modsig if detached
data cannot be supplied.

Patch 2 keeps the exact pkcs7_supply_detached_data() error for a
separate failed-modsig audit record. This part has been split out on
request from Mimi Zohar so that the verification fix can be backported
without this audit change.

---
Changes in v4:
- Keep the call to pkcs7_get_digest() unchanged.
- Adapt the calls to integrity_audit_msg() to ca1e0f3cce8a4.

Changes in v3:
- Split the verification fix from the audit change.

Changes in v2:
- If detached data cannot be supplied, discard the modsig but continue
  hashing and xattr appraisal.
- Keep signatures without exported digests; leave d-modsig empty.
- Audit failure to supply detached data separately, even with O_DIRECT.

v1: https://lore.kernel.org/all/20260822082434.489470-2-Jeremy.Jean@oss.cyber.gouv.fr/
v2: https://lore.kernel.org/all/20260924085516.3111521-2-Jeremy.Jean@oss.cyber.gouv.fr/
v3: https://lore.kernel.org/all/20261021210145.1851396-2-Jeremy.Jean@oss.cyber.gouv.fr/

Jérémy Jean (2):
  ima: discard modsig when detached data cannot be supplied
  ima: audit failure to supply detached data for modsig

 Documentation/security/IMA-templates.rst  |  3 ++-
 security/integrity/ima/ima.h              |  9 +++----
 security/integrity/ima/ima_api.c          | 29 ++++++++++++++++-------
 security/integrity/ima/ima_main.c         |  2 +-
 security/integrity/ima/ima_modsig.c       | 10 ++++++--
 security/integrity/ima/ima_template_lib.c |  4 ++--
 6 files changed, 39 insertions(+), 18 deletions(-)

-- 
2.47.3

[PATCH v4 1/2] ima: discard modsig when detached data cannot be supplied

[Jérémy Jean]

ima_collect_modsig() supplies the file contents to the parsed PKCS#7
message as detached data. If the message already contains embedded data,
pkcs7_supply_detached_data() returns -EINVAL, but IMA ignores the error.
ima_modsig_verify() then verifies the embedded data instead of the file
being appraised.

Make ima_collect_modsig() report whether the file data was supplied,
and discard the modsig if it was not.

Fixes: 15588227e086 ("ima: Collect modsig")
Cc: stable@vger.kernel.org
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 Documentation/security/IMA-templates.rst  |  3 ++-
 security/integrity/ima/ima.h              |  7 ++++---
 security/integrity/ima/ima_api.c          | 10 +++++++---
 security/integrity/ima/ima_main.c         |  2 +-
 security/integrity/ima/ima_modsig.c       | 10 ++++++++--
 security/integrity/ima/ima_template_lib.c |  4 ++--
 6 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 15b4add314fc9..de7a3dfee33db 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -69,7 +69,8 @@ descriptors by adding their identifier to the format string
    algorithm (field format: <hash algo>:digest);
  - 'd-ngv2': same as d-ng, but prefixed with the "ima" or "verity" digest type
    (field format: <digest type>:<hash algo>:digest);
- - 'd-modsig': the digest of the event without the appended modsig;
+ - 'd-modsig': the digest of the event without the appended modsig, empty
+   when no signature digest can be exported;
  - 'n-ng': the name of the event, without size limitations;
  - 'sig': the file signature, based on either the file's/fsverity's digest[1],
    or the EVM portable signature, if 'security.ima' contains a file hash.
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index b502854f28ee7..6280d7ec23326 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -431,7 +431,7 @@ int ima_get_action(const struct mnt_idmap *idmap, struct inode *inode,
 int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
 int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
 			    void *buf, loff_t size, enum hash_algo algo,
-			    struct modsig *modsig);
+			    struct modsig **modsig);
 void ima_store_measurement(struct ima_iint_cache *iint, struct file *file,
 			   const unsigned char *filename,
 			   struct evm_ima_xattr_data *xattr_value,
@@ -559,7 +559,7 @@ static inline void __init init_ima_appraise_lsm(const struct lsm_id *lsmid)
 #ifdef CONFIG_IMA_APPRAISE_MODSIG
 int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
 		    struct modsig **modsig);
-void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size);
+bool ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size);
 int ima_get_modsig_digest(const struct modsig *modsig, enum hash_algo *algo,
 			  const u8 **digest, u32 *digest_size);
 int ima_get_raw_modsig(const struct modsig *modsig, const void **data,
@@ -572,9 +572,10 @@ static inline int ima_read_modsig(enum ima_hooks func, const void *buf,
 	return -EOPNOTSUPP;
 }
 
-static inline void ima_collect_modsig(struct modsig *modsig, const void *buf,
+static inline bool ima_collect_modsig(struct modsig *modsig, const void *buf,
 				      loff_t size)
 {
+	return false;
 }
 
 static inline int ima_get_modsig_digest(const struct modsig *modsig,
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 45701f04ba62f..7859c9d1a27f5 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -235,6 +235,8 @@ static bool ima_get_verity_digest(struct ima_iint_cache *iint,
  *
  * Calculate the file hash, if it doesn't already exist,
  * storing the measurement and i_version in the iint.
+ * If modsig is provided, discard it when detached data cannot be supplied
+ * so ordinary measurement and xattr appraisal can proceed.
  *
  * Must be called with iint->mutex held.
  *
@@ -242,7 +244,7 @@ static bool ima_get_verity_digest(struct ima_iint_cache *iint,
  */
 int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
 			    void *buf, loff_t size, enum hash_algo algo,
-			    struct modsig *modsig)
+			    struct modsig **modsig)
 {
 	const char *audit_cause = "failed";
 	struct inode *inode = file_inode(file);
@@ -262,8 +264,10 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
 	 * the file digest without collecting the modsig in a previous
 	 * measurement rule.
 	 */
-	if (modsig)
-		ima_collect_modsig(modsig, buf, size);
+	if (modsig && *modsig && !ima_collect_modsig(*modsig, buf, size)) {
+		ima_free_modsig(*modsig);
+		*modsig = NULL;
+	}
 
 	if (iint->flags & IMA_COLLECTED)
 		goto out;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index fed80da3ee096..336e17ec4f7a3 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -422,7 +422,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
 
 	hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
 
-	rc = ima_collect_measurement(iint, file, buf, size, hash_algo, modsig);
+	rc = ima_collect_measurement(iint, file, buf, size, hash_algo, &modsig);
 	if (rc != 0 && rc != -EBADF && rc != -EINVAL)
 		goto out_locked;
 
diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
index 632c746fd81e4..e640a9a53508e 100644
--- a/security/integrity/ima/ima_modsig.c
+++ b/security/integrity/ima/ima_modsig.c
@@ -96,8 +96,12 @@ int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
  * Since the modsig is part of the file contents, the hash used in its signature
  * isn't the same one ordinarily calculated by IMA. Therefore PKCS7 code
  * calculates a separate one for signature verification.
+ *
+ * Digest export is best-effort; not all signatures expose a single digest.
+ *
+ * Return: true if the file data was supplied, false otherwise.
  */
-void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size)
+bool ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size)
 {
 	int rc;
 
@@ -109,11 +113,13 @@ void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size)
 		sizeof(struct module_signature);
 	rc = pkcs7_supply_detached_data(modsig->pkcs7_msg, buf, size);
 	if (rc)
-		return;
+		return false;
 
 	/* Ask the PKCS7 code to calculate the file hash. */
 	rc = pkcs7_get_digest(modsig->pkcs7_msg, &modsig->digest,
 			      &modsig->digest_size, &modsig->hash_algo);
+
+	return true;
 }
 
 int ima_modsig_verify(struct key *keyring, const struct modsig *modsig)
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 9bd80ca1159a2..585cd59f00a2e 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -478,8 +478,8 @@ int ima_eventdigest_modsig_init(struct ima_event_data *event_data,
 		if (rc)
 			return rc;
 		else if (hash_algo == HASH_ALGO__LAST || cur_digestsize == 0)
-			/* There was some error collecting the digest. */
-			return -EINVAL;
+			/* Keep the ordinary measurement without an exported digest. */
+			return 0;
 	}
 
 	return ima_eventdigest_init_common(cur_digest, cur_digestsize,
-- 
2.47.3

[PATCH v4 2/2] ima: audit failure to supply detached data for modsig

[Jérémy Jean]

ima_collect_measurement() discards a modsig when detached data cannot be
supplied, but the reason is lost.

Return the pkcs7_supply_detached_data() error from ima_collect_modsig()
and emit a separate failed-modsig audit record. Keep ordinary collection
errors on their existing audit path so both causes are reported when they
happen together.

Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 security/integrity/ima/ima.h        |  8 ++++----
 security/integrity/ima/ima_api.c    | 25 +++++++++++++++++--------
 security/integrity/ima/ima_modsig.c |  8 ++++----
 3 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 6280d7ec23326..a3670a479d095 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -559,7 +559,7 @@ static inline void __init init_ima_appraise_lsm(const struct lsm_id *lsmid)
 #ifdef CONFIG_IMA_APPRAISE_MODSIG
 int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
 		    struct modsig **modsig);
-bool ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size);
+int ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size);
 int ima_get_modsig_digest(const struct modsig *modsig, enum hash_algo *algo,
 			  const u8 **digest, u32 *digest_size);
 int ima_get_raw_modsig(const struct modsig *modsig, const void **data,
@@ -572,10 +572,10 @@ static inline int ima_read_modsig(enum ima_hooks func, const void *buf,
 	return -EOPNOTSUPP;
 }
 
-static inline bool ima_collect_modsig(struct modsig *modsig, const void *buf,
-				      loff_t size)
+static inline int ima_collect_modsig(struct modsig *modsig, const void *buf,
+				     loff_t size)
 {
-	return false;
+	return -EOPNOTSUPP;
 }
 
 static inline int ima_get_modsig_digest(const struct modsig *modsig,
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 7859c9d1a27f5..5f6178948eb3e 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -254,7 +254,7 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
 						struct ima_digest_data, hdr);
 	struct name_snapshot filename;
 	struct kstat stat;
-	int result = 0;
+	int result = 0, modsig_result = 0;
 	int length;
 	void *tmpbuf;
 	u64 i_version = 0;
@@ -264,9 +264,12 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
 	 * the file digest without collecting the modsig in a previous
 	 * measurement rule.
 	 */
-	if (modsig && *modsig && !ima_collect_modsig(*modsig, buf, size)) {
-		ima_free_modsig(*modsig);
-		*modsig = NULL;
+	if (modsig && *modsig) {
+		modsig_result = ima_collect_modsig(*modsig, buf, size);
+		if (modsig_result) {
+			ima_free_modsig(*modsig);
+			*modsig = NULL;
+		}
 	}
 
 	if (iint->flags & IMA_COLLECTED)
@@ -326,15 +329,21 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
 	if (!result)
 		iint->flags |= IMA_COLLECTED;
 out:
-	if (result) {
+	if (result || modsig_result) {
 		if (file->f_flags & O_DIRECT)
 			audit_cause = "failed(directio)";
 
 		take_dentry_name_snapshot(&filename, file->f_path.dentry);
 
-		integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
-				    filename.name.name, "collect_data",
-				    audit_cause, result, 0, result);
+		if (modsig_result)
+			integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
+					    filename.name.name, "collect_data",
+					    "failed-modsig", modsig_result, 0,
+					    modsig_result);
+		if (result)
+			integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
+					    filename.name.name, "collect_data",
+					    audit_cause, result, 0, result);
 
 		release_dentry_name_snapshot(&filename);
 	}
diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
index e640a9a53508e..c5bd4770298a1 100644
--- a/security/integrity/ima/ima_modsig.c
+++ b/security/integrity/ima/ima_modsig.c
@@ -99,9 +99,9 @@ int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
  *
  * Digest export is best-effort; not all signatures expose a single digest.
  *
- * Return: true if the file data was supplied, false otherwise.
+ * Return: 0 if the file data was supplied, error code otherwise.
  */
-bool ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size)
+int ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size)
 {
 	int rc;
 
@@ -113,13 +113,13 @@ bool ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size)
 		sizeof(struct module_signature);
 	rc = pkcs7_supply_detached_data(modsig->pkcs7_msg, buf, size);
 	if (rc)
-		return false;
+		return rc;
 
 	/* Ask the PKCS7 code to calculate the file hash. */
 	rc = pkcs7_get_digest(modsig->pkcs7_msg, &modsig->digest,
 			      &modsig->digest_size, &modsig->hash_algo);
 
-	return true;
+	return 0;
 }
 
 int ima_modsig_verify(struct key *keyring, const struct modsig *modsig)
-- 
2.47.3

Re: [PATCH v4 2/2] ima: audit failure to supply detached data for modsig

[sashiko-bot]

> ima_collect_measurement() discards a modsig when detached data cannot be
> supplied, but the reason is lost.
> 
> Return the pkcs7_supply_detached_data() error from ima_collect_modsig()
> and emit a separate failed-modsig audit record. Keep ordinary collection
> errors on their existing audit path so both causes are reported when they
> happen together.
> 
> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024093918.2929374-1-Jeremy.Jean@oss.cyber.gouv.fr?part=2

Re: [PATCH v4 1/2] ima: discard modsig when detached data cannot be supplied

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ima: ignored pkcs7_get_digest() failure in ima_collect_modsig()
- [Medium] ima: unintended measurement in process_measurement() when modsig is discarded
--

commit 6ba08dabf06dd0a8dc8646bcd331980059e5a114
Author: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>

ima: discard modsig when detached data cannot be supplied

This commit updates ima_collect_modsig() to return a boolean indicating
whether detached file data was supplied to the PKCS#7 message, and
discards the modsig in ima_collect_measurement() if supplying the
detached data fails.

> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index 45701f04ba62f..7859c9d1a27f5 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -235,6 +235,8 @@ static bool ima_get_verity_digest(struct ima_iint_cache *iint,
>   *
>   * Calculate the file hash, if it doesn't already exist,
>   * storing the measurement and i_version in the iint.
> + * If modsig is provided, discard it when detached data cannot be supplied
> + * so ordinary measurement and xattr appraisal can proceed.
>   *
>   * Must be called with iint->mutex held.
>   *
> @@ -242,7 +244,7 @@ static bool ima_get_verity_digest(struct ima_iint_cache *iint,
>   */
>  int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
>  			    void *buf, loff_t size, enum hash_algo algo,
> -			    struct modsig *modsig)
> +			    struct modsig **modsig)
>  {
>  	const char *audit_cause = "failed";
>  	struct inode *inode = file_inode(file);
> @@ -262,8 +264,10 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
>  	 * the file digest without collecting the modsig in a previous
>  	 * measurement rule.
>  	 */
> -	if (modsig)
> -		ima_collect_modsig(modsig, buf, size);
> +	if (modsig && *modsig && !ima_collect_modsig(*modsig, buf, size)) {
> +		ima_free_modsig(*modsig);
> +		*modsig = NULL;
> +	}
>  
>  	if (iint->flags & IMA_COLLECTED)
>  		goto out;
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index fed80da3ee096..336e17ec4f7a3 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -422,7 +422,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
>  
>  	hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
>  
> -	rc = ima_collect_measurement(iint, file, buf, size, hash_algo, modsig);
> +	rc = ima_collect_measurement(iint, file, buf, size, hash_algo, &modsig);

[Severity: Medium]
Can discarding modsig in ima_collect_measurement() leave IMA_MEASURE set
in action when the measurement was only requested to record the modsig?

Earlier in process_measurement(), if IMA_MODSIG_ALLOWED is set and
ima_read_modsig() succeeds on an already-measured file, IMA_MEASURE is
added to action:

	if (iint->flags & IMA_MODSIG_ALLOWED) {
		rc = ima_read_modsig(func, buf, size, &modsig);

		if (!rc && ima_template_has_modsig(template_desc) &&
		    iint->flags & IMA_MEASURED)
			action |= IMA_MEASURE;
	}

When ima_collect_measurement() subsequently discards modsig and sets it
to NULL because pkcs7_supply_detached_data() fails, action still has
IMA_MEASURE set.

If the earlier measurement that set IMA_MEASURED used a non-default PCR
and the current hook only matched an appraise rule (leaving pcr at
CONFIG_IMA_MEASURE_PCR_IDX), ima_store_measurement() will see modsig ==
NULL, find that the default PCR bit is not set in iint->measured_pcrs:

	if (iint->measured_pcrs & (0x1 << pcr) && !modsig)
		return;

and record an extra measurement with no modsig into the default PCR.
Should IMA_MEASURE only be added to action after modsig collection has
succeeded?

[ ... ]

> diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
> index 632c746fd81e4..e640a9a53508e 100644
> --- a/security/integrity/ima/ima_modsig.c
> +++ b/security/integrity/ima/ima_modsig.c
> @@ -96,8 +96,12 @@ int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
>   * Since the modsig is part of the file contents, the hash used in its signature
>   * isn't the same one ordinarily calculated by IMA. Therefore PKCS7 code
>   * calculates a separate one for signature verification.
> + *
> + * Digest export is best-effort; not all signatures expose a single digest.
> + *
> + * Return: true if the file data was supplied, false otherwise.
>   */
> -void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size)
> +bool ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size)
>  {
>  	int rc;
>  
> @@ -109,11 +113,13 @@ void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size)
>  		sizeof(struct module_signature);
>  	rc = pkcs7_supply_detached_data(modsig->pkcs7_msg, buf, size);
>  	if (rc)
> -		return;
> +		return false;
>  
>  	/* Ask the PKCS7 code to calculate the file hash. */
>  	rc = pkcs7_get_digest(modsig->pkcs7_msg, &modsig->digest,
>  			      &modsig->digest_size, &modsig->hash_algo);
> +
> +	return true;
>  }
>  
>  int ima_modsig_verify(struct key *keyring, const struct modsig *modsig)
> diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> index 9bd80ca1159a2..585cd59f00a2e 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -478,8 +478,8 @@ int ima_eventdigest_modsig_init(struct ima_event_data *event_data,
>  		if (rc)
>  			return rc;
>  		else if (hash_algo == HASH_ALGO__LAST || cur_digestsize == 0)
> -			/* There was some error collecting the digest. */
> -			return -EINVAL;
> +			/* Keep the ordinary measurement without an exported digest. */
> +			return 0;
>  	}

[Severity: High]
What happens in ima_check_blacklist() and ima_store_measurement() when
pkcs7_get_digest() fails in ima_collect_modsig() (note that the return
value of pkcs7_get_digest() also remains unchecked in the follow-up commit
"ima: audit failure to supply detached data for modsig")?

When pkcs7_get_digest() fails (for example, returning -EBADMSG when
sinfo->next != NULL, or -EINVAL when !sinfo->sig->m_free for algorithms
such as ML-DSA) or leaves modsig->hash_algo as HASH_ALGO__LAST,
ima_collect_modsig() still returns true. That leaves modsig non-NULL in
process_measurement() with modsig->digest == NULL, modsig->digest_size ==
0, and modsig->hash_algo == HASH_ALGO__LAST.

In security/integrity/ima/ima_appraise.c:ima_check_blacklist(), a
non-NULL modsig takes the first branch and skips checking iint->ima_hash:

	if (iint->flags & IMA_MODSIG_ALLOWED && modsig) {
		ima_get_modsig_digest(modsig, &hash_algo, &digest, &digestsize);

		rc = is_binary_blacklisted(digest, digestsize);
	} else if (iint->flags & IMA_DIGSIG_REQUIRED && iint->ima_hash)
		rc = is_binary_blacklisted(iint->ima_hash->digest, iint->ima_hash->length);

Because digest is NULL and digestsize is 0, is_binary_blacklisted(NULL, 0)
returns 0. Since pkcs7_get_digest() rejects messages with multiple
SignerInfos (sinfo->next != NULL) while ima_modsig_verify() ->
verify_pkcs7_message_sig() succeeds as long as at least one SignerInfo is
valid and trusted (and similarly for ML-DSA signatures where
!sinfo->sig->m_free), wouldn't a blacklisted binary bypass
ima_check_blacklist()?

In addition, because ima_eventdigest_modsig_init() now returns 0 instead
of -EINVAL when hash_algo == HASH_ALGO__LAST || cur_digestsize == 0 while
event_data->modsig is still non-NULL:

1. For templates like ima-modsig (d-ng|n-ng|sig|d-modsig|modsig),
   ima_eventdigest_modsig_init() leaves the d-modsig field empty while
   ima_eventmodsig_init() still populates the modsig field with the raw
   PKCS#7 signature, breaking the requirement in check_template_modsig()
   that modsig be paired with d-modsig.

2. If the file was already measured (iint->flags & IMA_MEASURED),
   process_measurement() and ima_store_measurement() will record a second
   measurement entry and extend the PCR again even though no modsig digest
   could be exported.

Should ima_collect_modsig() fail and discard the modsig when
pkcs7_get_digest() fails or leaves modsig->hash_algo == HASH_ALGO__LAST?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024093918.2929374-1-Jeremy.Jean@oss.cyber.gouv.fr?part=1

Re: [PATCH v4 1/2] ima: discard modsig when detached data cannot be supplied

[Jérémy Jean]

On 2026-10-24 11:39, Jérémy Jean wrote:
> ima_collect_modsig() supplies the file contents to the parsed PKCS#7
> message as detached data. If the message already contains embedded 
> data,
> pkcs7_supply_detached_data() returns -EINVAL, but IMA ignores the 
> error.
> ima_modsig_verify() then verifies the embedded data instead of the file
> being appraised.
> 
> Make ima_collect_modsig() report whether the file data was supplied,
> and discard the modsig if it was not.
> 
> Fixes: 15588227e086 ("ima: Collect modsig")

The public instance of Sashiko raised some relevant feedbacks for 1/2
that my local Sashiko run did not see.
I will address them in a new version.

Regards,
Jérémy