bubblewrap: Add --not-a-security-boundary flag to enable fail-open behavior

[ao2]

Some callers of bwrap (e.g. xdg-dbus-proxy, Steam Runtime) use it purely to adjust filesystem layout, without any expectation of a security boundary between the sandbox and the host.

For these callers, hard failures during sandbox setup (such as an automount timeout on a bind source) are unnecessarily fatal.

So add a new --not-a-security-boundary option that can be used to relax the bubblewrap behavior in these specific cases, and allow it to "fail-open".

[ao2]

cc @smcv even though it's still a draft

[smcv]

I'd like some smoke-test coverage for this in the test suite. Setting up a failing automounter (so that there will genuinely be an error to ignore) is probably too difficult to do in bubblewrap's test suite, but we can at least exercise the happy path where the new option makes no practical difference.

[smcv]

Why does only --bind fail open? If bubblewrap is not a security boundary, shouldn't --dev-bind and --ro-bind be equally willing to fail open?

Separately, I think we're getting enough flags that the repeated ?: operators make it harder to read, and it would be better more like:

bind_option_t bind_flags = 0;

if (opt_not_a_security_boundary)
  bind_flags |= BIND_FAIL_OPEN;

if (op->type == SETUP_RO_BIND_MOUNT)
  bind_flags |= BIND_READONLY;

if (op->type == SETUP_DEV_BIND_MOUNT)
  bind_flags |= BIND_DEVICES;

/* etc. */

setup_op_bind_mount (bind_flags, source, dest);

[ao2]

As mentioned in the commit message -dev-bind and --ro-bind seemed more critical on a first look.

For --dev-bind I thought that, since the argument is probably a device node needed for the intended operations, if remounting failed it might affect functionality and we might want to signal it.

For --ro-bind I thought that remounting ro was asked explicitly and so we might want to fail hard if we cannot satisfy an explicit request.

But maybe I am being overly cautious here, because of my limited familiarity with bwrap use cases, and the warning is good enough, since --not-a-security-boundary was also an explicit directive.

[ao2]

Done in the latest version.

[smcv]

The commit message should have a Resolves: https://github.com/containers/bubblewrap/issues/653, and optionally also:

• Helps: https://github.com/flatpak/flatpak/issues/5112
• Helps: https://github.com/ValveSoftware/steam-for-linux/issues/10571
• Helps: https://github.com/ValveSoftware/steam-runtime/issues/766

Optionally also a mention of steamrt/tasks#535.

[smcv]

For manual testing, I see you've described how to reproduce the problem in ValveSoftware/steam-for-linux#10571 (comment) (it might be useful to summarize that in the commit message). It would be useful if you could create other mount points - perhaps /mnt/bad for the broken automount, and /mnt/good for a loopback mount that is working correctly - and then inspect /proc/self/mountinfo inside the container to see what happens after the error is ignored.

What I expect will happen is that /mnt/good will be remounted nodev, but the broken automount at /mnt/bad might lack the nodev option.

Similarly, if you request mounting them read-only, something like --ro-bind /mnt /mnt, I expect that /mnt/good will be remounted ro, but /mnt/bad will still be rw, which is why we need to fail closed for the default "yes, security boundary" use-case.

But it would be good to confirm this.

[ao2]

Some steps to manually test the change.

0. Prepare a loop device:

   $ dd if=/dev/zero of=good.bin bs=1 count=1M
   $ sudo mkfs.ext4 good.bin 
   $ sudo modprobe loop 
   $ sudo losetup loop0 good.bin 
   $ export LOOP_UUID="$(lsblk -n -o uuid /dev/loop0)"
   

1. Add an available and an unavailable automount:

   $ sudo -E sh -c 'echo "UUID=${LOOP_UUID} /mnt/good  ext4 defaults,noatime,nofail,inode_readahead_blks=64,nobarrier,x-systemd.automount    0  1" >> /etc/fstab'
   $ sudo sh -c 'echo "UUID=00000000-0000-0000-0000-000000000000 /mnt/bad  ext4 defaults,noatime,nofail,inode_readahead_blks=64,nobarrier,x-systemd.automount    0  1" >> /etc/fstab'
   $ sudo systemctl daemon-reload
   $ sudo systemctl restart mnt-bad.automount 
   $ sudo systemctl restart mnt-good.automount 
   

