System-wide sandboxing framework - sandbox-app-launcher

Development
,
41

Probably not.
Debian Code Search: DPkg::Post-Invoke

At each DPkg::Post-Invoke event maybe the right thing to do is to iterate through all of PATH?

I still think an DPkg::Post-Invoke hook is OK to create a wrapper. But the actual questions should be asked at first start of the program (if any, non-avoidable). The wrapper could check at first start if questions already answered or still needed to ask. That way lots of applications which are never used don’t need to be configured.

…then also not sure how to solve the issue of starting applications without any input (graphical or console) connected, i.e. started by cron, systemd or other scripts/programs internally. Maybe that becomes more clear later during development when we discuss a few applications specifically and which defaults seem sensible.

1 Like
42

That feature used to cause breakage all over the place. I always disabled it because may major programs were never written to use memory that way and I had things to get on with.

1 Like
43

Hence the whitelist. W^X is really important. Allowing WX mappings allows an attacker to write whatever they want and execute it.

1 Like
44

What about new packages where we haven’t tested compatibility yet? I guess if the sandbox framework is opt-in this shouldn’t be a problem.

2 Likes
45

WX mappings are a well-known security risk. It’s unlikely for modern software to be using them outside of JIT.

46

I’ve been testing this with a few apps and it works fine. It would be nice if more people were testing this. You just need to run:

sudo /usr/share/sandbox-app-launcher/sandbox app-name

and it sets everything up for you.

1 Like
47

This should now make introduction of new arbitrary code impossible.

1 Like
48

Merged. And made some progress with packaging.

/usr/share/sandbox-app-launcher/sandbox

Could you please move to /usr/bin/sandbox or so?
Or should that be /usr/sbin/ since sudo/root will be required to start it?

1 Like
49

I’m not sure whether it should require root or not.

It’d make the wrappers harder to create although root is needed for some things but we can use sudoers exceptions.

1 Like
50

Ok. Minor detail. usr/bin good enough for now. Not a blocker. Please move.

1 Like
51
1 Like
52

Merged. Please kindly send pull requests to Whonix/Whonix next time.

Packaging will be done soon.

1 Like
53

Damnit. I keep doing that by accident.

1 Like
54

Could you please add the usual apparmor local line?

  # Site-specific additions and overrides. See local/README for details.
  #include <local/usr.bin.sdwdate>

Apparmor profile needs path update?

Packaging is done for now. sandbox-app-launcher is now available from Whonix developers repository. Does the package look as expected? If that is confirmed it would be easy to copy to all Whonix repositories including the stable repository as it isn’t installed by default, there are no existing users which could have a setup that would break, and therefore have almost zero chance of causing issues for anyone who doesn’t wish to test it. However, it would make thing a lot easier for testers to get it installed.

Does building the package from source work for you?

1 Like
55

Why would it?

Yes.

There was one issue which I resolved.

Yes.

1 Like
56

Btw one issue with the packaging is that when removing the package, the files the script creates (e.g. the seccomp filter) remain:

dpkg: warning: while removing sandbox-app-launcher, directory '/usr/share/sandbox-app-launcher' not empty so not removed

The users created (e.g. sandbox-firefox) are also not removed.

Maybe during removal, we can run:

rm -rf /usr/share/sandbox-app-launcher
for user in $(getent passwd | grep "sandbox-" | sed -e 's/:.*//g')
do
  userdel -rf "${user}"
done
1 Like
57

Will look into it.

These need to be quoted:

app_path=“$(type -P ${app_name})”

stat -c %a ${app_homedir}

Could use a root check since not running as root causes only a ton of errors?

   if [ "$(id -u)" != "0" ]; then
      echo "ERROR: Must run as root."
      exit 112
   fi

Script also needs error handling. At minimum set -e. Better a trap ERR (see Whonix source code error_handler in many places).

sudo -H -u “${app_user}” bash -c "

Why do we need bash -c? Reasonable to make it work without bash -c? Without bash -c we could use a lot more quotes.

  10< <(getent passwd root ${app_user} nobody) \
  11< <(getent group root ${app_user} nobody) \
  12< ${seccomp_filter} \

This looks rather strange. Opening a subshell and executing something there. Better done without a subshell. Easier to understand and less risky. Also easier to read xtrace.

What are these file descriptors 10, 11, 12 for?

We have to be careful with the sudo call to avoid any privilege escalation. I haven’t found any ways yet but the bash -c and subshell makes it hard to me to understand.

bwrap_args is currently not set under any circumstances. Therefore it could be set as an environment variable. When using sudo -E sandbox-app-launcher such environment variables.

sandbox-app-launcher needs to survive malicious user input such as

app_name=\$(echo execute this)

What if app_name contains spaces?

1 Like
58

Could you put the compiled / auto generated files into a different folder please? More clean to separate source code and compiled code. The folder could have autogenerated, compiled or something in its name to make clear we can wipe it in the Debian postrm script.

1 Like
59

Without bash -c the FDs break e.g.

bwrap: Can't read seccomp data: Bad file descriptor

I’ve no clue why that is though. If you just copy/paste in the command without bash -c and execute it manually, it works fine. It only breaks without bash -c in the script.

I just randomly thought bash -c might fix it and it did so it’s there now.

Bubblewrap can take in data from file descriptors. If you look at the bwrap command you’ll see:

--ro-bind-data 10 /etc/passwd \
--ro-bind-data 11 /etc/group \
--seccomp 12 \

10 gives us a private /etc/passwd, 11 gives a private /etc/group and 12 is the seccomp filter.

So something like /usr/share/sandbox-app-launcher-autogenerated?

I’d assume the home directories should also be there.

1 Like
60

Can’t we start the script with a new environment similar to rapt?

Or what about just unset’ing all the variables the script uses?

Haven’t tried. Would probably bork the script though.

1 Like