Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No device_path information when extending PCR4 done by shim_verify #642

Open
aplanas opened this issue Feb 27, 2024 · 9 comments
Open

No device_path information when extending PCR4 done by shim_verify #642

aplanas opened this issue Feb 27, 2024 · 9 comments

Comments

@aplanas
Copy link

aplanas commented Feb 27, 2024

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
Copy link
Contributor

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

@arvidjaar
Copy link

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
Copy link
Contributor

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
Copy link
Contributor

mikebeaton commented Feb 27, 2024

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
Copy link

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
Copy link
Contributor

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
Copy link

no bootloader had to implement its own PE loader

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

@lnussel
Copy link

lnussel commented Feb 27, 2024

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

@mikebeaton
Copy link
Contributor

mikebeaton commented Feb 27, 2024

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.

lnussel added a commit to lnussel/systemd that referenced this issue Mar 7, 2024
Don't mark records with empty device path as invalid. When shim is in
use, such records are normal :-/
rhboot/shim#642
lnussel added a commit to lnussel/systemd that referenced this issue Mar 14, 2024
Don't mark records with empty device path as invalid. When shim is in
use such records are normal :-/
rhboot/shim#642
lnussel added a commit to lnussel/systemd that referenced this issue Mar 14, 2024
Don't mark records with empty device path as invalid. When shim is in
use such records are normal :-/
rhboot/shim#642
lnussel added a commit to lnussel/systemd that referenced this issue Apr 3, 2024
Don't mark records with empty device path as invalid. When shim is in
use such records are normal :-/
rhboot/shim#642
lnussel added a commit to lnussel/systemd that referenced this issue Apr 4, 2024
Don't mark records with empty device path as invalid. When shim is in
use such records are normal :-/
rhboot/shim#642
bluca pushed a commit to lnussel/systemd that referenced this issue Apr 22, 2024
Don't mark records with empty device path as invalid. When shim is in
use such records are normal :-/
rhboot/shim#642
lnussel added a commit to lnussel/systemd that referenced this issue Apr 29, 2024
Don't mark records with empty device path as invalid. When shim is in
use such records are normal :-/
rhboot/shim#642
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants