Commit 11a90916 authored by Nick Mathewson's avatar Nick Mathewson 🦞
Browse files

arti-client: Report clock skew when it is noteworthy

(Also, blame clock skew when it is an explanation of why we cannot
finish a connection.)
parent 5f946b8d
Loading
Loading
Loading
Loading
+2 −0
Original line number Original line Diff line number Diff line
@@ -388,11 +388,13 @@ impl<R: Runtime> TorClient<R> {


        let conn_status = chanmgr.bootstrap_events();
        let conn_status = chanmgr.bootstrap_events();
        let dir_status = dirmgr.bootstrap_events();
        let dir_status = dirmgr.bootstrap_events();
        let skew_status = circmgr.skew_events();
        runtime
        runtime
            .spawn(status::report_status(
            .spawn(status::report_status(
                status_sender,
                status_sender,
                conn_status,
                conn_status,
                dir_status,
                dir_status,
                skew_status,
            ))
            ))
            .map_err(|e| ErrorDetail::from_spawn("top-level status reporter", e))?;
            .map_err(|e| ErrorDetail::from_spawn("top-level status reporter", e))?;


+45 −6
Original line number Original line Diff line number Diff line
@@ -8,6 +8,7 @@ use educe::Educe;
use futures::{Stream, StreamExt};
use futures::{Stream, StreamExt};
use tor_basic_utils::skip_fmt;
use tor_basic_utils::skip_fmt;
use tor_chanmgr::{ConnBlockage, ConnStatus, ConnStatusEvents};
use tor_chanmgr::{ConnBlockage, ConnStatus, ConnStatusEvents};
use tor_circmgr::{ClockSkewEvents, SkewEstimate};
use tor_dirmgr::DirBootstrapStatus;
use tor_dirmgr::DirBootstrapStatus;
use tracing::debug;
use tracing::debug;


@@ -29,6 +30,8 @@ pub struct BootstrapStatus {
    conn_status: ConnStatus,
    conn_status: ConnStatus,
    /// Status for our directory information.
    /// Status for our directory information.
    dir_status: DirBootstrapStatus,
    dir_status: DirBootstrapStatus,
    /// Current estimate of our clock skew.
    skew: Option<SkewEstimate>,
}
}


impl BootstrapStatus {
impl BootstrapStatus {
@@ -73,7 +76,15 @@ impl BootstrapStatus {
        if let Some(b) = self.conn_status.blockage() {
        if let Some(b) = self.conn_status.blockage() {
            let message = b.to_string().into();
            let message = b.to_string().into();
            let kind = b.into();
            let kind = b.into();
            if matches!(kind, BlockageKind::ClockSkewed) && self.skew_is_noteworthy() {
                Some(Blockage {
                    kind,
                    message: format!("Clock is {}", self.skew.as_ref().expect("logic error"))
                        .into(),
                })
            } else {
                Some(Blockage { kind, message })
                Some(Blockage { kind, message })
            }
        } else {
        } else {
            None
            None
        }
        }
@@ -88,6 +99,16 @@ impl BootstrapStatus {
    fn apply_dir_status(&mut self, status: DirBootstrapStatus) {
    fn apply_dir_status(&mut self, status: DirBootstrapStatus) {
        self.dir_status = status;
        self.dir_status = status;
    }
    }

    /// Adjust this status based on new estimated clock skew information.
    fn apply_skew_estimate(&mut self, status: Option<SkewEstimate>) {
        self.skew = status;
    }

    /// Return true if our current clock skew estimate is considered noteworthy.
    fn skew_is_noteworthy(&self) -> bool {
        matches!(&self.skew, Some(s) if s.noteworthy())
    }
}
}


/// A reason why a client believes it is stuck.
/// A reason why a client believes it is stuck.
@@ -115,6 +136,10 @@ pub enum BlockageKind {
    /// We have some other kind of problem connecting to Tor
    /// We have some other kind of problem connecting to Tor
    #[display(fmt = "Can't reach the Tor network")]
    #[display(fmt = "Can't reach the Tor network")]
    CantReachTor,
    CantReachTor,
    /// We believe our clock is set incorrectly, and that's preventing us from
    /// successfully with relays and/or from finding a directory that we trust.
    #[display(fmt = "Clock is skewed.")]
    ClockSkewed,
}
}


impl From<ConnBlockage> for BlockageKind {
impl From<ConnBlockage> for BlockageKind {
@@ -122,6 +147,7 @@ impl From<ConnBlockage> for BlockageKind {
        match b {
        match b {
            ConnBlockage::NoTcp => BlockageKind::Offline,
            ConnBlockage::NoTcp => BlockageKind::Offline,
            ConnBlockage::NoHandshake => BlockageKind::Filtering,
            ConnBlockage::NoHandshake => BlockageKind::Filtering,
            ConnBlockage::CertsExpired => BlockageKind::ClockSkewed,
            _ => BlockageKind::CantReachTor,
            _ => BlockageKind::CantReachTor,
        }
        }
    }
    }
@@ -136,14 +162,20 @@ impl fmt::Display for BootstrapStatus {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        let percent = (self.as_frac() * 100.0).round() as u32;
        let percent = (self.as_frac() * 100.0).round() as u32;
        if let Some(problem) = self.blocked() {
        if let Some(problem) = self.blocked() {
            write!(f, "Stuck at {}%: {}", percent, problem)
            write!(f, "Stuck at {}%: {}", percent, problem)?;
        } else {
        } else {
            write!(
            write!(
                f,
                f,
                "{}%: {}; {}",
                "{}%: {}; {}",
                percent, &self.conn_status, &self.dir_status
                percent, &self.conn_status, &self.dir_status
            )
            )?;
        }
        if let Some(skew) = &self.skew {
            if skew.noteworthy() {
                write!(f, ". Clock is {}", skew)?;
            }
        }
        }
        Ok(())
    }
    }
}
}


