Doesn't work yet but opening a draft PR in case some UEFI guru sees this and can help out with getting this working.

The accompanying PR in mkosi: systemd/mkosi#651

So the general approach is as follows: we use sbsiglist and sbvarsign in mkosi to write authenticated EFI signature list files to the EFI partition that we should in theory be able to pass directly into EFI's SetVariable() call. The current sd-boot code in this PR tries to do just add. Initially, when trying to enroll the platform key, I got invalid parameter errors until I added the EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS flag which turned the error into "Security Policy Violation".

In section 7.2.1 of the UEFI spec (2.6) https://uefi.org/sites/default/files/resources/UEFI%20Spec%202_6.pdf, it's mentioned that this happens because some validation checks might have failed.

The driver shall update the value of the variable only if all of these checks pass. If any of the checks fails, firmware must return EFI_SECURITY_VIOLATION

However, that same section also mentions that the checks should be ignored if SetupMode == 1

3. If the variable SetupMode==1, and the variable is a secure boot policy variable, then the firmware implementation shall consider the checks in the following steps 4 and 5 to have passed, and proceed with updating the variable value as outlined below

I've verified that SetupMode is indeed 1 when launching mkosi qemu which makes me wonder why we're still getting this error.

Another thing is that looking at the latest UEFI spec (https://uefi.org/sites/default/files/resources/UEFI%20Spec%202.8B%20May%202020.pdf) section 32.3.1 on enrolling the platform key, that section refers to section 8.2.1 for information on how to use SetVariable to enroll the platform key. 8.2.1 tells us to use EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS to enroll the platform key. However, this flag requires the data to be formatted in a different way than what is produced by sbvarsign.

My next steps are to try and figure out which version of the UEFI spec OVMF implements and to try enrolling the mkosi Qemu VM into Secure Boot using sbsigntools from the VM after booting (using efivarfs). That ensures that the problem is in our code and not because of some incompatibility between sbsigntools and OVMF.

Of course, any hints on what I'm missing that cause this to fail are much appreciated.

Isn't there a specific order certs have to be enrolled? PK -> KEK -> DB? Going from memory, it's been a while since I worked on SB

I got it working by using the binaries from efitools instead of sbsigntools to create the EFI signature lists. Cleaning up the PR now for review.

Test plan:

# checkout sb-esl branch on my mkosi fork
# checkout sd-boot-enroll-sb branch on my systemd fork
cd systemd
# Add SecureBoot=yes to a dropin file in mkosi.default.d/
mkosi -f genkey
mkosi -f qemu
# In VM
bootctl status
# Should show secure boot enabled and in user mode

So the current code works well if there's just one PK, KEK and db key to enroll. The problem with enrolling more than one key comes from the fact that to enroll these keys, they have to be authenticated (signature signed with the relevant key has to be provided to the UEFI firmware when updating one of the secure boot key variables). We generate these authenticated ESLs with sign-efi-sig-list from the efitools package. Unfortunately, the attributes used in the SetVariable call are part of this signature including the EFI_VARIABLE_APPEND_WRITE flag so we can't use that to append to one of these variables. Even if we tried to retrieve the current value and do the append ourselves, It seems like we'd have to recalculate the signature in sd-boot itself which seems rather hard given we don't have OpenSSL available there.

So my suggestion would be that we don't support combining multiple keys in sd-boot into a single ESL but instead, we leave it to the tools like mkosi to do this before putting the ESL in /efi/loader/secure-boot. That makes the logic in sd-boot simple, we just iterate over the ESLs and overwrite the previous value of the UEFI variable every time.

I'm also not entirely sure to do with partial keys in the secure-boot directory. For example, if we only find a kek.esl file, do we still try to enroll it or do we expect all three necessary keys to be provided (PK, ESL, db)?

I was also thinking of providing an enroll verb in bootctl that would combine the functionality of cert-to-efi-sig-list and sign-efi-sig-list so that eventually we can remove the efitools dependency in mkosi.

Would appreciate any thoughts here before I continue with the implementation.

For example, if we only find a kek.esl file, do we still try to enroll it or do we expect all three necessary keys to be provided (PK, ESL, db)?

Only a KEK or db cert is fine, as they are additive - but a new PK without anything else means an non-bootable system, so perhaps that's a good condition to check?

BTW support for enrolling certs/hashes in DBX would be good too

It seems that in setup mode, the signature validation is ignored by the firmware so we could actually use EFI_VARIABLE_APPEND_WRITE since we're guaranteed to be in setup mode anyway when this code is executed. I'll just need to modify the logic to enroll the PK last since the firmware transfers out of setup mode once the PK is enrolled.

Isn't there a specific order certs have to be enrolled? PK -> KEK -> DB? Going from memory, it's been a while since I worked on SB

afaik PK cert must be enrolled last. At that point the UEFI comes out of "setup mode".

edit, found at least one reference to the claim (but there are others): https://wiki.archlinux.org/index.php/Unified_Extensible_Firmware_Interface/Secure_Boot#Enrolling_keys_in_firmware

The order doesn't matter as long as the variable writes are signed correctly which sign-efi-sig-list does for us. But after the PK is enrolled, we exit setup mode so the signature of the authenticated variable is checked and has to be correct. In practice this means that after the PK is enrolled, we can't use EFI_VARIABLE_APPEND_WRITE anymore unless we explicitly configured sign-efi-sig-list to configure the signature for usage with EFI_VARIABLE_APPEND_WRITE. We can get around this by just configuring the PK last as before that happens, the signatures aren't checked by the firmware. I'll push an update to make sure we enroll the PK last so we can use EFI_VARIABLE_APPEND_WRITE for the KEK and db variables without having to worry whether the authenticated variable we're loading was explicitly configured for usage with EFI_VARIABLE_APPEND_WRITE.

I hope there will be some documentation before this is merged, but if I understand the code correctly, if the UEFI/Secureboot is in Setup mode, systemd-boot will automatically enroll the keys in $ESP/loader/secure-boot/*.esl right?

And yeah, it sounds like we should install the keys in some well defined order, to make this well-defined. I mean, the DB backing store might have a size limit, and we should probably handle that reasonably gracefully and in that case install the keys with earlier names. that way users can name their key failes 00-xyz.esl if they are important, and 99-xyz.esl if least important, if they want.

(you could add another commit to this, which drops the entry about this from TODO. The entry suggests using X.509, but i guess that is obsolete. it's one of the first few entries there)

I'd really like to see handling of DBX too (here or as a follow-up), as it's really needed for a complete solution

I'm also not entirely sure to do with partial keys in the secure-boot directory. For example, if we only find a kek.esl file, do we still try to enroll it or do we expect all three necessary keys to be provided (PK, ESL, db)?

Make it simple I'd say, just expect the stuff to be in order.

@bluca Preferably SBAT support should be added as dbx currently doesn't scale with the amount of shims that needs to be revoked.

https://github.com/rhboot/shim/blob/main/SBAT.md

(This is probably worthy of it's own issue I believe)

And generally speaking, the order of db and KEK does not matter. But it's important that PK is last so SetupMode is left. I'll follow the thread and help test the patches when they are ready :)

https://github.com/rhboot/shim/blob/main/SBAT.md

sbat support has been merged to git master.

@DaanDeMeyer any update on this?

@DaanDeMeyer should we close this one in favour of #20255? Currently work on both PRs unfortunately stalled. :-(

Yeah sorry about that. If #20255 isn't merged I might pick this up again next year. For now we can close this as it's just creating noise and I don't think I'll have the time to continue working on this in the near future.