madaidan/sandbox-app-launcher

An app launcher to start apps in a restrictive sandbox

sandbox-app-launcher's Issues

some questions

Hi, I know that this is a work in progress, but I'm intrigued by it and hope it's not too annoying at this stage if I ask a few naive questions about the implementation.

1. It looks like applications are run as the sandbox user via sudo -u ${app_user}. Does this mean that the main user will need to be a sudoer in order to use sandboxing, and will need to provide their password each time they run a sandboxed application?

2. The the name of the sandbox user appears to be sandbox-${app_name}, which is independent of the name of the main user, so it seems that in a multi-user setup, everyone would run $app as the same sandbox user. Can users safely share sandboxes without having access to each other's data (such as their $app dotfiles), or is sandbox-application-launcher only intended for single user setups?

3. My understanding is that data is shared across applications via a single /shared directory, which each app either has read/write, read-only, or no access to the entirety of. I was wondering if you had given any thought toward giving apps access to specific directories at intuitive locations in the main user's home directory. For example, the sandbox-chromium user's Downloads directory might be a symlink to /home/$MAINUSER/Downloads/Chromium, and have read/write access to that directory, but not the rest of the main user's Downloads directory. The sandbox-libreoffice user might have read-only access to /home/$MAINUSER/Downloads and all subdirectories in order to open documents downloaded by other applications, but only have write access to save documents to /home/$MAINUSER/Documents/LibreOffice. I think this is (at least superficially) similar to what SupgraphOS does, and seems like it would offer more fine-grained control over what access each application should have to certain subclasses of data than a single /shared directory, but maybe this has already been ruled out for reasons over my head.

Thanks, and keep up the great work!

I propose replacing shell with a proper scripting language.

I've used https://janet-lang.org/ to great success for my scripting needs.

Portability

sandbox-app-launcher uses adduser which is debian-specific. Its important to change that for portability.

Issues with mknod* seccomp-whitelist

TL;DR:

• The whitelist for mknod does not mask out file permission bits making it rather useless.
• The whitelist for mknodat checks the wrong argument making it (in theory) possible to call it with S_IFCHR/S_IFBLK.

---

Deep dive

seccomp-whitelist contains the following for mknod/mknodat:

# We don't need to allow creation of char/block devices.
mknod 1 S_IFREG
mknod 1 S_IFIFO
mknod 1 S_IFSOCK
mknodat 1 S_IFREG
mknodat 1 S_IFIFO
mknodat 1 S_IFSOCK

After running bash autogen-seccomp seccomp-whitelist this becomes

  ALLOW_ARG1 (mknod, S_IFREG);
  ALLOW_ARG1 (mknod, S_IFIFO);
  ALLOW_ARG1 (mknod, S_IFSOCK);
  ALLOW_ARG1 (mknodat, S_IFREG);
  ALLOW_ARG1 (mknodat, S_IFIFO);
  ALLOW_ARG1 (mknodat, S_IFSOCK);

Which gets expanded to the following (for x86_64) by running
bash autogen-seccomp seccomp-whitelist | gcc -Wall -x c -E -P -:

  { if (seccomp_rule_add (ctx, 0x7fff0000U, (133), 1, ((struct scmp_arg_cmp){1, SCMP_CMP_EQ, 0100000}), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, 0x7fff0000U, (133), 1, ((struct scmp_arg_cmp){1, SCMP_CMP_EQ, 0010000}), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, 0x7fff0000U, (133), 1, ((struct scmp_arg_cmp){1, SCMP_CMP_EQ, 0140000}), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, 0x7fff0000U, (259), 1, ((struct scmp_arg_cmp){1, SCMP_CMP_EQ, 0100000}), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, 0x7fff0000U, (259), 1, ((struct scmp_arg_cmp){1, SCMP_CMP_EQ, 0010000}), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, 0x7fff0000U, (259), 1, ((struct scmp_arg_cmp){1, SCMP_CMP_EQ, 0140000}), 0) < 0) goto out; };

Which looks like this if we manually revert the libseccomp macros:

  { if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(mknod), 1, SCMP_A1(SCMP_CMP_EQ, S_IFREG), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(mknod), 1, SCMP_A1(SCMP_CMP_EQ, S_IFIFO), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(mknod), 1, SCMP_A1(SCMP_CMP_EQ, S_IFSOCK), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(mknodat), 1, SCMP_A1(SCMP_CMP_EQ, S_IFREG), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(mknodat), 1, SCMP_A1(SCMP_CMP_EQ, S_IFIFO), 0) < 0) goto out; };
  { if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(mknodat), 1, SCMP_A1(SCMP_CMP_EQ, S_IFSOCK), 0) < 0) goto out; };

The first issue here is that the mode argument (arg 1) of mknod

