Github Issue #885 - qubes-dom0-update could check whether /boot is mounted
Very old issue opened by Marek on Mar 8, 2015, quoting a user from Google groups:
it might make sense to check before updates to /boot whether it is actually mounted by checking whether there are any files in /boot.
I just noticed that qubes-dom0-update wrote to /boot without it being mounted (i.e. it wrote to a previously empty folder on /). I usually don’t mount /boot in fstab, because it tends to make the boot process faster by a couple of seconds (& grub doesn’t really seem to need it as it is more low-level).
This is an extremely easy issue to solve. qubes-dom0-update is a bash script. And you could easily find information on how to check if a directory is empty in bash (like this nixCraft post). I leave this issue for other people who expressed their interest on contributing to Qubes OS. And I will come back to it in a month if it is not solved.
Requirements
bash scripting knowledge, being able to test it on an actual machine with split /boot. Reading the official guidelines on how to contribute.
If someone run qubes-dom0-update without mounting /boot before this change will be made then the /boot directory could already contain files even without /boot directory being mounted.
So checking if /boot directory is empty is not a certain way to check if it’s actually mounted.
If the /boot is present in /etc/fstab but has noauto option then it could be checked with certainty whether it’s mounted or not. If there is no separate /boot in /etc/fstab then it could be assumed that /boot doesn’t have a separate partition and it’s located in the root partition.
But if there is no /boot entry in /etc/fstab and user is mounting it manually then I don’t know of a certain way to check whether it’s mounted or not.
I think it would be a highly unlikely scenario. Doesn’t this require unencrypted root?
This is again a highly unlikely scenario. I am not sure if Anaconda installer would go forward without /boot access. And it should add it to fstab.
But in any case, the patch could check if fstab contains /boot and then check for /boot in mount command output. For safety measures. It could check for the existence of /boot/{inintramfs*, grub, vmlinuz*, …}
It can be done with encrypted root as well e.g. encrypted /boot scenario:
Yes, that’s unlikely but still possible scenario with some custom partitioning or user commenting out the /boot in fstab instead of adding noauto option.
I think the best way would be to just check if the /boot and /boot/efi (in case of EFI system) entries in fstab files are mounted if present. I think there is no need to check the commented out entries.
If the entries are present in fstab but not mounted then warn the user and ask to:
Try again - check again if entries in fstab files are mounted if present
Skip - continue with updates without /boot and/or /boot/efi mounted
Cancel - cancel update
If no entries are present in fstab then warn the user that:
“No separate /boot and/or /boot/efi partitions found, press Continue if you have /boot and root directory on the same partition or mount the /boot and/or /boot/efi partitions manually before pressing Continue”
And ask to:
Continue - continue with updates assuming that user took actions if needed
Cancel - cancel update
But this will require implementing interaction with user in update tools.
Related to this issue or should be a separate issue?
OK. What we have so far? Something like this?
#!/bin/bash
# Check if boot partition is a mount point and is mounted.
check_for_boot () {
local boot
# These are the default boot mountpoints on Fedora 37
[ -d /sys/firmware/efi ] && boot=/boot/efi || boot=/boot
# No reason to check further if boot partition is not in fstab
mountpoint --quiet "${boot}" || return
# No reason to interact with user if boot is already mounted
mount | grep --quiet "${boot}" || return
# Here we should either go interactive or raise an error and exit
echo boot is not mounted. What to do? Exit with warning or interact?
exit 1
}
P.S.: I am personally against the idea of making this interactive. A user with split boot is usually an advanced user. Simple mistake if forgetting to attach and/or mount the the boot media is possible. But showing a simple warning should be sufficient. We only have to consider to abort it entirely if boot partition is not mounted. Or add something like --ignore-boot option to command if user wants to force it. Maybe they are just installing emacs or something similar that has nothing to do with boot partition.
I think it’s better to check both /boot and /boot/efi mountpoints:
#!/bin/bash
# Check if boot partition is a mount point and is mounted.
check_for_boot () {
local boot
# No reason to check further if boot partition is not mounted
mountpoint --quiet /boot || return
# No reason to check further if EFI partition is not mounted on EFI system
[ -d /sys/firmware/efi ] && ( mountpoint --quiet /boot/efi || return )
# Here we should either go interactive or raise an error and exit
echo boot is not mounted. What to do? Exit with warning or interact?
exit 1
}
Checking mount command output by just grepping “/boot” string is not good since it can also match some other user mounts e.g. /home/user/tmp/bootdisk.
The output of mount command needs to be parsed strictly e.g. like this:
mount | awk '{print $3}' | grep -Fxq "${boot}"
For console update tool sure, but for GUI update tool it’ll be inconvenient for user if it’ll just exit if /boot won’t be mounted. Imagine if user will open Qubes Update GUI tool, choose a few tens of templates to be updated from a hundred templates along with dom0 and then press Next only for GUI tool to exit with an error and user will be forced to open Qubes Update GUI tool again and select the templates all over again. That’d be frustrating.
You are right. But this needs to be tested. I will try to emulate the scenario. This needs to be tested thoroughly to see if works well with GUI updater.
OK. We have a working prototype. With some caveats. Here is the patch:
qubes-dom0-update patch
--- qubes-dom0-update.orig 2024-06-15 11:42:31.823712605 +0330
+++ qubes-dom0-update 2024-06-15 15:13:19.851313032 +0330
@@ -234,6 +234,43 @@
fi
rm -f /var/lib/qubes/updates/errors
+# Synopsis: check_mounted MOUNTPOINT
+check_mounted() {
+ local CHOICE
+ # No reason to check further if partition is not in fstab
+ awk -v PART="${1}" '!/^[ \t]*#/{ if ( $2 == PART ) { exit 0}} ENDFILE {exit -1}' < /etc/fstab
+ [[ ${?} -ne 0 ]] && return
+ # No reason to check further if partition is already mounted
+ awk -v PART="${1}" '{if ($2 == PART ) { exit 0 }} ENDFILE{exit -1}' < /proc/mounts
+ [[ ${?} -ne 0 ]] || return
+ # Ask user to manually mount partition if user is using GUI Updater
+ if [ ! -t 1 ]; then
+ echo "Could not decide about unmounted ${1} partition in non-interactive/GUI mode!"
+ echo "Please mount ${1} manually before proceeding with updates"
+ exit 1
+ fi
+ read -p "${1} partition is not mounted! mount it now? (y)es, (n)o, (a)bort operation " CHOICE
+ case ${CHOICE} in
+ y|Y)
+ mount "${1}"
+ if [[ ${?} -ne 0 ]]; then
+ echo "Mounting of ${1} was unsuccessful! aborting."
+ exit 1
+ fi
+ ;;
+ n|N) echo "Warning! Proceeding forward without mounting ${1}";;
+ a|A) echo Operation aborted!; exit 1;;
+ *) echo Invalid choice. Aborting!; exit 1;;
+ esac
+}
+
+if [ "$CHECK_ONLY" != "1" ]; then
+ # Check if /boot is mounted on split root systems
+ check_mounted "/boot"
+ # Check if efi partition is mounted on UEFI systems
+ [ -d /sys/firmware/efi ] && check_mounted "/boot/efi"
+fi
+
echo "Using $UPDATEVM as UpdateVM to download updates for Dom0; this may take some time..." >&2
# qvm-run by default auto-starts the VM if not running
We can not go interactive for GUI Updater. Since it needs zenity or something similar which is removed in R4.2. Here is the related Github issue. I guess aborting update with a warning is better than proceeding with update if user forgets to mount /boot & /boot/efi. Or Maybe force mount and then abort if fails in the GUI updater?
If qubes-dom0-update is aborted in GUI updater because of missing /boot (or /boot/efi), it still shows the green checkmark. This has nothing to do with this patch. GUI Updater does not check for subprocess.returncode of qubes-dom0-update (only does it for refresh). So unrelated bug and not our fault. It will show the green checkmark even if the actual update fails (maybe due to interrupted internet connection)
I could not understand the scenario of no entries in fstab but somehow needed to be mounted individually. I studied this scenario but it implies that boot resides within root. So no need to mount it.
I guess aborting update is the only way for now.
I think it could be done without interaction with user in GUI updater and lessen the user frustration if it was possible to go back to the main Qubes Updater GUI tool screen where you select the qubes to update after the update will finish and the qubes/dom0 that were checked to be updated and failed to update will stay checked there if user go back there so user can retry these failed qubes/dom0 update after fixing the issue that caused them to fail.
I don’t think that’s a good idea.
Used could have fstab /boot entry without using UUID e.g.:
/dev/sdb1 /boot ...
And at the time of update the /dev/sdb1 could be not the actual device containing /boot partition.
It’s unlikely situation, but still possible.
If user don’t know how fstab works and didn’t want to read the doc or search the net to understand how to properly fix the issue with missing fstab entry during boot then user could just comment out or remove the /boot entry that is causing the system boot to fail instead of adding noauto option that user don’t know about.
I could imagine such situation:
User installed Qubes OS with /boot partition on USB disk and without creating sys-usb qube.
Later the user wants to create sys-usb qube, but after rebooting the system user can’t boot anymore and sees that it’s waiting for the /boot partition to be mounted.
Then user boots with disabled qubes autostart and usbguard to fix the issue.
User knows that mounts are stored in /etc/fstab file but doesn’t know how it works.
User doesn’t want to read the docs or search the net for a proper fix so user just removes the /boot entry from /etc/fstab and decides to just mount it manually when updating dom0.
It is probable but I believe this scenario to be highly unlikely. A user who is using split boot with detached LUKS header is an advanced user. They are probably following a proper guide such as your community guide or know what they are doing.
The proposed patch might not help the unlikely scenario, but will neither create further issues. I guess the patch could be tested for one or two weeks and then submitted for review.
It’s not only about detached LUKS header.
Some user could multiboot to Windows or some other OS on the same machine and after reading the warning that this could lead to Qubes OS compromise by some malware writing to the Qubes OS /boot partition user could decide to install Qubes OS /boot partition on a removable USB disk and only connect it to boot Qubes OS.
But anyway these are some specific cases and I guess they could be ignored.
Related to this issue or should be a separate issue?
I don’t know. Perhaps both.
If it was my script, I would refactor it completely. I don’t claim expertise but as I write shell scripts almost on a daily basis, I think there is a lot that can be improved in regards to the coding style (this applies to many scripts used in Qubes). Clean code is easier to maintain long term and editing just a few lines to improve something, only sustains the mess. Personally, that is a big stopper for me to contribute. I don’t know if there have been any discussions about this.
Well. The current trend is to avoid bash as much as possible and rewrite everything in Python whenever possible. I quote this directly from official Coding Style Guidelines:
Avoid writing scripts in bash whenever possible. Use python instead. Bash-scripts are Unix-specific and will not work under Windows VMs, or in Windows admin domain, or Windows gui domain.
And I have seen multiple tools which used to be bash scripts are written from scratch in Python.