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

Allow exporting keys without efivars present and update CI staticcheck #363

Merged
merged 2 commits into from
Aug 16, 2024

Conversation

Cornelicorn
Copy link
Contributor

@Cornelicorn Cornelicorn commented Aug 15, 2024

Closes #364

@Foxboron
Copy link
Owner

Generally it looks fine, but what is the usecase?

@Cornelicorn
Copy link
Contributor Author

Generally it looks fine, but what is the usecase?

Just added the issue, maybe should do that the other way around next time :D

@Foxboron
Copy link
Owner

Ah, cool. This makes sense.

But then I suspect it would be better to entierly decouple the export stuff from the enroll functions. Currently they are quite entangled but there isn't a reason why everything is refactored into a RunExportKeys and then we just have an early if-check for a non-empty Export.

We don't really care about whether or not enrollment fails to export the keys, and exporting them could probably be done before enrollment.

Don't have to do this for this PR though, but it would be trivial to write and also test for in the current setup.

switch to errors.New() for error messages without formatting to avoid this lint error:

> printf-style function with dynamic format string and no further arguments should use print-style function instead (SA1006)
@Cornelicorn
Copy link
Contributor Author

Ah, cool. This makes sense.

But then I suspect it would be better to entierly decouple the export stuff from the enroll functions. Currently they are quite entangled but there isn't a reason why everything is refactored into a RunExportKeys and then we just have an early if-check for a non-empty Export.

We don't really care about whether or not enrollment fails to export the keys, and exporting them could probably be done before enrollment.

Don't have to do this for this PR though, but it would be trivial to write and also test for in the current setup.

Yes, enrolling and exporting should probably be split up.
I won't be able to get started with this in the next 4 weeks sadly, but the error is relatively easy to reproduce with mkosi:

mkosi.conf

[Distribution]
Distribution=arch

[Content]
Bootable=yes
Packages=
    linux
    systemd

mkosi.postinst.chroot

#!/bin/bash

sbctl create-keys
mkdir -p /efi/loader/keys/auto
cd /efi/loader/keys/auto
sbctl enroll-keys --disable-landlock --custom --yes-this-might-brick-my-machine --export auth
sbctl enroll-keys --disable-landlock --custom --yes-this-might-brick-my-machine --export esl

Then run

mkdir -p mkosi.extra/usr/bin
cp /my/sbctl/binary mkosi.extra/usr/bin/sbctl
mkosi

@Foxboron
Copy link
Owner

I won't be able to get started with this in the next 4 weeks sadly

We'll see if I have time for it if I get bored for an hour. Should not be very hard.

I'll accept this PR regardless though. Gives me a little bit of motivation to do it :)

@Foxboron Foxboron merged commit 2669fe9 into Foxboron:master Aug 16, 2024
6 checks passed
@Foxboron
Copy link
Owner

Thanks!

@Cornelicorn Cornelicorn deleted the export-keys-without-efivars branch August 18, 2024 08:34
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.

Allow exporting keys without efivarfs
2 participants