stub: allow loading and verifying kernel command line addons #27358

bluca · 2023-04-22T01:56:54Z

No description provided.

bluca · 2023-04-22T01:57:39Z

@medhefgo this works when going directly shim -> uki, but when going shim -> sd-boot -> uki the shim protocol is no longer available, do we need to do something to keep it around?

src/boot/efi/pe.c

medhefgo · 2023-04-22T07:28:57Z

@medhefgo this works when going directly shim -> uki, but when going shim -> sd-boot -> uki the shim protocol is no longer available, do we need to do something to keep it around?

That's just one of the many delights that shim brings us. It will always uninstall the protocol before executing the image (and then also fail to reinstall it if it happens to return successfully). And you can't insert an extra shim in between as it kills auto-detection of UKIs. Best thing is to get shim to change or call shim from a known location with the UKI cmdline as arguments.

This code assumes use of shim, but what about support for custom sb keys? Also, I really don't see how this code is any better than having a cmdline allowlist with globs.

bluca · 2023-04-22T11:10:26Z

@medhefgo this works when going directly shim -> uki, but when going shim -> sd-boot -> uki the shim protocol is no longer available, do we need to do something to keep it around?

That's just one of the many delights that shim brings us. It will always uninstall the protocol before executing the image (and then also fail to reinstall it if it happens to return successfully). And you can't insert an extra shim in between as it kills auto-detection of UKIs. Best thing is to get shim to change or call shim from a known location with the UKI cmdline as arguments.

Interesting, thanks, I had no idea - I've asked the shim folks, we'll see what they say

This code assumes use of shim, but what about support for custom sb keys?

Is there a way to get the firmware to validate a PE (without actually executing it) in case there's no first stage?

Also, I really don't see how this code is any better than having a cmdline allowlist with globs.

Fully cryptographically verified payload to me is better than partially or not verified payload. Even if allowlist happens, I don't think it would disqualify supporting this use case, they could both be supported.

bluca · 2023-04-23T10:22:12Z

This code assumes use of shim, but what about support for custom sb keys?

Is there a way to get the firmware to validate a PE (without actually executing it) in case there's no first stage?

I ask because I tried to use LoadImage, which naively I thought should suffice, but I'm getting "invalid parameter" out of it

bluca · 2023-04-23T12:37:34Z

This code assumes use of shim, but what about support for custom sb keys?

Is there a way to get the firmware to validate a PE (without actually executing it) in case there's no first stage?

I ask because I tried to use LoadImage, which naively I thought should suffice, but I'm getting "invalid parameter" out of it

Never mind, turns out I am an eejit. Works now - if there's no shim or shim validation fails, falls back to LoadImage.

src/boot/efi/meson.build

src/boot/efi/shim.c

src/boot/efi/stub.c

src/boot/efi/boot.c

src/boot/efi/stub.c

poettering · 2023-04-24T11:43:15Z

also needs docs for the sd-stub addition

If the SHIM_RETAIN_PROTOCOL variable is set, avoid uninstalling our protocol. For example, this allows sd-stub in a UKI to use the shim protocol to validate PE binaries, even if it is executed by a second stage, before the kernel is loaded. Example use case in sd-boot/sd-stub: systemd/systemd#27358

If the SHIM_RETAIN_PROTOCOL variable is set, avoid uninstalling our protocol. For example, this allows sd-stub in a UKI to use the shim protocol to validate PE binaries, even if it is executed by a second stage, before the kernel is loaded. Example use case in sd-boot/sd-stub: systemd/systemd#27358 Signed-off-by: Luca Boccassi <bluca@debian.org>

If the SHIM_RETAIN_PROTOCOL variable is set, avoid uninstalling our protocol. For example, this allows sd-stub in a UKI to use the shim protocol to validate PE binaries, even if it is executed by a second stage, before the kernel is loaded. Ensure that the variable is volatile and for BootServices access. Example use case in sd-boot/sd-stub: systemd/systemd#27358 Signed-off-by: Luca Boccassi <bluca@debian.org>

If the SHIM_RETAIN_PROTOCOL variable is set, avoid uninstalling our protocol. For example, this allows sd-stub in a UKI to use the shim protocol to validate PE binaries, even if it is executed by a second stage, before the kernel is loaded. Ensure that the variable is volatile and for BootServices access. Also delete it on startup, so that we can be sure it was really set by a second stage. Example use case in sd-boot/sd-stub: systemd/systemd#27358 Signed-off-by: Luca Boccassi <bluca@debian.org>

