From d38aafa054a89d7e5217b48d9161d899d18553aa Mon Sep 17 00:00:00 2001 From: Nick Mathewson <nickm@torproject.org> Date: Fri, 18 Mar 2022 11:57:39 -0400 Subject: [PATCH] Expose more peer information from circuit build failures We already have the ability to get peer information from ChanMgr errors, and therefore from any RetryErrors that contain ChanMgr errors. This commit adds optional peer information to tor-proto errors, and a function to expose whatever peer information is available. --- crates/tor-circmgr/src/build.rs | 27 ++++++++++++++++++++++----- crates/tor-circmgr/src/err.rs | 28 ++++++++++++++++++++++++---- doc/semver_status.md | 2 ++ 3 files changed, 48 insertions(+), 9 deletions(-) diff --git a/crates/tor-circmgr/src/build.rs b/crates/tor-circmgr/src/build.rs index 12ec9150b8..1768cdbd1b 100644 --- a/crates/tor-circmgr/src/build.rs +++ b/crates/tor-circmgr/src/build.rs @@ -80,7 +80,10 @@ async fn create_common<RT: Runtime, CT: ChanTarget>( peer: OwnedChanTarget::from_chan_target(target), cause, })?; - let (pending_circ, reactor) = chan.new_circ().await?; + let (pending_circ, reactor) = chan.new_circ().await.map_err(|error| Error::Protocol { + error, + peer: None, // we don't blame the peer, because new_circ() does no networking. + })?; rt.spawn(async { let _ = reactor.run().await; @@ -99,7 +102,12 @@ impl Buildable for ClientCirc { params: &CircParameters, ) -> Result<Self> { let circ = create_common(chanmgr, rt, ct).await?; - Ok(circ.create_firsthop_fast(params).await?) + circ.create_firsthop_fast(params) + .await + .map_err(|error| Error::Protocol { + peer: Some(ct.clone()), + error, + }) } async fn create<RT: Runtime>( chanmgr: &ChanMgr<RT>, @@ -108,7 +116,12 @@ impl Buildable for ClientCirc { params: &CircParameters, ) -> Result<Self> { let circ = create_common(chanmgr, rt, ct).await?; - Ok(circ.create_firsthop_ntor(ct, params.clone()).await?) + circ.create_firsthop_ntor(ct, params.clone()) + .await + .map_err(|error| Error::Protocol { + peer: Some(OwnedChanTarget::from_chan_target(ct)), + error, + }) } async fn extend<RT: Runtime>( &self, @@ -116,8 +129,12 @@ impl Buildable for ClientCirc { ct: &OwnedCircTarget, params: &CircParameters, ) -> Result<()> { - self.extend_ntor(ct, params).await?; - Ok(()) + self.extend_ntor(ct, params) + .await + .map_err(|error| Error::Protocol { + error, + peer: None, // we don't know who caused the error. + }) } } diff --git a/crates/tor-circmgr/src/err.rs b/crates/tor-circmgr/src/err.rs index 41498e20e6..93f554a50c 100644 --- a/crates/tor-circmgr/src/err.rs +++ b/crates/tor-circmgr/src/err.rs @@ -76,8 +76,16 @@ pub enum Error { }, /// Protocol issue while building a circuit. - #[error("Problem building a circuit: {0}")] - Protocol(#[from] tor_proto::Error), + #[error("Problem building a circuit with {peer:?}")] + Protocol { + /// The peer that created the protocol error. + /// + /// This is set to None if we can't blame a single party. + peer: Option<OwnedChanTarget>, + /// The underlying error. + #[source] + error: tor_proto::Error, + }, /// We have an expired consensus #[error("Consensus is expired")] @@ -143,7 +151,7 @@ impl HasKind for Error { .map(|e| e.kind()) .unwrap_or(EK::Internal), E::CircCanceled => EK::TransientFailure, - E::Protocol(e) => e.kind(), + E::Protocol { error, .. } => error.kind(), E::State(e) => e.kind(), E::GuardMgr(e) => e.kind(), E::Guard(e) => e.kind(), @@ -179,11 +187,23 @@ impl Error { E::Guard(_) => 40, E::RequestFailed(_) => 40, E::Channel { .. } => 40, - E::Protocol(_) => 45, + E::Protocol { .. } => 45, E::ExpiredConsensus => 50, E::Spawn { .. } => 90, E::State(_) => 90, E::Bug(_) => 100, } } + + /// Return a list of the peers to "blame" for this error, if there are any. + pub fn peers(&self) -> Vec<&OwnedChanTarget> { + match self { + Error::RequestFailed(errors) => errors.sources().flat_map(|e| e.peers()).collect(), + Error::Channel { peer, .. } => vec![peer], + Error::Protocol { + peer: Some(peer), .. + } => vec![peer], + _ => vec![], + } + } } diff --git a/doc/semver_status.md b/doc/semver_status.md index 59bc6ecd59..3e75b1c5d3 100644 --- a/doc/semver_status.md +++ b/doc/semver_status.md @@ -54,6 +54,8 @@ tor-circmgr: api-break: The fallbacks case of DirInfo now wants a slice of references to fallbacks. + api-break: Some error types have changed to include peer info. + tor-dirmgr: new-api: DirMgrConfig object now has accessors. DirMgrCfg: totally changed, builder abolished. -- GitLab