Re: [v6 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 3 potential issue(s) to consider:
- [High] State persistence across validations causes false blacklist rejections and API breakage if a pkcs7_message is evaluated multiple times.
- [Medium] verify_pkcs7_message_sig is exported using EXPORT_SYMBOL, creating a licensing mismatch with its prerequisite GPL functions.
- [Low] The commit message references a nonexistent function.
--

commit f9218a5ea80d770fbf301c4067d4e4b1febdf4ab
Author: James Bottomley <James.Bottomley@HansenPartnership.com>

crypto: pkcs7: add flag for validated trust on a signed info block

This commit adds a verified flag to the pkcs7_signed_info struct to track
if a signer has passed trust validation. It also exports
verify_pkcs7_message_sig so consumers can retrieve trusted pkcs7 objects.

> Allow consumers of struct pkcs7_message to tell if any of the sinfo
> fields has passed a trust validation.  Note that this does not happen
> in parsing, pkcs7_validate_trust() must be explicitly called or called
> via validate_pkcs7_trust().  Since the way to get this trusted pkcs7

This isn't a bug, but does validate_pkcs7_trust() exist in the tree?
It looks like this might be a typo for verify_pkcs7_signature() or
verify_pkcs7_message_sig().

> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index e0761436ec7f..9bda49295bd0 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);

Could this cause an API licensing mismatch?

To legitimately use verify_pkcs7_message_sig(), a consumer must parse the
raw payload into a pkcs7_message and eventually free it using
pkcs7_parse_message() and pkcs7_free_message().

Since those prerequisite functions are restricted to EXPORT_SYMBOL_GPL,
non-GPL modules wouldn't be able to legally acquire or manage the required
structures to use this new export. Should this be EXPORT_SYMBOL_GPL instead?

> diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
> index 9a87c34ed173..78ebfb6373b6 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;
>  	kleave(" = 0");
>  	return 0;
>  }

Will these verified flags cause issues if a parsed pkcs7_message is evaluated
multiple times?

Exporting verify_pkcs7_message_sig() allows a caller to evaluate a parsed
PKCS#7 message against multiple keyrings sequentially. If a message is
validated successfully against the first keyring, the certificates and
sinfo are marked verified = true.

When the same message is then validated against a second keyring, the internal
blacklist check is_key_on_revocation_list() invokes pkcs7_validate_trust()
against the blacklist keyring.

In pkcs7_validate_trust_one(), the code checks:

	for (x509 = sinfo->signer; x509; x509 = x509->signer) {
		if (x509->seen) {
			if (x509->verified)
				goto verified;

Because the certificate's verified flag is still true from the first keyring
check, won't the blacklist check hit this goto and falsely conclude the
certificate is blacklisted?

This seems like it would cause the second validation to unconditionally return
-EKEYREJECTED for a valid message.

Additionally, if detached data is provided, pkcs7_supply_detached_data()
unconditionally returns -EINVAL on a second call. Does the state need to
be cleared before a subsequent validation?

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