Removal of SD_TPM_PCR option breaks grub + sd-stub set-ups

[alfonsosanchezbeato]

systemd version the issue has been seen with
v250

Used distribution
Ubuntu

Linux kernel version used (uname -a)
Linux host 5.4.0-100-generic #113-Ubuntu SMP Thu Feb 3 18:43:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

CPU architecture issue was seen on
x86

Expected behaviour you didn't see
NA

Unexpected behaviour you saw

Commit faacf18 breaks Ubuntu setups. In Ubuntu, we configure grub with PCR8 whilst we configure sd-stub with PCR12. This allows us to seal, compute, and differentiate cmdlines as measured by grub and as measured by sd-stub, on the same system.

Could the commit be reverted please?

Or for example use distinct from grub pcr by default, i.e. 12 - which i guess will not fly, given it is a breaking change for anybody who only ever used sdstub only without grub.

Simultaneous measurements of cmdline to the same pcr by either grub & sd-stub and harder to compute sealing policies for if one wants to support one/another/either.

Steps to reproduce the problem
NA

Additional program output to the terminal or log subsystem illustrating the issue
NA

[alfonsosanchezbeato]

@xnox ^^

[medhefgo]

I don't quite see why grub/sd-boot/sd-stub should be using different PCRs to measure the same thing. That's literally what a PCR measurement is supposed to be for, no?

/cc @poettering

[poettering]

I am not sure I follow? Using the kernel cmdline PCR to distinguish boot loaders sounds quite a surprising use for them? Can you elaborate?

Why not just check for contents of /sys/firmware/efi/efivars/LoaderInfo-4a67b082-0a4c-41cf-b6c7-440b29bb8c4f if you want to know the boot loader?

[poettering]

Dropping from milestone, I don#t think it's obvious what the right approach here is, I simply understand the rationale here.

[xnox]

I will provide bios measurements log to explain what is going on.

[xnox]

tl;dr it is impossible to compute PCR values when grub chainloads systemd-boot or Boot Loader Specification type 2 binary, because grub measures a lot of things into PCR 8.

