Add an ostree-admin-esp-upgrade subcommand to update the EFI System Partition

[martinezjavier]

This pull-request addresses #1649 by adding a new ostree admin esp-upgrade subcommand that can be used to upgrade the EFI System Partition (ESP) with files from the /usr/lib/ostree-boot/efi directory in the current OSTree deployment.

The command only works on systems that are booted with an EFI firmware and where the ESP is mounted in /boot/efi.

[rh-atomic-bot]

Can one of the admins verify this patch?
I understand the following commands:

• bot, add author to whitelist
• bot, test pull request
• bot, test pull request once

[cgwalters]

bot, add author to whitelist

[cgwalters]

In theory the code should probably be a library function but...eh. Not really worth it right now.

I know it is probably next to impossible to make this truly transactional but can we at least do what the ostree core does and copy everything into temporary names, then syncfs(), then rename into place?

Related to that, is there a safer order to update the components? For example, update shim first?

I'm also wondering if what we really need to do is have the bootloader know how to do this rather than ostree, but as is it's fine for now.

[cgwalters]

I mentioned this before I think also but soon at least FCOS is probably going to move away from installing the grub binaries into the ostree itself because they're only needed for Anaconda, and bloat the image. See coreos/coreos-assembler#556

So if we go that route...we'd need to e.g. publish a container or ostree ref or RPM or whatever that users could download to update the bootloader "out of band" - really quite similar to what LVFS is doing?

[cgwalters]

Also thanks for working on this!

[cgwalters]

