rhboot / shim

UEFI shim loader
Other
816 stars 284 forks source link

No device_path information when extending PCR4 done by shim_verify #642

Open aplanas opened 4 months ago

aplanas commented 4 months ago

When secure boot gets enabled we found that the TPM2 event log does not contain the "DevicePath" information related to the EV_EFI_BOOT_SERVICES_APPLICATION event during the PCR 4 extension for the kernel.

After some debugging (see cc) the cause seems to be that the communication between systemd-boot and shim is done via shim_verify(), that does not accept any "DevicePath" information that can be later be communicated to tpm_log_pe()

This is an issue because some tools, like pcr-oracle use the "DevicePath" form the event log to locate the PE binary and do a rehash to calculate new predictions and policies. The proposed workaround is very brittle and will be invalidated if more that one pathless extension happens in PCR4, so we are looking for a more robust solution.

Do makes sense to have a shim_verify2() or some other mechanism that can be used to pass the device path information?

cc: @lnussel, @arvidjaar

mikebeaton commented 4 months ago

Does the existing (but barely supported, and possibly to-be-removed) OVERRIDE_SECURITY_POLICY mechanism avoid this issue? https://github.com/rhboot/shim/issues/596#issuecomment-1908352329

arvidjaar commented 4 months ago

Does the existing (but barely supported, and possibly to-be-removed) OVERRIDE_SECURITY_POLICY mechanism avoid this issue?

No. It calls exactly the same shim_verify(). If anything, it makes things worse because now there will be multiple "anonymous" entries without any reliable way to know what each entry measures.

mikebeaton commented 4 months ago

Right, the security policy hook (the added security verifier) is shim_verify, and in the case where the original policy doesn't 'pass', it is shim_verify which does the TPM measurement - with the problems described. Okay.

mikebeaton commented 4 months ago

FWIW It looks like there would be no problem in updating OVERRIDE_SECURITY_POLICY to support this (if something like the feature proposed in this issue was added, and if anyone wanted to maintain support for OVERRIDE_SECURITY_POLICY...), since device path is available to both (older and newer) overridden protocols.

arvidjaar commented 4 months ago

It looks like there would be no problem in updating OVERRIDE_SECURITY_POLICY to support this

This will require cooperation from bootloaders anyway (both grub and sd-boot explicitly call ->shim_verify()) at which point it is much easier to add the V2 of shim protocol with additional parameter. OVERRIDE_SECURITY_POLICY may be useful with non-participating bootloaders, but that is orthogonal to this issue.

mikebeaton commented 4 months ago

I don't want to (try to) completely hijack your issue, but OVERRIDE_SECURITY_POLICY is arguably not completely orthogonal due to two connections:

a) If something like the update suggested here was made, it would definitely make sense to upgrade OVERRIDE_SECURITY_POLICY to use it, too, if OVERRIDE_SECURITY_POLICY is still going to be supported b) If all boot loaders were non-participating in the first place, i.e. if OVERRIDE_SECURITY_POLICY had been the default approach, and no bootloader had to implement its own PE loader and directly call shim_verify, then - it is arguably worth noting - an update such as this could have been done transparently, without requiring co-operation from bootloaders

I don't disagree about what is required now for the issue raised here (i.e. cooperation from participating bootloaders), and I agree that whether or not OVERRIDE_SECURITY_POLICY continues to be supported can be decided completely separately from whether or not the issue here is fixed.

arvidjaar commented 4 months ago

no bootloader had to implement its own PE loader

grub can load kernel from anywhere and cannot rely on LoadImage.

lnussel commented 4 months ago

If participation from boot loaders is expected anyway, wouldn't it be easier if shim just exported start_image()?

mikebeaton commented 4 months ago

grub can load kernel from anywhere and cannot rely on LoadImage.

Actually, LoadImage can be called with an already loaded buffer from anywhere.

Also, possibly less relevant, if OVERRIDE_SECURITY_POLICY and non-participating (in the sense 'not needing updates for things like this') bootloaders were the norm, then bootloaders could also call the (overridden, if shim is in place) security protocols directly; but I don't think they'd actually need to, for the previous reason?

Apologies, I appreciate this is going rather off topic, though I don't think it's completely orthogonal. Thanks for taking the time to address these issues.