System-wide sandboxing framework - sandbox-app-launcher

visudo is a good idea but insufficient. We can write to a temporary file. Then use visudo as a sanity check. But even if we verified using visudo that the file is valid, writing into /etc/sudoers.d needs to be atomic. I am worried about file system inconsistency. For example, power loss during writing the file. Either the file creation is complete and successful or partial and not really part of the file system.

I.e. can

echo "0123456789" > testfile

result in only ending up


on the hard drive if power is lost in the middle of the operation?

1 Like

References on atomic writes to disk:

(We currently use ext4 as file system. Not btrfs. Just to show why this might be an issue.)

1 Like

set -x
set -e
set -o pipefail


sudoers_file_content="## test"

mkdir -p /etc/sandbox-app-launcher

## Non-ideal location for temporary file but required for following atomic rename.
## Should not cross potential file system barriers such as /tmp on a different partition than /etc.
echo "$sudoers_file_content" | tee "/etc/sandbox-app-launcher/sandbox-app-launcher_${app_user}" >/dev/null

## Using '>/dev/null' to hide success messages on stdout.
## Would still show error messages if any on stderr.
SUDO_EDITOR='/bin/false' VISUAL='/bin/false' EDITOR='/bin/false' visudo --strict --check --file "/etc/sandbox-app-launcher/sandbox-app-launcher_${app_user}" >/dev/null

## Atomic rename.
mv "/etc/sandbox-app-launcher/sandbox-app-launcher_${app_user}" "/etc/sudoers.d/sandbox-app-launcher_${app_user}"
1 Like

But even if we verified using visudo that the file is valid, writing into /etc/sudoers.d needs to be atomic.

Does visudo not write to /etc/sudoers.d/ atomically? I thought the point of visudo was to write to /etc/sudoers.d/ safely by first writing to a temporary file, similarly to what your example code does, plus checks to prevent multiple users from editing sudoers simultaneously.

1 Like

I don’t think visudo is designed for use in scripts. Can’t find many examples of using it that way.

Above answer doesn’t consider bash pipefail. I.e.

set -o pipefail


echo 'foobar ALL=(ALL:ALL) ALL'  | visudo ... something ...

doesn’t return non-zero on the shell unless we set set -o pipefail. Will edit my above draft to add set -o pipefail.

visudo supports various environment variables for editor setting. It might be best if we set these to /bin/false? Just in case? I am not sure in our scenario there is any chance for these environment variables to be set.

SUDO_EDITOR='/bin/false' VISUAL='/bin/false' EDITOR='/bin/false' visudo

(Will edit above pseudo code to add.)

Related quote from man page:

If set, visudo will use the value of the SUDO_EDITOR, VISUAL or EDITOR environment variables before falling back on the default editor list. Note that this may create a security hole as it allows the user to run any arbitrary command as root without logging. A safer alternative is to place a colon-separated list of editors in the editor variable. visudo will then only use SUDO_EDITOR, VISUAL or EDITOR if they match a value specified in editor. If the env_reset flag is enabled, the SUDO_EDITOR, VISUAL and/or EDITOR environment variables must be present in the env_keep list for the env_editor flag to function when visudo is invoked via sudo. The default value is on, which can be set at compile time via the --with-env-editor configure option.

Above link suggests to use tee as editor for sudo.


set -x
set -e


sudoers_file_content="## test"

mkdir -p /etc/sandbox-app-launcher

## Should probably set after "set -e" for better style. Setting it here to
## illustrate importance.
set -o pipefail

## visudo does not check exit code of the editor.

echo 'foobar ALL=(ALL:ALL) ALL' | SUDO_EDITOR='tee' VISUAL='/bin/false' EDITOR='/bin/false' visudo --strict --file /tmp/x >/dev/null

But if you replace tee with nonexistent then sudo doesn’t notice that the editor actually failed (non-zero exit code) and doesn’t exit with an error message.

