generate-efi : only sign what has been updated

[ShellCode33]

Currently the pacman hook will re-sign all the EFI binaries if a single one of them is updated.

It would be nice to only sign what has been updated.

alpm-hooks provides a NeedsTargets which according to the man page does the following:

Causes the list of matched trigger targets to be passed to the running hook on stdin.

Currently the hook looks like this :

https://github.com/maximbaz/arch-secure-boot/blob/928c6981138f33a733c8d8c2347a948d46369f5f/pacman-hooks/95-arch-secure-boot-generate-efi.hook#L9-L12

Instead we could do something like this :

Exec = /usr/bin/arch-secure-boot generate-efi --from-stdin

Or maybe just the standard trailing -.

That way generate-efi would be able to sign only the updated EFI image, instead of signing them all.

To consider: an update of *-ucode.img must trigger the recreation of $NAME-unsigned.efi + signing it

[maximbaz]

While that is certainly true, I would like to ask what value do you think it would provide? I imagine it would be quite a big complexity increase to correctly keep track of which dependencies should regenerate and resign which .efi files :thinking:

To give you the context for why I'm asking this, I strongly believe that the less code is involved in the critical part of the system, the boot process, the less chances there are to make accidental bugs that can be used for security breaches, and the easier it is for users to actually read and review the code, instead of just hoping that "if it's open source, it must be secure, surely someone looked at the code already".

That's also the reason for some opinionated choices, e.g. the hardcoded combos of generated .efi files :slightly_smiling_face:

(That's not to say that we can't add any new features, only that I'd like to consider pros & cons perhaps a bit more carefully than I'd normally do for other projects :slightly_smiling_face:)

[ShellCode33]

only that I'd like to consider pros & cons perhaps a bit more carefully than I'd normally do for other projects

Totally understandable and reassuring ;)

I do share your taste for minimalism, especially in such an early stage of the boot process. And you're definitely right that the less code there is, the less likely it is that bugs will sneak in. This is not a "must do" issue, it was more of a suggestion, to see what you think. However I'm not sure I agree with you on the fact that it would be a big complexity increase, the logic would be something among those lines :

files_to_update = stdin

if empty(files_to_update) {
     files_to_update = ["vmlinuz-linux", "vmlinuz-linux-lts", "etc"]
}

if "*-ucode" in files_to_update || "vmlinuz-linux" in files_to_update {
    objcopy vmlinuz-linux
    sign vmlinuz-linux
}

if "vmlinuz-linux-lts" in files_to_update {
    objcopy vmlinuz-linux-lts
    sign vmlinuz-linux-lts
}

if "efi-shell" in files_to_update {
    objcopy efi-shell
    sign efi-shell
}

if "fwupd" in files_to_update {
    sign fwupd
}

Looks pretty straightforward to me :) It would even remove the loops after that because signing would be inlined in each condition, increasing readability and maintainability.

The added value is better performance (even if it's probably insignificant), and a more (I believe) robust approach.

Let's imagine today's a really bad day, and you have a power failure right here on line 105:

https://github.com/maximbaz/arch-secure-boot/blob/928c6981138f33a733c8d8c2347a948d46369f5f/arch-secure-boot#L102-L110

Your PC goes kaboom. Whereas if you update only what needs to be updated, this is more unlikely to happen I guess. This is probably a bad exemple but I believe that having the lowest footprint possible on the system is the right thing to do in such situation.

Btw what I just said about the divine power failure is I think a good case for removing the find rm, I didn't think of that before. Obviously this is unlikely to happen, but as you said, touching to such critical components require that we take extra care and make things as robust as possible.

PS1: It made me notice that usr/lib/fwupd/efi/fwupdx64.efi should probably be added to the pacman hook as well

PS2: Someone on #archlinux:matrix.org sent this, I thought it would be of some interest to you :

• https://github.com/systemd/systemd/pull/25180
• https://github.com/systemd/systemd/blob/main/src/ukify/ukify.py

[maximbaz]

However I'm not sure I agree with you on the fact that it would be a big complexity increase, the logic would be something among those lines

I agree with you that it's easy to read, what I had in mind was more about cyclomatic complexity (the number of independent paths a code can take), which grows from 1 (one test-case can cover everything) to 2*number_of_conditons=12 (...right? :thinking: every condition, including every OR, can result in two outcomes for the body execution). Cyclomatic complexity is often a source of bugs not because it's difficult to read, but because people don't test all combinations, I feel like it does increase the maintenance cost...

Not saying that I am against this, just wanted to clarify what I am afraid of :slightly_smiling_face: At the same time, to tell you completely honestly, I'm not quite sold yet.

Btw what I just said about the divine power failure is I think a good case for removing the find rm, I didn't think of that before.

This sounds important, I like this argument. Even if it's theoretical, let's not increase any chances of getting an unbootable system!

What do you think about excluding the newly generated files from find, for example using -prune command? The copy would become atomic operation (... kinda...) like it used to be, and we will still cleanup any garbage we produce.

PS1: It made me notice that usr/lib/fwupd/efi/fwupdx64.efi should probably be added to the pacman hook as well

:+1:

PS2: Someone on #archlinux:matrix.org sent this, I thought it would be of some interest to you :

Thanks for sharing! Looks interesting, and very feature complete! And big :grin: But regardless, I think it's a very useful source, if nothing else, to take inspiration from on how things ought to be done!

[ShellCode33]

I'm not quite sold yet

Alright, no problem with that, even if I don't think that's the choice I would have made, I completely get your point and it's a sensible one. Plus, in the end I really don't mind. Though you're only considering the cyclomatic complexity of your script and not the one of the external programs you call ! Their code is way bigger and certainly has many potential points of failure. In my book, calling ~15 programs instead of ~3, it's a pretty big increase of the overall cyclomatic complexity :stuck_out_tongue_winking_eye: Even if those 15 programs are the same, there are many external factors that could cause it to go wrong (OOM, FS failure, config file change, signal handling, etc.)

---

What do you think about excluding the newly generated files from find, for example using -prune command?

Sounds great ! The best of both worlds

---

Thanks for sharing! Looks interesting, and very feature complete! And big :grin:

(interesting out of context quote)

They have unit tests though :grin: But true, this is why I will stick to your tool for now. Even if I have to admit that if it's shipped by default along side systemd, I might consider it at some point... I'd like to keep the number of installed AUR packages as low as possible for safety reasons (even if I tend to review the AUR code I install, I could definitely miss some nasty lines of code hidden somewhere in a random GitHub repo)