[HOME] [DOWNLOAD] [DOCS] [NEWS] [SUPPORT] [TIPS] [ISSUES] [CONTRIBUTE] [DONATE]

System-wide sandboxing framework - sandbox-app-launcher

That’s a cool idea. It might break an exploit chain, disable some exploit payloads, make exploitation harder. Non-perfect as per Are non-perfect Defenses that defeat off-the-shelf Viruses a worthwhile Development Goal? or could it be in some cases really strong?

Similar to:

  • Automatically creating a mandatory access control profile based on parsing the binary (or source code).
  • Automatically creating an apparmor profile based on parsing the binary (or source code).

I am wondering why nobody earlier got that idea? Or did someone?

A bit similar to this:

Design:

  • For sure keep this in a separate file for better abstraction. Might even be useful for some other purpose. A good addition to https://gitlab.com/whonix/helper-scripts? (Not important if you prefer to keep all inside sandbox-app-launcher package.)
  • helper-scripts package shouldn’t change anything on the system. Should have most minimal risk of breaking unrelated packages that don’t depend on it. Should just be a vehicle to ship files. Therefore the parser could be added there.
  • The caching of parsed information, folder creation however belong into sandbox-app-launcher package since that is making more intrusive file system modifications.
  • Have a feature in sandbox-app-launcher to disable/enable this feature per application using the usual config snippets. (Default: enabled, if attainable/realistic development milestone.)

With sandbox-app-launcher we have to worry about too things:

  • local privilege escalation vulnerabilities
  • sandbox escape vulnerabilities

And sandbox-app-launcher might be used to start two different types of binaries:

  • those which “only” contain vulnerabilities and/or bugdoors
  • those containing specifically crafted code intended to exploit the parser and its dependencies (currently objdump, ldconfig, pacman (not on Debian), sandbox-app-launcher (not sure the attack surface of itself), its dependencies sudo, bash, bubblewrap

Therefore it might make sense to run the parser (and its dependencies) itself through either sandbox-app-launcher, bubblewrap and/or some mandatory access control?

At the end of the script, check that variable all_paths contains only white listed characters? (To avoid crafted, malicious contents of being re-used in command line options later by sandbox-app-launcher calling sudo, bwrap.)

printf, /usr/bin/printf by coreutils: What’s the security record of that? Any way that printf can lead to unwanted code execution? Really required to use printf instead of bash’s built-in echo?

1 Like

Merged.

This is to have as few levels of indentation/logic as possible. Closing an if/then statement whenever possible to keep complexity, mental load low.

I’ll look into improvement of some checks now as discussions earlier.

1 Like

I am experiencing one strange issue.

sudo sandbox-app-launcher setup ristretto

INFO: Done, setup complete, OK.

sudo sandbox-app-launcher remove ristretto

userdel: sal-ristretto mail spool (/var/mail/sal-ristretto) not found
INFO: Done, removal complete, OK.

sal does the right things in bash xtrace as far as I can see…

adduser --home /home/sandbox-app-launcher-appdata/ristretto --no-create-home --> disabled-login --gecos '' sal-ristretto

Ok.

userdel --remove --force sal-ristretto

userdel: sal-ristretto mail spool (/var/mail/sal-ristretto) not found

Any idea?


cat /etc/passwd | grep ristretto

sal-ristretto:x:1001:1001:,:/home/sandbox-app-launcher-appdata/ristretto:/bin/bash

Should we change from default login shell /bin/bash to /bin/false?

To do so we would add to adduser:

--shell /bin/false

Will stop making changes for now.

1 Like

Should remove also do the following?

  ## Don't leave any left-over processes such as the D-Bus daemon.
  killall -9 -u "${app_user}"

Though, is going to be slightly more complicated as killall is asynchronous. Sends only a signal to process. Doesn’t block/wait until really killed the process.


Need to fix some bugs that I just introduced. Working on it now.

1 Like

Well, that’s fixed and was an interesting exercise. However, I think the wrapper creation inside sal is quite ugly code wise.

Do we really need a dynamically generated wrapper script?

The only dynamic parts are really just A) set -x, bash xtrace for debugging or not which is not super important and could be implemented to only do that if some variable is set and B)

"/path/to/app" "${@}"

i.e. path to binary and command line options. All three variables (debug, app_path and args) could be set in sal. Then we could have a “static” script in a different file. sal would set appropriate variables and the script would serve the same purpose. No more need to have 1 script per application or dynamic wrapper script generation.

What do you think?

1 Like

I’m not sure if it would be very strong. The impacts need more research.

It could make our dynamic native code execution / W^X restrictions stronger. Currently those don’t cover interpreters (i.e. shell, python, etc. scripts; this should probably be documented in the wiki) but with these restrictions, if the app doesn’t actually need them, those interpreters won’t be available within the sandbox for an attacker to possibly abuse. For example, with Chromium, bash and python aren’t accessible in the sandbox.

