linux-kernel - Re: [PATCH v2] integrity: Extract secure boot enquiry function out of IMA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <15ae451cf47f45a7b540200107ef1f5d1d1543f9.camel@linux.ibm.com>
Date: Wed, 02 Jul 2025 23:35:20 -0400
From: Mimi Zohar <zohar@...ux.ibm.com>
To: GONG Ruiqi <gongruiqi1@...wei.com>,
        Roberto Sassu
 <roberto.sassu@...wei.com>,
        Dmitry Kasatkin <dmitry.kasatkin@...il.com>,
        Jarkko Sakkinen <jarkko@...nel.org>,
        Madhavan Srinivasan	
 <maddy@...ux.ibm.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        Heiko
 Carstens	 <hca@...ux.ibm.com>,
        Vasily Gorbik <gor@...ux.ibm.com>,
        Alexander
 Gordeev	 <agordeev@...ux.ibm.com>
Cc: Eric Snowberg <eric.snowberg@...cle.com>,
        Christophe Leroy	
 <christophe.leroy@...roup.eu>,
        Nicholas Piggin <npiggin@...il.com>,
        Christian Borntraeger <borntraeger@...ux.ibm.com>,
        Sven Schnelle
 <svens@...ux.ibm.com>, "Lee, Chun-Yi" <jlee@...e.com>,
        linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
        linux-s390@...r.kernel.org, linux-integrity@...r.kernel.org,
        keyrings@...r.kernel.org, Lu Jialin <lujialin4@...wei.com>,
        Nayna Jain	
 <nayna@...ux.ibm.com>
Subject: Re: [PATCH v2] integrity: Extract secure boot enquiry function out
 of IMA

On Thu, 2025-07-03 at 10:07 +0800, GONG Ruiqi wrote:
> Hi Mimi,
> 
> On 7/3/2025 9:38 AM, Mimi Zohar wrote:
> > [CC: Nayna Jain]
> > 
> > On Sat, 2025-06-28 at 14:32 +0800, GONG Ruiqi wrote:
> > > ...
> > 
> > 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
> 
> > 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