diff --git a/crates/tor-cell/src/chancell/msg.rs b/crates/tor-cell/src/chancell/msg.rs
index 77b905f20feb1d9c9458af004e00c7ce69334c9f..777d527e3a072b6220a9f61fb6ee2dfdb743394c 100644
--- a/crates/tor-cell/src/chancell/msg.rs
+++ b/crates/tor-cell/src/chancell/msg.rs
@@ -587,8 +587,11 @@ impl DestroyReason {
/// channel and sending data.
#[derive(Clone, Debug)]
pub struct Netinfo {
- /// Time when this cell was sent, or 0 if this cell is sent by
- /// a client.
+ /// Time when this cell was sent, or 0 if this cell is sent by a client.
+ ///
+ /// TODO-SPEC(nickm): Y2038 issue here. Better add a new handshake version
+ /// to solve it. See
+ /// [torspec#80](https://gitlab.torproject.org/tpo/core/torspec/-/issues/80).
timestamp: u32,
/// Observed address for party that did not send the netinfo cell.
their_addr: Option<IpAddr>,
@@ -652,6 +655,15 @@ impl Netinfo {
my_addr,
}
}
+ /// Return the time reported in this NETINFO cell.
+ pub fn timestamp(&self) -> Option<std::time::SystemTime> {
+ use std::time::{Duration, SystemTime};
+ if self.timestamp == 0 {
+ None
+ } else {
+ Some(SystemTime::UNIX_EPOCH + Duration::from_secs(self.timestamp.into()))
+ }
+ }
}
impl Body for Netinfo {
fn into_message(self) -> ChanMsg {
diff --git a/crates/tor-chanmgr/src/builder.rs b/crates/tor-chanmgr/src/builder.rs
index cabbb4b56f665d1efb8a4fc098e2f63a232fe8d7..adca41dfb3130ad84eb6d50ccd32721fd40d2823 100644
--- a/crates/tor-chanmgr/src/builder.rs
+++ b/crates/tor-chanmgr/src/builder.rs
@@ -181,7 +181,10 @@ impl<R: Runtime> ChanBuilder<R> {
// 2. Set up the channel.
let mut builder = ChannelBuilder::new();
builder.set_declared_addr(addr);
- let chan = builder.launch(tls).connect().await?;
+ let chan = builder
+ .launch(tls)
+ .connect(|| self.runtime.wallclock())
+ .await?;
let now = self.runtime.wallclock();
let chan = chan.check(target, &peer_cert, Some(now))?;
let (chan, reactor) = chan.finish().await?;
diff --git a/crates/tor-proto/src/channel/handshake.rs b/crates/tor-proto/src/channel/handshake.rs
index 577211cbf9a37eac9ce1d627cdfdbb001335a69a..111b660df7cd16f627e99e2d3be139cd97a4fd88 100644
--- a/crates/tor-proto/src/channel/handshake.rs
+++ b/crates/tor-proto/src/channel/handshake.rs
@@ -9,11 +9,13 @@ use tor_error::internal;
use crate::channel::codec::{ChannelCodec, CodecError};
use crate::channel::UniqId;
+use crate::util::skew::ClockSkew;
use crate::{Error, Result};
use tor_cell::chancell::{msg, ChanCmd};
use std::net::SocketAddr;
use std::sync::Arc;
+use std::time::SystemTime;
use tor_bytes::Reader;
use tor_linkspec::ChanTarget;
@@ -61,6 +63,8 @@ pub struct UnverifiedChannel<T: AsyncRead + AsyncWrite + Send + Unpin + 'static>
/// The netinfo cell that we got from the relay.
#[allow(dead_code)] // Relays will need this.
netinfo_cell: msg::Netinfo,
+ /// How much clock skew did we detect in this handshake?
+ clock_skew: ClockSkew,
/// Logging identifier for this stream. (Used for logging only.)
unique_id: UniqId,
}
@@ -108,7 +112,13 @@ impl<T: AsyncRead + AsyncWrite + Send + Unpin + 'static> OutboundClientHandshake
/// Negotiate a link protocol version with the relay, and read
/// the relay's handshake information.
- pub async fn connect(mut self) -> Result<UnverifiedChannel<T>> {
+ ///
+ /// Takes a function that reports the current time. In theory, this can just be
+ /// `SystemTime::now()`.
+ pub async fn connect<F>(mut self, now_fn: F) -> Result<UnverifiedChannel<T>>
+ where
+ F: FnOnce() -> SystemTime,
+ {
/// Helper: wrap an IoError as a HandshakeIoErr.
fn io_err_to_handshake(err: std::io::Error) -> Error {
Error::HandshakeIoErr(Arc::new(err))
@@ -128,6 +138,8 @@ impl<T: AsyncRead + AsyncWrite + Send + Unpin + 'static> OutboundClientHandshake
.map_err(io_err_to_handshake)?;
self.tls.flush().await.map_err(io_err_to_handshake)?;
}
+ let versions_flushed_at = coarsetime::Instant::now();
+ let versions_flushed_wallclock = now_fn();
// Get versions cell.
trace!("{}: waiting for versions", self.unique_id);
@@ -172,7 +184,7 @@ impl<T: AsyncRead + AsyncWrite + Send + Unpin + 'static> OutboundClientHandshake
// Read until we have the netinfo cells.
let mut certs: Option<msg::Certs> = None;
- let mut netinfo: Option<msg::Netinfo> = None;
+ let mut netinfo: Option<(msg::Netinfo, coarsetime::Instant)> = None;
let mut seen_authchallenge = false;
// Loop: reject duplicate and unexpected cells
@@ -207,7 +219,7 @@ impl<T: AsyncRead + AsyncWrite + Send + Unpin + 'static> OutboundClientHandshake
"Somehow tried to record a duplicate NETINFO cell"
)));
}
- netinfo = Some(n);
+ netinfo = Some((n, coarsetime::Instant::now()));
break;
}
// No other cell types are allowed.
@@ -226,13 +238,24 @@ impl<T: AsyncRead + AsyncWrite + Send + Unpin + 'static> OutboundClientHandshake
"Missing netinfo or closed stream".into(),
)),
(None, _) => Err(Error::HandshakeProto("Missing certs cell".into())),
- (Some(certs_cell), Some(netinfo_cell)) => {
+ (Some(certs_cell), Some((netinfo_cell, netinfo_rcvd_at))) => {
trace!("{}: received handshake, ready to verify.", self.unique_id);
+ let clock_skew = if let Some(netinfo_timestamp) = netinfo_cell.timestamp() {
+ let delay = netinfo_rcvd_at - versions_flushed_at;
+ ClockSkew::from_handshake_timestamps(
+ versions_flushed_wallclock,
+ netinfo_timestamp,
+ delay.into(),
+ )
+ } else {
+ ClockSkew::None
+ };
Ok(UnverifiedChannel {
link_protocol,
tls,
certs_cell,
netinfo_cell,
+ clock_skew,
target_addr: self.target_addr,
unique_id: self.unique_id,
})
@@ -242,6 +265,14 @@ impl<T: AsyncRead + AsyncWrite + Send + Unpin + 'static> OutboundClientHandshake
}
impl<T: AsyncRead + AsyncWrite + Send + Unpin + 'static> UnverifiedChannel<T> {
+ /// Return the reported clock skew from this handshake.
+ ///
+ /// Note that the skew reported by this function might not be "true": the
+ /// relay might have its clock set wrong, or it might be lying to us.
+ pub fn clock_skew(&self) -> ClockSkew {
+ self.clock_skew
+ }
+
/// Validate the certificates and keys in the relay's handshake.
///
/// 'peer' is the peer that we want to make sure we're connecting to.
@@ -479,7 +510,13 @@ pub(super) mod test {
// no certificates in this cell, but connect() doesn't care.
const NOCERTS: &[u8] = &hex!("00000000 81 0001 00");
const NETINFO_PREFIX: &[u8] = &hex!(
- "00000000 08 085F9067F7
+ "00000000 08 00000000
+ 04 04 7f 00 00 02
+ 01
+ 04 04 7f 00 00 03"
+ );
+ const NETINFO_PREFIX_WITH_TIME: &[u8] = &hex!(
+ "00000000 08 48949290
04 04 7f 00 00 02
01
04 04 7f 00 00 03"
@@ -505,18 +542,21 @@ pub(super) mod test {
#[test]
fn connect_ok() -> Result<()> {
tor_rtcompat::test_with_one_runtime!(|_rt| async move {
+ let now = SystemTime::UNIX_EPOCH + Duration::from_secs(1217696400);
let mut buf = Vec::new();
// versions cell
buf.extend_from_slice(VERSIONS);
// certs cell -- no certs in it, but this function doesn't care.
buf.extend_from_slice(NOCERTS);
// netinfo cell -- quite minimal.
- add_netinfo(&mut buf);
+ add_padded(&mut buf, NETINFO_PREFIX);
let mb = MsgBuf::new(&buf[..]);
let handshake = OutboundClientHandshake::new(mb, None);
- let unverified = handshake.connect().await?;
+ let unverified = handshake.connect(|| now).await?;
assert_eq!(unverified.link_protocol, 4);
+ // No timestamp in the NETINFO, so no skew.
+ assert_eq!(unverified.clock_skew(), ClockSkew::None);
// Try again with an authchallenge cell and some padding.
let mut buf = Vec::new();
@@ -525,10 +565,22 @@ pub(super) mod test {
buf.extend_from_slice(VPADDING);
buf.extend_from_slice(AUTHCHALLENGE);
buf.extend_from_slice(VPADDING);
- add_netinfo(&mut buf);
+ add_padded(&mut buf, NETINFO_PREFIX_WITH_TIME);
let mb = MsgBuf::new(&buf[..]);
let handshake = OutboundClientHandshake::new(mb, None);
- let _unverified = handshake.connect().await?;
+ let unverified = handshake.connect(|| now).await?;
+ // Correct timestamp in the NETINFO, so no skew.
+ assert_eq!(unverified.clock_skew(), ClockSkew::None);
+
+ // Now pretend our clock is fast.
+ let now2 = now + Duration::from_secs(3600);
+ let mb = MsgBuf::new(&buf[..]);
+ let handshake = OutboundClientHandshake::new(mb, None);
+ let unverified = handshake.connect(|| now2).await?;
+ assert_eq!(
+ unverified.clock_skew(),
+ ClockSkew::Fast(Duration::from_secs(3600))
+ );
Ok(())
})
@@ -537,7 +589,7 @@ pub(super) mod test {
async fn connect_err<T: Into<Vec<u8>>>(input: T) -> Error {
let mb = MsgBuf::new(input);
let handshake = OutboundClientHandshake::new(mb, None);
- handshake.connect().await.err().unwrap()
+ handshake.connect(SystemTime::now).await.err().unwrap()
}
#[test]
@@ -635,11 +687,13 @@ pub(super) mod test {
fn make_unverified(certs: msg::Certs) -> UnverifiedChannel<MsgBuf> {
let localhost = std::net::IpAddr::V4(std::net::Ipv4Addr::LOCALHOST);
let netinfo_cell = msg::Netinfo::for_client(Some(localhost));
+ let clock_skew = ClockSkew::None;
UnverifiedChannel {
link_protocol: 4,
tls: futures_codec::Framed::new(MsgBuf::new(&b""[..]), ChannelCodec::new(4)),
certs_cell: certs,
netinfo_cell,
+ clock_skew,
target_addr: None,
unique_id: UniqId::new(),
}
diff --git a/crates/tor-proto/src/lib.rs b/crates/tor-proto/src/lib.rs
index 196f85552ab34abb100669d270563a590d71b7ba..24a34c47f3b846494686192b7769aef2785bbc63 100644
--- a/crates/tor-proto/src/lib.rs
+++ b/crates/tor-proto/src/lib.rs
@@ -119,6 +119,7 @@ pub mod stream;
mod util;
pub use util::err::Error;
+pub use util::skew::ClockSkew;
/// A vector of bytes that gets cleared when it's dropped.
type SecretBytes = zeroize::Zeroizing<Vec<u8>>;
diff --git a/crates/tor-proto/src/util.rs b/crates/tor-proto/src/util.rs
index 515ef5a26b77e2ecd66a5d4e1dae0ea5a3534629..043dbad57469424167eb418fa7f98221f5bf4aea 100644
--- a/crates/tor-proto/src/util.rs
+++ b/crates/tor-proto/src/util.rs
@@ -2,4 +2,5 @@
pub(crate) mod ct;
pub(crate) mod err;
+pub(crate) mod skew;
pub(crate) mod ts;
diff --git a/crates/tor-proto/src/util/skew.rs b/crates/tor-proto/src/util/skew.rs
new file mode 100644
index 0000000000000000000000000000000000000000..c85c86a2a56bc8fb3f2cba9ec73b19f0a568c34f
--- /dev/null
+++ b/crates/tor-proto/src/util/skew.rs
@@ -0,0 +1,102 @@
+//! Tools and types for reporting declared clock skew.
+
+use std::time::{Duration, SystemTime};
+
+/// A reported amount of clock skew from a relay or other source.
+///
+/// Note that this information may not be accurate or trustworthy: the relay
+/// could be wrong, or lying.
+///
+/// The skews reported here are _minimum_ amounts; the actual skew may
+/// be a little higher, depending on latency.
+#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd)]
+#[allow(clippy::exhaustive_enums)]
+pub enum ClockSkew {
+ /// Our own clock is "running slow": the relay's clock is at least this far
+ /// ahead of ours.
+ Slow(Duration),
+ /// Our own clock is not necessarily inconsistent with the relay's clock.
+ None,
+ /// Own own clock is "running fast": the relay's clock is at least this far
+ /// behind ours.
+ Fast(Duration),
+}
+
+impl ClockSkew {
+ /// Construct a ClockSkew from a set of channel handshake timestamps.
+ ///
+ /// Requires that `ours_at_start` is the timestamp at the point when we
+ /// started the handshake, `theirs` is the timestamp the relay reported in
+ /// its NETINFO cell, and `delay` is the total amount of time between when
+ /// we started the handshake and when we received the NETINFO cell.
+ pub(crate) fn from_handshake_timestamps(
+ ours_at_start: SystemTime,
+ theirs: SystemTime,
+ delay: Duration,
+ ) -> Self {
+ /// We treat clock skew as "zero" if it less than this long.
+ ///
+ /// (Since the relay only reports its time to the nearest second, we
+ /// can't reasonably infer that differences less than this much reflect
+ /// accurate differences in our clocks.)
+ const MIN: Duration = Duration::from_secs(2);
+
+ // The relay may have generated its own timestamp any time between when
+ // we sent the handshake, and when we got the reply. Therefore, at the
+ // time we started, it was between these values.
+ let theirs_at_start_min = theirs - delay;
+ let theirs_at_start_max = theirs;
+
+ if let Ok(skew) = theirs_at_start_min.duration_since(ours_at_start) {
+ ClockSkew::Slow(skew).if_above(MIN)
+ } else if let Ok(skew) = ours_at_start.duration_since(theirs_at_start_max) {
+ ClockSkew::Fast(skew).if_above(MIN)
+ } else {
+ // Either there is no clock skew, or we can't detect any.
+ ClockSkew::None
+ }
+ }
+
+ /// 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,
+ }
+ }
+}
+
+#[cfg(test)]
+mod test {
+ use super::*;
+
+ #[test]
+ fn make_skew() {
+ let now = SystemTime::now();
+ let later = now + Duration::from_secs(777);
+ let earlier = now - Duration::from_secs(333);
+ let window = Duration::from_secs(30);
+
+ // Case 1: they say our clock is slow.
+ let skew = ClockSkew::from_handshake_timestamps(now, later, window);
+ // The window is only subtracted in this case, since we're reporting the _minimum_ skew.
+ assert_eq!(skew, ClockSkew::Slow(Duration::from_secs(747)));
+
+ // Case 2: they say our clock is fast.
+ let skew = ClockSkew::from_handshake_timestamps(now, earlier, window);
+ assert_eq!(skew, ClockSkew::Fast(Duration::from_secs(333)));
+
+ // Case 3: The variation in our clock is less than the time window it took them to answer.
+ let skew = ClockSkew::from_handshake_timestamps(now, now + Duration::from_secs(20), window);
+ assert_eq!(skew, ClockSkew::None);
+
+ // Case 4: The variation in our clock is less than the limits of the timer precision.
+ let skew = ClockSkew::from_handshake_timestamps(
+ now,
+ now + Duration::from_millis(500),
+ Duration::from_secs(0),
+ );
+ assert_eq!(skew, ClockSkew::None);
+ }
+}
diff --git a/doc/semver_status.md b/doc/semver_status.md
index 07dbb24ba79fcc594bed1ed5c4230884cd7ce827..1f4f86d220732999968e900a7c867dfb098c4690 100644
--- a/doc/semver_status.md
+++ b/doc/semver_status.md
@@ -58,6 +58,12 @@ tor-netdoc:
tor-protover:
new-api: Protocols now implements Eq, PartialEq, and Hash.
+tor-proto:
+ api-break: OutboundClientHandshake::connect() now takes now_fn.
+
+tor-cell:
+ new-api: Netinfo message now has a timestamp() accessor.
+
tor-basic-utils:
Remove `humantime_serde_option` module.