sdboot: add support for autoenroll in `bootc install` #1818

gerblesh · 2025-12-02T02:20:13Z

add autoenroll to sdboot

installs .auth files from /usr/lib/bootc/keys into ESP/loader/keys/auto. Currently uses cfs file APIs and is a little rough. Not sure if this should be in composefs-boot rather than bootc. The directory /usr/lib/bootc/keys probably isn't the best choice although at least it means that the auth files are "measured" when the cfs digest is taken.

Note that this is missing some of the bits from CI for this to be properly tested. I will try to get that done when I can though

gemini-code-assist

Code Review

This pull request adds support for auto-enrolling Secure Boot keys for sd-boot from .auth files located in /usr/lib/bootc/keys within a composefs image. The changes look good overall. My review includes a suggestion to make the key discovery process more robust by handling cases where the keys directory does not exist, and a minor performance improvement to avoid an unnecessary allocation within a loop.

crates/lib/src/bootc_composefs/boot.rs

crates/lib/src/bootloader.rs

crates/lib/src/bootc_composefs/boot.rs

Johan-Liebert1 · 2025-12-03T04:47:52Z

We also use "topic prefixes" for commits, so if you could prefix your commit messages with sd-boot: that would be great. Also, I think most of these commits could be squashed under add support for key autoenroll to sdboot

cgwalters

As far as tests; we could unit test the key parsing easily, but not a blocker.