2. Check mount options on the host:

   $ mount | grep /mnt
   systemd-1 on /mnt/bad type autofs (rw,relatime,fd=76,pgrp=1,timeout=0,minproto=5,maxproto=5,direct,pipe_ino=15854)
   systemd-1 on /mnt/good type autofs (rw,relatime,fd=77,pgrp=1,timeout=0,minproto=5,maxproto=5,direct,pipe_ino=15857)
   

3. Check that mount options inside the container are not applied to unavailable automounts when passing --not-a-security-boundary, while automounts are being accessed:

   $ ls /mnt/good &>/dev/null &
   $ ls /mnt/bad &>/dev/null &
   $ ./_build/bwrap --not-a-security-boundary --bind / / grep /mnt /proc/self/mountinfo
   bwrap: Can't remount /newroot/mnt/bad submount (No such device), ignoring error
   860 769 0:41 / /mnt/bad rw,relatime master:45 - autofs systemd-1 rw,fd=107,pgrp=1,timeout=0,minproto=5,maxproto=5,direct,pipe_ino=144522
   861 769 0:66 / /mnt/good rw,relatime master:576 - autofs systemd-1 rw,fd=109,pgrp=1,timeout=0,minproto=5,maxproto=5,direct,pipe_ino=145727
   863 861 7:0 / /mnt/good rw,nosuid,nodev,noatime master:599 - ext4 /dev/loop0 rw,nobarrier,inode_readahead_blks=64
   

   We can see that nosuid,nodev are applied to the /mnt/good actually autmounted from /dev/loop0 but not to the inactive automounts.

4. Check that ro option is not applied when using --ro-bind on an unavailable automout that is accessed:

   In order to test the behavior with --ro-bind in a representative way we have
   to prevent /mnt/bad to be remounted twice, otherwise we may loose the
   reproducibility window related to the unavailable mount point being accessed at the same time when bubblewrap tries to remount it.

   Remember that if the unavailable automount is inactive (i.e. is not being accesses when bubblerwap runs) then systemd will just accept the new flags even without --not-a-security-boundary.

   We can achieve the desired behavior by using an alternative rootfs for /, which does not include /mnt.

   $ sudo debootstrap trixie test-rootfs
   $ ls /mnt/good &>/dev/null &
   $ ls /mnt/bad &>/dev/null &
   $ ./_build/bwrap --not-a-security-boundary --bind ./test-rootfs / --bind /proc /proc  --dev-bind /dev /dev --ro-bind /mnt /mnt grep /mnt /proc/self/mountinfo
   bwrap: Can't remount /newroot/mnt/bad submount (No such device), ignoring error
   742 725 254:1 /mnt /mnt ro,nosuid,nodev,relatime master:1 - ext4 /dev/mapper/Z13--vg-root rw,errors=remount-ro
   743 742 0:40 / /mnt/bad rw,relatime master:43 - autofs systemd-1 rw,fd=76,pgrp=1,timeout=0,minproto=5,maxproto=5,direct,pipe_ino=15854
   744 742 0:41 / /mnt/good rw,relatime master:45 - autofs systemd-1 rw,fd=77,pgrp=1,timeout=0,minproto=5,maxproto=5,direct,pipe_ino=15857
   745 744 7:0 / /mnt/good ro,nosuid,nodev,noatime master:568 - ext4 /dev/loop0 rw,nobarrier,inode_readahead_blks=64
   

   Where we can see that the ro option is not applied to /mnt/bad but is applied to the active /mnt/good.

[ao2]

I'll try to condense the comment above about the manual tests into something suitable for the commit message.

[ao2]

