Re: [PATCH v7 01/10] crypto: pkcs7: add flag for validated trust on a signed info block

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Exporting `verify_pkcs7_message_sig` as `EXPORT_SYMBOL` creates a non-GPL wrapper around strictly GPL-only PKCS7 APIs.
- [High] The `verified` flags on `x509_certificate` and `pkcs7_signed_info` are not cleared between validations, causing severe state leakage and API breakage on subsequent validations.
- [Medium] The `sinfo->verified` flag is set to `true` for blacklisted signatures, creating a dangerous API hazard.
- [Medium] Concurrency data race on internal flags in `pkcs7_message` when `verify_pkcs7_message_sig` is called.
--

commit 4629d1adf43a0253ba7082b6c379d2bf57a8c8ce
Author: James Bottomley <James.Bottomley@HansenPartnership.com>
crypto: pkcs7: add flag for validated trust on a signed info block

This commit adds a flag to struct pkcs7_message to indicate if any sinfo
fields have passed trust validation. It sets the verified flag during
trust validation and exports verify_pkcs7_message_sig for module use.

> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index e0761436ec7f4..9bda49295bd02 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -380,6 +380,7 @@ int verify_pkcs7_message_sig(const void *data, size_t len,
>  	pr_devel("<==%s() = %d\n", __func__, ret);
>  	return ret;
>  }
> +EXPORT_SYMBOL(verify_pkcs7_message_sig);

Since the underlying subsystem functions like pkcs7_verify and
pkcs7_validate_trust, as well as the alternative wrapper
verify_pkcs7_signature, are all explicitly exported as EXPORT_SYMBOL_GPL,
does exporting verify_pkcs7_message_sig as EXPORT_SYMBOL allow proprietary
modules to bypass the GPL requirement and access internal cryptographic
evaluations?

Should this use EXPORT_SYMBOL_GPL to match the rest of the subsystem?

> diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
> index 9a87c34ed1733..78ebfb6373b61 100644
> --- a/crypto/asymmetric_keys/pkcs7_trust.c
> +++ b/crypto/asymmetric_keys/pkcs7_trust.c

[ ... ]

> @@ -127,6 +127,7 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
>  		for (p = sinfo->signer; p != x509; p = p->signer)
>  			p->verified = true;
>  	}
> +	sinfo->verified = true;

If a consumer validates a message successfully against a keyring, the verified
flags are permanently set to true since they are not cleared between
validations.

If the same object is subsequently verified again, verify_pkcs7_message_sig
first runs is_key_on_revocation_list using the blacklist_keyring. During this
blacklist check, if the message has multiple sinfo blocks signed by the same
key, the first block fails but sets seen to true.

The second block observes seen as true and the stale verified flag as true
from the previous run. Will this falsely evaluate to success, causing
is_key_on_revocation_list to return -EKEYREJECTED and making a previously
validated message fail subsequent verifications?

Additionally, when verify_pkcs7_message_sig checks the blacklist, it calls
pkcs7_validate_trust with the blacklist_keyring. If a signature matches a
revoked key, pkcs7_validate_trust returns success and sets sinfo->verified
to true.

Even though the outer function correctly returns -EKEYREJECTED, does this
leave the parsed pkcs7 object with verified set to true for a revoked key?
Could this confuse consumers inspecting the verified state after a failed
call?

Finally, if multiple threads concurrently call verify_pkcs7_message_sig on
the same pkcs7_message object, will the unprotected modifications to internal
flags like p->seen, x509->verified, and sinfo->verified cause data races?

Could this lack of synchronization lead to random validation failures or
incorrect trust bypasses?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260507191416.2984054-1-bboscaccy@linux.microsoft.com?part=1