specifies both the file mode to use and the type of node to be created.

So it is only allowed to create a file with 000 permissions.

In order to check the type but not the mode all permission bit should be masked out like

-  { if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(mknod), 1, SCMP_A1(SCMP_CMP_EQ, S_IFREG), 0) < 0) goto out; };
+  { if (seccomp_rule_add (ctx, SCMP_ACT_ALLOW, SCMP_SYS(mknod), 1, SCMP_A1(SCMP_CMP_MASKED_EQ, S_IFMT, S_IFREG), 0) < 0) goto out; };

The second issue is that mknodat is defined as
int mknodat(int dirfd, const char *pathname, mode_t mode, dev_t dev);
meaning mode is arg 2 and arg 1 is the pathname. This makes using mknodat
pratically impossible (and mknod too because glibc implements mknod with
mknodat) because it is only allowed if pathname is at S_IFREG, S_IFIFO
or S_IFSOCK. And it makes it in theory possible to call mknodat with
S_IFCHR/S_IFBLK though this seems to be very hard in practice because
S_IFREG, S_IFIFO and S_IFSOCK are very low numbers.

---

Test programs

# Open autogen-seccomp and change 'seccomp_filter_path='
# Generate the seccomp-filter
bash autogen-seccomp seccomp-whitelist | gcc -Wall -l seccomp -o autogen-seccomp-whitelist -x c - && ./autogen-seccomp-whitelist`
# Comopile the test programs
gcc -Wall -o mknod mknod.c
gcc -Wall -o mknodat mknodat.c
# Run the test programs w/o seccomp-filter
./mknod
> mknod("/tmp/reg1", S_IFREG) = 0
> mknod("/tmp/reg2", S_IFREG|0644) = 0
./mknodat
> mknodat(AT_FDCWD, "/tmp/reg1", S_IFREG) = 0
> mknodat(AT_FDCWD, "/tmp/reg2", S_IFREG|0644) = 0
> ptr: c000
> ptr2: ffffffffffffffff    <== -1
> 1 Operation not permitted
> zsh: segmentation fault (core dumped)  ./mknodat
# Run the test programs with seccomp-filter
bwrap --dev-bind   --seccomp 3 ./mknod 3<./seccomp-whitelist.bpf
> mknod("/tmp/reg1", S_IFREG) = 0
> killed
bwrap --dev-bind   --seccomp 3 ./mknodat 3<./seccomp-whitelist.bpf
> killed

mknod.c:

#include <stdio.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <unistd.h>

// glibc uses mknodat to implement mknod (in newer versions).
int mknod(const char *pathname, mode_t mode, dev_t dev) {
    return syscall(SYS_mknod, pathname, mode, dev);
}

int main(int argc, char **argv) {
    int rv;

    unlink("/tmp/reg1");
    rv = mknod("/tmp/reg1", S_IFREG, 0);
    printf("mknod(\"/tmp/reg1\", S_IFREG) = %i\n", rv);

    unlink("/tmp/reg2");
    rv = mknod("/tmp/reg2", S_IFREG|0644, 0);
    printf("mknod(\"/tmp/reg2\", S_IFREG|0644) = %i\n", rv);
}

mknodat.c:

#include <errno.h>
#include <fcntl.h>
#include <stdint.h>
#include <stdio.h>
#include <string.h>
#include <sys/mman.h>
#include <sys/stat.h>
#include <sys/sysmacros.h>
#include <unistd.h>

int main(int argc, char **argv) {
    int rv;

    unlink("/tmp/reg1");
    rv = mknodat(AT_FDCWD, "/tmp/reg1", S_IFREG, 0);
    printf("mknodat(AT_FDCWD, \"/tmp/reg1\", S_IFREG) = %i\n", rv);

    unlink("/tmp/reg2");
    rv = mknodat(AT_FDCWD, "/tmp/reg2", S_IFREG|0644, 0);
    printf("mknodat(AT_FDCWD, \"/tmp/reg2\", S_IFREG|0644) = %i\n", rv);

    char *ptr = (char *)S_IFSOCK;
    char *ptr2 = mmap(ptr, strlen("/tmp/reg3") + 1, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_FIXED, -1, 0);
    printf("ptr: %lx\nptr2: %lx\n%i %s\n", (uintptr_t)ptr, (uintptr_t)ptr2, errno, strerror(errno));
    strcpy(ptr, "/tmp/reg3"); // SEGV
    unlink("/tmp/reg3");
    rv = mknodat(AT_FDCWD, ptr, S_IFCHR, makedev(0, 0));
    printf("mknodat(AT_FDCWD, \"/tmp/reg3\", S_IFCHR, makedev(0, 0)) = %i\n", rv);
}