Re: [PATCH] ima: reject modsig if detached data cannot be supplied

On Sat, 2026-08-22 at 08:24 +0000, Jérémy Jean wrote:
> ima_collect_modsig() binds the file being appraised to the parsed PKCS#7
> message with pkcs7_supply_detached_data().  If the message already contains
> embedded data, the helper returns -EINVAL.  However, ima_collect_modsig()
> silently returns because its return type is void.
> 
> ima_modsig_verify() subsequently calls verify_pkcs7_message_sig() with a
> NULL data pointer.  That verifies the embedded PKCS#7 data instead of the
> file being appraised.  An attacker can therefore take a valid
> detached signature, insert the original signed bytes as embedded content
> without the private key, and append the resulting message to an unrelated
> file.  IMA accepts the unrelated file.
> 
> This bypasses the module authenticity boundary when an IMA modsig appraisal
> rule is enforced while CONFIG_MODULE_SIG is disabled.
> 
> Return detached-data binding errors and stop measurement collection when
> binding fails.  Keep digest extraction optional because algorithms such as
> ML-DSA can verify the message directly.  Appraisal then rejects the file
> rather than reaching verification with embedded content active.
> 
> Fixes: 15588227e086 ("ima: Collect modsig")
> Signed-off-by: Jérémy Jean <[email protected]>
> Assisted-by: Codex:gpt-5
> ---
>  security/integrity/ima/ima.h        |  7 ++++---
>  security/integrity/ima/ima_api.c    |  7 +++++--
>  security/integrity/ima/ima_modsig.c |  9 +++++++--
>  3 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 10214f73ca1e..ac1d593a42c1 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);
> -void 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,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,
> -				      loff_t size)
> +static inline int ima_collect_modsig(struct modsig *modsig, const void *buf,
> +				     loff_t size)
>  {
> +	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 122d127e108d..e2b167f24778 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -262,8 +262,11 @@ 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) {
> +		result = ima_collect_modsig(modsig, buf, size);
> +		if (result)
> +			goto out;
> +	

The normal processing of getting the xattr needs to continue even if the
appended signatures doesn't exist.  Exiting here is too early. Consider saving
the result as a different variable and returning an error later, if needed.

Mimi

>  
>  	if (iint->flags & IMA_COLLECTED)
>  		goto out;
> diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
> index 632c746fd81e..57dc8dc43b55 100644
> --- a/security/integrity/ima/ima_modsig.c
> +++ b/security/integrity/ima/ima_modsig.c
> @@ -96,8 +96,10 @@ 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.
> + *
> + * Return: 0 if the file data was supplied, error code otherwise.
>   */
> -void 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;
>  
> @@ -109,11 +111,14 @@ 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 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);
> +
> +	/* Some signature algorithms operate on the message without a digest. */
> +	return 0;
>  }
>  
>  int ima_modsig_verify(struct key *keyring, const struct modsig *modsig)