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

Have securedrop-proxy drop unnecessary prilveges #2061

Open
legoktm opened this issue Jun 6, 2024 · 0 comments
Open

Have securedrop-proxy drop unnecessary prilveges #2061

legoktm opened this issue Jun 6, 2024 · 0 comments

Comments

@legoktm
Copy link
Member

legoktm commented Jun 6, 2024

Description

Inspired by today's gVisor discussion, I started looking at how difficult it would be to use seccomp to drop privileges in securedrop-proxy. I found the extrasafe crate to provide a nice abstraction for what we want. In code it ends up being:

    extrasafe::SafetyContext::new()
        .enable(
            extrasafe::builtins::Networking::nothing()
                .allow_start_tcp_clients(),
        )
        .expect("failed to enable networking rules")
        /*.enable(
            extrasafe::builtins::SystemIO::nothing()
                .allow_stdin()
                .allow_stdout()
                .allow_stderr(),
        )
        .expect("failed to enable system IO rules")*/
        .apply_to_current_thread()
        .expect("failed to apply safety context");

This disables all networking except starting a TCP socket (but not listening, nor UDP, etc.). Allowing stdin/stdout/stderr isn't necessary (and is problematic) because the networking part allows the write() syscall, so it's already enabled. It also supports using landlock to restrict access to files but I didn't get there yet.

Trying it out, all tests pass except the one that tries the missing.test hostname:

thread 'main' panicked at /home/user/.cargo/registry/src/index.crates.io-6f17d22bba15001f/hyper-util-0.1.3/src/client/legacy/connect/dns.rs:121:24:
OS can't spawn worker thread: Operation not permitted (os error 1)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

because it tries to spawn a threadpool that calls the getaddrinfo syscall, except we lost the capability to start new threads (good?). This is a bit of a tangent, because we don't actually want reqwest resolving DNS, since we only use .onion names that Tor resolves. But let's swap in hickory-dns, which is the Rust DNS client that properly handles onion addresses (hickory-dns/hickory-dns#1479).

-reqwest = { version = "0.12", features = ["gzip", "stream"] }
+reqwest = { version = "0.12", features = ["gzip", "hickory-dns", "stream"] }

Tests now pass and everything is happy (minus the cargo vet backlog).

Ignoring the DNS portion for now, I am not sure where this would fit in our security mitigation stack. We already have:

  • Qubes/Xen VMs - isolation from other components and underlying system/hardware
  • AppArmor - restricts what program can do inside the VM, controlled by root/kernel
  • seccomp/landlock - also restricts what the program can do from inside the VM, controlled by the program itself

Primarily it seems to me as defense in depth but also very duplicative/overlapping with AppArmor.

@legoktm legoktm changed the title Have securedrop-proxy drop unncessary prilveges Have securedrop-proxy drop unnecessary prilveges Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants