Commit 9160b55c authored by Nick Mathewson's avatar Nick Mathewson 🦞
Browse files

chanmgr: bubble ClockSkew up through the Error object. 

Fortunately, we don't need a separate type here: authenticated
clock skew can only come attached to a `tor_proto::Error`.

We also remove skew from `tor_proto::Error::HandshakeCertsExpired`,
since it would now be redundant.
parent 7656ab09

crates/tor-chanmgr/src/builder.rs

+15 −3
Original line number Original line Diff line number Diff line
@@ -184,10 +184,22 @@ impl<R: Runtime> ChanBuilder<R> { 
        let chan = builder

        let chan = builder

            .launch(tls)

            .launch(tls)

            .connect(|| self.runtime.wallclock())

            .connect(|| self.runtime.wallclock())

            .await?;

            .await

            .map_err(Error::from_proto_no_skew)?;

        let clock_skew = Some(chan.clock_skew()); // Not yet authenticated; can't use it till `check` is done.

        let now = self.runtime.wallclock();

        let now = self.runtime.wallclock();

        let chan = chan.check(target, &peer_cert, Some(now))?;

        let chan = chan

        let (chan, reactor) = chan.finish().await?;

            .check(target, &peer_cert, Some(now))

            .map_err(|source| match &source {

                tor_proto::Error::HandshakeCertsExpired { .. } => {

                    Error::Proto { source, clock_skew }

                }

                _ => Error::from_proto_no_skew(source),

            })?;

        let (chan, reactor) = chan

            .finish()

            .await

            .map_err(|source| Error::Proto { source, clock_skew })?;





        {

        {

            self.event_sender

            self.event_sender

crates/tor-chanmgr/src/err.rs

+41 −5
Original line number Original line Diff line number Diff line
@@ -7,6 +7,7 @@ use futures::task::SpawnError; 
use thiserror::Error;

use thiserror::Error;





use tor_error::{internal, ErrorKind};

use tor_error::{internal, ErrorKind};

use tor_proto::ClockSkew;





/// An error returned by a channel manager.

/// An error returned by a channel manager.

#[derive(Debug, Error, Clone)]

#[derive(Debug, Error, Clone)]
@@ -25,8 +26,15 @@ pub enum Error { 
    ChanTimeout,

    ChanTimeout,





    /// A protocol error while making a channel

    /// A protocol error while making a channel

    #[error("Protocol error while opening a channel: {0}")]

    #[error("Protocol error while opening a channel.")]

    Proto(#[from] tor_proto::Error),

    Proto {

        /// The underlying error

        #[source]

        source: tor_proto::Error,

        /// An authenticated ClockSkew (if available) that we received from the

        /// peer.

        clock_skew: Option<ClockSkew>,

    },





    /// Network IO error or TLS error

    /// Network IO error or TLS error

    #[error("Network IO error, or TLS error, in {action}, talking to {peer}")]

    #[error("Network IO error, or TLS error, in {action}, talking to {peer}")]
@@ -82,9 +90,14 @@ impl tor_error::HasKind for Error { 
        use Error as E;

        use Error as E;

        use ErrorKind as EK;

        use ErrorKind as EK;

        match self {

        match self {

            E::ChanTimeout | E::Io { .. } | E::Proto(ProtoErr::ChanIoErr(_)) => EK::TorAccessFailed,

            E::ChanTimeout

            | E::Io { .. }

            | E::Proto {

                source: ProtoErr::ChanIoErr(_),

                ..

            } => EK::TorAccessFailed,

            E::Spawn { cause, .. } => cause.kind(),

            E::Spawn { cause, .. } => cause.kind(),

            E::Proto(e) => e.kind(),

            E::Proto { source, .. } => source.kind(),

            E::PendingFailed => EK::TorAccessFailed,

            E::PendingFailed => EK::TorAccessFailed,

            E::UnusableTarget(_) | E::Internal(_) => EK::Internal,

            E::UnusableTarget(_) | E::Internal(_) => EK::Internal,

            Error::ChannelBuild { .. } => EK::TorAccessFailed,

            Error::ChannelBuild { .. } => EK::TorAccessFailed,
@@ -104,7 +117,7 @@ impl tor_error::HasRetryTime for Error { 
            //

            //

            // TODO: Someday we might want to distinguish among different kinds of IO

            // TODO: Someday we might want to distinguish among different kinds of IO

            // errors.

            // errors.

            E::PendingFailed | E::Proto(_) | E::Io { .. } => RT::AfterWaiting,

            E::PendingFailed | E::Proto { .. } | E::Io { .. } => RT::AfterWaiting,





            // This error reflects multiple attempts, but every failure is an IO

            // This error reflects multiple attempts, but every failure is an IO

            // error, so we can also retry this after a delay.

            // error, so we can also retry this after a delay.
@@ -131,4 +144,27 @@ impl Error { 
            cause: Arc::new(err),

            cause: Arc::new(err),

        }

        }

    }

    }



    /// Construct a new `Error` from a `tor_proto::Error`, with no additional

    /// clock skew information.

    ///

    /// This is not an `Into` implementation because we don't want to call it

    /// accidentally when we actually do have clock skew information.

    pub(crate) fn from_proto_no_skew(source: tor_proto::Error) -> Self {

        Error::Proto {

            source,

            clock_skew: None,

        }

    }



    /// Return the clock skew information from this error (or from an internal

    /// error).

    ///

    /// Only returns the clock skew information if it is authenticated.

    pub fn clock_skew(&self) -> Option<ClockSkew> {

        match self {

            Error::Proto { clock_skew, .. } => *clock_skew,

            _ => None,

        }

    }

}
 
}

crates/tor-chanmgr/src/lib.rs

+2 −1
Original line number Original line Diff line number Diff line
@@ -125,7 +125,8 @@ impl<R: Runtime> ChanMgr<R> { 
        let chan = self.mgr.get_or_launch(*ed_identity, targetinfo).await?;

        let chan = self.mgr.get_or_launch(*ed_identity, targetinfo).await?;

        // Double-check the match to make sure that the RSA identity is

        // Double-check the match to make sure that the RSA identity is

        // what we wanted too.

        // what we wanted too.

        chan.check_match(target)?;

        chan.check_match(target)

            .map_err(Error::from_proto_no_skew)?;

        Ok(chan)

        Ok(chan)

    }

    }

crates/tor-proto/src/channel/handshake.rs

+1 −1
Original line number Original line Diff line number Diff line
@@ -328,7 +328,7 @@ impl<T: AsyncRead + AsyncWrite + Send + Unpin + 'static> UnverifiedChannel<T> { 
                (TimeValidityError::Expired(expired_by), ClockSkew::Fast(skew))

                (TimeValidityError::Expired(expired_by), ClockSkew::Fast(skew))

                    if expired_by < skew =>

                    if expired_by < skew =>

                {

                {

                    Error::HandshakeCertsExpired { expired_by, skew }

                    Error::HandshakeCertsExpired { expired_by }

                }

                }

                // As it so happens, we don't need to check for this case, since the certs in use

                // As it so happens, we don't need to check for this case, since the certs in use

                // here only have an expiration time in them.

                // here only have an expiration time in them.

crates/tor-proto/src/util/err.rs

+0 −2
Original line number Original line Diff line number Diff line
@@ -50,8 +50,6 @@ pub enum Error { 
    HandshakeCertsExpired {

    HandshakeCertsExpired {

        /// For how long has the circuit been expired?

        /// For how long has the circuit been expired?

        expired_by: Duration,

        expired_by: Duration,

        /// How fast does the relay claim that our clock is?

        skew: Duration,

    },

    },

    /// Protocol violation at the channel level, other than at the handshake

    /// Protocol violation at the channel level, other than at the handshake

    /// stage.

    /// stage.