Re: [PATCH RFC 1/1] module: Make use of platform keyring for module signature verify

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Mon, 2025-06-02 at 15:25 +0200, Vitaly Kuznetsov wrote:
> This patch complements commit 278311e417be ("kexec, KEYS: Make use of
> platform keyring for signature verify") and commit 6fce1f40e951
> ("dm verity: add support for signature verification with platform
> keyring")
> and allows for signing modules using keys from SecureBoot 'db'. This
> may
> come handy when the user has control over it, e.g. in a virtualized
> or a
> cloud environment.
> 
> Suggested-by: Robert Holmes <robeholmes@gmail.com>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  Documentation/admin-guide/module-signing.rst |  6 ++++++
>  kernel/module/Kconfig                        | 11 +++++++++++
>  kernel/module/signing.c                      |  9 ++++++++-
>  security/integrity/Kconfig                   |  2 +-
>  4 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/admin-guide/module-signing.rst
> b/Documentation/admin-guide/module-signing.rst
> index a8667a777490..44ed93e586b9 100644
> --- a/Documentation/admin-guide/module-signing.rst
> +++ b/Documentation/admin-guide/module-signing.rst
> @@ -118,6 +118,12 @@ This has a number of options available:
>       additional certificates which will be included in the system
> keyring by
>       default.
>  
> + (5) :menuselection:`Use .platform keyring for verifying kernel
> modules signatures`
> +     (``CONFIG_MODULE_SIG_PLATFORM``)
> +
> +     This option additionally allows modules to be signed with a key
> present
> +     in ``.platform`` keyring, e.g. a SecureBoot 'db' key.
> +
>  Note that enabling module signing adds a dependency on the OpenSSL
> devel
>  packages to the kernel build processes for the tool that does the
> signing.
>  
> diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
> index 39278737bb68..f1b85c14548a 100644
> --- a/kernel/module/Kconfig
> +++ b/kernel/module/Kconfig
> @@ -340,6 +340,17 @@ config MODULE_SIG_HASH
>  	default "sha3-384" if MODULE_SIG_SHA3_384
>  	default "sha3-512" if MODULE_SIG_SHA3_512
>  
> +config MODULE_SIG_PLATFORM
> +	bool "Use .platform keyring for verifying kernel modules
> signatures"
> +	depends on INTEGRITY_PLATFORM_KEYRING
> +	depends on MODULE_SIG
> +	help
> +	  When selected, keys from .platform keyring can be used for
> verifying
> +	  modules signatures. In particular, this allows to use UEFI
> SecureBoot
> +	  'db' for verification.
> +
> +	  If unsure, say N.
> +
>  config MODULE_COMPRESS
>  	bool "Module compression"
>  	help
> diff --git a/kernel/module/signing.c b/kernel/module/signing.c
> index a2ff4242e623..3327e7243211 100644
> --- a/kernel/module/signing.c
> +++ b/kernel/module/signing.c
> @@ -61,10 +61,17 @@ int mod_verify_sig(const void *mod, struct
> load_info *info)
>  	modlen -= sig_len + sizeof(ms);
>  	info->len = modlen;
>  
> -	return verify_pkcs7_signature(mod, modlen, mod + modlen,
> sig_len,
> +	ret = verify_pkcs7_signature(mod, modlen, mod + modlen,
> sig_len,
>  				      VERIFY_USE_SECONDARY_KEYRING,
>  				      VERIFYING_MODULE_SIGNATURE,
>  				      NULL, NULL);
> +	if (ret == -ENOKEY &&
> IS_ENABLED(CONFIG_MODULE_SIG_PLATFORM)) {
> +		ret = verify_pkcs7_signature(mod, modlen, mod +
> modlen, sig_len,
> +				VERIFY_USE_PLATFORM_KEYRING,
> +				VERIFYING_MODULE_SIGNATURE,
> +				NULL, NULL);
> +	}
> +	return ret;
>  }

I don't think this is the correct way to do it.  If, as you say, db is
controlled by the end user and therefore has trusted contents, then I
think you want to update certs/system_keyring.c to link the platform
keyring into the secondary trusted one (like it does today for the
machine keyring), so it can be used by *every* application that checks
keyrings rather than just modules.

Also, are you sure a config option is the right thing?  Presumably Red
Hat wants to limit its number of kernels and the design of just linking
the machine keyring (i.e. MoK) was for the use case where trust is
being pivoted away from db by shim, so users don't want to trust the db
keys they don't control.  If the same kernel gets used for both
situations (trusted and untrusted db) you might want a runtime means to
distinguish them.

Regards,

James