Another possible advantage is that it could limit the amount of code reuse gadgets available to an attacker in the sandbox to complement W^X. However, I highly doubt this will be significant and it will most likely be negligible since all necessary gadgets would probably be in libc, etc. already. There has been research on the ineffectiveness of removing gadgets within a singular binary (https://arxiv.org/abs/1902.10880) and when considering a complete sandboxed environment with a lot of accessible binaries/libraries, it’d be even less effective.

grsecurity’s RBAC did.

https://grsecurity.net/featureset/rbac

Maybe. Vulnerabilities have been discovered in objdump before: https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=objdump

We could just use echo.

1 Like

That’s a harmless error. userdel --remove attempts to delete the mail spool, as well as the home folder but since we don’t setup a mail spool, there’s nothing to delete.

Yes.

That would be great.

1 Like

Done.

Currently we don’t have proper whitespace handling.

    bash -c "
  bwrap \
  --ro-bind /bin /bin \
...
  --ro-bind ${main_app_dir}/machine-id /etc/machine-id \
...
  10< <(getent passwd root ${app_user} nobody) \
  11< <(getent group root ${app_user} nobody) \
  12< ${seccomp_filter} \
  ${bwrap_args} \
  ${wrapper_script} ${@}"

User names or folder names with white spaces wouldn’t work, but we don’t have these and check that there are no such characters.

Arguments however ${@} would be broken. Such as

sandbox-app-launcher start vlc "my video"

would break. That would result in VLC trying to open a file my and a file video.

It could be made to work similar to: https://github.com/Whonix/apparmor-profile-everything/commit/d3eccd40b1547114159ef5309518a75f14800391

But since the brwap command is very lengthy, that would be rather ugly.

I guess we couldn’t even use

command+=("--ro-bind /bin /bin")
command+=("--ro-bind /usr/bin /usr/bin")
...

Because that would be interpreted as:

‘–ro-bind /bin /bin’

And not as intended as:

–ro-bind /bin /bin

However, the following would probably work

command+=("--ro-bind")
command+=("/bin")
command+=("/bin")
...
bash -c "${command[@]}"

but make code look much worse. (60 lines of bwrap command would become I guestimate 180 lines.)

The issue comes from bash subshell opening with another double quote bash -c ":

  sudo \
...
    bash -c "
  bwrap \
...
  --seccomp 12 \
  10< <(getent passwd root ${app_user} nobody) \
  11< <(getent group root ${app_user} nobody) \
  12< ${seccomp_filter} \
  ${bwrap_args} \
  ${wrapper_script} ${@}"

Do you think it would be a good idea to move that into its own /usr/share/sandbox-app-launcher/bwrap-wrapper script? Then I believe whitespace handling could be easily fixed.

(No need for command+=("--ro-bind") or "${command[@]}".)

I think it’s better to just lay a few files in the root mount namespace shared by multiple applications and guard the files with apparmor so that only applications that need the shared files can access them.

  • /run/user-id/sway-ipc-*
  • /run/user-id/wayland-*
  • shared dbus sessions.

How would that fix it? We’d still need the subshell for the file descriptors.

1 Like

Yes, but that is OK…?

We’d have proper whitespace handling… The pseudo code:

sandbox-app-launcher:

  sudo \
    --set-home \
    --user="${app_user}" \
    sandbox_app_launcher_debug="$sandbox_app_launcher_debug" \
    app_path="$app_path" \
    /usr/share/sandbox-app-launcher/bwrap-wrapper

bwrap-wrapper:

  bwrap \
...
  --ro-bind "${main_app_dir}/machine-id /etc/machine-id" \
  --ro-bind "${wrapper_script}" "${wrapper_script}" \
  10< <(getent passwd root "${app_user}" nobody) \
  11< <(getent group root "${app_user}" nobody) \
  12< "${seccomp_filter}" \
  "${bwrap_args}" \
  "${wrapper_script}" "${@}"

Using "${@}" (or "$@") would fix whitespace handling.

If that sounds good, I am eager to implement this.

2 Likes

Ah, I understand now. That would be good to implement.

2 Likes

It’s implemented. We have now proper whitespace support. Please have a look.
Not too beautiful code. Some imperfections.

  • bwrap_args still does not have proper whitespace support but that’s not user facing since path to shared_dir is hardcoded.
  • Lots of variables have to be passed from sandbox-app-launcher to brwap-wrapper since we cannot preserve environment (sudo -E).
2 Likes
sandbox-app-launcher start vlc "abc cde" "file 2"

This is actually good:

[0000738ee3f89d00] filesystem stream error: cannot open file /home/sandbox-app-launcher-appdata/vlc/abc cde (No such file or directory)
[0000738ee3f8f480] filesystem stream error: cannot open file /home/sandbox-app-launcher-appdata/vlc/file 2 (No such file or directory)

Your input can’t be opened:
VLC is unable to open the MRL ‘file:///home/sandbox-app-launcher-appdata/vlc/abc%20cde’. Check the log for details.
Your input can’t be opened:
VLC is unable to open the MRL ‘file:///home/sandbox-app-launcher-appdata/vlc/file%202’. Check the log for details.

It shows proper whitespace hanlding. Previously VLC (example, any application) would have attempted to open 4 files (abc, def, file, 2) instead of 2 files (“abc def”, “file 2”).

1 Like

Looks good to me.

1 Like

It would be useful to have a list argument to list all the currently configured sandboxes. Could simply be:

getent passwd | grep "sal" | sed -e 's/:.*//g' | str_replace "sal-" ""

Also since replace dynamic wrapper script creation with static script · Whonix/sandbox-app-launcher@f939fe8 · GitHub, the AppArmor profile is now broken: sandbox-app-launcher/sandbox-app-launcher at master · Whonix/sandbox-app-launcher · GitHub

Profile sandbox-app-launcher applies to /var/cache/sandbox-app-launcher-autogenerated/wrappers/** and profile sandbox-app-launcher-wx applies to /var/cache/sandbox-app-launcher-autogenerated/wrappers-wx/**

Since those directories no longer exist, we will need to create 2 copies of the static wrapper script: wrapper-script and wrapper-script-wx, confined by their respective AppArmor profiles.

1 Like
1 Like
2 Likes

Awesome! All merged. Left some inline comments.

1 Like
[Imprint] [Privacy Policy] [Cookie Policy] [Terms of Use] [E-Sign Consent] [DMCA] [Contributors] [Investors] [Priority Support] [Professional Support]