If the ShimRetainProtocol variable is set, avoid uninstalling our protocol. For example, this allows sd-stub in a UKI to use the shim protocol to validate PE binaries, even if it is executed by a second stage, before the kernel is loaded. Ensure that the variable is volatile and for BootServices access. Also delete it on startup, so that we can be sure it was really set by a second stage. Example use case in sd-boot/sd-stub: systemd/systemd#27358 Signed-off-by: Luca Boccassi <bluca@debian.org>

poettering · 2023-05-24T13:00:44Z

I can deploy with binary with a sbat section that mentions any version I like, such as 99999999+1. sbat is about revoking past signed binaries, not stopping new binaries from being signed.

And so what? not sure what that has to do with anything?

The point i was making is that sbat data allows blocking any kind of old resources and concepts, not just one kind of sw package. the first line in sbat is the version of the sbat spec basically, and that means you can block the concept that "sbat version 1" is. that's the key take-away.

Files placed in /EFI/Linux/UKI.efi.extra.d/ and /loader/addons/ are opened and verified using the LoadImage protocol, and will thus get verified via shim/firmware. If they are valid signed PE files, the .cmdline section will be extracted and appended. If there are multiple addons in each directory, they will be parsed in alphanumerical order. Optionally the .uname sections are also matched if present, so that they can be used to filter out addons as well if needed, and only addons that correspond exactly to the UKI being loaded are used. It is recommended to also always add a .sbat section to addons, so that they can be mass-revoked with just a policy update. The files must have a .addon.efi suffix. Files in the per-UKI directory are parsed, sorted, measured and appended first. Then, files in the generic directory are processed.

bluca · 2023-05-24T14:06:27Z

To paraphrase, "everything you sign can and will be used against you in a court of law". IE: don't sign stuff that can be used to screw you, and if you realize too late, revoke it via hash/cert/sbat.

Well, I think that's too simplistic a view. It's hard to do things right if noone tells you all the pitfalls, even if you want to. People generally are not aware that signing components doesn't imply they are necessarily used in the combination and under the name the signer originally intended them to be used.

At the most basic the docs should discuss these kinds of attacks clearly.

The fact that sbat exists is not an excuse to not document things, and not an excuse to not give a fuck anymore about vulnerabilities with combining signed things in unexpected ways..

No issue with documenting stuff - updated now. Using measurements is not only about measured boot, but also attestation - if someone is playing games with ordering of addons, PCR12 will change, and an attestation service will notice it.

poettering · 2023-05-25T10:55:59Z

still think we should quickly add osrel matching. but anyway, can come later. PR looks good. lets merge.

esposem · 2023-05-30T12:19:50Z

@bluca I have some issues running this stuff. In particular, it seems that shim_load_image does not like the addons. Even though I tested it and shim_validate returns always EFI_SUCCESS. On the other side, shim_load_image returns EFI_ACCESS_DEINED. Opinions?

bluca · 2023-05-30T12:35:16Z

How are you testing it? IE, what's the chain, which shim version, and how did you create the addon

esposem · 2023-05-30T12:39:39Z

chain is UEFI -> shim -> systemd-stub -> vmlinuz
shim is shim-x64-15.6-1.el9.x86_64
addons are created for testing using this script https://gitlab.com/eesposit/eesposit-scripts/-/blob/main/qemu_machine/create_test_cmdline_pe.sh
devel.efi is my UKI created with https://gitlab.com/eesposit/eesposit-scripts/-/blob/main/qemu_machine/load_efi_devel.sh
Note that when I initially acked this PR (when you were just using shim_validate) it worked.

bluca · 2023-05-30T12:40:58Z

What if you add sd-boot after shim?

bluca · 2023-05-30T12:42:31Z

also you are creating the addon with the linux stub - use the addon stub instead

medhefgo · 2023-05-30T12:42:43Z

shim-x64-15.6-1.el9.x86_64 probably does not have the protocol retaining patch.

esposem · 2023-05-30T12:42:46Z

I don't think we plan to use sd-boot atm

esposem · 2023-05-30T12:43:46Z

how do I generate the addon stub?

bluca · 2023-05-30T12:44:52Z

sure, but it's to figure out what's the difference in the setups, for me shim -> sd-boot -> sd-stub -> addon works, and also sd-boot -> sd-stub -> addon works

the addon stub is built automatically on latest main after this PR, it's in the same build dir as the linux one

esposem · 2023-05-30T13:00:37Z

