On Thu, 2025-07-03 at 10:07 +0800, GONG Ruiqi wrote:

Hi Mimi,

On 7/3/2025 9:38 AM, Mimi Zohar wrote:

quoted

[CC: Nayna Jain]

On Sat, 2025-06-28 at 14:32 +0800, GONG Ruiqi wrote:

quoted

...

The original reason for querying the secure boot status of the system was in
order to differentiate IMA policies.  Subsequently, the secure boot check was
also added to safely allow loading of the certificates stored in MOK. So loading
IMA policies and the MOK certificates ARE dependent on the secure boot mode.
                                                                                
What is your real motivation for moving the secure boot checking out of IMA?    
                                                                                

Sorry for not stating that clearly in this patch. I think the cover
letter of V3 I just sent few minutes ago can answer your question, and I
quote:

"We encountered a boot failure issue in an in-house testing, where the
kernel refused to load its modules since it couldn't verify their
signature. The root cause turned out to be the early return of
load_uefi_certs(), where arch_ima_get_secureboot() returned false
unconditionally due to CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT=n, even
though the secure boot was enabled.

This patch set attempts to remove this implicit dependency by shifting
the functionality of efi secure boot enquiry from IMA to the integrity
subsystem, so that both certificate loading and IMA can make use of it
independently."

Here's the link of V3, and please take a look:
https://lore.kernel.org/all/20250703014353.3366268-1-gongruiqi1@huawei.com/T/#mef6d5ea47a4ee19745c5292ab8948eba9e16628d (local)

quoted

FYI, there are a number of problems with the patch itself.  From a very high
level:  
                                                                                
- The EFI secure boot check is co-located with loading the architecture specific
policies.  By co-locating the secure boot check with loading the architecture
specific IMA policies, there aren't any ifdef's in C code.  Please refer to the
"conditional compilation" section in the kernel coding-style documentation on
avoiding ifdef's in C code.
                                                                                
- Each architecture has it's own method of detecting secure boot. Originally the
x86 code was in arch/x86, but to prevent code duplication it was moved to IMA. 
The new file should at least be named efi_secureboot.c.  

You're right. I didn't realize it's arch-specific in the first place,
and moving and renaming arch_ima_get_secureboot() turned out to be a
real mess ...

So the V3 keeps the prototype of arch_ima_get_secureboot(), and only
moves out its body, which I think can also better represent the
intention of the patch.

It's definitely much better.  To summarize, arch_ima_get_secureboot() becomes a
wrapper for integrity_get_efi_secureboot().  Before loading the MOK/MOKx keys,
load_uefi_certs() calls integrity_get_efi_secureboot() directly.

With load_uefi_certs() calling integrity_get_efi_secureboot() directly, please
check to see whether an integrity_get_efi_secureboot() stub function needs to be
defined.

Mimi

As of the name of the new file, as V3 has been sent earlier and still
uses secureboot.c, I can't change it there. I can do it in V4.

-Ruiqi