[PATCH] Revert "integrity: Do not load MOK and MOKx when secure boot be disabled"

[Mimi Zohar]

On Thu, 2025-03-20 at 13:02 +0100, Lennart Poettering wrote:
> This reverts commit 92ad19559ea9a8ec6f158480934ae26ebfe2c14f.
> 
> This original commit this reverts creates a strange situation: it
> ensures more restrictive behaviour if SecureBoot is off then when it
> is on, which is the opposite of what one would expect.
> 
> Typically, one would expect that if SB is off the validation of
> resources during the pre-kernel and kernel initialization is less
> restrictive, not more restrictive. But this check turned the world on
> its head.

Hi Lennart,

I'm really sorry for the long delay ...

>From an IMA perspective, the default is to only trust keys built into the kernel
or certificates signed by the builtin keys and loaded onto the
.secondary_trusted_keys keyring.
                                                                                
The ability of loading MOK keys onto the .machine keyring and linked to the
.secondary_trusted_keys keyring is an exception based on the assumption that
that there is a secure boot chain of trust.  Allowing untrusted keys onto or
linked to the .secondary_trusted_keys keyring, would potentially allow loading
code signing keys onto the IMA keyring signed by untrusted MOK keys.
                                                                                
I was really hesitant to allow this exception of loading MOK keys onto the
.machine keyring in the first place.  I'm now even more concerned.
                                                                                
This is not just an issue of being more or less restrictive, but of adding a new
integrity gap when one didn't exist previously.

Mimi


> 
> I'd like to ask for this commit to be reverted. If SB is on all bets are
> off regarding integrity of boot loaders and stuff, hence it makes no
> sense to be restrictive here: you cannot regain integrity once you gave
> it up once, hence if all bets are off anyway we might as well import any
> Mok keys passed to us into the kernel keyring.
> 
> Or to say this differently: if an attacker got control of the pre-kernel
> boot phase they might as well patch around in the firmware apis to make
> the kernel believe it is in SB mode even if it is not. Hence the check
> carries no value. It doesn't protect anything in any effective way.
> 
> The reason i'd like this check to go is that I'd like a nice way to
> insert keys from pre-boot into into the kernel keyring for use with
> signed dm-verity, without requiring recompilation of the kernel, and
> without SB database games. i.e. i'd like to use a regular, signed
> distro kernel, and pass to it additional keys to insert into the
> kernel keyring in a reasonable way. The mok stuff would be great for that,
> except it all falls apart once SB is off.
> 
> You might wonder what signed dm-verity gives me if I have SB off. If
> we authenticate the boot phase up to Linux userspace via TPM-based PCR
> policies (i.e. measured boot) we can be sure of the boot integrity
> without having to rely on SB. But then we'd still like to use
> dm-verity based code signing for userspace.
> ---
>  security/integrity/platform_certs/load_uefi.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
> index d1fdd113450a..7783bcacd26c 100644
> --- a/security/integrity/platform_certs/load_uefi.c
> +++ b/security/integrity/platform_certs/load_uefi.c
> @@ -7,7 +7,6 @@
>  #include <linux/err.h>
>  #include <linux/efi.h>
>  #include <linux/slab.h>
> -#include <linux/ima.h>
>  #include <keys/asymmetric-type.h>
>  #include <keys/system_keyring.h>
>  #include "../integrity.h"
> @@ -211,10 +210,6 @@ static int __init load_uefi_certs(void)
>  		kfree(dbx);
>  	}
> 
> -	/* the MOK/MOKx can not be trusted when secure boot is disabled */
> -	if (!arch_ima_get_secureboot())
> -		return 0;
> -
>  	mokx = get_cert_list(L"MokListXRT", &mok_var, &mokxsize, &status);
>  	if (!mokx) {
>  		if (status == EFI_NOT_FOUND)
> --
> 2.48.1
> 
> 
> Lennart
> 
> --
> Lennart Poettering, Berlin
>