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

Do not sign in-place, always require the "-o" parameter. #228

Open
conrad-heimbold opened this issue Jul 5, 2023 · 5 comments
Open

Do not sign in-place, always require the "-o" parameter. #228

conrad-heimbold opened this issue Jul 5, 2023 · 5 comments

Comments

@conrad-heimbold
Copy link

conrad-heimbold commented Jul 5, 2023

The README.md mentions signing EFI files on the EFI system partition (ESP) in-place:

# sbctl sign -s /efi/EFI/BOOT/BOOTX64.EFI
✔ Signed /efi/EFI/BOOT/BOOTX64.EFI...

# sbctl sign -s /efi/EFI/arch/fwupdx64.efi
✔ Signed /efi/EFI/arch/fwupdx64.efi...

# sbctl sign -s /efi/EFI/systemd/systemd-bootx64.efi
✔ Signed /efi/EFI/systemd/systemd-bootx64.efi...

I think this is bad and should be avoided and discouraged:

  • first of all, because overwriting files is always a bad idea. At least make a backup!
  • second of all, because this is bad in terms of security: the files on the EFI system partition can be manipulated, because they are not encrypted (when someone else had your laptop and replaced these files; you then sign the manipulated files, thereby defeating the whole purpose of secure boot). Do not use the files on the ESP for signing; instead always use the files from /boot or /usr (which can be encrypted, so protected).
  • third of all, because the files on the EFI system partition can be outdated (if for example an update failed). The files inside /usr however always have the latest locally installed version.

Signing in-place might be easier; but in the long run, it just introduces more problems; I think.

The README.md should therefore instead suggest (if BOOTX64.EFI is systemd-bootx64.efi):

# sbctl sign -s /usr/lib/systemd/boot/efi/ -o /efi/EFI/BOOT/BOOTX64.EFI  
✔ Signed /efi/EFI/BOOT/BOOTX64.EFI...

# sbctl sign -s /usr/lib/fwupd/efi/fwupdx64.efi -o /efi/EFI/arch/fwupdx64.efi 
✔ Signed /efi/EFI/arch/fwupdx64.efi... 

# sbctl sign -s /usr/lib/systemd/boot/efi/ -o /efi/EFI/systemd/systemd-bootx64.efi 
✔ Signed /efi/EFI/systemd/systemd-bootx64.efi...

Here are some locations on ArchLinux if you are searching for where to find them:

  • KeyTool.efi comes from /usr/share/efitools/efi/KeyTool.efi
  • systemd-bootx64.efi comes from /usr/lib/systemd/boot/efi/systemd-bootx64.efi
  • fwupdx64.efi comes from /usr/lib/fwupd/efi/fwupdx64.efi
  • linux.efi usually comes from /boot/vmlinuz-linux
  • linux-hardened.efi usually comes from /boot/vmlinuz-linux-hardened
  • shimx64.efi comes from /usr/share/shim-signed/shimx64.efi
  • mmx64.efi (MokManager) comes from /usr/share/shim-signed/mmx64.efi
@conrad-heimbold conrad-heimbold changed the title Do not sign "in-place", always require the "-o" parameter. Do not sign in-place, always require the "-o" parameter. Jul 5, 2023
@gilbsgilbs
Copy link

I second this. The README currently encourages signing images in-place on the EFI partition and add them to sbctl database¹ which is a high security risk as it could lead to signing arbitrary files. A malicious actor could replace an unused image with a rogue one and patiently wait for the user to trigger sbctl sign-all (for example, after a kernel update, thanks to the pacman hook on Arch).

I think the best practice would be to always keep the unsigned images in a secure, encrypted, partition, and have sbctl sign the trusted unsigned file into the EFI partition. This way, the signed image could always be trusted.

Preventing in-place signing would indeed be preferable for all the reasons you mentioned, but insufficient as it wouldn't prevent signing untrusted files. At very least, sbctl should display a warning when signing a file from an unencrypted partition, and certainly not encourage such practice in the README.

@Foxboron
Copy link
Owner

Foxboron commented Sep 4, 2023

Changing a 3 year old README is not high up on my priority list of "things I'll work work on soon'ish". So please send patches if you would like to see this changed.

Some of these files have their own "move signed file into ESP" semantics with a .signed suffix on the files.

@conrad-heimbold
Copy link
Author

Sorry if I sounded harsh or rude. I am already working on a patch.

@Ferdi265
Copy link

Ferdi265 commented Dec 22, 2023

Depending on the hooks that users/distros have set up for generating UKIs and signing EFI images provided by packages, the only thing that might be needed is an option to skip signing EFI images on the ESP when running sign-all, and emitting a warning when images on the ESP are signed in-place.

This would likely be a simple-to-implement alternative that avoids breaking existing users, while still giving users that care a way to be secure.

@00-kat
Copy link
Contributor

00-kat commented Apr 24, 2024

By the way, this isn't only in the the README, it recommends signing in-place in the manpage too.

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

5 participants