For the actual functionality - it looks like (combined with the systemd-boot signing fixes in #1809 ) we could actually have a test flow that does this in addition to/instead of setting up the libvirt firmware too, which would be cool. (Actually we do need to test both paths)

cgwalters · 2025-12-04T15:59:40Z

crates/lib/src/bootc_composefs/boot.rs

 const INITRD: &str = "initrd";
 const VMLINUZ: &str = "vmlinuz";

+const BOOTC_AUTOENROLL_PATH: &str = "usr/lib/bootc/keys";


If these are only copied from the container to the bootloader state at install time, then I think this should be /usr/lib/bootc/install/secureboot-keys or so.

(Note install/ subdir)

Also can you add a section to the install manpage about this?

sounds good, the path I chose was pretty arbitrary. Added a section in the manpage about this as well

crates/lib/src/bootloader.rs

cgwalters · 2025-12-04T16:08:16Z

crates/lib/src/bootc_composefs/boot.rs

+        let path = match Utf8PathBuf::from_os_string(name.clone()) {
+            Ok(p) => p,
+            Err(_) => bail!("couldn't get pathbuf: /{p}/{name:?}"),
+        };


There's more idiomatic ways to do this. One is to use DirUtf8 which hard enforces this, or use let path: Utf8PathBuf = name.try_into()?;

Or on this case we could probably just use PathBuf (or OSString).

Used try_into()? here. esp_path is already a Utf8Path so I figure it's better to keep the types consistent but I dunno if there's a more preferred type here

crates/lib/src/bootc_composefs/boot.rs

cgwalters · 2025-12-04T16:11:11Z

crates/lib/src/bootc_composefs/boot.rs

+        };
+        entries.push(path);
+    }
+    if entries.len() > 0 {


Converting an empty result to None isn't wrong, but it's not clear we actually gain much versus just handling the empty vector in the callers instead.

To really get the benefit of combining Option + Vec, we'd use https://docs.rs/vec1/latest/vec1/ instead I think (but let's not do that yet, not clear it's worth the nontrivial external dep just for this)

moved to the install function, not really sure what the idiomatic Rust way of doing this is but things don't break with no entries just nothing is really copied over. Added some logging for when the directory exists but no keys are to be enrolled

cgwalters · 2025-12-04T21:39:33Z

To xref I also did systemd/systemd#39993 while investigating this

cgwalters

Thanks! A few more nits

crates/lib/src/bootloader.rs

docs/src/man/bootc-install.8.md

Signed-off-by: Gareth Widlansky <gareth.widlansky@proton.me> cleanup Signed-off-by: Gareth Widlansky <gareth.widlansky@proton.me> don't expect `/usr/lib/bootc/keys` to exist Signed-off-by: Gareth Widlansky <gareth.widlansky@proton.me> remove comment Signed-off-by: Gareth Widlansky <gareth.widlansky@proton.me> don't use ambient authority Signed-off-by: Gareth Widlansky <gareth.widlansky@proton.me> use option rather than an enum Signed-off-by: Gareth Widlansky <gareth.widlansky@proton.me> use fs.open_dir_optional Signed-off-by: Gareth Widlansky <gareth.widlansky@proton.me> use basedir, link spec, add docs to `bootc install` docs: rework manpage Signed-off-by: Gareth Widlansky <gareth.widlansky@proton.me> nits typo

gerblesh · 2025-12-04T22:32:14Z

Alright should have addressed all of the nits, lmk if there's more I should fix/tweak before merge

cgwalters

Thanks so much! I'm ok getting this in without integration tests for now as the whole composefs is still experimental; I hope we'll continue momentum on fleshing out a lot of those though.

cgwalters · 2025-12-05T14:46:20Z

@Johan-Liebert1 you need to clear your requested changes state here

bootc-bot bot requested a review from jmarrero December 2, 2025 02:20

gerblesh mentioned this pull request Dec 2, 2025

Fix systemd-boot signing on sealed image test #1809

Open

gemini-code-assist bot reviewed Dec 2, 2025

View reviewed changes

crates/lib/src/bootc_composefs/boot.rs Outdated Show resolved Hide resolved

crates/lib/src/bootloader.rs Outdated Show resolved Hide resolved

Johan-Liebert1 reviewed Dec 2, 2025

View reviewed changes

crates/lib/src/bootc_composefs/boot.rs Outdated Show resolved Hide resolved

gerblesh force-pushed the sdboot-autoenroll branch from 3c2bb96 to d8a145d Compare December 2, 2025 18:06

Johan-Liebert1 requested changes Dec 3, 2025

View reviewed changes

crates/lib/src/bootc_composefs/boot.rs Outdated Show resolved Hide resolved

gerblesh force-pushed the sdboot-autoenroll branch from 7825e03 to d900f21 Compare December 3, 2025 16:40

gerblesh changed the title ~~add support for key autoenroll to sdboot~~ sdboot: support for key autoenroll Dec 3, 2025

gerblesh changed the title ~~sdboot: support for key autoenroll~~ sdboot: add support for autoenroll in bootc install Dec 3, 2025

cgwalters requested changes Dec 4, 2025

View reviewed changes

gerblesh force-pushed the sdboot-autoenroll branch from d900f21 to cabe1ab Compare December 4, 2025 19:48

github-actions bot added the area/documentation Updates to the documentation label Dec 4, 2025

gerblesh force-pushed the sdboot-autoenroll branch 2 times, most recently from 49fbaca to e56fb5f Compare December 4, 2025 20:15

cgwalters reviewed Dec 4, 2025

View reviewed changes

gerblesh force-pushed the sdboot-autoenroll branch from e56fb5f to 35633bc Compare December 4, 2025 22:26

gerblesh force-pushed the sdboot-autoenroll branch from 35633bc to f80a211 Compare December 4, 2025 22:30

cgwalters approved these changes Dec 5, 2025

View reviewed changes

cgwalters enabled auto-merge (rebase) December 5, 2025 01:39

Johan-Liebert1 approved these changes Dec 8, 2025

View reviewed changes

sdboot: add support for autoenroll in bootc install #1818

Are you sure you want to change the base?

sdboot: add support for autoenroll in bootc install #1818

Uh oh!

Conversation

gerblesh commented Dec 2, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Johan-Liebert1 commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cgwalters left a comment

Choose a reason for hiding this comment

Uh oh!

cgwalters Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

gerblesh Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cgwalters Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

gerblesh Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cgwalters Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

gerblesh Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cgwalters commented Dec 4, 2025

Uh oh!

cgwalters left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gerblesh commented Dec 4, 2025

Uh oh!

cgwalters left a comment

Choose a reason for hiding this comment

Uh oh!

cgwalters commented Dec 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sdboot: add support for autoenroll in `bootc install` #1818

sdboot: add support for autoenroll in `bootc install` #1818

Johan-Liebert1 commented Dec 3, 2025 •

edited

Loading

gerblesh Dec 4, 2025 •

edited

Loading

gerblesh Dec 4, 2025 •

edited

Loading