-
Notifications
You must be signed in to change notification settings - Fork 339
bubblewrap: Add --not-a-security-boundary flag to enable fail-open behavior
#751
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -614,6 +614,33 @@ | |
| command line. Please be careful to the order they are specified. | ||
| </para></listitem> | ||
| </varlistentry> | ||
| <varlistentry> | ||
| <term><option>--not-a-security-boundary</option></term> | ||
| <listitem> | ||
| <para> | ||
| Declare that this invocation of <command>bwrap</command> is not | ||
| intended to create a security boundary between the sandbox and the | ||
| 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, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, technically it's not the bind mount that is failing in the scenario that motivated this option. That scenario is something like this:
"A bind mount failing" would be the So I think it would be more accurate to say something like 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 |
||
| rather than causing <command>bwrap</command> to exit with an error. | ||
|
smcv marked this conversation as resolved.
|
||
| The effect of this option might be extended to make other sandbox | ||
| setup operations non-fatal in future releases of bubblewrap. | ||
| </para> | ||
| <para> | ||
| This option is intended for callers such as | ||
| <application>xdg-dbus-proxy</application> or Steam that use | ||
| <command>bwrap</command> to adjust the filesystem layout for a | ||
| process, but do not rely on it to create a security boundary. | ||
| </para> | ||
| <para> | ||
| Other operations that are fundamental to establishing the sandbox | ||
| (creating namespaces, <function>pivot_root</function>, | ||
| dropping capabilities) will still cause a hard failure | ||
| regardless of this option. | ||
| </para> | ||
| </listitem> | ||
| </varlistentry> | ||
| </variablelist> | ||
| </refsect1> | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I verified that having the check under the for loop that start with
i = 1seem to be enough:This seems to cover also host mount points like
/hdddirectly under/, because even when--bind / /is usedmount_tab[0].mountpointwill be/newroot, and so the code will handle/newroot/hddAnd we can assume that
/newrootis not on an automount?So we shouldn't nee to add the check also for
mount_tab[0].mountpoint.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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/amis an automount) then I thinkmount_tab[0].mountpointmight be the automounter? But in that case, I thinkmount(2)would trigger the automount, mount the underlying filesystem, and either succeed or fail as appropriate. Please could you confirm that?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's reasonable to say that if
/mnt/badis a broken automount, thenbwrap --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/badonto$DEST, and if we're unable to do so, we should fail.Conversely, if
/mnt/badis a broken automount and we mount/mntonto$DESTsuccessfully, it's a lot more reasonable to succeed even though were unable to remount/mnt/badasnodev.