bubblewrap: Add --not-a-security-boundary flag to enable fail-open behavior#751
bubblewrap: Add --not-a-security-boundary flag to enable fail-open behavior#751ao2 wants to merge 2 commits into
--not-a-security-boundary flag to enable fail-open behavior#751Conversation
--not-a-security-boundary flag to enable fail-open …--not-a-security-boundary flag to enable fail-open behavior
|
cc @smcv even though it's still a draft |
e64137e to
418a4f1
Compare
smcv
left a comment
There was a problem hiding this comment.
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.
| (op->type == SETUP_DEV_BIND_MOUNT ? BIND_DEVICES : 0) | | ||
| ((op->type == SETUP_BIND_MOUNT && | ||
| opt_not_a_security_boundary) ? BIND_FAIL_OPEN : 0), | ||
| source, dest); |
There was a problem hiding this comment.
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);|
The commit message should have a
Optionally also a mention of |
|
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 What I expect will happen is that Similarly, if you request mounting them read-only, something like But it would be good to confirm this. |
| host system. When this option is given, certain non-fatal sandbox | ||
| setup failures (such as a bind mount failing because an automounter | ||
| did not respond in time) will produce a warning and will be skipped, | ||
| rather than causing <command>bwrap</command> to exit with an error. |
There was a problem hiding this comment.
Perhaps worthwhile to say something like
| rather than causing <command>bwrap</command> to exit with an error. | |
| rather than causing <command>bwrap</command> to exit with an error. | |
| The effect of this option might be extended to make other sandbox | |
| setup operations non-fatal in future releases of bubblewrap. |
to give us space to make more operations fail open if we find that it's desirable.
Rework how flags are passed to setup_op_bind_mount() when calling it in setup_newroot(); no functional changes are made, this is just in preparation for adding add more flags in future commits and still keep the code readable. Signed-off-by: Antonio Ospite <antonio.ospite@collabora.com>
|
Some steps to manually test the change.
|
|
I'll try to condense the comment above about the manual tests into something suitable for the commit message. |
418a4f1 to
c93289e
Compare
…behavior 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". The behavior can be tested by setting up an unavailable automount and check that: 1. bubblewrap succeeds even when accessing the unavailable automount fails; 2. remount flags like `nosuid,nodev` are not applied to the unavailable automount. ``` $ 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 $ mount | grep /mnt/bad systemd-1 on /mnt/bad type autofs (rw,relatime,fd=76,pgrp=1,timeout=0,minproto=5,maxproto=5,direct,pipe_ino=15854) $ 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 ``` steamrt/tasks#535 Resolves: containers#653 Helps: flatpak/flatpak#5112 Helps: ValveSoftware/steam-for-linux#10571 Helps: ValveSoftware/steam-runtime#766 Signed-off-by: Antonio Ospite <antonio.ospite@collabora.com>
c93289e to
d983f77
Compare
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-boundaryoption that can be used to relax the bubblewrap behavior in these specific cases, and allow it to "fail-open".