so: using the addon stub doesn't change the outcome, still access denied :/
@medhefgo I can try downloading a newer shim, but I am not sure if that's the issue. If I understand correctly, that is to make sure that shim_validate is still present, and it is since I can also call the function myself from the stub. Something is weird though, since when I call it myself it returns 0 even when I provide an unsigned PE. Weird, since it used to work before (using the first versions of the PR)...
@bluca could you provide the steps/a guide on how to add sd-boot between shim and sd-stub?

esposem · 2023-05-30T13:07:01Z

actually I got confused, also shim_validate returns 0 (meaining validation was not successful)...
shim_validate returns 1 on success, 0 on failure
but shim_load_image returns 0 (EFI_SUCCESS) on success, != 0 on failure

bluca · 2023-05-30T13:30:49Z

You need to build shim from the main branch, then install systemd-boot normally with bootctl, and install shim as the entry point, the ESP layout will look like:

/tmp/img/boot/
/tmp/img/boot/EFI
/tmp/img/boot/EFI/systemd
/tmp/img/boot/EFI/systemd/mmx64.efi
/tmp/img/boot/EFI/systemd/shimx64.efi
/tmp/img/boot/EFI/systemd/BOOTX64.CSV
/tmp/img/boot/EFI/systemd/systemd-bootx64.efi
/tmp/img/boot/EFI/Linux
/tmp/img/boot/EFI/Linux/mkosi-debian-6.1.0-9-cloud-amd64.efi
/tmp/img/boot/EFI/BOOT
/tmp/img/boot/EFI/BOOT/mmx64.efi
/tmp/img/boot/EFI/BOOT/BOOTX64.EFI
/tmp/img/boot/EFI/BOOT/fbx64.efi
/tmp/img/boot/loader
/tmp/img/boot/loader/entries.srel
/tmp/img/boot/loader/random-seed
/tmp/img/boot/loader/entries
/tmp/img/boot/loader/addons
/tmp/img/boot/loader/addons/test4.addon.efi
/tmp/img/boot/loader/addons/test2.addon.efi
/tmp/img/boot/loader/addons/test5.addon.efi
/tmp/img/boot/loader/addons/test.addon.efi
/tmp/img/boot/loader/addons/test3.addon.efi
/tmp/img/boot/loader/loader.conf
/tmp/img/boot/loader/keys
/tmp/img/boot/loader/keys/auto
/tmp/img/boot/loader/keys/auto/PK.auth
/tmp/img/boot/loader/keys/auto/db.auth
/tmp/img/boot/loader/keys/auto/KEK.auth

bluca · 2023-05-30T13:32:09Z

and shim is built with DEFAULT_LOADER=\\\\systemd-bootx64.efi EFIDIR=systemd

esposem · 2023-05-30T15:26:58Z

Ok apparently the issue was with ukify. Giving --stub=sthing is not the same as --stub sthing. And it wasn't printing any kind of error or wrong pe. Weird. Thanks for the help!

bluca · 2023-05-30T16:10:05Z

mmh that sounds wrong @keszybz shouldn't --stub=foo and --stub foo both work?

esposem · 2023-05-31T11:31:33Z

Anyways, I am happy to confirm that this works using shim->stub (without sd-boot) 🎉
There's some minor things to modify, probably in ukify but I'll open issues/PR for that.
When you have a minute, please review #27621

If the ShimRetainProtocol variable is set, avoid uninstalling our protocol. For example, this allows sd-stub in a UKI to use the shim protocol to validate PE binaries, even if it is executed by a second stage, before the kernel is loaded. Ensure that the variable is volatile and for BootServices access. Also delete it on startup, so that we can be sure it was really set by a second stage. Example use case in sd-boot/sd-stub: systemd/systemd#27358 Signed-off-by: Luca Boccassi <bluca@debian.org>

bluca added the sd-boot/sd-stub/bootctl label Apr 22, 2023

bluca requested a review from medhefgo April 22, 2023 01:56

github-actions bot added meson uki labels Apr 22, 2023

bluca commented Apr 22, 2023

View reviewed changes

src/boot/efi/pe.c Outdated Show resolved Hide resolved

bluca mentioned this pull request Apr 22, 2023

ukify: allow building PE mule #27362

Merged

bluca force-pushed the pe_mule branch from ed76319 to 7b0e7af Compare April 23, 2023 10:21

github-actions bot added the documentation label Apr 23, 2023

bluca force-pushed the pe_mule branch from 7b0e7af to b5cd36b Compare April 23, 2023 12:35

