Repository: cloudflare/boringtun (language: rust)

Issue:
Platform differences on timers when OS is asleep
boringtun relies on on std::time::Instant to deal with handshake timeout, session timeout, keep-alive, etc. Under the hood, std::time::Instant uses the following APIs (https://doc.rust-lang.org/std/time/struct.Instant.html#underlying-system-calls)

Windows => QueryPerformanceCounter
macOS => mach_absolute_time
Linux => clock_gettime

These timers do not exhibit the same behavior when the OS is asleep

Windows:

> QueryPerformanceCounter reads the performance counter and returns the total number of ticks that have occurred since the Windows operating system was started, including the time when the machine was in a sleep state such as standby, hibernate, or connected standby.

https://docs.microsoft.com/en-us/windows/win32/sysinfo/acquiring-high-resolution-time-stamps

macOS:

> this clock does not increment while the system is asleep.

https://developer.apple.com/documentation/kernel/1462446-mach_absolute_time

Linux (I couldn't find any docs).

I think std::time::Instant maybe the wrong clock to use here? My main concern is macOS. After waking from sleep the timers are not properly updated.

There is already a change out for rust std::time::Instant here: https://github.com/rust-lang/rust/pull/88714


Discussion / hints:

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index 96a52af5..3de0db5e 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -610,9 +610,9 @@ checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646"
 
 [[package]]
 name = "libc"
-version = "0.2.126"
+version = "0.2.132"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "349d5a591cd28b49e1d1037471617a32ddcda5731b99419008085f72d5a53836"
+checksum = "8371e4e5341c3a96db127eb2465ac681ced4c433e01dd0e938adbef26ba93ba5"
 
 [[package]]
 name = "lock_api"
@@ -656,14 +656,14 @@ checksum = "717e29a243b81f8130e31e24e04fb151b04a44b5a7d05370935f7d937e9de06d"
 
 [[package]]
 name = "nix"
-version = "0.24.1"
+version = "0.25.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8f17df307904acd05aa8e32e97bb20f2a0df1728bbc2d771ae8f9a90463441e9"
+checksum = "e322c04a9e3440c327fca7b6c8a63e6890a32fa2ad689db972425f07e0d22abb"
 dependencies = [
+ "autocfg",
  "bitflags",
  "cfg-if",
  "libc",
- "memoffset",
 ]
 
 [[package]]
diff --git a/boringtun/Cargo.toml b/boringtun/Cargo.toml
index f0197c58..95ccf5d1 100644
--- a/boringtun/Cargo.toml
+++ b/boringtun/Cargo.toml
@@ -34,8 +34,8 @@ hmac = "0.12"
 jni = { version = "0.19.0", optional = true }
 mock_instant = { version = "0.2", optional = true }
 
-[target.'cfg(target_os="macos")'.dependencies]
-nix = "0.24.1"
+[target.'cfg(unix)'.dependencies]
+nix = { version = "0.25", default-features = false, features = ["time", "user"] }
 
 [dev-dependencies]
 etherparse = "0.12"
diff --git a/boringtun/src/lib.rs b/boringtun/src/lib.rs
index 619d02e7..530544a9 100644
--- a/boringtun/src/lib.rs
+++ b/boringtun/src/lib.rs
@@ -10,9 +10,11 @@ pub mod device;
 
 #[cfg(feature = "ffi-bindings")]
 pub mod ffi;
-pub mod noise;
-
 #[cfg(feature = "jni-bindings")]
 pub mod jni;
+pub mod noise;
+
+#[cfg(not(feature = "mock-instant"))]
+pub(crate) mod sleepyinstant;
 
 pub(crate) mod serialization;
diff --git a/boringtun/src/noise/handshake.rs b/boringtun/src/noise/handshake.rs
index 9ceaf522..a17df60a 100644
--- a/boringtun/src/noise/handshake.rs
+++ b/boringtun/src/noise/handshake.rs
@@ -4,6 +4,8 @@
 use super::{HandshakeInit, HandshakeResponse, PacketCookieReply};
 use crate::noise::errors::WireGuardError;
 use crate::noise::session::Session;
+#[cfg(not(feature = "mock-instant"))]
+use crate::sleepyinstant::Instant;
 use aead::{Aead, Payload};
 use blake2::digest::{FixedOutput, KeyInit};
 use blake2::{Blake2s256, Blake2sMac, Digest};
@@ -11,8 +13,6 @@ use chacha20poly1305::XChaCha20Poly1305;
 use rand_core::OsRng;
 use ring::aead::{Aad, LessSafeKey, Nonce, UnboundKey, CHACHA20_POLY1305};
 use std::convert::TryInto;
-#[cfg(not(feature = "mock-instant"))]
-use std::time::Instant;
 use std::time::{Duration, SystemTime};
 
 #[cfg(feature = "mock-instant")]
diff --git a/boringtun/src/noise/rate_limiter.rs b/boringtun/src/noise/rate_limiter.rs
index 7e559d4d..32839f85 100644
--- a/boringtun/src/noise/rate_limiter.rs
+++ b/boringtun/src/noise/rate_limiter.rs
@@ -8,7 +8,7 @@ use std::net::IpAddr;
 use std::sync::atomic::{AtomicU64, Ordering};
 
 #[cfg(not(feature = "mock-instant"))]
-use std::time::Instant;
+use crate::sleepyinstant::Instant;
 
 use aead::generic_array::GenericArray;
 use aead::{AeadInPlace, KeyInit};
diff --git a/boringtun/src/noise/timers.rs b/boringtun/src/noise/timers.rs
index a16f5df6..6b91d576 100644
--- a/boringtun/src/noise/timers.rs
+++ b/boringtun/src/noise/timers.rs
@@ -12,7 +12,7 @@ use std::time::Duration;
 use mock_instant::Instant;
 
 #[cfg(not(feature = "mock-instant"))]
-use std::time::Instant;
+use crate::sleepyinstant::Instant;
 
 // Some constants, represent time in seconds
 // https://www.wireguard.com/papers/wireguard.pdf#page=14
diff --git a/boringtun/src/sleepyinstant/mod.rs b/boringtun/src/sleepyinstant/mod.rs
new file mode 100644
index 00000000..542beea3
--- /dev/null
+++ b/boringtun/src/sleepyinstant/mod.rs
@@ -0,0 +1,77 @@
+#![forbid(unsafe_code)]
+//! Attempts to provide the same functionality as std::time::Instant, except it
+//! uses a timer which accounts for time when the system is asleep
+use std::time::Duration;
+
+#[cfg(target_os = "windows")]
+mod windows;
+#[cfg(target_os = "windows")]
+use windows as inner;
+
+#[cfg(unix)]
+mod unix;
+#[cfg(unix)]
+use unix as inner;
+
+/// A measurement of a monotonically nondecreasing clock.
+/// Opaque and useful only with [`Duration`].
+///
+/// Instants are always guaranteed, barring [platform bugs], to be no less than any previously
+/// measured instant when created, and are often useful for tasks such as measuring
+/// benchmarks or timing how long an operation takes.
+///
+/// Note, however, that instants are **not** guaranteed to be **steady**. In other
+/// words, each tick of the underlying clock might not be the same length (e.g.
+/// some seconds may be longer than others). An instant may jump forwards or
+/// experience time dilation (slow down or speed up), but it will never go
+/// backwards.
+///
+/// Instants are opaque types that can only be compared to one another. There is
+/// no method to get "the number of seconds" from an instant. Instead, it only
+/// allows measuring the duration between two instants (or comparing two
+/// instants).
+///
+/// The size of an `Instant` struct may vary depending on the target operating
+/// system.
+///
+#[derive(Clone, Copy, Debug)]
+pub struct Instant {
+    t: inner::Instant,
+}
+
+impl Instant {
+    /// Returns an instant corresponding to "now".
+    pub fn now() -> Self {
+        Self {
+            t: inner::Instant::now(),
+        }
+    }
+
+    /// Returns the amount of time elapsed from another instant to this one,
+    /// or zero duration if that instant is later than this one.
+    ///
+    /// # Panics
+    ///
+    /// panics when `earlier` was later than `self`.
+    pub fn duration_since(&self, earlier: Instant) -> Duration {
+        self.t.duration_since(earlier.t)
+    }
+
+    /// Returns the amount of time elapsed since this instant was created.
+    pub fn elapsed(&self) -> Duration {
+        Self::now().duration_since(*self)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn time_increments_after_sleep() {
+        let sleep_time = Duration::from_millis(10);
+        let start = Instant::now();
+        std::thread::sleep(sleep_time);
+        assert!(start.elapsed() >= sleep_time);
+    }
+}
diff --git a/boringtun/src/sleepyinstant/unix.rs b/boringtun/src/sleepyinstant/unix.rs
new file mode 100644
index 00000000..40aaebab
--- /dev/null
+++ b/boringtun/src/sleepyinstant/unix.rs
@@ -0,0 +1,48 @@
+use std::time::Duration;
+
+use nix::sys::time::TimeSpec;
+use nix::time::{clock_gettime, ClockId};
+
+#[cfg(any(target_os = "macos", target_os = "ios"))]
+const CLOCK_ID: ClockId = ClockId::CLOCK_MONOTONIC;
+#[cfg(not(any(target_os = "macos", target_os = "ios")))]
+const CLOCK_ID: ClockId = ClockId::CLOCK_BOOTTIME;
+
+#[derive(Clone, Copy, Debug)]
+pub(crate) struct Instant {
+    t: TimeSpec,
+}
+
+impl Instant {
+    pub(crate) fn now() -> Self {
+        // std::time::Instant unwraps as well, so feel safe doing so here
+        let t = clock_gettime(CLOCK_ID).unwrap();
+        Self { t }
+    }
+
+    fn checked_duration_since(&self, earlier: Instant) -> Option<Duration> {
+        const NANOSECOND: nix::libc::c_long = 1_000_000_000;
+        let (tv_sec, tv_nsec) = if self.t.tv_nsec() < earlier.t.tv_nsec() {
+            (
+                self.t.tv_sec() - earlier.t.tv_sec() - 1,
+                self.t.tv_nsec() - earlier.t.tv_nsec() + NANOSECOND,
+            )
+        } else {
+            (
+                self.t.tv_sec() - earlier.t.tv_sec(),
+                self.t.tv_nsec() - earlier.t.tv_nsec(),
+            )
+        };
+
+        if tv_sec < 0 {
+            None
+        } else {
+            Some(Duration::new(tv_sec as _, tv_nsec as _))
+        }
+    }
+
+    pub(crate) fn duration_since(&self, earlier: Instant) -> Duration {
+        self.checked_duration_since(earlier)
+            .unwrap_or(Duration::ZERO)
+    }
+}
diff --git a/boringtun/src/sleepyinstant/windows.rs b/boringtun/src/sleepyinstant/windows.rs
new file mode 100644
index 00000000..ac852292
--- /dev/null
+++ b/boringtun/src/sleepyinstant/windows.rs
@@ -0,0 +1,1 @@
+pub(crate) use std::time::Instant;


Test patch: