Restrict socket syscall arguments to TCP/UDP/UNIX

Previously, even if `Networking::allow_start_{tcp,udp,unix}_servers` was
called, the socket syscall wasn't restricted to just sockets of that
specific type.

This prevents many CVEs that exploit, e.g. the netlink subsystem,
although they were already prevented if you didn't need networking or
pre-opened sockets and used `Networking::allow_running_*_servers`.

examples/ipc_server_with_database.rs

@@ -271,6 +271,8 @@ fn run_client_read() {
     // enable extrasafe context
     SafetyContext::new()
         .enable(Networking::nothing()
+            // Necessary for DNS
+            .allow_start_udp_servers().yes_really()
             .allow_start_tcp_clients()).unwrap()
         // For some reason only if we make two requests with a client does it use multiple threads,
         // so we only need them in the reader thread rather than the writer.

examples/server_with_database.rs

@@ -224,6 +224,8 @@ fn run_client_read() {
     // enable extrasafe context
     SafetyContext::new()
         .enable(Networking::nothing()
+            // Necessary for DNS
+            .allow_start_udp_servers().yes_really()
             .allow_start_tcp_clients()).unwrap()
         // For some reason only if we make two requests with a client does it use multiple threads,
         // so we only need them in the reader thread rather than the writer.

src/builtins/network.rs

@@ -2,6 +2,7 @@
 
 use std::collections::{HashMap, HashSet};
 
+use libseccomp::scmp_cmp;
 use syscalls::Sysno;
 
 use super::YesReally;
@@ -38,10 +39,8 @@ const NET_READ_SYSCALLS: &[Sysno] = &[Sysno::listen,
 const NET_WRITE_SYSCALLS: &[Sysno] = &[Sysno::sendto, Sysno::sendmsg, Sysno::sendmmsg,
                                        Sysno::sendfile,
                                        Sysno::write, Sysno::writev, Sysno::pwritev, Sysno::pwritev2];
-const NET_CREATE_SERVER_SYSCALLS: &[Sysno] = &[Sysno::socket, Sysno::bind];
-const NET_CREATE_CLIENT_SYSCALLS: &[Sysno] = &[Sysno::socket, Sysno::connect];
 
-// TODO: refactor
+// TODO: refactor Socket rule creation to reduce duplication in the allow_start_*_server functions
 
 /// A [`RuleSet`] representing syscalls that perform network operations - accept/listen/bind/connect etc.
 ///
@@ -81,8 +80,8 @@ impl Networking {
         }
     }
 
-    /// Allow a running TCP server to continue running. Does not allow `socket` or `bind` to
-    /// prevent new sockets from being created.
+    /// Allow a running TCP server to continue running. Does not allow `socket` or `bind`,
+    /// preventing new sockets from being created.
     #[must_use]
     pub fn allow_running_tcp_servers(mut self) -> Networking {
         self.allowed.extend(NET_IO_SYSCALLS);
@@ -101,16 +100,37 @@ impl Networking {
     /// `examples/network_server.rs` for an example with warp.
     #[must_use]
     pub fn allow_start_tcp_servers(mut self) -> YesReally<Networking> {
-        self.allowed.extend(NET_CREATE_SERVER_SYSCALLS);
+        const AF_INET: u64 = libc::AF_INET as u64;
+        const AF_INET6: u64 = libc::AF_INET6 as u64;
+        const SOCK_STREAM: u64 = libc::SOCK_STREAM as u64;
+
+        // IPv4
+        let rule = Rule::new(Sysno::socket)
+            .and_condition(scmp_cmp!($arg0 & AF_INET == AF_INET))
+            .and_condition(scmp_cmp!($arg1 & SOCK_STREAM == SOCK_STREAM));
+        self.custom.entry(Sysno::socket)
+            .or_insert_with(Vec::new)
+            .push(rule);
+        // IPv6
+        let rule = Rule::new(Sysno::socket)
+            .and_condition(scmp_cmp!($arg0 & AF_INET6 == AF_INET6))
+            .and_condition(scmp_cmp!($arg1 & SOCK_STREAM == SOCK_STREAM));
+        self.custom.entry(Sysno::socket)
+            .or_insert_with(Vec::new)
+            .push(rule);
+
+        // Bind is unconditional here because besides the socket fd, the other argument is a
+        // struct we can't look into due to seccomp restrictions.
+        self.allowed.extend(&[Sysno::bind]);
         self.allowed.extend(NET_IO_SYSCALLS);
         self.allowed.extend(NET_READ_SYSCALLS);
         self.allowed.extend(NET_WRITE_SYSCALLS);
 
         YesReally::new(self)
     }
 
-    /// Allow a running UDP socket to continue running. Does not allow `socket` or `bind` to
-    /// prevent new sockets from being created.
+    /// Allow a running UDP socket to continue running. Does not allow `socket` or `bind`,
+    /// preventing new sockets from being created.
     #[must_use]
     pub fn allow_running_udp_sockets(mut self) -> Networking {
         self.allowed.extend(NET_IO_SYSCALLS);
@@ -128,7 +148,27 @@ impl Networking {
     /// use [`allow_running_udp_sockets`](Self::allow_running_udp_sockets).
     #[must_use]
     pub fn allow_start_udp_servers(mut self) -> YesReally<Networking> {
-        self.allowed.extend(NET_CREATE_SERVER_SYSCALLS);
+        const AF_INET: u64 = libc::AF_INET as u64;
+        const AF_INET6: u64 = libc::AF_INET6 as u64;
+        const SOCK_DGRAM: u64 = libc::SOCK_DGRAM as u64;
+
+        // IPv4
+        let rule = Rule::new(Sysno::socket)
+            .and_condition(scmp_cmp!($arg0 & AF_INET == AF_INET))
+            .and_condition(scmp_cmp!($arg1 & SOCK_DGRAM == SOCK_DGRAM));
+        self.custom.entry(Sysno::socket)
+            .or_insert_with(Vec::new)
+            .push(rule);
+        // IPv6
+        let rule = Rule::new(Sysno::socket)
+            .and_condition(scmp_cmp!($arg0 & AF_INET6 == AF_INET6))
+            .and_condition(scmp_cmp!($arg1 & SOCK_DGRAM == SOCK_DGRAM));
+        self.custom.entry(Sysno::socket)
+            .or_insert_with(Vec::new)
+            .push(rule);
+
+        self.allowed.extend(&[Sysno::bind]);
+        self.allowed.extend(NET_IO_SYSCALLS);
         self.allowed.extend(NET_READ_SYSCALLS);
         self.allowed.extend(NET_WRITE_SYSCALLS);
 
@@ -143,16 +183,35 @@ impl Networking {
     /// reqwest, so we allow socket but not bind here.
     #[must_use]
     pub fn allow_start_tcp_clients(mut self) -> Networking {
-        self.allowed.extend(NET_CREATE_CLIENT_SYSCALLS);
+        const AF_INET: u64 = libc::AF_INET as u64;
+        const AF_INET6: u64 = libc::AF_INET6 as u64;
+        const SOCK_STREAM: u64 = libc::SOCK_STREAM as u64;
+
+        // IPv4
+        let rule = Rule::new(Sysno::socket)
+            .and_condition(scmp_cmp!($arg0 & AF_INET == AF_INET))
+            .and_condition(scmp_cmp!($arg1 & SOCK_STREAM == SOCK_STREAM));
+        self.custom.entry(Sysno::socket)
+            .or_insert_with(Vec::new)
+            .push(rule);
+        // IPv6
+        let rule = Rule::new(Sysno::socket)
+            .and_condition(scmp_cmp!($arg0 & AF_INET6 == AF_INET6))
+            .and_condition(scmp_cmp!($arg1 & SOCK_STREAM == SOCK_STREAM));
+        self.custom.entry(Sysno::socket)
+            .or_insert_with(Vec::new)
+            .push(rule);
+
+        self.allowed.extend(&[Sysno::connect]);
         self.allowed.extend(NET_IO_SYSCALLS);
         self.allowed.extend(NET_READ_SYSCALLS);
         self.allowed.extend(NET_WRITE_SYSCALLS);
 
         self
     }
 
-    /// Allow a running TCP client to continue running. Does not allow `socket` or `connect` to
-    /// prevent new sockets from being created.
+    /// Allow a running TCP client to continue running. Does not allow `socket` or `connect`,
+    /// preventing new sockets from being created.
     ///
     /// This is technically the same as
     /// [`allow_running_tcp_servers`](Self::allow_running_tcp_servers).
@@ -172,17 +231,36 @@ impl Networking {
     /// You probably don't need to use this. In most cases you can just run your server and then
     /// use [`allow_running_unix_servers`](Self::allow_running_unix_servers).
     #[must_use]
-    pub fn allow_start_unix_server(mut self) -> YesReally<Networking> {
-        self.allowed.extend(NET_CREATE_SERVER_SYSCALLS);
+    pub fn allow_start_unix_servers(mut self) -> YesReally<Networking> {
+        const AF_UNIX: u64 = libc::AF_UNIX as u64;
+        const SOCK_STREAM: u64 = libc::SOCK_STREAM as u64;
+        const SOCK_DGRAM: u64 = libc::SOCK_DGRAM as u64;
+
+        // We allow both stream and dgram unix sockets
+        let rule = Rule::new(Sysno::socket)
+            .and_condition(scmp_cmp!($arg0 & AF_UNIX == AF_UNIX))
+            .and_condition(scmp_cmp!($arg1 & SOCK_STREAM == SOCK_STREAM));
+        self.custom.entry(Sysno::socket)
+            .or_insert_with(Vec::new)
+            .push(rule);
+        // DGRAM
+        let rule = Rule::new(Sysno::socket)
+            .and_condition(scmp_cmp!($arg0 & AF_UNIX == AF_UNIX))
+            .and_condition(scmp_cmp!($arg1 & SOCK_DGRAM == SOCK_DGRAM));
+        self.custom.entry(Sysno::socket)
+            .or_insert_with(Vec::new)
+            .push(rule);
+
+        self.allowed.extend(&[Sysno::bind]);
         self.allowed.extend(NET_IO_SYSCALLS);
         self.allowed.extend(NET_READ_SYSCALLS);
         self.allowed.extend(NET_WRITE_SYSCALLS);
 
         YesReally::new(self)
     }
 
-    /// Allow a running Unix server to continue running. Does not allow `socket` or `bind` to
-    /// prevent new sockets from being created.
+    /// Allow a running Unix server to continue running. Does not allow `socket` or `bind`,
+    /// preventing new sockets from being created.
     #[must_use]
     pub fn allow_running_unix_servers(mut self) -> Networking {
         self.allowed.extend(NET_IO_SYSCALLS);
@@ -192,8 +270,8 @@ impl Networking {
         self
     }
 
-    /// Allow a running Unix socket client to continue running. Does not allow `socket` or `connect` to
-    /// prevent new sockets from being created.
+    /// Allow a running Unix socket client to continue running. Does not allow `socket` or `connect`,
+    /// preventing new sockets from being created.
     ///
     /// This is technically the same as
     /// [`allow_running_unix_servers`](Self::allow_running_unix_servers).

tests/network.rs

@@ -146,3 +146,91 @@ fn test_tcp() {
     let res = std::net::TcpListener::bind("127.0.0.1:31359");
     assert!(res.is_err(), "Incorrectly succeeded in binding to socket");
 }
+
+#[test]
+/// Demonstrating opening new TCP sockets and binding them. Ideally you do not need do this and can
+/// instead open and bind before applying your policy.
+fn test_start_tcp() {
+    SafetyContext::new()
+        .enable(
+            Networking::nothing()
+                .allow_start_tcp_servers().yes_really()
+        ).unwrap()
+        .enable(
+            Threads::nothing()
+                .allow_create()
+        ).unwrap()
+        .apply_to_current_thread()
+        .unwrap();
+    let tcp_res = std::net::TcpListener::bind("127.0.0.1:0");
+    assert!(tcp_res.is_ok(), "Failed to bind tcp server");
+
+    let udp_res = std::net::UdpSocket::bind("127.0.0.1:0");
+    assert!(udp_res.is_err(), "Incorrectly succeeded in binding udp socket");
+
+    // test ipv6 as well
+    let tcp_res = std::net::TcpListener::bind("[::1]:0");
+    assert!(tcp_res.is_ok(), "Failed to bind tcp server");
+
+    let udp_res = std::net::UdpSocket::bind("[::1]:0");
+    assert!(udp_res.is_err(), "Incorrectly succeeded in binding udp socket");
+}
+
+#[test]
+/// Demonstrating opening new UDP sockets and binding them. Ideally you do not need do this and can
+/// instead open and bind before applying your policy.
+fn test_start_udp() {
+    SafetyContext::new()
+        .enable(
+            Networking::nothing()
+                .allow_start_udp_servers().yes_really()
+        ).unwrap()
+        .enable(
+            Threads::nothing()
+                .allow_create()
+        ).unwrap()
+        .apply_to_current_thread()
+        .unwrap();
+    let udp_res = std::net::UdpSocket::bind("127.0.0.1:0");
+    assert!(udp_res.is_ok(), "Failed to bind udp server");
+
+    let udp_res = std::net::TcpListener::bind("127.0.0.1:0");
+    assert!(udp_res.is_err(), "Incorrectly succeeded in binding udp socket");
+
+    // test ipv6 as well
+    let udp_res = std::net::UdpSocket::bind("[::1]:0");
+    assert!(udp_res.is_ok(), "Failed to bind udp server");
+
+    let tcp_res = std::net::TcpListener::bind("[::1]:0");
+    assert!(tcp_res.is_err(), "Incorrectly succeeded in binding tcp socket");
+}
+
+#[test]
+/// Demonstrating opening new Unix domain sockets and binding them. Ideally you do not need do this
+/// and can instead open and bind before applying your policy.
+fn test_start_unix() {
+    let dir = tempfile::TempDir::new().unwrap();
+    let mut path = dir.path().to_path_buf();
+    path.push("test.sock");
+
+    SafetyContext::new()
+        .enable(
+            Networking::nothing()
+                .allow_start_unix_servers().yes_really()
+        ).unwrap()
+        .enable(
+            Threads::nothing()
+                .allow_create()
+        ).unwrap()
+        .apply_to_current_thread()
+        .unwrap();
+
+    let unix_res = std::os::unix::net::UnixListener::bind(&path);
+    assert!(unix_res.is_ok(), "Failed to bind tcp server");
+
+    let udp_res = std::net::UdpSocket::bind("127.0.0.1:0");
+    assert!(udp_res.is_err(), "Incorrectly succeeded in binding udp socket");
+
+    let tcp_res = std::net::TcpListener::bind("[::1]:0");
+    assert!(tcp_res.is_err(), "Incorrectly succeeded in binding tcp socket");
+}