add virtiofs systemd credential handling#4230
Conversation
There was a problem hiding this comment.
Code Review
This pull request restructures the build manifests by consolidating package definitions and postprocess scripts into fedora-coreos.yaml and moving shared configurations to shared-el.yaml. It also introduces a new dracut module, 40import-virtiofs-systemd-credentials, to import systemd credentials from a tagged virtiofs share during initramfs. The review feedback highlights critical path mismatches in shared-el.yaml where the new overlay is incorrectly referenced under overlay/ instead of overlay.d/, which would cause build failures. Additionally, a documentation mismatch in the systemd service file was identified regarding the target credentials directory.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| # Run in the initramfs so that systemd credentials are in | ||
| # /run/credentials/@system/ before the initramfs-to-real-root transition. |
There was a problem hiding this comment.
The comment mentions /run/credentials/@system/, but the script import-virtiofs-systemd-credentials.sh actually copies the credentials to /run/credentials/@initrd. Please update the comment to match the actual path used in the script to avoid confusion.
# Run in the initramfs so that systemd credentials are in
# /run/credentials/@initrd before the initramfs-to-real-root transition.
|
This is built on top of #4227 Please just consider the last commit here. |
a3b0a03 to
914cc8b
Compare
|
Looking at coreos/coreos-assembler#4469 it looks like a desire here is to be able to run tests against other bootable container images (bootc) so adding this functionality maybe should go in https://gitlab.com/fedora/bootc/base-images instead of here. |
|
@dustymabe Thanks for working on this! One question I have is: why use virtiofs instead of SMBIOS? Is there a specific reason to choose virtiofs? I'd just like to understand better and get a bit more context. |
Good question. Basically systemd/systemd#29175 (comment) We need something that will work across every architecture we want to run tests on if at all possible. |
c594a82 to
15c2f2b
Compare
|
Got it, makes sense! Thank you for explain! I will make changes in my PR to see if is possible to support virtiofs instead of SMBIOS so we can be in the same page :) |
Not quite ready to push this up yet but you can see the COSA side changes in the |
|
@travier interested in your thoughts on this from a security perspective.. and also the question of:
If we don't put it in bootc then we could add a |
15c2f2b to
bb2d251
Compare
|
Pushed a second commit here to fix the problem mentioned in #4227 (comment) |
This feels unrelated to this PR. Let's split into another one so that we can get it in directly? |
|
|
Let's drop the second commit from this one. This looks very interesting but I need to think about it a bit more and wrap my head around it. It's interesting that there is discussion upstream and it feels like something reasonable that we could tweak once upstream approves it. Also related: https://systemd.io/VM_INTERFACE/ |
bb2d251 to
9a9a6a5
Compare
Done. rebased this PR. on top of latest
👍
Is there anything there that would cause us to change this? |
|
Small note, the |
|
So if I understand correctly, this is only for the QEMU virtualization case as this requires virtiofs. But in this case, shouldn't we use the existing smbios-type-11 support in systemd as that would work as well and should work in limit virtualization context that we have here? |
Not directly in this PR but we could consider using |
I'm looking for something cross architecture. smbios doesn't work everywhere we need it to (kernel args would, but I think cracking open images to inject kernel args for every test would negate some of the test) and maintaining different solutions for different architectures is fraught with problems IMO. |
I actually have AF_SOCK working (among some of the commits in https://github.com/dustymabe/coreos-assembler/commits/dusty-running/), but I haven't opened a PR for it yet. Part of the problem there is coreos/fedora-coreos-tracker#2098 (comment) Basically I was relying on a Ignition provided SSH key, but using VSOCK to connect, but that doesn't work. But in general, there are two problems:
The implementation for them is independent (one doesn't depend on the other). |
Ah yes indeed. I remembered that this was not supported on all architectures (thus why this can not replace Ignition) but I thought maybe QEMU could still do it. Apparently that's unlikely and it's only documented under the i686 target: https://www.qemu.org/docs/master/system/invocation.html#hxtool-4 Also from https://www.dmtf.org/standards/smbios:
|
|
So overall this looks good, but this now becomes another access point that we would have to measure for confidential computing. So if that works for you, I would prefer we either restrict that to testing releases/builds (likely not ideal) or we add a condition to disable it when booted from a measured UKI (systemd has this option now). |
hmm. how does systemd handle this today for say smbios based credential injection? Do they measure that?
I really only care about testing and not trying to enable this generically for users.
Yeah not sure how we would do the "restrict to testing releases/builds" if we also wanted to use this to test our production builds. I'm happy to add a Also, this question still remains:
If we keep it here in CoreOS we can do a |
Yes, they started measuring SMBIOS Type 11 entries read from system-stub into a PCR (12 I think).
👍🏻 but once it's in, someone will use it 🙂.
Will do.
Agree that it would be best to have that in the bootc base image for wider testing. Or we could use that for FCOS testing on PRs to the bootc repo / pipeline. |
|
From https://www.freedesktop.org/software/systemd/man/latest/systemd.unit.html#ConditionSecurity=: |
As part of an effort to test more on non-CoreOS bootable container images we'd like a cross architecture way to expose systemd credentials into a VM. This commit adds a new import-virtiofs-systemd-credentials.service that runs in the initramfs and will copy credentials exposed via the io.systemd.credentials virtiofs tag into the /run/credentials/@initrd directory, which will then get imported by systemd itself. Related to systemd/systemd#29175 upstream. Assisted-by: <anthropic/claude-opus-4.6>
added
Do you think I should open this against bootc then? Or maybe we do it in parallel and don't block on it being there just in case? I updated this PR to have |
9a9a6a5 to
76f82eb
Compare
|
I think we can do it in parallel and we can start testing this here. |
|
@travier anything blocking this at this point? |
| # Clean up the mount | ||
| umount "${MOUNTPOINT}" |
There was a problem hiding this comment.
We should be able to skip that by using a dedicated mount namespace like we do in other units that mount filesystems. https://github.com/coreos/bootupd/blob/main/systemd/bootloader-update.service#L16
|
|
||
| # Clean up the mount | ||
| umount "${MOUNTPOINT}" | ||
| rmdir "${MOUNTPOINT}" 2>/dev/null || true |
There was a problem hiding this comment.
Ah, maybe not if we need to cleanup the mountpoint
| } | ||
|
|
||
| installkernel() { | ||
| instmods -c virtiofs |
There was a problem hiding this comment.
This will make our initrd 35K (xz compressed) bigger so should be fine.
travier
left a comment
There was a problem hiding this comment.
One potential nit but could be a followup if actually possible.
LGTM
Uh oh!
There was an error while loading. Please reload this page.