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

Add export-enrolled-keys command #303

Merged
merged 1 commit into from
May 7, 2024

Conversation

jimmykarily
Copy link
Contributor

This PR introduces an export-enrolled-keys command that allows the user to export the already enrolled keys to a directory for backup reasons (e.g. before making any changes on new hardware) and to prepare a directory that can be used as a "custom certs" when enrolling keys.

User scenario:

  • User buys some new hardware with MS and vendor keys already enrolled
  • User backs up the built-in keys using this new command
  • User generates new keys (intended to sign their own OSes)
  • User puts the machine in "setup mode" by deleting PK, KEK and db from bios
  • User enrolls the new keys including the original ones as "custom keys"

This PR is only one of the things needed to implement the above scenario. Currently the export directory structure is the one expected by the enroll-keys command but the location is arbitrary. The user would have to copy the directory to the appropriate location (the db directory) in order to be read by the "enroll-keys" command.

In my opinion, it's the enroll-keys command that should be changed to be more flexible and read keys from arbitrary locations but this can be discussed on another ticket/PR.

Note: I would have written a test for this but waiting for this refactoring first: #302

I'm opening this draft PR to collect some early feedback, until I can write the tests.

@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 25, 2024 13:50 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 25, 2024 13:50 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 25, 2024 13:50 — with GitHub Actions Inactive
@Foxboron
Copy link
Owner

We already have sbctl enroll-keys --export which exports the keys sbctl intends to enroll into esl or auth files. I'm wondering if we should try to mold this into something you can use instead of adding another command.

Would sbctl enroll-keys --export der be a thing you could use?

@jimmykarily
Copy link
Contributor Author

jimmykarily commented Apr 26, 2024

We already have sbctl enroll-keys --export which exports the keys sbctl intends to enroll into esl or auth files. I'm wondering if we should try to mold this into something you can use instead of adding another command.

Would sbctl enroll-keys --export der be a thing you could use?

I may be missing something but the use case of the new command is to export the build-in certificates from brand new hardware. I thought that maybe a combination of flags of the enroll-keys could achieve the same thing but it doesn't seem to be the case:

~/workspace/sbctl (export-enrolled-keys)*$ sudo go run ./cmd/sbctl enroll-keys --firmware-builtin --export esl
Found OptionROM in the bootchain. This means we should not enroll keys into UEFI without some precautions.

There are three flags that can be used:
    --microsoft: Enroll the Microsoft OEM certificates into the signature database.
    --tpm-eventlog: Enroll OpRom checksums into the signature database (experimental!).
    --yes-this-might-brick-my-machine: Ignore this warning and continue regardless.

Please read the FAQ for more information: https://github.com/Foxboron/sbctl/wiki/FAQ#option-rom
exit status 1

I could try setting --yes-this-might-brick-my-machine but even if this worked, it still feels wrong. Exporting keys is a non mutating action and shouldn't print any warnings at all (or required "dangerous" flags).

Kairos specific background for better understanding

To make it even more clear, this is our full user scenario:

  • User maintains their own PKI infrastructure. This means they generate private and public keys by different means (not necessarily sbctl).
  • User wants to sign Kairos OS with their own keys but we also want the full bundle of keys (the user's + the original ones from the hardware) to be enrolled automatically on first boot in setup mode (systemd-boot feature).

The above is an one-off procedure. As soon as the Kairos OS has been prepared, any machine booting that OS in setup mode, will automatically get the same certificates enrolled. This allows the user to prepare thousands of machines with less effort.

We already have tooling that will prepare the OS and the input to that is the set of keys to sign the efi file but also the keys to enroll automatically when the system boots.

So, what we need from sbctl is to allow the user to prepare such a set of keys in an easy way. The process I had in mind was:

  • use sbctl to export the original keys
  • use sbctl to combine user supplied keys with the above exported keys into authenticated variable files that can be enrolled with systemd-boot

the result of the above will be fed to the enki build-uki command (linked above) to prepare the OS.

@jimmykarily
Copy link
Contributor Author

It seems that sbctl can do all things already for us except maybe exporting the builtin keys to be used later on another machine to prepare the rest of the keys. We are investigating here: kairos-io/enki#91

@nianyush will give it a try with --yes-this-might-brick-my-machine in a VM and maybe if that works, we can just improve the wording or skip the need for the flag when the --export flags is also set.

@Foxboron
Copy link
Owner

I suspect the code paths is a bit non-obvious which is why the optionrom check is being triggered. I think we should be able to move things around for this to work properly :)

If this doesn't work at all or becomes too confusing I'm totes open for the original idea of having a new command.

@nianyush
Copy link

nianyush commented Apr 26, 2024

i can confirm that these two commands works for me to generate the auth/esl file.

sbctl enroll-keys --export auth --yes-this-might-brick-my-machine -a
sbctl enroll-keys --export auth --yes-this-might-brick-my-machine -f

Although in qemu vm, -f option didnt work for me. It seems my qemu vm is missing dbDefault efivar. However, i can still achieve my goal with -a since i have my factory keys already enrolled in the system.

