Message-ID: <c119df40-5cd9-40f8-b7b7-4085b87e90e5@kernel.org>
Date: Mon, 2 Dec 2024 08:56:11 +0100
From: Jiri Slaby <jirislaby@...nel.org>
To: James Bottomley <James.Bottomley@...senPartnership.com>,
 Linus Torvalds <torvalds@...ux-foundation.org>,
 Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Cc: Peter Hüwe <PeterHuewe@....de>,
 Jarkko Sakkinen <jarkko@...nel.org>, Jason Gunthorpe <jgg@...pe.ca>,
 linux-integrity@...r.kernel.org, Ard Biesheuvel <ardb@...nel.org>,
 "linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>
Subject: Re: TPM/EFI issue [Was: Linux 6.12]

On 29. 11. 24, 22:08, James Bottomley wrote:
> On Fri, 2024-11-29 at 11:03 -0500, James Bottomley wrote:
>> On Fri, 2024-11-29 at 07:36 +0100, Jiri Slaby wrote:
>>> On 28. 11. 24, 17:13, James Bottomley wrote:
>> [...]
>>>> Yes, it tells me the entries in the log for PCR0-7,14 match the
>>>> log entries (for both sha1 and sha256).  However there are
>>>> entries for PCR9,12 which don't match.  The log shows shim
>>>> starting at entry 32, grub starting at entry 37 and the kernel
>>>> loading at entry 39 the kernel command line logged at 40 to PCR
>>>> 12, which is mismatching.
>>>>
>>>> The next two entries (41,42) are for the mismatching PCR9 and are
>>>> of the initrd and the options and come from the libstub code in
>>>> the kernel early boot (efi-stub-helper.c).
>>>
>>> Note that ovmf logged:
>>> Called TcgDxeHashLogExtendEvent 0 58683000 1B1E78C 5FE63C00
>>> 5E3492AA Data 28 B5 2F FD ... E1 29 FE 0
>>>
>>> But initrd on disk is 1B1E78B long, not 1B1E78C. So the excessive 0
>>> at the end above brews the mismatch. See:
>>>     https://bugzilla.suse.com/show_bug.cgi?id=1233752#c14
>>> "By adding the 0 byte I can replicate the measured digest."
>>>
>>> So there is something aligning the initrd. kernel's libstub just
>>> uses and passes load_file2's size down to TcgDxeHashLogExtendEvent,
>>> AIUI. So it'd be sdb, ovmf or something. BTW how are sizes stored
>>> in/fetched from vfat?
>>
>> Well, I was going to explain what EFI does, but it doesn't look
>> relevant now I've had a crash course reading the systemd-boot code.
>> It looks like run() calls image_start() which loads the initrd
>> itself. Then in initrd.c:initrd_prepare() it actually installs its
>> own load file2 protocol which is the protocol the kernel picks up
>> when it loads the initrd.  So whatever length the kernel is picking
>> up is, in fact, provided by systemd-boot.
>>
>> I'd suspect something in this double indirection of load file
>> protocols is causing your length mismatch.
> 
> OK, confirmed it's the Load File2 protocol installed by systemd-boot
> that's doing this.  It seems to be by design: it zero pads and aligns
> on 4 bytes:
> 
> https://github.com/systemd/systemd/blob/3f3b4959e2cb9bca1e1ed527a0692c9a8b6a18ea/src/boot/boot.c#L2498-L2504
> 
> I managed to construct a debian secure boot vm image with the latest
> systemd just to check and sure enough the linux boot stub is using the
> systemd-boot Load file2 protocol module and so does have this zero
> padding issue.
> 
> Although it's a problem if you do a flat file hash, and obviously
> violates the linux stub assumption that that's how we compute the hash,
> I'd have to be reasonably certain that the systemd tools take the zero
> padding into account when constructing the pcr lock values.

Thanks for the investigation. Just for info, the ball looks to be on the 
systemd side now. So as noted to the downstream bug [1], this was also 
reported to systemd [2].

[1] https://bugzilla.suse.com/show_bug.cgi?id=1233752#c19
[2] https://github.com/systemd/systemd/issues/35439

thanks,
-- 
js
suse labs

Powered by blists - more mailing lists