$ sudo ./tcglog-dump -verbose -with-grub -pcr 8 -pcr 12 -with-systemd-efi-stub -systemd-efi-stub-pcr 12
 8 75409120452bbbee30abe289af973ecdd7e0ef6b EV_IPL [ grub_cmd{ set default=0 } ]
 8 68dc3c3065536575e511fa144ffa84566038d499 EV_IPL [ grub_cmd{ set timeout=3 } ]
 8 58795e8592d9ff3b6b39add68ddba958eff547a2 EV_IPL [ grub_cmd{ set timeout_style=hidden } ]
 8 403106f2cb2ac1a455a11e5389b4fda22a8bebd6 EV_IPL [ grub_cmd{ [ -e /EFI/ubuntu/grubenv ] } ]
 8 2f839cb57fd3cc1d0fd99d433d03c5b20f292412 EV_IPL [ grub_cmd{ load_env --file /EFI/ubuntu/grubenv snapd_recovery_mode snapd_recovery_system } ]
 8 6fb18f99d8b408a6579c3a8d3743fab711db63b6 EV_IPL [ grub_cmd{ set snapd_static_cmdline_args=console=ttyS0 console=tty1 panic=-1 } ]
 8 853ac1db3a64324a4ec7cb358beff2f9c2b523c0 EV_IPL [ grub_cmd{ [ -z run ] } ]
 8 3b405c80e702dc42e6c6c2c65b4a5419df5cf6c3 EV_IPL [ grub_cmd{ [ run = run ] } ]
 8 bc0b1baf44d19756f53d874913e2fba5c165b0ab EV_IPL [ grub_cmd{ default=run } ]
 8 0af5c5511a8ca6b31a1e1903bd64364e7acaaa55 EV_IPL [ grub_cmd{ search --no-floppy --set=boot_fs --label ubuntu-boot } ]
 8 27b2ca659b3ab88b527d9b8e9be81bdb655c051e EV_IPL [ grub_cmd{ [ -n hd0,gpt3 ] } ]
 8 76854ebe2e900df83de9ad7ee4b2dc25466b59f4 EV_IPL [ grub_cmd{ menuentry Continue to run mode --hotkey=n --id=run {
        chainloader ($boot_fs)/EFI/boot/grubx64.efi
    } } ]
 8 cdc887f8aab7dd652443ea45fdad35e65122f3e1 EV_IPL [ grub_cmd{ regexp --set 1:label /([a-z0-9](-?[a-z0-9])*)$ /systems/20220308 } ]
 8 3d5dd651e2d9207f5d07fbd7a2a8830b81a3f983 EV_IPL [ grub_cmd{ [ -z 20220308 ] } ]
 8 41fac31dcc3bd1de44cfe085227442289a9f6b4d EV_IPL [ grub_cmd{ [ -z  -o 20220308 <  ] } ]
 8 4196b6619027fed2ed6e6ddd2a905f13a8ca4661 EV_IPL [ grub_cmd{ set best=20220308 } ]
 8 3d5dd651e2d9207f5d07fbd7a2a8830b81a3f983 EV_IPL [ grub_cmd{ [ -z 20220308 ] } ]
 8 09cd4678c1207de0671a8b13c839b2486512e647 EV_IPL [ grub_cmd{ set snapd_recovery_kernel= } ]
 8 9ec0359dd31f8350a84234119a55d698111828e5 EV_IPL [ grub_cmd{ load_env --file /systems/20220308/grubenv snapd_recovery_kernel snapd_extra_cmdline_args snapd_full_cmdline_args } ]
 8 9dd8006abf23e134f7ffac8ef7c033e950ee52e3 EV_IPL [ grub_cmd{ set cmdline_args=console=ttyS0 console=tty1 panic=-1  } ]
 8 7a86009dc1f23867d8951bb95471618bde2d1918 EV_IPL [ grub_cmd{ [ -n  ] } ]
 8 7aad2b26922eaedd01c91a1fb6554fbcbb36f38b EV_IPL [ grub_cmd{ menuentry Recover using 20220308 --hotkey=r --id=recover-20220308 /snaps/pc-kernel_910.snap recover 20220308 {
        loopback loop $2
        chainloader (loop)/kernel.efi snapd_recovery_mode=$3 snapd_recovery_system=$4 $cmdline_args
    } } ]
 8 43255af81090360108b7ff214f3abb7ced4a9980 EV_IPL [ grub_cmd{ menuentry Install using 20220308 --hotkey=i --id=install-20220308 /snaps/pc-kernel_910.snap install 20220308 {
        loopback loop $2
        chainloader (loop)/kernel.efi snapd_recovery_mode=$3 snapd_recovery_system=$4 $cmdline_args
    } } ]
 8 5232ffa8d1d3e2627b7a145ba432f8f0e64eb5b7 EV_IPL [ grub_cmd{ menuentry UEFI Firmware Settings --hotkey=f uefi-firmware {
    fwsetup
} } ]
 8 162947d1c42efa7389af2b7f1b6a5bef8ff30d3c EV_IPL [ grub_cmd{ setparams Continue to run mode } ]
 8 fd02d1830fee90a4ef2cd872497da8ab8a7feed7 EV_IPL [ grub_cmd{ chainloader (hd0,gpt3)/EFI/boot/grubx64.efi } ]
 8 75409120452bbbee30abe289af973ecdd7e0ef6b EV_IPL [ grub_cmd{ set default=0 } ]
 8 68dc3c3065536575e511fa144ffa84566038d499 EV_IPL [ grub_cmd{ set timeout=3 } ]
 8 58795e8592d9ff3b6b39add68ddba958eff547a2 EV_IPL [ grub_cmd{ set timeout_style=hidden } ]
 8 da67154aaa49fa0bf5789120508233cb84432ab7 EV_IPL [ grub_cmd{ load_env --file /EFI/ubuntu/grubenv kernel_status snapd_extra_cmdline_args snapd_full_cmdline_args } ]
 8 6fb18f99d8b408a6579c3a8d3743fab711db63b6 EV_IPL [ grub_cmd{ set snapd_static_cmdline_args=console=ttyS0 console=tty1 panic=-1 } ]
 8 9dd8006abf23e134f7ffac8ef7c033e950ee52e3 EV_IPL [ grub_cmd{ set cmdline_args=console=ttyS0 console=tty1 panic=-1  } ]
 8 7a86009dc1f23867d8951bb95471618bde2d1918 EV_IPL [ grub_cmd{ [ -n  ] } ]
 8 949bc093f440b10ea6bdb290a2f27ce838ef47be EV_IPL [ grub_cmd{ set kernel=kernel.efi } ]
 8 ff05b936a1cd2283cb83e411595533c731d5b8d4 EV_IPL [ grub_cmd{ [  = try ] } ]
 8 f88562e61aea6775ed04b7d41e2cd99caa81814c EV_IPL [ grub_cmd{ [  = trying ] } ]
 8 7a86009dc1f23867d8951bb95471618bde2d1918 EV_IPL [ grub_cmd{ [ -n  ] } ]
 8 0ca9658407083f0ee88ac8ea26a25d862748840c EV_IPL [ grub_cmd{ [ -e (hd0,gpt3)/EFI/ubuntu/kernel.efi ] } ]
 8 f2246906d1df87686a9189fa1bbd8b6ab5014a9b EV_IPL [ grub_cmd{ menuentry Run Ubuntu Core 20 {
    # use $prefix because the symlink manipulation at runtime for kernel snap
    # upgrades, etc. should only need the /boot/grub/ directory, not the
    # /EFI/ubuntu/ directory
    chainloader $prefix/$kernel snapd_recovery_mode=run $cmdline_args
} } ]
 8 492c75a6f141a566e02329925774080874935914 EV_IPL [ grub_cmd{ setparams Run Ubuntu Core 20 } ]
 8 963b15a84c382336bd0d82a3ab389377ddecbc8f EV_IPL [ grub_cmd{ chainloader (hd0,gpt3)/EFI/ubuntu/kernel.efi snapd_recovery_mode=run console=ttyS0 console=tty1 panic=-1 } ]
