Closed Bug 1320834 Opened 3 years ago Closed 2 years ago
Restrict prctl() in desktop content processes
59 bytes, text/x-review-board-request
Currently, content processes on Linux can use any prctl() command, which exposes a nontrivial amount of kernel attack surface. We should do something about that. The allowed list for GMP almost certainly isn't large enough, and a default-deny policy is likely to cause breakage on unusual and/or newer distributions (like everything else sandboxing-related), but this is not insurmountable.
In particular: PR_SET_SECCOMP allows adding additional seccomp-bpf programs; while this can't be used to allow any syscalls that an existing filter has denied, there have been exploitable bugs in the kernel BPF engine in the past.
It looks like we can just use the "generic" prctl policy, with no additions, at least for the systems we use for automation: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9f328522f6093e12d3b1eef6a364bd02d8ec85f (We'd need to add the FP context stuff to support PPC or MIPS, it looks like, but that wouldn't be the only problem facing a new architecture.) A look at the current glibc source doesn't show anything besides PR_SET_NAME and PR_GET_NAME, so hopefully we won't get any surprises from there with uncommon/newer distros. There's also arch_prctl, which doesn't look too dangerous but we might be able to drop it — it exists only on x86_64, and the only part of it glibc uses is ARCH_SET_FS, and that's only once during startup (there's a separate syscall used for setting the TLS base for a new thread), so before sandboxing. Looks like it was added from the original experiments that strace(1)d the content process (bug 935111), which didn't prevent it from doing fork/exec.
Comment on attachment 8918480 [details] Bug 1320834 - Reduce prctl policy for desktop content processes. https://reviewboard.mozilla.org/r/189334/#review194900 For reference, that means the new policy is: return Switch(op) .CASES((PR_GET_SECCOMP, // BroadcastSetThreadSandbox, etc. PR_SET_NAME, // Thread creation PR_SET_DUMPABLE, // Crash reporting PR_SET_PTRACER), // Debug-mode crash handling Allow()) DUMPABLE and PTRACER look...worthy of investigation? In any case that's better than just allow-all.
Attachment #8918480 - Flags: review?(gpascutto) → review+
I edited the commit message to point out the inheritance. DUMPABLE and PTRACER are about protecting the calling process from attack by other processes, so restricting them probably isn't too useful. DUMPABLE is for Breakpad to turn dumpability back on while crashing, although in Firefox's case we (probably?) never turned it off in the first place. PTRACER is similar but it's for attaching a debugger, and we do need it if we're on a system where the Yama LSM is restricting ptrace by default. (Chromium, I believe, runs or at least used to run all child processes in the same pid namespace, so before seccomp-bpf the only way to keep renderers from trivially attacking each other was to make them all undumpable.)
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/c006ddf45ea8 Reduce prctl policy for desktop content processes. r=gcp
You need to log in before you can comment on or make changes to this bug.