Is there a nicer way for us to detect if we need to do this? I guess just doing a diff is the dumb/simple way but perhaps it'd be nicer to have a version file that's the result of rpm -qf on those files or whatever? (This starts to make this more RPM specific but we could define a "protocol" where the ostree core only reads version files, it doesn't write them)

[martinezjavier]

In theory the code should probably be a library function but...eh. Not really worth it right now.

Ok, I didn't see that other ot_admin_builtin_*() handlers where calling to library functions so I didn't think about that. But I will move it in the next iteration.

I know it is probably next to impossible to make this truly transactional but can we at least do what the ostree core does and copy everything into temporary names, then syncfs(), then rename into place?

I thought that temporary files where already used when calling ot_copy_dir_recurse() since it calls glnx_file_copy_at(). But maybe I misread the code? (I'm not that familiar with libglnx).

It doesn't do syncfs() though so I guess we will need a custom copy function for this anyways. But even with temporary files, we can't really make it atomic on vfat I think since renameat2(..., RENAME_EXCHANGE) isn't supported there.

I tried to reuse ot_copy_dir_recurse() just to keep the implementation simple, but agreed that we could do something smarter here.

Related to that, is there a safer order to update the components? For example, update shim first?

Yes, I also thought about that. We could determine the order in which the files need to be copied to reduce the risk of loosing power during the ESP upgrade and messing things up.

As you mention, things could fail if for example grub is upgraded without upgrading shim first.

My worry was that adding such logic would make the command less flexible. For example users can choose to not use shim at all and just have an EFI Boot Entry that points directly to the grub binary. Or someone could be using a different EFI bootloader, like sd-boot.

In other words, I wondered if the command should be aware of the content of the ESP or just copy whatever was installed by packages in /boot/efi. I leaned towards the latter but I agree with you that this is less safe.

I'm also wondering if what we really need to do is have the bootloader know how to do this rather than ostree, but as is it's fine for now.

The problem is that all these files are not owned by the same component. There are EFI binaries installed by at least the shim, grub2 and fwupd packages. So even if someday shim and grub could know how to update themselves, there are other EFI binaries like fwupdx64 that are managed by a user-space component (fwupd).

[martinezjavier]

I mentioned this before I think also but soon at least FCOS is probably going to move away from installing the grub binaries into the ostree itself because they're only needed for Anaconda, and bloat the image. See coreos/coreos-assembler#556

Yes, but I understood that was only for the user-space tools (since the grub.cfg would be static). I thought that at least the grub2-efi (on EFI installs) and grub2-pc (on legacy BIOS installs) would still be present in the ostree.

Unrelated, but in the pull you referenced I noticed that you are creating your own grub EFI binary. That will break Secure Boot since the image won't be signed.

So if we go that route...we'd need to e.g. publish a container or ostree ref or RPM or whatever that users could download to update the bootloader "out of band" - really quite similar to what LVFS is doing?

As mentioned I don't think that's a requirement since we could keep the grub2 packages that have the bootloader even if the ones for the user-space tools are dropped.

But yes, it would be good to explore having an out-of-band update mechanism for shim and grub that's similar to how we update the firmware with LVFS.

[martinezjavier]

Is there a nicer way for us to detect if we need to do this? I guess just doing a diff is the dumb/simple way but perhaps it'd be nicer to have a version file that's the result of rpm -qf on those files or whatever? (This starts to make this more RPM specific but we could define a "protocol" where the ostree core only reads version files, it doesn't write them)

That's a very interesting idea. So IIUC rpm-ostree could generate this /usr/lib/ostree-boot/efi/version file that contains the list of files that need to be upgrade and their sha512 checksum to compare. The file could even have the files in the order that need to be updated, so that way the shim+grub logic is not in the ostree-admin-upgrade-esp command but in rpm-ostree.

So a different version file could be created depending of the bootloader used (grub vs sd-boot).

[cmurf]

Looks like on vfat mv falls back to rename() which is atomic, with caveat:
If newpath already exists, it will be atomically replaced, so that there is no point at which another process attempting to access newpath will find it missing. However, there will probably be a window in which both oldpath and newpath refer to the file being renamed. https://www.unix.com/man-page/linux/2/renameat2/

renameat2(AT_FDCWD, "/boot/efi/testzero.txt.bak", AT_FDCWD, "/boot/efi/EFI/test.txt", RENAME_NOREPLACE) = -1 EEXIST (File exists)
stat("/boot/efi/EFI/test.txt", {st_mode=S_IFREG|0700, st_size=131072, ...}) = 0
lstat("/boot/efi/testzero.txt.bak", {st_mode=S_IFREG|0700, st_size=131072, ...}) = 0
newfstatat(AT_FDCWD, "/boot/efi/EFI/test.txt", {st_mode=S_IFREG|0700, st_size=131072, ...}, AT_SYMLINK_NOFOLLOW) = 0
geteuid()                               = 0
rename("/boot/efi/testzero.txt.bak", "/boot/efi/EFI/test.txt") = 0
lseek(0, 0, SEEK_CUR)                   = -1 ESPIPE (Illegal seek)

What about writing new bootloader files to /boot/efi/.tmp/, then sync(), then rename() by absolute paths? While there's not much that can be done to clean up the failure case, at least they don't result in boot failure.

[cgwalters]

I want to emphasize I really appreciate your work on this PR! This is attempting to address a real-world and important problem.

However...the more I think about this, the more I feel (as noted before) that the long term end state should be shipping the EFI bits separately, and have a separate tool (which could be a privileged container image, or a tool on the host that downloads things on its own).

This way the layering and ownership of things is very clear - ostree handles OSTrees (I know this seems obvious, but right now having /usr/lib/ostree-boot is I think a weird layer-crossing in-between state).

Further, we should define a versioning mechanism for data in /boot which is independent of the RPM (or other package) database that lives in /usr.

And then in an ideal world, we we have a package-system independent (in the same way ostree is) mechanism to update that data. Now, distributions which use rpm may choose to ship e.g. a separate rpmdb for /boot or so in addition.

Maybe all of this is to say - we need something like https://github.com/fwupd/fwupd except for bootloaders! (Which would get us into a nice layering of "ostree updates bootupdated, which updates fwupd-efi, which updates your firmware...)

Or I guess, we could just expand the scope of fwupd to include bootloaders.

That all said...in practice there may be tricky situations where in order to update the bootloader, one needs to update the ostree or vice versa...ideally we can always update the bootloader first and it e.g. compatibly handles older config files though.

[cgwalters]

Summoning @hughsie for thoughts on ⬆️

[cgwalters]

Random other side thought: Updating the ESP may not be at all easy to do truly transactionally in the general case, but it'd probably work to just ensure that the system remains bootable, and if we're interrupted halfway through, we just "reconcile" on reboot. We were leaning towards doing something like that in ostree to handle the "/boot/loader can't be a symlink on FAT" problem.

[hughsie]

From a fwupd point of view, we just need to be able to write /boot/efi/EFI/fedora/fwupdx64.efi when a firmware is going to be deployed (we can't install files onto the ESP reliably from a package) and then also write random blobs of firmware into /boot/efi/EFI/fedora/fw at runtime. As long as ostree admin esp-upgrade doesn't \remove\ files, then I don't see a problem.

Also, the ESP is nearly always FAT, for better or worse.

[martinezjavier]

@cgwalters thanks a lot for your feedback and comments.

This way the layering and ownership of things is very clear - ostree handles OSTrees (I know this seems obvious, but right now having /usr/lib/ostree-boot is I think a weird layer-crossing in-between state).

Yes, I guess that most users just don't know that the bootloader is never updated since the rpmdb would report a different package version than the one installed in the ESP, but it's confusing that ostree handles some of the files under /usr/lib/ostree-boot (i.e: the kernel) but not others like everything under /usr/lib/ostree-boot/efi/.

I know this is because the ESP is a fat partition so can't be part of the deployment transaction, but this is not evident at first sight. We should probably document this to raise awareness.

Further, we should define a versioning mechanism for data in /boot which is independent of the RPM (or other package) database that lives in /usr.

And then in an ideal world, we we have a package-system independent (in the same way ostree is) mechanism to update that data. Now, distributions which use rpm may choose to ship e.g. a separate rpmdb for /boot or so in addition.

Would this versioning mechanism be for data in /boot or /boot/efi? If the former, then it won't be used for kernel images?

Maybe all of this is to say - we need something like https://github.com/fwupd/fwupd except for bootloaders! (Which would get us into a nice layering of "ostree updates bootupdated, which updates fwupd-efi, which updates your firmware...)

Or I guess, we could just expand the scope of fwupd to include bootloaders.

Not sure about expanding fwupd scope since the bootloaders are not really firmware, but I understand your point. And that's why I tried to use a separate command for this. Although it may not be a ostree subcommand and instead a separate daemon that updates the ESP.

For now I'll hold this PR until we have decided how the solution should be.

I would also like to know @vathpela's opinion on this, and also from @wjt and @carlocaione who also were involved in previous discussions about this.

[dsd]

I've been also involved in these discussions behind the scenes from the Endless perspective (which I think is why you mentioned @wjt and @carlocaione).

From that angle we're additionally interested in updating bootloaders such as legacy grub and u-boot so we're looking for something not limited to ESP. We're also looking for something that would be run automatically, without requiring the user to run an ostree command from the terminal.

So to consider this a bit more from the general case: should ostree handle bootloader updates?

After a bit of reflection I think the answer of "no, a separate tool should handle it" is actually pretty reasonable based on the fact that ostree did not install the bootloader, so why should it be in charge of updating it? Conceptually, I don't think bootloader installation/updating belongs in ostree.

I was wondering if there is a counter-argument around practicality (since EFI bootloaders are so easy to update - just copy files), in that if ostree did at least handle the EFI case it'd solve this gaping hole for many system integrators, but actually considering the challenges around which order to update files and can it be done atomically on VFAT..., maybe its actually not so easy & practical, and the argument to keep it separate holds strong.

[cgwalters]

I'm also not opposed to try to bring bootloader updates into the ostree codebase BTW; the logic for "ostree handles ostrees" is obviously circular here because "ostree" is defined to not include /boot.

Yes, I guess that most users just don't know that the bootloader is never updated since the rpmdb would report a different package version

Right, hence the idea to remove the bootloader stuff from the ostree entirely. I imagine some people today are shipping builds in this way, and as mentioned above we are poised to do it for FCOS.

than the one installed in the ESP, but it's confusing that ostree handles some of the files under /usr/lib/ostree-boot (i.e: the kernel) but not others like everything under /usr/lib/ostree-boot/efi/.

The canonical location for the kernel (and initramfs) is now /usr/lib/modules. See #1079

Would this versioning mechanism be for data in /boot or /boot/efi? If the former, then it won't be used for kernel images?

OSTree always owns kernel images - the entire codebase is oriented around the concept of a "bootable commit" which is a pair of (kernel+initramfs, userspace) effectively; then a "deployment" is a (bootable commit, kernel arguments, idserial) etc.

This property is really important to me; if you type ostree admin status (or rpm-ostree status) and see the commit identifier, it (should be) truth - you are booted into that checksum.

The bootloader has always been outside of this.

[martinezjavier]

@dsd thanks a lot for your comments.

After a bit of reflection I think the answer of "no, a separate tool should handle it" is actually pretty reasonable based on the fact that ostree did not install the bootloader, so why should it be in charge of updating it? Conceptually, I don't think bootloader installation/updating belongs in ostree.

Yes, I'm also convinced now that bootloader updates don't really belong to ostree and that should be handled by a separate tool.

In fact, this tool could be used even in non-ostree based distros since we also have the problem of bootloader update there. For example we never update the bootloader in traditional Fedora for legacy BIOS, only for EFI because copying the binaries in the ESP is handled by the shim and grub rpm packages.

[cgwalters]

xref coreos/fedora-coreos-tracker#510