bootupd: Fix /sys read-only mount issue

[henrywang]

Bootc daily CI test failed on bootc install to-existing-root test on RHEL 9.x and 10.x with the following error. The failure comes from bootc commit 0a757685ee631ca9d53df6941af4fb71e4ac1bf3 or 0a757685ee631ca9d53df6941af4fb71e4ac1bf3, because commit 51dabaa5cb7b6313b8cd75a002862c64de3c7b85 passed.

efibootmgr needs write access to /sys/firmware/efi/efivars to modify EFI boot variables (like deleting boot entries with -B)
This PR is a simple fix, mount /sys as read-write.

Installing bootloader via bootupd
Executing: "efibootmgr" "-b" "0002" "-B"
Could not delete variable: Read-only file system
error: boot data installation failed: installing component EFI: Updating EFI firmware variables: Clearing EFI boot entries that match target Red Hat Enterprise Linux: Failed to run command: Command {
    program: "efibootmgr",
    args: [
        "efibootmgr",
        "-b",
        "0002",
        "-B",
    ],
    create_pidfd: false,
}
error: Installing to filesystem: Installing bootloader: Failed to run command: Command {
    program: "bwrap",
    args: [
        "bwrap",
        "--bind",
        "/target/ostree/deploy/default/deploy/96b094a8419f0fd81c0434730e7906d00a3571017a8755761aab66df4c7270ea.0",
        "/",
        "--proc",
        "/proc",
        "--dev",
        "/dev",
        "--ro-bind",
        "/sys",
        "/sys",
        "--bind",
        "/target/boot",
        "/boot",
        "--dev-bind",
        "/dev/nvme0n1",
        "/dev/nvme0n1",
        "--dev-bind",
        "/dev/nvme0n1p1",
        "/dev/nvme0n1p1",
        "--dev-bind",
        "/dev/nvme0n1p2",
        "/dev/nvme0n1p2",
        "--dev-bind",
        "/dev/nvme0n1p3",
        "/dev/nvme0n1p3",
        "--setenv",
        "PATH",
        "/bin:/usr/bin:/sbin:/usr/sbin:/usr/local/bin:/usr/local/sbin",
        "--",
        "bootupctl",
        "backend",
        "install",
        "--write-uuid",
        "--update-firmware",
        "--auto",
        "--device",
        "/dev/nvme0n1",
        "/",
    ],
    create_pidfd: false,
}

[gemini-code-assist]

Code Review

This pull request fixes an issue where efibootmgr fails due to a read-only /sys mount inside a bwrap sandbox. The change makes the /sys mount read-write, which resolves the immediate problem. However, this change affects all users of the BwrapCmd utility, potentially weakening the sandbox for commands that do not require write access to /sys. My review includes a suggestion to make this behavior configurable to adhere to the principle of least privilege.

[gemini-code-assist]

While this change fixes the issue with efibootmgr, it makes /sys writable for all commands executed via BwrapCmd. This could be a security concern as it weakens the sandbox for commands that don't require write access to /sys, violating the principle of least privilege.

A more robust approach would be to make this configurable. For example, you could add a field to BwrapCmd to control this behavior, and default to read-only. Callers that need a writable /sys would then explicitly opt-in.

Here's an example of how you could implement this:

// In BwrapCmd struct:
sys_rw: bool,

// In BwrapCmd::new():
// ...
sys_rw: false,

// Add a new builder method:
pub fn sys_rw(mut self, rw: bool) -> Self {
    self.sys_rw = rw;
    self
}

// In BwrapCmd::run():
if self.sys_rw {
    cmd.args(["--bind", "/sys", "/sys"]);
} else {
    cmd.args(["--ro-bind", "/sys", "/sys"]);
}

Could you please consider refactoring to this approach for better security?

[cgwalters]

Yeah, could be a followup

[henrywang]

Or I can mount /sys read-only but add a read-write bind mount for /sys/firmware/efi/efivars. This should be more secure, I think.

[cgwalters]

Yeah in this specific case I think that's the right thing to do.