bluca force-pushed the pe_mule branch from b5cd36b to 8608959 Compare April 23, 2023 12:47

medhefgo reviewed Apr 24, 2023

View reviewed changes

src/boot/efi/meson.build Outdated Show resolved Hide resolved

src/boot/efi/shim.c Outdated Show resolved Hide resolved

src/boot/efi/shim.c Outdated Show resolved Hide resolved

src/boot/efi/stub.c Outdated Show resolved Hide resolved

poettering reviewed Apr 24, 2023

View reviewed changes

src/boot/efi/boot.c Outdated Show resolved Hide resolved

poettering reviewed Apr 24, 2023

View reviewed changes

src/boot/efi/stub.c Outdated Show resolved Hide resolved

poettering reviewed Apr 24, 2023

View reviewed changes

src/boot/efi/stub.c Outdated Show resolved Hide resolved

poettering reviewed Apr 24, 2023

View reviewed changes

src/boot/efi/stub.c Show resolved Hide resolved

poettering added the needs-rebase label Apr 24, 2023

bluca mentioned this pull request Apr 25, 2023

Optionally allow to keep shim protocol installed rhboot/shim#565

Merged

bluca force-pushed the pe_mule branch from 8608959 to b8285cd Compare April 25, 2023 23:39

bluca force-pushed the pe_mule branch from 0ff18c1 to 4c36a4a Compare May 24, 2023 13:55

bluca added 2 commits May 24, 2023 15:02

TODO: remove fixed item

f19b627

bluca force-pushed the pe_mule branch from 4c36a4a to f19b627 Compare May 24, 2023 14:02

poettering merged commit be2e631 into systemd:main May 25, 2023
46 of 47 checks passed

github-actions bot removed the please-review PR is ready for (re-)review by a maintainer label May 25, 2023

bluca deleted the pe_mule branch May 25, 2023 11:01

poettering mentioned this pull request Jun 2, 2023

EFI Stub: Support for hashed external UKI components #26096

Open

Itxaka mentioned this pull request Oct 3, 2023

Improve uki iso stuff kairos-io/kairos#1854

Merged

ElvishJerricco mentioned this pull request Nov 29, 2023

nixos/systemd-boot: add option to boot from systemd-stub NixOS/nixpkgs#265273

Open

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stub: allow loading and verifying kernel command line addons #27358

stub: allow loading and verifying kernel command line addons #27358

bluca commented Apr 22, 2023

bluca commented Apr 22, 2023

medhefgo commented Apr 22, 2023

bluca commented Apr 22, 2023

bluca commented Apr 23, 2023 •

edited

bluca commented Apr 23, 2023

poettering commented Apr 24, 2023

poettering commented May 24, 2023

bluca commented May 24, 2023

poettering commented May 25, 2023

esposem commented May 30, 2023

bluca commented May 30, 2023

esposem commented May 30, 2023

bluca commented May 30, 2023

bluca commented May 30, 2023

medhefgo commented May 30, 2023

esposem commented May 30, 2023

esposem commented May 30, 2023

bluca commented May 30, 2023

esposem commented May 30, 2023

esposem commented May 30, 2023 •

edited

bluca commented May 30, 2023

bluca commented May 30, 2023

esposem commented May 30, 2023 •

edited

bluca commented May 30, 2023

esposem commented May 31, 2023

stub: allow loading and verifying kernel command line addons #27358

stub: allow loading and verifying kernel command line addons #27358

Conversation

bluca commented Apr 22, 2023

bluca commented Apr 22, 2023

medhefgo commented Apr 22, 2023

bluca commented Apr 22, 2023

bluca commented Apr 23, 2023 • edited

bluca commented Apr 23, 2023

poettering commented Apr 24, 2023

poettering commented May 24, 2023

bluca commented May 24, 2023

poettering commented May 25, 2023

esposem commented May 30, 2023

bluca commented May 30, 2023

esposem commented May 30, 2023

bluca commented May 30, 2023

bluca commented May 30, 2023

medhefgo commented May 30, 2023

esposem commented May 30, 2023

esposem commented May 30, 2023

bluca commented May 30, 2023

esposem commented May 30, 2023

esposem commented May 30, 2023 • edited

bluca commented May 30, 2023

bluca commented May 30, 2023

esposem commented May 30, 2023 • edited

bluca commented May 30, 2023

esposem commented May 31, 2023

bluca commented Apr 23, 2023 •

edited

esposem commented May 30, 2023 •

edited

esposem commented May 30, 2023 •

edited