12 3f60bb68b1c88f0b7d79aea4644ece09857558fd EV_IPL [ snapd_recovery_mode=run console=ttyS0 console=tty1 panic=-1 ]

On Ubuntu Core we boot into shim (because we must boot shim to support Secureboot on systems with MS certificates).
Shim singing policy requires one to only boot grub and/or linux.
We boot grub which generates a dynamic set of finite menu items with a finite set of commandline boot options.
And then we support chainloading Boot Loader Specification Type 2 binary.
We use full disk encryption, and seal the keys against PCR 7 (secureboot variables state), PCR 4 (measurements of shim / grub / kernel.efi), and we also want to seal against a finite sub-set of allowed kernel command-lines.

And here lies the problem. For example offline addition of recovery systems, which one doesn't seal secret against, invalidate the valid boot entry to boot the encrypted system, because grub measures extra commands into PCR8 because it parsed them even when user didn't pick to boot them.

Using the https://github.com/canonical/tcglog-parser/tree/master/tcglog-dump you can see that by default grub measures every step of its execution into PCR 8 making a complete mess of it. Furthermore, the way it does measurements are hardware dependend i.e. $prefix can be different, and are not exactly round-trip safe (for example there is a lot of whitespace mangling and quoting issues). There is no grub.cfg tpm measurements simulator either that can be used to predict a finite set of measurements for the kernel commandline that one wants to seal against.