I verified that having the check under the for loop that start with i = 1 seem to be enough:

  if (recursive)
    {
      for (i = 1; mount_tab[i].mountpoint != NULL; i++)
        {
           ...

This seems to cover also host mount points like /hdd directly under /, because even when --bind / / is used mount_tab[0].mountpoint will be /newroot, and so the code will handle /newroot/hdd

And we can assume that /newroot is not on an automount?

So we shouldn't nee to add the check also for mount_tab[0].mountpoint.

[smcv]

And we can assume that /newroot is not on an automount?

Normally, yes: it's on a tmpfs, look for comment /* Create a tmpfs which we will use as / in the namespace */.

If we're mounting an automount onto the root (like --bind /mnt/am / where /mnt/am is an automount) then I think mount_tab[0].mountpoint might be the automounter? But in that case, I think mount(2) would trigger the automount, mount the underlying filesystem, and either succeed or fail as appropriate. Please could you confirm that?

[smcv]

I think it's reasonable to say that if /mnt/bad is a broken automount, then bwrap --not-a-security-boundary ... --bind /mnt/bad $DEST ... should still fail: even though we're not imposing a security boundary, we still asked to mount /mnt/bad onto $DEST, and if we're unable to do so, we should fail.

Conversely, if /mnt/bad is a broken automount and we mount /mnt onto $DEST successfully, it's a lot more reasonable to succeed even though were unable to remount /mnt/bad as nodev.

[ao2]

Does it make sense to still fail closed for --ro-bind and --dev-bind ?

[ao2]

Addressed this in the latest version.

[ao2]

As mentioned in the commit message -dev-bind and --ro-bind seemed more critical on a first look.

For --dev-bind I thought that, since the argument is probably a device node needed for the intended operations, if remounting failed it might affect functionality and we might want to signal it.

For --ro-bind I thought that remounting ro was asked explicitly and so we might want to fail hard if we cannot satisfy an explicit request.

But maybe I am being overly cautious here, because of my limited familiarity with bwrap use cases, and the warning is good enough, since --not-a-security-boundary was also an explicit directive.

[ao2]

Done in the latest version.

[smcv]

And we can assume that /newroot is not on an automount?

Normally, yes: it's on a tmpfs, look for comment /* Create a tmpfs which we will use as / in the namespace */.

If we're mounting an automount onto the root (like --bind /mnt/am / where /mnt/am is an automount) then I think mount_tab[0].mountpoint might be the automounter? But in that case, I think mount(2) would trigger the automount, mount the underlying filesystem, and either succeed or fail as appropriate. Please could you confirm that?

[smcv]

I think it's reasonable to say that if /mnt/bad is a broken automount, then bwrap --not-a-security-boundary ... --bind /mnt/bad $DEST ... should still fail: even though we're not imposing a security boundary, we still asked to mount /mnt/bad onto $DEST, and if we're unable to do so, we should fail.

Conversely, if /mnt/bad is a broken automount and we mount /mnt onto $DEST successfully, it's a lot more reasonable to succeed even though were unable to remount /mnt/bad as nodev.

[smcv]

Hmm, technically it's not the bind mount that is failing in the scenario that motivated this option. That scenario is something like this:

• user wants to bwrap --ro-bind (or bwrap --bind), let's say /mnt onto /mnt
• bwrap --ro-bind implies that they want the whole /mnt hierarchy to be mounted ro, and similarly either bwrap --ro-bind or bwrap --bind implies that they want the whole /mnt hierarchy to be mounted nodev
• but mount(2) can't do that atomically, so we can only achieve that[1] with the C equivalent of:
  1. mount --bind /oldroot/mnt /newroot/mnt
  2. mount -o remount,ro,nodev,nosuid /newroot/mnt
  3. mount -o remount,ro,nodev,nosuid /newroot/mnt/bad
  4. mount -o remount,ro,nodev,nosuid /newroot/mnt/good
  5. ...

"A bind mount failing" would be the mount --bind-equivalent, but that's not what is failing here: what is failing here is the remount of /newroot/mnt/bad, specifically.

So I think it would be more accurate to say something like

          setup failures (such as a subdirectory remount failing because an automounter
          did not respond in time) will produce a warning and will be skipped,

If you're touching this text anyway, consider also doing semantic line-breaks after each sentence, after each comma, before and after the part in parentheses and so on, even if the line-length limit has not yet been reached, to allow later edits with minimal diff. Ordinary flowing text in Docbook gets line-wrapped automatically, so that won't affect the rendered output.

---

[1] assuming we don't want to increase our minimum kernel version, which I think we don't. In newer kernels we'd be able to use the "new mount APIs", like open_tree(2), mount_setattr(2) and move_mount(2), but using those APIs exclusively would mean losing the ability to run on old host OSs like Ubuntu 20.04.