root@ubuntu:/home/nianyu/work# sbctl enroll-keys --export auth --yes-this-might-brick-my-machine -f
Exporting keys to EFI files...
With vendor certificates built into the firmware...✗ 
couldn't sync keys: could not enroll built-in firmware keys: open /sys/firmware/efi/efivars/dbDefault-8be4df61-93ca-11d2-aa0d-00e098032b8c: no such file or directory
root@ubuntu:/sys/firmware/efi/efivars# ls | grep db
certdb-d9bee56e-75dc-49d9-b4d7-b534210f637a
certdbv-d9bee56e-75dc-49d9-b4d7-b534210f637a
db-d719b2cb-3d3a-4596-a3bc-dad00e67656f
dbx-d719b2cb-3d3a-4596-a3bc-dad00e67656f

@jimmykarily @Foxboron

@Foxboron
Copy link
Owner

Cool, so we can fix the UX here to make it better and easier to work with then introducing a new command :)

@nianyush
Copy link

nianyush commented Apr 26, 2024

Yes, and one more command i think it might be helpful is to export auth/esl from enrolled keys in the system directly. Because currently enroll-keys -a --export auth would need me to first create some keys and then merge my created keys with system keys. If we have this command, i would more flexibility on exporting and appending keys with different machines.

@nianyush
Copy link

nianyush commented Apr 26, 2024

it would be similar to

efi-readvar -v db -o 'db.esl'

@jimmykarily
Copy link
Contributor Author

Yes, and one more command i think it might be helpful is to export auth/esl from enrolled keys in the system directly.

You mean like the command introduced in this PR? I'm confused now :D

@nianyush
Copy link

nianyush commented Apr 26, 2024

@jimmykarily --export works if you have your custom keys imported using sbctl import-keys. But if you dont have keys created/imported by sbctl. It would fail

root@ubuntu:/usr/share/secureboot# sbctl enroll-keys --export auth  --yes-this-might-brick-my-machine
open /usr/share/secureboot/GUID: no such file or directory
``

@jimmykarily
Copy link
Contributor Author

@nianyush so the desired command would simply export the enrolled keys without importing anything first. If you check this PR code, that's exactly what it does (exports the "der" version but that we can change or make configurable). So what I'm trying to understand is whether the command introduced in this PR is still needed or not.

@nianyush
Copy link

nianyush commented Apr 26, 2024

Then the answer would probably be yes for me:) @jimmykarily

@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 29, 2024 07:27 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 29, 2024 07:27 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 29, 2024 07:27 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 29, 2024 16:03 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 29, 2024 16:03 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 29, 2024 16:03 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 30, 2024 06:15 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 30, 2024 06:15 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 30, 2024 06:15 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 30, 2024 07:49 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 30, 2024 07:49 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 30, 2024 07:49 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 30, 2024 08:29 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 30, 2024 08:29 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries April 30, 2024 08:29 — with GitHub Actions Inactive
@jimmykarily
Copy link
Contributor Author

I've added a couple of tests and added a flag for the export format ("der" or "esl").

@Foxboron if you prefer this functionality to be implemented by the enroll-keys with additional flags let me know and I can move it there. In my opinion, one command should do one thing. enroll-keys should enroll, export-whatever should export. And there is the distinction between "export enrolled keys to the database" and "export the enrolled key to the specified directory". I would like to keep the option to export to a specified directory because the database path may not be writable (e.g. in Kairos).

@jimmykarily jimmykarily marked this pull request as ready for review April 30, 2024 08:47
@Foxboron
Copy link
Owner

Foxboron commented May 1, 2024

if you prefer this functionality to be implemented by the enroll-keys with additional flags let me know and I can move it there. In my opinion, one command should do one thing. enroll-keys should enroll, export-whatever should export.

I agree that lumping this up with enroll-keys would be confusing. So we can have this command while we figure out a better approach to exporting the auth lists.

tests/integration_test.go Outdated Show resolved Hide resolved
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries May 1, 2024 13:52 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries May 1, 2024 13:52 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries May 1, 2024 13:52 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries May 1, 2024 15:36 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries May 1, 2024 15:36 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries May 1, 2024 15:36 — with GitHub Actions Inactive
Signed-off-by: Dimitris Karakasilis <dimitris@karakasilis.me>
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries May 1, 2024 15:38 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries May 1, 2024 15:38 — with GitHub Actions Inactive
@jimmykarily jimmykarily temporarily deployed to Build, sign, release binaries May 1, 2024 15:38 — with GitHub Actions Inactive
@jimmykarily
Copy link
Contributor Author

Squashed commits

@Foxboron
Copy link
Owner

Foxboron commented May 7, 2024

Thanks!

@Foxboron Foxboron merged commit 71fa775 into Foxboron:master May 7, 2024
6 checks passed
@jimmykarily jimmykarily deleted the export-enrolled-keys branch May 13, 2024 08:10
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

Successfully merging this pull request may close these issues.

3 participants