Thus instead of trying to predict the final states of the grub's PCR 8 that is in the below form of to seal against cmdline args

 8 963b15a84c382336bd0d82a3ab389377ddecbc8f EV_IPL [ grub_cmd{ chainloader (hd0,gpt3)/EFI/ubuntu/kernel.efi snapd_recovery_mode=run console=ttyS0 console=tty1 panic=-1 } 

We instead configure systemd-boot to measure to PCR 12 (currently universally unused). Which results in a clean measurement of the commandline that is passed to the kernel image i.e.

12 3f60bb68b1c88f0b7d79aea4644ece09857558fd EV_IPL [ snapd_recovery_mode=run console=ttyS0 console=tty1 panic=-1 

Which is hardware independent, and easy to parse and predict. For example we allow a few PCR12 values those that match a few different allowed snapd_recovery_mode= mode values, and potentially with user defined cmdline options etc.

To continue using BLS type 2 binaries that are chainloader loaded by grub, with predictable PCR measurements, it would be nice if systemd-boot would switch by default to a clean PCR 12 (i.e. explicitly different PCR from grub's one), or for it to keep the PCR measurement number build time configurable to allow distributions to sefl-allocate different PCRs to different bootloaders, and thus allowing easy ways to precompute policies when multiple bootloaders are used during boot. I.e. on Ubuntu so far we have been using PCR 8 for grub mess, PCR 12 for clean declarative systemd-boot measurements across our Desktop, Server and IoT product lines.

[medhefgo]

Shim singing policy requires one to only boot grub and/or linux.

From shim-review: Note that we really only have experience with using GRUB2 on Linux, so asking us to endorse anything else for signing is going to require some convincing on your part.. So it doesn't seem to be a hard requirement, and I would very much like to see an explanation for any sd-boot rejections (other than "we don't wanna bother").

Ultimately, this looks to me like grub and sd-boot (mayber others?) aren't on an agreement/standard as to what to measure into what. In particular, the PCR8 measurement doesn't look very useful to me for a "it's the kernel command line" when it puts everything and the kitchen sink in it (which can easily change on boot loader upgrades)…
Wouldn't it be more prudent to get this right across distros/bootloaders and come up with an agreement as to what measure into which PCR in whatever format? Seem better than the wild west we have now with everyone using something different as they please?

[xnox]

Shim singing policy requires one to only boot grub and/or linux.

From shim-review: Note that we really only have experience with using GRUB2 on Linux, so asking us to endorse anything else for signing is going to require some convincing on your part.. So it doesn't seem to be a hard requirement, and I would very much like to see an explanation for any sd-boot rejections (other than "we don't wanna bother").

sd-boot has not had security review and at the moment the bootloader efi app is not signed by any of the linux distros (including the major ones like debian/fedora/ubuntu/rhel/suse/oracle/etc.). I think it is out of scope to discuss this.

As far as i can tell the only apps that are signed are shim, shim-fallback, shim-mokmanager, grub in various configurations, fwupd firmware updater, and linux kernels with various EFI entry point stubs (i.e. sd-boot stub and/or kernel efi stub).

Ultimately, this looks to me like grub and sd-boot (mayber others?) aren't on an agreement/standard as to what to measure into what. In particular, the PCR8 measurement doesn't look very useful to me for a "it's the kernel command line" when it puts everything and the kitchen sink in it (which can easily change on boot loader upgrades)… Wouldn't it be more prudent to get this right across distros/bootloaders and come up with an agreement as to what measure into which PCR in whatever format? Seem better than the wild west we have now with everyone using something different as they please?

Whilst that is true, and a good goal, it is currently the state of affairs that grub imho over-measures a lot. and as a distribution i have no option to backwards compat support that grub behaviour, whilst still trying to land new OS features that use sd-boot stub.

But also note that measuring pcr8 was a way to detect and protect oneself from grub acpi-bypass vulnerability, precisely because grub measures everything it does via all of its commands. Given that grub's complexity which looks like a turing complete mini-OS, I am not sure they would be open to measuring less things.

It is also not nice for the systemd-boot project to remove a feature that is actively used for 3 years and shipped in production by one vendor, with what I hope I demonstrated as a valid use case whilst being stuck between a rock and a hard place.

I'm not too sure what I can propose to be universal going forward.

I.e. PCR#8 at this point looks like a bootloader-specific freeform whatever. And I kind of want / need to define a PCR#12 to be "the args/cmdline passed to StartImage of an EFI app binary that happens to be a linux kernel" which is also a very tough sell, as it is very EFI & OS specific. Given that many other architectures use TPM measurements without any args, or not using EFI at all.

Please resurrect compile time configuration of where sd-boot sends TPM measurements, as removal of this features breaks backwards compat in production systems.

[poettering]

yikes, those measurements grub is doing there are some serious garbage.

I'd really be interested in removing any variables of PCR assignment from the game in the long run, i.e. the others typically hard code the PCR numbers, and so should we. i.e. I'd like to have reasonably universially adopted PCR meanings if possible.

If grub does such a mess with PCR 8, maybe we should switch to PCR 12 altogether for our measurements? and leave PCR 8 in the mess it is?

I wonder what a good approach could be there. Just hard switch our stuff to PCR 12? or switch to PCR 12, but optionally additionally measure into PCR 8 as compat ifdeffery?

[poettering]

I wonder what a good approach could be there. Just hard switch our stuff to PCR 12? or switch to PCR 12, but optionally additionally measure into PCR 8 as compat ifdeffery?

I am leaning towards this approach. i.e. hardcode PCR 12 now in our codebase, document it in our man pages. Then, add a boolean compat knob in meson that if enabled also enables PCR 8 measurement, but now defaults to off, plus a NEWS story that hints we'll drop that in 1 year or two and people should migrate to PCR 12.

[poettering]

Would be happy to review a patch along those lines. i.e. not reintroduce the old configurability of the PCR to use, but just a build-time to choose between "only new-style" or "new-style + old-style simultaneously".

[xnox]

Yeap, squatting a PCR # seems to be the way to go here. And I like your proposed strategy here.

[Foxboron]

Just a headsup that Bitlocker reserves the use of PCR12 for "Data events and highly volatile events". I'm not sure what that means or if it's relevant if someone winds up dual-booting Linux and Windows with bitlocker?

https://github.com/tianocore-docs/edk2-TrustedBootChain/blob/main/4_Other_Trusted_Boot_Chains.md

[xnox]

Just a headsup that Bitlocker reserves the use of PCR12 for "Data events and highly volatile events". I'm not sure what that means or if it's relevant if someone winds up dual-booting Linux and Windows with bitlocker?

https://github.com/tianocore-docs/edk2-TrustedBootChain/blob/main/4_Other_Trusted_Boot_Chains.md

reading that, it sounds like it should be ok. Because we don't want to boot windows bootmgr from systemd-boot probably, instead we'd want people to use direct UEFI windows boot entry instead. Thus systemd-boot is not a boot sequence to affect windows boot sequence PCR measurements.

[xnox]

But maybe we should add ourselves to that table. It looks nice.

[Foxboron]

Not sure how well maintained it is. Been trying to keep the archwiki updated on my end.
https://wiki.archlinux.org/title/Trusted_Platform_Module#Accessing_PCR_registers

[poettering]

Dropping from the milestone for now. Let's not delay the release for this. I think there's sufficient consensus on how to fix this properly, just somebody needs to prep the patch for it. If none is supplied by release time I think while this is technically a regression it's trivial to work around in Ubuntu's private builds, so not sure it's worth delaying the release for.

[poettering]

Ah, well, to make it easy I prepped PR #22761.