Skip to content
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

Add hooks to run code in the child process before and after setting up the sandbox #37

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

rocallahan
Copy link

I have two needs not currently supported by gaol:

  • I want to run code in the child process after forking but before the sandbox is in place, e.g. to run prctl(PR_SET_PDEATHSIG) and to set up stdio redirection.
  • I want to activate the sandbox before exec because I don't control the execed binary. I only need this to work on Linux.

I think these should be supported by providing hooks similar to std::os::unix::process::CommandExt::before_exec. I've named them before_sandbox and before_exec. On Linux a before_exec hook can call ChildSandbox::activate and do extra setup work after entering the sandbox, if desired (e.g. configuring the new namespaces).

To get this to work I had to fix a number of bugs and other issues. In particular, to test that ChildSandbox::activate works in before_exec I had to make it possible to start a glibc-based process in the Linux sandbox, which meant adding Operation::CreateNewProcesses and whitelisting some more system calls. I also had to make substantial fixes the error handling in Linux's start().

The Mac support doesn't propagate errors from before_sandbox/before_exec because I didn't want to try to write that code without being able to test it.

Parallelizing the forbidden_syscalls test exposes the bug and
also reduces test time on my 4-core Skylake laptop:

Before:
[roc@glory gaol]$ time target/debug/deps/forbidden_syscalls-6fdc4dd40a646c6f
real	0m41.322s
user	0m0.590s
sys	0m1.452s

After:
[roc@glory gaol]$ time target/debug/deps/forbidden_syscalls-6fdc4dd40a646c6f
real	0m14.915s
user	0m0.248s
sys	0m0.980s
@rocallahan
Copy link
Author

Github isn't showing the commits in the correct order for some reason. My branch has them in the correct order.

glibc uses this during process startup.
glibc uses this during startup
Sandboxed processes should be able to use these to reduce their limits.
In a future commit we will turn all soft limits into hard limits so
it's impossible for a sandboxed child to increase any of its limits.

glibc uses prlimit64 during startup.
time/gettimeofday are generally called through the vDSO without
entering the kernel so blocking them with seccomp doesn't really work
anyway.

Having sandboxed children fail only when the vDSO is disabled (e.g.
when running under rr) is a problem.
…ironment

This commit is a bit oversized... adding support for these callbacks required
creating a way to pass errors back to the parent process, which inspired
fixing the error handling in start(), which uncovered some bugs in start():
* Immediate child process leaked as a zombie
* pipe_fds[0] leaked in parent
* pipe_fds[1] leaked into grandchild
@rocallahan
Copy link
Author

BTW the current patches don't support CreateNewProcesses on Mac. It looks like that could be supported, but again I hesitate to try without being able to test.

@bors-servo
Copy link

☔ The latest upstream changes (presumably #51) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants