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)