In light of that, I think my above pseudo code (System-wide sandboxing framework - sandbox-app-launcher - #291 by Patrick) is best suited.

Are we partially re-inventing flatpak which is internally using bubblewrap?
I guess we have a nicer decoupled design.

Also interesting, perhaps worth not re-inventing, regarding /shared:

sandbox-app-launcher (bubblewrap based) will probably be incompatible with applications from flatpak since these have their own bubblewrap based mandatory sandbox?

Have you found a way to connect wayland clients to wayland even if they are on different mount namespaces?

Flatpak suffers from numerous fundamental flaws in its sandboxing design.


Not yet.

1 Like

Good. Merged.

Quotes these variables otherwise spaces in variable names (even though we shouldn’t have any) would break that loop.

This line looks unnecessarily complex:

: ${app_path:="$(type -P "${app_name}" || true)"}
  • Starting a line with : is very uncommon style.
  • If we need || true would be nice to have it outside of the variable assignment.
  • := - Set $FOO to val if unset (or null) - why do we need that there?

shellcheck reports:

In usr/bin/sandbox-app-launcher line 41:
: ${app_path:="$(type -P “${app_name}” || true)"}
^-- SC2223: This default assignment may cause DoS due to globbing. Quote it.

Could we simplify please?

app_path="$(type -P "${app_name}")" || true


1 Like

Could you please add /etc/X11/Xsession.d/20software_rendering_in_vms (from vm-config-dist) to xsession_d_file_list_executable if that makes sense?

1 Like

It was intentionally made like that as to support manually setting app_path. It allows a user to execute:

app_path="/path/to/example" sandbox-app-launcher setup example

Why would it be necessary?

1 Like

See file /etc/X11/Xsession.d/20software_rendering_in_vms.

  • monero-gui in sandbox
  • any other Qt based application being slow inside VMs because of this
1 Like

Also, is there any need for the full file path in the array?

We could have:


for file_name in "/etc/X11/Xsession.d/${xsession_d_file_list_executable[@]}" ; do

Makes it simpler.

madaidan via Whonix Forum:

Add 20software_rendering_in_vms by madaidan · Pull Request #51 · Kicksecure/sandbox-app-launcher · GitHub



for file_name in "/etc/X11/Xsession.d/${xsession_d_file_list_executable[@]}" ; do

I don’t think this code would work. Tested. Would expand to:



String /etc/X11/Xsession.d/ could be inside the for loop. That would

But I don’t see much advantage in that. Trading a bit shorter contents
inside the variable assignment at the cost of higher complexity inside
the for loop. Using the full file names such as
/etc/X11/Xsession.d/20uwt makes it easier to grep (the whole Whonix)
source code. As currently implemented, code is easier to understand.
Currently one can copy/paste one string, the file name, and then use cat
or open in an editor to view the file. By making it
/etc/X11/Xsession.d/ + variable it gets harder. Then one has to
understand this a bit better, copy/paste /etc/X11/Xsession.d/ (or
manually write) + the variable name (such as 20uwt).

1 Like

Would it be possible to combine sandbox-app-launcher with constrained system resources program starter wrapper? Either by one script calling the other. Which one would run which? Or by adding similar code to sandbox-app-launcher?

sandbox-app-launcher currently attempts to improve security by isolating applications in a sandbox. It however does not yet have any defenses against denial of service (DOS) by exhausting system resources. Either as part of an actual DOS attack (compromised application doing DOS). Or as an attempt to prevent a buggy application from slowing down or even freezing the system.

1 Like

what general purpose system utilities can be integrated for DoS resistance? Are the defaults sane?

1 Like

There doesn’t seem to be much available against local source DoS (equals misbehaving buggy applications). See constrained system resources program starter wrapper.

Are the defaults sane?

I don’t think there are any default restrictions in Debian at all.

1 Like

We should probably add similar code to sandbox-app-launcher.

We do use AppArmor to enforce a maximum of 200 processes as a basic protection but we should definitely invest in more.

sandbox-app-launcher/sandbox-app-launcher at master · Kicksecure/sandbox-app-launcher · GitHub

More on rlimit rules: apparmor.d(5) — apparmor — Debian buster — Debian Manpages

No, the dreaded :(){ :|:& };: can still be used to trivially wreck a system.

1 Like