@@ -159,7 +191,8 @@ impl fmt::Display for BootstrapStatus {
pub(crate) async fn report_status(
pub(crate) async fn report_status(
    mut sender: postage::watch::Sender<BootstrapStatus>,
    mut sender: postage::watch::Sender<BootstrapStatus>,
    conn_status: ConnStatusEvents,
    conn_status: ConnStatusEvents,
    dir_status: impl Stream<Item = DirBootstrapStatus> + Unpin,
    dir_status: impl Stream<Item = DirBootstrapStatus> + Send + Unpin,
    skew_status: ClockSkewEvents,
) {
) {
    /// Internal enumeration to combine incoming status changes.
    /// Internal enumeration to combine incoming status changes.
    enum Event {
    enum Event {
@@ -167,15 +200,21 @@ pub(crate) async fn report_status(
        Conn(ConnStatus),
        Conn(ConnStatus),
        /// A directory status change
        /// A directory status change
        Dir(DirBootstrapStatus),
        Dir(DirBootstrapStatus),
        /// A clock skew change
        Skew(Option<SkewEstimate>),
    }
    }
    let mut stream =
    let mut stream = futures::stream::select_all(vec![
        futures::stream::select(conn_status.map(Event::Conn), dir_status.map(Event::Dir));
        conn_status.map(Event::Conn).boxed(),
        dir_status.map(Event::Dir).boxed(),
        skew_status.map(Event::Skew).boxed(),
    ]);


    while let Some(event) = stream.next().await {
    while let Some(event) = stream.next().await {
        let mut b = sender.borrow_mut();
        let mut b = sender.borrow_mut();
        match event {
        match event {
            Event::Conn(e) => b.apply_conn_status(e),
            Event::Conn(e) => b.apply_conn_status(e),
            Event::Dir(e) => b.apply_dir_status(e),
            Event::Dir(e) => b.apply_dir_status(e),
            Event::Skew(e) => b.apply_skew_estimate(e),
        }
        }
        debug!("{}", *b);
        debug!("{}", *b);
    }
    }
+5 −0
Original line number Original line Diff line number Diff line
@@ -96,6 +96,11 @@ impl SkewEstimate {
        self.estimate
        self.estimate
    }
    }


    /// Return true if this estimate is worth telling the user about.
    pub fn noteworthy(&self) -> bool {
        !matches!(self.estimate, ClockSkew::None) && !matches!(self.confidence, Confidence::None)
    }

    /// Compute an estimate of how skewed we think our clock is, based on the
    /// Compute an estimate of how skewed we think our clock is, based on the
    /// reports in `skews`.
    /// reports in `skews`.
    pub(crate) fn estimate_skew<'a>(
    pub(crate) fn estimate_skew<'a>(