systemd icon indicating copy to clipboard operation
systemd copied to clipboard

sysupdate: parse currently booted UKI version if any for ProtectVersion=

Open bluca opened this issue 6 months ago • 19 comments

Currently ProtectVersion= only looks at the os-release in the rootfs. If a UKI is updated, the protection mechanism doesn't work and the currently booted UKI is deleted.

Find the current UKI via the StubImageIdentifier (if any) and parse its version. If the resource being updated is under ESP/XBOOTLDR/$BOOT then check its version against the UKI's.

bluca avatar Jun 14 '25 19:06 bluca

Hmm, I am a bit puzzled about your choice to add this as side-effect of ProtectVersion=.

So I presume you are looking into the scenario if you allow UKIs and system images to be updated somewhat independently? Do we even support that? I mean /usr/lib/modules will then not have the UKI#s matching kmod dir, so how can this work?

What am I missing?

poettering avatar Jun 16 '25 11:06 poettering

Hmm, I am a bit puzzled about your choice to add this as side-effect of ProtectVersion=.

It's not a side effect really, it's just that option currently completely ignores UKIs, so with this transfer file:

[Source]
Type=url-file
Path=https://download.opensuse.org/repositories/system:/systemd/Debian_Testing_images/
MatchPattern=debian-trixie_@v_%a.efi

[Target]
Type=regular-file
Path=/EFI/Linux
PathRelativeTo=boot
MatchPattern=debian-trixie_@v_%a+@[email protected] \
  debian-trixie_@v_%[email protected] \
  debian-trixie_@v_%a.efi
Mode=0644
TriesLeft=3
TriesDone=0
InstancesMax=2

[Transfer]
ProtectVersion=%A

ProtectVersion= is completely ignored and doesn't work. So it's not a side effect, it's fixing a bug where that field doesn't work

So I presume you are looking into the scenario if you allow UKIs and system images to be updated somewhat independently? Do we even support that? I mean /usr/lib/modules will then not have the UKI#s matching kmod dir, so how can this work?

Of course, it works just fine, I have been using a normal rootfs, package-managed, with a UKI for months and months. When a new kernel package is published the UKI gets rebuilt and delivered, and the kernel package also gets updated and delivered, as the source for both is the same (kernel package in the distro repo) and the updates (apt and sysupdate) are both on the same timer.

bluca avatar Jun 16 '25 11:06 bluca

Of course, it works just fine, I have been using a normal rootfs, package-managed, with a UKI for months and months. When a new kernel package is published the UKI gets rebuilt and delivered, and the kernel package also gets updated and delivered, as the source for both is the same (kernel package in the distro repo) and the updates (apt and sysupdate) are both on the same timer.

that's a pretty strange setup, no?

ProtectVersion= is completely ignored and doesn't work. So it's not a side effect, it's fixing a bug where that field doesn't work

ProtectVersion=%A means: protect the image version of your current rootfs image's os-release. If you have no rootfs image of course that is not going to do anything? But it's really weird making up another version check here implicitly.

I guess if you really want to support that you#d have to add additional specifiers that use the osrel data from the UKI instead of from the the host os. That would probably mean you'd have to introduce a bunch of new specifiers that use the 2nd info, i.e. counterparts for %A, %B, %M, %o, %w, %W. And then you could do ProtectVersion=%A and ProtectVersion=%X (if %X is like %a, but uses the other os-release data) in combination, to properly protect the OS version of the UKi an the OS version of the rootfs image.

poettering avatar Jun 16 '25 11:06 poettering

Oh, and you can just read the UKI's os-release data from /run/systemd/stub/os-release these days. We place it there via tmpfiles.d/20-systemd-stub.conf.

That's much simpler and works unpriv and so on.

poettering avatar Jun 16 '25 11:06 poettering

Or in other words: if you want this, then please introduce counterparts for those 6 specifiers, that look in /run/systemd/stub/os-release instead of /etc/os-release, and then add one ProtectVersion= line so that the UKI version is protected too.

poettering avatar Jun 16 '25 11:06 poettering

Of course, it works just fine, I have been using a normal rootfs, package-managed, with a UKI for months and months. When a new kernel package is published the UKI gets rebuilt and delivered, and the kernel package also gets updated and delivered, as the source for both is the same (kernel package in the distro repo) and the updates (apt and sysupdate) are both on the same timer.

