Vlad/minor fixes (cloudflare#96)

* Handle unused Result in an idiomatic way

Fixes cloudflare#62

* Fix compilation error on OpenVZ

Fixes cloudflare#89

* Allow --disable-drop-privileges being set using the WG_SUDO env variable

Fixes cloudflare#86

* Clippy

* Rework stats

* Add tunnel stats for estimated downstream packet loss and handshake rtt
* Use tunnel stats for tx and rx bytes, instead peer stats

* Require read to clear signalfd events

* Implement FromIterator for AllowedIps

* Enable mutable events in epoll

README.md

@@ -38,7 +38,7 @@ The tunnel can then be configured using [wg](https://git.zx2c4.com/WireGuard/abo
 
 It is also possible to use with [wg-quick](https://git.zx2c4.com/WireGuard/about/src/tools/man/wg-quick.8) by setting the environment variable `WG_QUICK_USERSPACE_IMPLEMENTATION` to `boringtun`. For example:
 
-`sudo WG_QUICK_USERSPACE_IMPLEMENTATION=boringtun wg-quick up CONFIGURATION`
+`sudo WG_QUICK_USERSPACE_IMPLEMENTATION=boringtun WG_SUDO=1 wg-quick up CONFIGURATION`
 
 ### Testing
 
@@ -78,7 +78,7 @@ arm-linux-androideabi         |      |   ✓   |JNI bindings
 
 `x86-64`, `aarch64` and `armv7` architectures are supported. The behaviour should be identical to that of [wireguard-go](https://git.zx2c4.com/wireguard-go/about/), with the following difference:
 
-`boringtun` will drop privileges when started. When privileges are dropped it is not possible to set `fwmark`. If `fwmark` is required, instead running with `sudo`, give the executable the `CAP_NET_ADMIN` capability using: `sudo setcap cap_net_admin+epi boringtun`. Alternatively run with `--disable-drop-privileges`.
+`boringtun` will drop privileges when started. When privileges are dropped it is not possible to set `fwmark`. If `fwmark` is required, such as when using `wg-quick`, instead running with `sudo`, give the executable the `CAP_NET_ADMIN` capability using: `sudo setcap cap_net_admin+epi boringtun`. Alternatively run with `--disable-drop-privileges` or set the environment variable `WG_SUDO=1`.
 
 #### macOS
 

src/device/allowed_ips.rs

@@ -1,7 +1,10 @@
 // Copyright (c) 2019 Cloudflare, Inc. All rights reserved.
 // SPDX-License-Identifier: BSD-3-Clause
 
+use crate::device::peer::AllowedIP;
+
 use std::cmp::min;
+use std::iter::FromIterator;
 use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
 
 /// A trie of IP/cidr addresses
@@ -16,6 +19,18 @@ impl<D> Default for AllowedIps<D> {
     }
 }
 
+impl<'a> FromIterator<&'a AllowedIP> for AllowedIps<()> {
+    fn from_iter<I: IntoIterator<Item = &'a AllowedIP>>(iter: I) -> Self {
+        let mut allowed_ips: AllowedIps<()> = Default::default();
+
+        for ip in iter {
+            allowed_ips.insert(ip.addr, ip.cidr as usize, ());
+        }
+
+        allowed_ips
+    }
+}
+
 impl<D> AllowedIps<D> {
     pub fn clear(&mut self) {
         self.v4 = None;

src/device/api.rs

@@ -16,7 +16,7 @@ use std::sync::atomic::Ordering;
 const SOCK_DIR: &str = "/var/run/wireguard/";
 
 fn create_sock_dir() {
-    create_dir(SOCK_DIR).is_ok(); // Create the directory if it does not exist
+    let _ = create_dir(SOCK_DIR); // Create the directory if it does not exist
 
     if let Ok((saved_uid, saved_gid)) = get_saved_ids() {
         unsafe {
@@ -40,7 +40,7 @@ impl Device {
 
         create_sock_dir();
 
-        remove_file(&path).is_ok(); // Attempt to remove the socket if already exists
+        let _ = remove_file(&path); // Attempt to remove the socket if already exists
 
         let api_listener = UnixListener::bind(&path).map_err(Error::ApiSocket)?; // Bind a new socket to the path
 
@@ -156,8 +156,10 @@ fn api_get(writer: &mut BufWriter<&UnixStream>, d: &Device) -> i32 {
             writeln!(writer, "last_handshake_time_nsec={}", time.subsec_nanos());
         }
 
-        writeln!(writer, "rx_bytes={}", p.get_rx_bytes());
-        writeln!(writer, "tx_bytes={}", p.get_tx_bytes());
+        let (_, tx_bytes, rx_bytes, ..) = p.tunnel.stats();
+
+        writeln!(writer, "rx_bytes={}", tx_bytes);
+        writeln!(writer, "tx_bytes={}", rx_bytes);
     }
     0
 }

src/device/epoll.rs

@@ -30,7 +30,7 @@ pub struct EventPoll<H: Sized> {
 /// Once the EventGuard goes out of scope, the underlying Event will be re-enabled
 pub struct EventGuard<'a, H> {
     epoll: RawFd,
-    event: &'a Event<H>,
+    event: &'a mut Event<H>,
     poll: &'a EventPoll<H>,
 }
 
@@ -96,6 +96,26 @@ impl<H: Sync + Send> EventPoll<H> {
         self.register_event(ev)
     }
 
+    /// Add and enable a new write event with the factory.
+    /// The event is triggered when a Write operation on the provided trigger becomes possible
+    /// For TCP sockets it means that the socket was succesfully connected
+    pub fn new_write_event(&self, trigger: RawFd, handler: H) -> Result<EventRef, Error> {
+        // Create an event descriptor
+        let flags = EPOLLOUT | EPOLLET | EPOLLONESHOT;
+        let ev = Event {
+            event: epoll_event {
+                events: flags as _,
+                u64: 0,
+            },
+            fd: trigger,
+            handler,
+            notifier: false,
+            needs_read: false,
+        };
+
+        self.register_event(ev)
+    }
+
     /// Add and enable a new timed event with the factory.
     /// The even will be triggered for the first time after period time, and henceforth triggered
     /// every period time. Period is counted from the moment the appropriate EventGuard is released.
@@ -189,7 +209,7 @@ impl<H: Sync + Send> EventPoll<H> {
             fd: sfd,
             handler,
             notifier: false,
-            needs_read: false,
+            needs_read: true,
         };
 
         self.register_event(ev)
@@ -205,11 +225,11 @@ impl<H: Sync + Send> EventPoll<H> {
             return WaitResult::Error(errno_str());
         }
 
-        let event_data = unsafe { (event.u64 as *mut Event<H>).as_ref().unwrap() };
+        let event_data = unsafe { (event.u64 as *mut Event<H>).as_mut().unwrap() };
 
         let guard = EventGuard {
             epoll: self.epoll,
-            event: &event_data,
+            event: event_data,
             poll: self,
         };
 
@@ -346,6 +366,11 @@ impl<'a, H> Drop for EventGuard<'a, H> {
 }
 
 impl<'a, H> EventGuard<'a, H> {
+    /// Get a mutable reference to the stored value
+    pub fn get_mut(&mut self) -> &mut H {
+        &mut self.event.handler
+    }
+
     /// Cancel and remove the event referenced by this guard
     pub fn cancel(self) {
         unsafe { self.poll.clear_event_by_fd(self.event.fd) };

src/device/mod.rs

@@ -176,7 +176,7 @@ impl DeviceHandle {
     pub fn clean(&mut self) {
         for path in &self.device.read().cleanup_paths {
             // attempt to remove any file we created in the work dir
-            std::fs::remove_file(&path).ok();
+            let _ = std::fs::remove_file(&path);
         }
     }
 
@@ -268,6 +268,7 @@ impl Device {
         }
     }
 
+    #[allow(clippy::too_many_arguments)]
     fn update_peer(
         &mut self,
         pub_key: X25519PublicKey,
@@ -510,7 +511,9 @@ impl Device {
         self.queue.new_periodic_event(
             // Reset the rate limiter every second give or take
             Box::new(|d, _| {
-                d.rate_limiter.as_ref().map(|r| r.reset_count());
+                if let Some(r) = d.rate_limiter.as_ref() {
+                    r.reset_count()
+                }
                 Action::Continue
             }),
             std::time::Duration::from_secs(1),
@@ -540,10 +543,10 @@ impl Device {
                         }
                         TunnResult::Err(e) => eprintln!("Timer error {:?}", e),
                         TunnResult::WriteToNetwork(packet) => {
-                            peer.add_tx_bytes(match endpoint_addr {
+                            match endpoint_addr {
                                 SocketAddr::V4(_) => udp4.sendto(packet, endpoint_addr),
                                 SocketAddr::V6(_) => udp6.sendto(packet, endpoint_addr),
-                            });
+                            };
                         }
                         _ => panic!("Unexpected result from update_timers"),
                     };
@@ -622,16 +625,16 @@ impl Device {
                         TunnResult::Err(_) => continue,
                         TunnResult::WriteToNetwork(packet) => {
                             flush = true;
-                            peer.add_tx_bytes(udp.sendto(packet, addr));
+                            udp.sendto(packet, addr);
                         }
                         TunnResult::WriteToTunnelV4(packet, addr) => {
                             if peer.is_allowed_ip(addr) {
-                                peer.add_rx_bytes(t.iface.write4(packet))
+                                t.iface.write4(packet);
                             }
                         }
                         TunnResult::WriteToTunnelV6(packet, addr) => {
                             if peer.is_allowed_ip(addr) {
-                                peer.add_rx_bytes(t.iface.write6(packet))
+                                t.iface.write6(packet);
                             }
                         }
                     };
@@ -641,7 +644,7 @@ impl Device {
                         while let TunnResult::WriteToNetwork(packet) =
                             peer.tunnel.decapsulate(None, &[], &mut t.dst_buf[..])
                         {
-                            peer.add_tx_bytes(udp.write(packet));
+                            udp.write(packet);
                         }
                     }
 
@@ -691,16 +694,16 @@ impl Device {
                         TunnResult::Err(e) => eprintln!("Decapsulate error {:?}", e),
                         TunnResult::WriteToNetwork(packet) => {
                             flush = true;
-                            peer.add_tx_bytes(udp.write(packet));
+                            udp.write(packet);
                         }
                         TunnResult::WriteToTunnelV4(packet, addr) => {
                             if peer.is_allowed_ip(addr) {
-                                peer.add_rx_bytes(iface.write4(packet))
+                                iface.write4(packet);
                             }
                         }
                         TunnResult::WriteToTunnelV6(packet, addr) => {
                             if peer.is_allowed_ip(addr) {
-                                peer.add_rx_bytes(iface.write6(packet))
+                                iface.write6(packet);
                             }
                         }
                     };
@@ -710,7 +713,7 @@ impl Device {
                         while let TunnResult::WriteToNetwork(packet) =
                             peer.tunnel.decapsulate(None, &[], &mut t.dst_buf[..])
                         {
-                            peer.add_tx_bytes(udp.write(packet));
+                            udp.write(packet);
                         }
                     }
 
@@ -761,11 +764,11 @@ impl Device {
                                 let endpoint = peer.endpoint();
                                 if let Some(ref conn) = endpoint.conn {
                                     // Prefer to send using the connected socket
-                                    peer.add_tx_bytes(conn.write(packet));
+                                    conn.write(packet);
                                 } else if let Some(addr @ SocketAddr::V4(_)) = endpoint.addr {
-                                    peer.add_tx_bytes(udp4.sendto(packet, addr));
+                                    udp4.sendto(packet, addr);
                                 } else if let Some(addr @ SocketAddr::V6(_)) = endpoint.addr {
-                                    peer.add_tx_bytes(udp6.sendto(packet, addr));
+                                    udp6.sendto(packet, addr);
                                 } else {
                                     eprintln!("No endpoint for peer");
                                 }

src/device/peer.rs

@@ -6,7 +6,6 @@ use crate::device::*;
 use std::net::IpAddr;
 use std::net::SocketAddr;
 use std::str::FromStr;
-use std::sync::atomic::{AtomicUsize, Ordering};
 
 #[derive(Default, Debug)]
 pub struct Endpoint {
@@ -17,8 +16,6 @@ pub struct Endpoint {
 pub struct Peer {
     pub(crate) tunnel: Box<Tunn>, // The associated tunnel struct
     index: u32,                   // The index the tunnel uses
-    rx_bytes: AtomicUsize,
-    tx_bytes: AtomicUsize,
     endpoint: spin::RwLock<Endpoint>,
     allowed_ips: AllowedIps<()>,
     preshared_key: Option<[u8; 32]>,
@@ -56,24 +53,16 @@ impl Peer {
         allowed_ips: &[AllowedIP],
         preshared_key: Option<[u8; 32]>,
     ) -> Peer {
-        let mut peer = Peer {
+        Peer {
             tunnel,
             index,
-            rx_bytes: AtomicUsize::new(0),
-            tx_bytes: AtomicUsize::new(0),
             endpoint: spin::RwLock::new(Endpoint {
                 addr: endpoint,
                 conn: None,
             }),
-            allowed_ips: Default::default(),
+            allowed_ips: allowed_ips.iter().collect(),
             preshared_key,
-        };
-
-        for AllowedIP { addr, cidr } in allowed_ips {
-            peer.allowed_ips.insert(*addr, *cidr as _, ());
         }
-
-        peer
     }
 
     pub fn update_timers<'a>(&self, dst: &'a mut [u8]) -> TunnResult<'a> {
@@ -145,22 +134,6 @@ impl Peer {
         Ok(udp_conn)
     }
 
-    pub fn add_rx_bytes(&self, amt: usize) {
-        self.rx_bytes.fetch_add(amt, Ordering::Relaxed);
-    }
-
-    pub fn add_tx_bytes(&self, amt: usize) {
-        self.tx_bytes.fetch_add(amt, Ordering::Relaxed);
-    }
-
-    pub fn get_rx_bytes(&self) -> usize {
-        self.rx_bytes.load(Ordering::Relaxed)
-    }
-
-    pub fn get_tx_bytes(&self) -> usize {
-        self.tx_bytes.load(Ordering::Relaxed)
-    }
-
     pub fn is_allowed_ip<I: Into<IpAddr>>(&self, addr: I) -> bool {
         self.allowed_ips.find(addr.into()).is_some()
     }

src/device/tun_linux.rs

@@ -74,7 +74,7 @@ impl TunSocket {
         let mut ifr = ifreq {
             ifr_name: [0; IFNAMSIZ],
             ifr_ifru: IfrIfru {
-                ifru_flags: IFF_TUN | IFF_NO_PI | IFF_MULTI_QUEUE,
+                ifru_flags: (IFF_TUN | IFF_NO_PI | IFF_MULTI_QUEUE) as _,
             },
         };
 

src/ffi/mod.rs

@@ -49,7 +49,9 @@ pub struct stats {
     pub time_since_last_handshake: i64,
     pub tx_bytes: usize,
     pub rx_bytes: usize,
-    reserved: [u8; 64], // Make sure to add new fields in this space, keeping total size constant
+    pub estimated_loss: f32,
+    pub estimated_rtt: i32,
+    reserved: [u8; 56], // Make sure to add new fields in this space, keeping total size constant
 }
 
 impl<'a> From<TunnResult<'a>> for wireguard_result {
@@ -294,12 +296,14 @@ pub unsafe extern "C" fn wireguard_force_handshake(
 #[no_mangle]
 pub unsafe extern "C" fn wireguard_stats(tunnel: *mut Tunn) -> stats {
     let tunnel = tunnel.as_ref().unwrap();
-    let (time, tx_bytes, rx_bytes) = tunnel.stats();
+    let (time, tx_bytes, rx_bytes, estimated_loss, estimated_rtt) = tunnel.stats();
     stats {
         time_since_last_handshake: time.map(|t| t as i64).unwrap_or(-1),
         tx_bytes,
         rx_bytes,
-        reserved: [0u8; 64],
+        estimated_loss,
+        estimated_rtt: estimated_rtt.map(|r| r as i32).unwrap_or(-1),
+        reserved: [0u8; 56],
     }
 }
 

src/main.rs

@@ -75,6 +75,7 @@ fn main() {
                 .default_value("/tmp/boringtun.err"),
             Arg::with_name("disable-drop-privileges")
                 .long("disable-drop-privileges")
+                .env("WG_SUDO")
                 .help("Do not drop sudo privileges"),
             Arg::with_name("disable-connected-udp")
                 .long("disable-connected-udp")