Message-ID: <87r001yzob.fsf@redhat.com>
Date: Tue, 03 Jun 2025 10:52:52 +0200
From: Vitaly Kuznetsov <vkuznets@...hat.com>
To: James Bottomley <James.Bottomley@...senPartnership.com>,
 linux-security-module@...r.kernel.org, linux-integrity@...r.kernel.org,
 linux-modules@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
 keyrings@...r.kernel.org, David Howells <dhowells@...hat.com>, David
 Woodhouse <dwmw2@...radead.org>, Jonathan Corbet <corbet@....net>, Luis
 Chamberlain <mcgrof@...nel.org>, Petr Pavlu <petr.pavlu@...e.com>, Sami
 Tolvanen <samitolvanen@...gle.com>, Daniel Gomez <da.gomez@...sung.com>,
 Mimi Zohar <zohar@...ux.ibm.com>, Roberto Sassu
 <roberto.sassu@...wei.com>, Dmitry Kasatkin <dmitry.kasatkin@...il.com>,
 Eric Snowberg <eric.snowberg@...cle.com>, Paul Moore
 <paul@...l-moore.com>, James Morris <jmorris@...ei.org>, "Serge E. Hallyn"
 <serge@...lyn.com>, Peter Jones <pjones@...hat.com>, Robert Holmes
 <robeholmes@...il.com>, Jeremy Cline <jcline@...hat.com>, Coiby Xu
 <coxu@...hat.com>, Gerd Hoffmann <kraxel@...hat.com>
Subject: Re: [PATCH RFC 1/1] module: Make use of platform keyring for module
 signature verify

James Bottomley <James.Bottomley@...senPartnership.com> writes:

> 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@...il.com>
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@...hat.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.

Yea, that would be the solution I allude to at the end of my cover
letter: make .platform globally trusted so we don't need the 'trusted
for kexec', 'trusted for dm-verity' zoo we already have.

>
> 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.

I was not personally involved when RH put the patch downstream (and
wasn't very successful in getting the background story) but it doesn't
even have an additional Kconfig, e.g.:
https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-10/-/commit/03d4694fa6511132989bac0da11fa677ea5d29f6
so apparently there's no desire to limit anything, basically, .platform
is always trusted on Fedora/RHEL systems (for a long time already).

As part of the RFC, I'd like to try to understand under which conditions
people may not want to trust 'db'. In the most common use case, 'db' is
used to authorize shim and the kernel is signed by a cert from shim's
vendor_db, not trusting 'db' for modules after that seems somawhat
silly. Maybe we can detect the fact that the user took control over the
system with MOK and untrust .platform only then (while trusting it by
default)?

A runtime toggle is not something I thought much about: the sole purpose
of this part of 'lockdown' (limitimg unsigned modules load) seems to be
to prevent someone who already has 'root' on the system to gain kernel
level access to e.g. hide its activities. In case root can decide which
keys are trusted, isn't it all in vain? Or maybe if the toggle is to
just trust/not trust .platform (and not e.g. disable signatures
verification completely, inject a new key,...) this is acceptable?
Another option is to have a kernel command line parameter but this is
complicated for users.

-- 
Vitaly

Powered by blists - more mailing lists