Re: [RFC PATCH 1/3] certs: blacklist: define skid: entries and add is_key_blacklisted()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] certs: blacklist: check-blacklist-hashes.awk rejects skid: prefix
- [Medium] certs: blacklist: undefined key_type_asymmetric in is_key_blacklisted()
--

commit 28646d9ec2c355c55a1dbd687d050b270243210e
Author: Mimi Zohar <zohar@linux.ibm.com>

certs: blacklist: define skid: entries and add is_key_blacklisted()

This commit introduces a new "skid:<hex>" blacklist entry type for revoking
already-loaded asymmetric keys by their Subject Key Identifier and adds
is_key_blacklisted() to query the system blacklist for a key's SKID.

> diff --git a/certs/Kconfig b/certs/Kconfig
> index 9d2bf7fb5b9e4..9e51f988ba3bd 100644
> --- a/certs/Kconfig
> +++ b/certs/Kconfig
> @@ -155,10 +155,11 @@ config SYSTEM_BLACKLIST_HASH_LIST
>  	  If set, this option should be the filename of a list of hashes in the
>  	  form "<hash>", "<hash>", ... .  This will be included into a C
>  	  wrapper to incorporate the list into the kernel.  Each <hash> must be a
> -	  string starting with a prefix ("tbs" or "bin"), then a colon (":"), and
> -	  finally an even number of hexadecimal lowercase characters (up to 128).
> +	  string starting with a prefix ("tbs", "bin" or "skid"), then a colon
> +	  (":"), and finally an even number of hexadecimal lowercase characters
> +	  (up to 128).

[Severity: Medium]
When CONFIG_SYSTEM_BLACKLIST_HASH_LIST is set, certs/Makefile runs
certs/check-blacklist-hashes.awk via cmd_check_and_copy_blacklist_hash_list
to validate the hash list file during the build:

certs/check-blacklist-hashes.awk:
	if (!match(part1[1], "^(tbs|bin):(.*)$", part2)) {
		print "Unknown prefix (item " NR "):", part1[1];
		exit 1;
	}

Since certs/check-blacklist-hashes.awk only allows tbs and bin prefixes,
won't any skid:<hex> entry in the file configured by
CONFIG_SYSTEM_BLACKLIST_HASH_LIST be rejected with an "Unknown prefix" error
and fail the kernel build?

> diff --git a/certs/blacklist.c b/certs/blacklist.c
> index fa561c12eabfe..b1e2c178d3782 100644
> --- a/certs/blacklist.c
> +++ b/certs/blacklist.c

[ ... ]

> @@ -244,6 +243,38 @@ int is_hash_blacklisted(const u8 *hash, size_t hash_len,
>  }
>  EXPORT_SYMBOL_GPL(is_hash_blacklisted);
>  
> +/**
> + * is_key_blacklisted - Determine if a key is blacklisted by its SKID
> + * @key: The key to be checked, e.g. a trust anchor found in a keyring
> + *
> + * Unlike a "tbs:" entry, which is only compared against certificates being
> + * added or presented in a chain, this checks a key that is already loaded, so
> + * that keys that cannot be unlinked from their keyring can still be revoked.
> + * The identifier is taken from the key itself, not from the caller.
> + *
> + * Returns -EKEYREJECTED if the key's Subject Key Identifier is blacklisted,
> + * 0 if it is not (or the key has no SKID), or another negative error.
> + */
> +int is_key_blacklisted(const struct key *key)
> +{
> +	const struct asymmetric_key_ids *kids;
> +	const struct asymmetric_key_id *skid;
> +
> +	if (!key || key->type != &key_type_asymmetric)
> +		return 0;

[Severity: Medium]
In certs/Kconfig, SYSTEM_BLACKLIST_KEYRING only depends on KEYS and does not
depend on or select ASYMMETRIC_KEY_TYPE:

certs/Kconfig:
config SYSTEM_BLACKLIST_KEYRING
	bool "Provide system-wide ring of blacklisted keys"
	depends on KEYS

However, key_type_asymmetric is defined in
crypto/asymmetric_keys/asymmetric_type.c, which is only built when
CONFIG_ASYMMETRIC_KEY_TYPE is enabled.

If the kernel is built with CONFIG_SYSTEM_BLACKLIST_KEYRING=y and
CONFIG_ASYMMETRIC_KEY_TYPE=n, won't this unconditional reference to
key_type_asymmetric in is_key_blacklisted() cause a link failure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105121227.4089147-1-zohar@linux.ibm.com?part=1