cloudflare / foundations

Cloudflare's Rust service foundations library.
https://blog.cloudflare.com/introducing-foundations-our-open-source-rust-service-foundation-library
BSD 3-Clause "New" or "Revised" License
1.25k stars 51 forks source link

OXY-1404: Avoid crashes resulting from double seccomp initialization #60

Closed OmegaJak closed 1 week ago

OmegaJak commented 1 month ago

If a service using foundations accidentally initializes seccomp in a thread that already seccomp initialized, the seccomp violation (depending on configuration) may be violated and crash the process.

An easy way to enter into this scenario is by having a hook in tokio::runtime::Builder::on_thread_start that initializes seccomp for threads that are initialized before the main thread. In this scenario, any threads created from tokio threads (say, inside a tokio task) may lead to a seccomp violation.

This PR uses the syscall prctl(PR_GET_SECCOMP) (PR_GET_SECCOMP) to find out definitively whether seccomp is already enabled for the current thread. If it is, then the default behavior is to not initialize seccomp again, thus avoiding what may have been a crash otherwise.

OmegaJak commented 1 month ago

There are lots of ways to implement this from an API perspective, I'm totally open to alternatives. We could leave enable_syscall_sandboxing untouched and just provide the new get_current_thread_seccomp_mode() method, expecting callers to check this method first if they care. But that felt to me like leaving a footgun in place for the standard path of using enable_syscall_sandboxing.

We could also not provide the enable_syscall_sandboxing_ignore_existing method, but it's technically possible that there are currently applications that initialize seccomp twice, which that would break.

I'm happy to implement any of those alternative approaches.

OmegaJak commented 1 month ago

This now has a dependency on #54 - it removed a bit of code that needs error type conversion now. So the compilation will fail until that PR is merged.