From eedee51899eb3365a21c37be3803846373265284 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 7 Apr 2022 12:16:13 -0400 Subject: [PATCH] Initial functions to determine and expose a clock skew estimate. (This is just a placeholder; I'm going to make the functions smarter in the next commit.) --- Cargo.lock | 1 + crates/tor-guardmgr/Cargo.toml | 1 + crates/tor-guardmgr/src/fallback/set.rs | 7 +++ crates/tor-guardmgr/src/guard.rs | 6 ++ crates/tor-guardmgr/src/lib.rs | 17 ++++++ crates/tor-guardmgr/src/sample.rs | 5 ++ crates/tor-guardmgr/src/skew.rs | 76 ++++++++++++++++++++++++- crates/tor-proto/src/util/skew.rs | 17 ++++-- 8 files changed, 125 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4115ecb25..f3fbc14fa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3463,6 +3463,7 @@ dependencies = [ "derive_more", "educe", "futures", + "humantime 2.1.0", "humantime-serde", "itertools", "pin-project", diff --git a/crates/tor-guardmgr/Cargo.toml b/crates/tor-guardmgr/Cargo.toml index df329d129..36f2e2874 100644 --- a/crates/tor-guardmgr/Cargo.toml +++ b/crates/tor-guardmgr/Cargo.toml @@ -33,6 +33,7 @@ derive_builder = "0.11" derive_more = "0.99" educe = "0.4.6" futures = "0.3.14" +humantime = "2" humantime-serde = "1.1.1" itertools = "0.10.1" pin-project = "1" diff --git a/crates/tor-guardmgr/src/fallback/set.rs b/crates/tor-guardmgr/src/fallback/set.rs index 0ebd5584b..cc208045f 100644 --- a/crates/tor-guardmgr/src/fallback/set.rs +++ b/crates/tor-guardmgr/src/fallback/set.rs @@ -215,6 +215,13 @@ impl FallbackState { entry.status.clock_skew = Some(observation); } } + + /// Return an iterator over all the clock skew observations we've made for fallback directories + pub(crate) fn skew_observations(&self) -> impl Iterator { + self.fallbacks + .iter() + .filter_map(|fb| fb.status.clock_skew.as_ref()) + } } #[cfg(test)] diff --git a/crates/tor-guardmgr/src/guard.rs b/crates/tor-guardmgr/src/guard.rs index 12d5f8cde..8be9e3ccd 100644 --- a/crates/tor-guardmgr/src/guard.rs +++ b/crates/tor-guardmgr/src/guard.rs @@ -673,6 +673,12 @@ impl Guard { self.clock_skew = Some(observation); } + /// Return the most recent clock skew observation for this guard, if we have + /// made one. + pub(crate) fn skew(&self) -> Option<&SkewObservation> { + self.clock_skew.as_ref() + } + /// Testing only: Return true if this guard was ever contacted successfully. #[cfg(test)] pub(crate) fn confirmed(&self) -> bool { diff --git a/crates/tor-guardmgr/src/lib.rs b/crates/tor-guardmgr/src/lib.rs index a29ecf115..dddd26579 100644 --- a/crates/tor-guardmgr/src/lib.rs +++ b/crates/tor-guardmgr/src/lib.rs @@ -163,6 +163,7 @@ pub use err::{GuardMgrError, PickGuardError}; pub use filter::GuardFilter; pub use ids::FirstHopId; pub use pending::{GuardMonitor, GuardStatus, GuardUsable}; +pub use skew::SkewEstimate; use pending::{PendingRequest, RequestId}; use sample::GuardSet; @@ -568,6 +569,14 @@ impl GuardMgr { ); } + /// Return our best estimate of our current clock skew, based on reports from the + /// guards and fallbacks we have contacted. + pub fn skew_estimate(&self) -> Option { + let inner = self.inner.lock().expect("Poisoned lock"); + let now = self.runtime.now(); + SkewEstimate::estimate_skew(inner.skew_observations(), now) + } + /// Ensure that the message queue is flushed before proceeding to /// the next step. Used for testing. #[cfg(test)] @@ -835,6 +844,14 @@ impl GuardMgrInner { } } + /// Return an iterator over all of the clock skew observations we've made + /// for guards or fallbacks. + fn skew_observations(&self) -> impl Iterator { + self.fallbacks + .skew_observations() + .chain(self.guards.active_guards().skew_observations()) + } + /// If the circuit built because of a given [`PendingRequest`] may /// now be used (or discarded), return `Some(true)` or /// `Some(false)` respectively. diff --git a/crates/tor-guardmgr/src/sample.rs b/crates/tor-guardmgr/src/sample.rs index f6f9cbf08..66021bdde 100644 --- a/crates/tor-guardmgr/src/sample.rs +++ b/crates/tor-guardmgr/src/sample.rs @@ -669,6 +669,11 @@ impl GuardSet { } } + /// Return an iterator over all stored clock skew observations. + pub(crate) fn skew_observations(&self) -> impl Iterator { + self.guards.values().filter_map(|g| g.skew()) + } + /// Return whether the circuit manager can be allowed to use a /// circuit with the `guard_id`. /// diff --git a/crates/tor-guardmgr/src/skew.rs b/crates/tor-guardmgr/src/skew.rs index bba261b15..6d79b8166 100644 --- a/crates/tor-guardmgr/src/skew.rs +++ b/crates/tor-guardmgr/src/skew.rs @@ -1,6 +1,6 @@ //! Code for creating and manipulating observations about clock skew. -use std::time::Instant; +use std::time::{Duration, Instant}; use tor_proto::ClockSkew; @@ -13,3 +13,77 @@ pub(crate) struct SkewObservation { /// The time when we added this observation. pub(crate) when: Instant, } + +impl SkewObservation { + /// Return true if this observation has been made more recently than + /// `cutoff`. + pub(crate) fn more_recent_than(&self, cutoff: Instant) -> bool { + self.when > cutoff + } +} + +/// An estimate of how skewed our clock is, plus a summary of why we think so. +// +// XXXX This is a placeholder for now. +#[derive(Clone, Debug)] +pub struct SkewEstimate { + /// Our best guess for the magnitude of the skew. + estimate: ClockSkew, + /// The number of observations leading to this estimate. + n_observations: usize, +} + +impl std::fmt::Display for SkewEstimate { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + use humantime::format_duration; + match self.estimate { + ClockSkew::Slow(d) => write!(f, "slow by {}", format_duration(d)), + ClockSkew::None => write!(f, "not skewed"), + ClockSkew::Fast(d) => write!(f, "fast by {}", format_duration(d)), + }?; + + write!(f, " (based on {} recent observations)", self.n_observations) + } +} + +impl SkewEstimate { + /// Return our best estimate for the current clock skew. + pub fn skew(&self) -> ClockSkew { + self.estimate + } + + /// Compute an estimate of how skewed we think our clock is, based on the + /// reports in `skews`. + pub(crate) fn estimate_skew<'a>( + skews: impl Iterator, + now: Instant, + ) -> Option { + // Only consider skew observations reported at least this recently. + let cutoff = now - Duration::from_secs(3600); + + // Don't believe observations unless we have at least this many. (This + // value is chosen somewhat arbitrarily.) + // + // Note that under normal client operation, we won't connect to this + // many guards or fallbacks. That's fine: clock skew is only a problem + // when it keeps us from bootstrapping, and when we are having + // bootstrapping problems, we _will_ connect to many guards or + // fallbacks. + let min_observations = 8; + + let mut skews: Vec<_> = skews + .filter_map(|obs| obs.more_recent_than(cutoff).then(|| obs.skew)) + .collect(); + let n_observations = skews.len(); + if n_observations < min_observations { + return None; + } + + let (_, median, _) = skews.select_nth_unstable(n_observations / 2); + // TODO: Consider the quartiles as well, as a rough estimate of confidence. + Some(SkewEstimate { + estimate: *median, + n_observations, + }) + } +} diff --git a/crates/tor-proto/src/util/skew.rs b/crates/tor-proto/src/util/skew.rs index c85c86a2a..b9c18a533 100644 --- a/crates/tor-proto/src/util/skew.rs +++ b/crates/tor-proto/src/util/skew.rs @@ -57,12 +57,21 @@ impl ClockSkew { } } + /// Return the magnitude of this clock skew. + pub fn magnitude(&self) -> Duration { + match self { + ClockSkew::Slow(d) => *d, + ClockSkew::None => Duration::from_secs(0), + ClockSkew::Fast(d) => *d, + } + } + /// Return this value if it is greater than `min`; otherwise return None. pub fn if_above(self, min: Duration) -> Self { - match self { - ClockSkew::Slow(d) if d > min => ClockSkew::Slow(d), - ClockSkew::Fast(d) if d > min => ClockSkew::Fast(d), - _ => ClockSkew::None, + if self.magnitude() > min { + self + } else { + ClockSkew::None } } }