that's a pretty strange setup, no?

I don't think so? Works really nicely, don't have to waste time building initrds locally, etc. You should try it, Fedora UKIs are built in the same place too ;-)

ProtectVersion= is completely ignored and doesn't work. So it's not a side effect, it's fixing a bug where that field doesn't work

ProtectVersion=%A means: protect the image version of your current rootfs image's os-release. If you have no rootfs image of course that is not going to do anything? But it's really weird making up another version check here implicitly.

That's not what the documentation says?

ProtectVersion=
Takes one or more version strings to mark as "protected". Protected versions are never removed while
making room for new, updated versions. This is useful to ensure that the currently booted OS version (or
auxiliary resources associated with it) is not replaced/overwritten during updates, in order to avoid
runtime file system corruptions.

It doesn't mention any restrictions that make it apply to the rootfs only, and in fact it explicitly mentions auxiliary resources too. And the other documentation and examples explicitly show it working with UKIs too:

3. Finally, a file "https://download.example.com/foobarOS_47.efi" (a unified kernel, as per Boot Loader
   Specification[2] Type #2) should be downloaded, decompressed and written to the $BOOT file system, i.e. to
   EFI/Linux/foobarOS_47.efi in the ESP or XBOOTLDR partition.
 # /usr/lib/sysupdate.d/70-kernel.transfer
 [Transfer]
 ProtectVersion=%A

 [Source]
 Type=url-file
 Path=https://download.example.com/
 [email protected]

 [Target]
 Type=regular-file
 Path=/EFI/Linux
 PathRelativeTo=boot
 MatchPattern=foobarOS_@v+@[email protected] \
              foobarOS_@[email protected] \
              [email protected]
 Mode=0644
 TriesLeft=3
 TriesDone=0
 InstancesMax=2

The above installs a unified kernel image into the $BOOT partition, as per Boot Loader Specification[2] Type
#2.

I guess if you really want to support that you#d have to add additional specifiers that use the osrel data from the UKI instead of from the the host os. That would probably mean you'd have to introduce a bunch of new specifiers that use the 2nd info, i.e. counterparts for %A, %B, %M, %o, %w, %W. And then you could do ProtectVersion=%A and ProtectVersion=%X (if %X is like %a, but uses the other os-release data) in combination, to properly protect the OS version of the UKi an the OS version of the rootfs image.

But why? A transfer file already covers a single resource, if I tell it to protect its version, it should figure it out by itself, without having to duplicate everything and figure out that actually if the resource I am pointing to is a UKI, I need to use some encoding or another. That makes things more confusing for users

bluca avatar Jun 16 '25 11:06 bluca

Oh, and you can just read the UKI's os-release data from /run/systemd/stub/os-release these days. We place it there via tmpfiles.d/20-systemd-stub.conf.

That's much simpler and works unpriv and so on.

ooft, wish I had known about that one yesterday

bluca avatar Jun 16 '25 12:06 bluca

Please just add those 6 new specifiers that use the UKI's os-release files, it's a very short patch then, and makes things less magic. People really should have the freedom to pick the versions they want to protect and which ones they don't.

poettering avatar Jun 17 '25 07:06 poettering

I don't think that's the right approach. As per documentation transfer files refer to different kind of resources, and the ProtectVersion field should do the right thing rather than providing complex footguns. Just as it doesn't make sense to compare a UKI version to the rootfs, the opposite is also true, when downloading a new rootfs it does not make sense to compare it to a UKI. Apples and oranges. It should just do the right thing out of the box, it's simpler, easier to use and harder to mis-use.

bluca avatar Jun 17 '25 09:06 bluca

sorry, but this cannot work. UKIs are not as unified as one might think in some cases, for example, a pcrlock.d golden measurement file for a UKI might be something you want to drop into /var/ at the same time as you install a new kernel. or you must drop in kmods, or other resources. So you might need to update UKI-associated files outside of ESP/XBOOTLDR at the same time as the UKI. But the reverse is also possible: there area lots of reasons why you need to update OS-bound resources in the ESP/XBOOTLDR partitions that should be glued to the OS version, not the kernel version.

Hence it's essential that we can manage them explicitly correctly: i.e. reference by UKI version and by OS version in ESP/XBOOTLDR and on the host. And that means we cannot overload the specifiers with different meaning... That's both broken, and very very confusing.

poettering avatar Jun 17 '25 09:06 poettering

If there are more cases where context matters, then the right thing should be done there too, depending on the resource being handled. Leaving it up to the users to figure out what combinations to use for what resource is just impossibly annoying to deal with. We can't keep adding more and more and more duplicated specifiers for anything that comes along, not only it's really bad UX but at some point there are only 26*2 characters available...

bluca avatar Jun 17 '25 11:06 bluca

do you se any further contexts like this showing up? i don't...

But again, you cannot just do "the right thing", because you never know if you want to match UKI versions on the rootfs, or match OS versions on the ESP. But are OK, and should be something we can express.

6 chars is not that much...

poettering avatar Jun 17 '25 12:06 poettering

do you se any further contexts like this showing up? i don't...

You mentioned a bunch?

But again, you cannot just do "the right thing", because you never know if you want to match UKI versions on the rootfs, or match OS versions on the ESP. But are OK, and should be something we can express.

But we do? When updating a UKI, I don't want the running UKI to be deleted. This is what this PR does. And it's also backward compatible, so if you also want to match it to the rootfs, that also still works.

bluca avatar Jun 17 '25 12:06 bluca

do you se any further contexts like this showing up? i don't...

You mentioned a bunch?

uh? there are only ever two sets of versions involved: the UKI's and the rootfs'. All I am saying is to have 6 for the UKI fields, and 6 for the rootfs fields.

poettering avatar Jun 17 '25 12:06 poettering

you mentioned pcrlock, kmods and other resources

bluca avatar Jun 17 '25 12:06 bluca

those are auxiliary resources store on disk that are versioned like the context they belong to, i.e. the UKI or the root fs.

All I am saying is that you cannot use the directory path as hint which os-release file to use, because there are files everywhere that use either version.

poettering avatar Jun 17 '25 13:06 poettering

But why not? Everything under EFI/Linux/ needs to look also at the booted UKI version. Can you show a concrete example of something that is shipped under there, and should ignore the currently booted UKI version (if any)?

bluca avatar Jun 17 '25 13:06 bluca

Let's say people actually go fully for independent UKI and rootfs versioning. They start and version their UKIs 1, 2, 3, 4, 5, 6. And they also version their roofs 1, 2, 3, 4, 5, 6.

Now they want to protect the UKI itself (stored in the esp), the pcrlock golden measurement (stored on the rootfs), maybe a sysext with kmods (ditto) via the UKI's version.

And they want to protect the rootfs itself, maybe a credential file or two, and a confext matching the rootfs (the latter two stored on the esp) via the rootfs' version.

and there's little space.

as we said, versioning of UKI and rootfs is independent in this scenario, like in your setup. So because there's little space we don't want to protect rootfs v1 just because UKI v1 is still the only version around, even though rootfs v6 is already current.

poettering avatar Jun 17 '25 13:06 poettering

Sure but that doesn't happen? The rootfs is not checked against the uki version with this PR. And if you are worried about the opposite, I can change it so that the reverse doesn't happen anymore, and ukis are only checked against the running uki and nothing else

bluca avatar Jun 17 '25 14:06 bluca

I am not sure why this is so hard to grok.

Let me try another way: let's say you want to ship an UKI together with a pcrlock golden file that needs to be dropped in /var/lib/pcrlock.d/. How are you gonna protect that? with your current patch you simply cannot. because you cannot apply the UKI version protection to stuff outside of the ESP/XBOOTLDR.

poettering avatar Jun 19 '25 16:06 poettering

and let me emphasize: that scenario is not made up, this is where i think we should be going.

poettering avatar Jun 19 '25 16:06 poettering

(and also, tricking out our own functions via the temporary dir logic is ugly as fuck and not mergeable anyway. but please fix this properly by adding those 6 specifiers)

poettering avatar Jun 19 '25 16:06 poettering

I am not sure why this is so hard to grok.

Let me try another way: let's say you want to ship an UKI together with a pcrlock golden file that needs to be dropped in /var/lib/pcrlock.d/. How are you gonna protect that? with your current patch you simply cannot. because you cannot apply the UKI version protection to stuff outside of the ESP/XBOOTLDR.

My point is that we should recognize that as a resource that needs to be tied to the running UKI version, rather than forcing those who write the config file to know they have to do that manually themselves

bluca avatar Jun 19 '25 19:06 bluca

My point is that we should recognize that as a resource that needs to be tied to the running UKI version, rather than forcing those who write the config file to know they have to do that manually themselves

That's not realistic, for pcrlock.d golden measurements it's not realistic for sysupdate to decide whether the file is associated with the UKI and needs to be versioned like it, or is associated with the OS and needs to be versioned like the OS.

Btw, I think we don't really need to bother with all 6 specifiers btw. I think it's sufficient to add specifiers for the actual versioning related fields of the UKI's os-release, i.e. counterparts for %A (i.e IMAGE_VERSION), %B (BUILD_VERSION), %w (VERSION_ID). That#s only three new specifiers.

poettering avatar Jun 20 '25 07:06 poettering

That's not realistic, for pcrlock.d golden measurements it's not realistic for sysupdate to decide whether the file is associated with the UKI and needs to be versioned like it, or is associated with the OS and needs to be versioned like the OS.

But the pcrlock files explicitly list the pcr, no? And we know which pcr we use for things that are part of the UKI, which one for things that are part of the rootfs, and so on, no? So it should be possible to make the right decision based on the content of the file?

bluca avatar Jun 20 '25 10:06 bluca

PCR 4 gets measurements from a lot of things, it's not clear cut.

Also, seriously, let's maybe not teach sysupdate.d to recognize pcrlock json drop-ins specifically and treat them differently, that'd be impossible for users to understand. It's also entirely unnecessary. If you people want disjoint update cycles for UKI and root fs, then I think it's not too much to ask to pick the right of two specifiers.

poettering avatar Jun 20 '25 13:06 poettering

PCR 4 gets measurements from a lot of things, it's not clear cut.

Then it can be checked against both. That's entirely my point: Joe random user has no way to figure out which is the right incantation for each of these mysterious things. I'm pretty sure you are the only one that understands how any of that works, and encoding that knowledge in the implementation is far better than keep adding dimensions to the matrix of magic letters and hoping everyone who uses it can make the right choice or end up with an expensive brick instead of a laptop

Also, seriously, let's maybe not teach sysupdate.d to recognize pcrlock json drop-ins specifically and treat them differently, that'd be impossible for users to understand. It's also entirely unnecessary. If you people want disjoint update cycles for UKI and root fs, then I think it's not too much to ask to pick the right of two specifiers.

It's exactly because things are already nearly impossible to understand that it shouldn't be let entirely up to every random user to decide whether %B or %z is the right magic incantation to select to avoid making their machine unbootable. "Use this magic letter if you don't want the automated tool running in the background on a timer to blast away the bits that make your machine working" is already quite far off on the spectrum of user friendliness in my view. I am very much convinced that adding yet another dimension to the problem and requiring everyone to figure out the right incantation from a combination of 2x2[x2 next time something new gets added, rinse and repeat] is not the right design choice

bluca avatar Jun 20 '25 13:06 bluca

"Use this magic letter if you don't want the automated tool running in the background on a timer to blast away the bits that make your machine working"

And once again a regression somewhere else (https://github.com/systemd/systemd/issues/38349) has left me with an unbootable laptop because sd-sysupdate just silently blasted away the working UKI, and left only broken ones installed. This stuff is really not ready for prime time.

bluca avatar Jul 26 '25 11:07 bluca

"Use this magic letter if you don't want the automated tool running in the background on a timer to blast away the bits that make your machine working"

And once again a regression somewhere else (#38349) has left me with an unbootable laptop because sd-sysupdate just silently blasted away the working UKI, and left only broken ones installed. This stuff is really not ready for prime time.

Just in case anyone faces the same issue and finds this by googling, as a workaround a drop in can be created as /etc/systemd/system/systemd-sysupdate.service.d/override.conf:

[Service]
BindReadOnlyPaths=-/run/systemd/stub/os-release:/usr/lib/os-release

bluca avatar Jul 27 '25 12:07 bluca