diff --git a/Cargo.lock b/Cargo.lock index 13231100a..db642797f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3256,6 +3256,7 @@ dependencies = [ "tor-cell", "tor-cert", "tor-checkable", + "tor-error", "tor-linkspec", "tor-llcrypto", "tor-protover", diff --git a/crates/tor-proto/Cargo.toml b/crates/tor-proto/Cargo.toml index c441ed6be..432739f84 100644 --- a/crates/tor-proto/Cargo.toml +++ b/crates/tor-proto/Cargo.toml @@ -21,6 +21,7 @@ traffic-timestamp = ["coarsetime"] tor-llcrypto = { path = "../tor-llcrypto", version = "0.0.3"} tor-bytes = { path = "../tor-bytes", version = "0.0.3"} tor-cert = { path = "../tor-cert", version = "0.0.3"} +tor-error = { path = "../tor-error", version = "0.0.1"} tor-linkspec = { path = "../tor-linkspec", version = "0.0.3"} tor-checkable = { path = "../tor-checkable", version = "0.0.3"} tor-protover = { path = "../tor-protover", version = "0.0.3"} diff --git a/crates/tor-proto/src/channel.rs b/crates/tor-proto/src/channel.rs index 2a8f53fde..4142b94eb 100644 --- a/crates/tor-proto/src/channel.rs +++ b/crates/tor-proto/src/channel.rs @@ -70,6 +70,7 @@ use crate::util::ts::OptTimestamp; use crate::{Error, Result}; use std::pin::Pin; use tor_cell::chancell::{msg, ChanCell, CircId}; +use tor_error::internal; use tor_linkspec::ChanTarget; use tor_llcrypto::pk::ed25519::Ed25519Identity; use tor_llcrypto::pk::rsa::RsaIdentity; @@ -344,12 +345,12 @@ impl Channel { use msg::ChanMsg::*; let msg = cell.msg(); match msg { - Created(_) | Created2(_) | CreatedFast(_) => Err(Error::InternalError(format!( + Created(_) | Created2(_) | CreatedFast(_) => Err(Error::Internal(internal!( "Can't send {} cell on client channel", msg.cmd() ))), Certs(_) | Versions(_) | Authenticate(_) | Authorize(_) | AuthChallenge(_) - | Netinfo(_) => Err(Error::InternalError(format!( + | Netinfo(_) => Err(Error::Internal(internal!( "Can't send {} cell after handshake is done", msg.cmd() ))), @@ -474,17 +475,13 @@ pub(crate) mod test { let cell = ChanCell::new(7.into(), msg::Created2::new(&b"hihi"[..]).into()); let e = chan.check_cell(&cell); assert!(e.is_err()); - assert_eq!( - format!("{}", e.unwrap_err()), - "Internal programming error: Can't send CREATED2 cell on client channel" - ); + assert!(format!("{}", e.unwrap_err()) + .contains("Can't send CREATED2 cell on client channel")); let cell = ChanCell::new(0.into(), msg::Certs::new_empty().into()); let e = chan.check_cell(&cell); assert!(e.is_err()); - assert_eq!( - format!("{}", e.unwrap_err()), - "Internal programming error: Can't send CERTS cell after handshake is done" - ); + assert!(format!("{}", e.unwrap_err()) + .contains("Can't send CERTS cell after handshake is done")); let cell = ChanCell::new(5.into(), msg::Create2::new(2, &b"abc"[..]).into()); let e = chan.check_cell(&cell); diff --git a/crates/tor-proto/src/channel/handshake.rs b/crates/tor-proto/src/channel/handshake.rs index ff7b2e1c5..bbd3dda2e 100644 --- a/crates/tor-proto/src/channel/handshake.rs +++ b/crates/tor-proto/src/channel/handshake.rs @@ -5,6 +5,7 @@ use asynchronous_codec as futures_codec; use futures::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; use futures::sink::SinkExt; use futures::stream::StreamExt; +use tor_error::internal; use crate::channel::codec::ChannelCodec; use crate::channel::UniqId; @@ -178,9 +179,9 @@ impl OutboundClientHandshake if netinfo.is_some() { // This should be impossible, since we would // exit this loop on the first netinfo cell. - return Err(Error::InternalError( - "Somehow tried to record a duplicate NETINFO cell".into(), - )); + return Err(Error::Internal(internal!( + "Somehow tried to record a duplicate NETINFO cell" + ))); } netinfo = Some(n); break; diff --git a/crates/tor-proto/src/channel/reactor.rs b/crates/tor-proto/src/channel/reactor.rs index 03cce26cf..1159e92bd 100644 --- a/crates/tor-proto/src/channel/reactor.rs +++ b/crates/tor-proto/src/channel/reactor.rs @@ -18,6 +18,7 @@ use futures::channel::{mpsc, oneshot}; use futures::sink::SinkExt; use futures::stream::Stream; use futures::Sink; +use tor_error::internal; use std::convert::TryInto; use std::fmt; @@ -312,7 +313,9 @@ impl Reactor { // TODO(nickm) I think that this one actually means the other side // is closed. See arti#269. target.send(created).map_err(|_| { - Error::InternalError("Circuit queue rejected created message. Is it closing?".into()) + Error::Internal(internal!( + "Circuit queue rejected created message. Is it closing?" + )) }) } @@ -332,9 +335,7 @@ impl Reactor { // TODO(nickm) I think that this one actually means the other side // is closed. See arti#269. .map_err(|_| { - Error::InternalError( - "pending circuit wasn't interested in Destroy cell?".into(), - ) + internal!("pending circuit wasn't interested in destroy cell?").into() }) } // It's an open circuit: tell it that it got a DESTROY cell. @@ -345,7 +346,7 @@ impl Reactor { // TODO(nickm) I think that this one actually means the other side // is closed. See arti#269. .map_err(|_| { - Error::InternalError("circuit wasn't interested in destroy cell?".into()) + internal!("open circuit wasn't interested in destroy cell?").into() }) } // We've sent a destroy; we can leave this circuit removed. diff --git a/crates/tor-proto/src/circuit.rs b/crates/tor-proto/src/circuit.rs index 4a9c0073b..efb0e487f 100644 --- a/crates/tor-proto/src/circuit.rs +++ b/crates/tor-proto/src/circuit.rs @@ -60,6 +60,7 @@ use tor_cell::{ relaycell::msg::{Begin, RelayMsg, Resolve, Resolved, ResolvedVal}, }; +use tor_error::internal; use tor_linkspec::{CircTarget, LinkSpec}; use futures::channel::{mpsc, oneshot}; @@ -240,9 +241,9 @@ impl ClientCirc { let num_hops = self.hops.load(Ordering::SeqCst); if num_hops == 0 { - return Err(Error::InternalError( - "Number of hops specified is zero, cannot continue".into(), - )); + return Err(Error::Internal(internal!( + "Can't begin a stream at the 0th hop" + ))); } let hop_num: HopNum = (num_hops - 1).into(); let (sender, receiver) = mpsc::channel(STREAM_READER_BUFFER); diff --git a/crates/tor-proto/src/circuit/halfstream.rs b/crates/tor-proto/src/circuit/halfstream.rs index a89d117a2..79003a00a 100644 --- a/crates/tor-proto/src/circuit/halfstream.rs +++ b/crates/tor-proto/src/circuit/halfstream.rs @@ -6,6 +6,7 @@ use crate::circuit::sendme::{StreamRecvWindow, StreamSendWindow}; use crate::{Error, Result}; use tor_cell::relaycell::msg::RelayMsg; +use tor_error::internal; /// Type to track state of half-closed streams. /// @@ -66,9 +67,9 @@ impl HalfStream { )) } } - RelayMsg::End(_) => Err(Error::InternalError( - "END cell in HalfStream::handle_msg().".into(), - )), + RelayMsg::End(_) => Err(Error::Internal(internal!( + "END cell in HalfStream::handle_msg()" + ))), _ => Err(Error::CircProto(format!( "Bad {} cell on a closed stream!", msg.cmd() diff --git a/crates/tor-proto/src/circuit/reactor.rs b/crates/tor-proto/src/circuit/reactor.rs index f631bb190..ce3e235e2 100644 --- a/crates/tor-proto/src/circuit/reactor.rs +++ b/crates/tor-proto/src/circuit/reactor.rs @@ -22,6 +22,7 @@ use tor_cell::relaycell::{RelayCell, RelayCmd, StreamId}; use futures::channel::{mpsc, oneshot}; use futures::Sink; use futures::Stream; +use tor_error::internal; use std::sync::atomic::{AtomicU8, Ordering}; use std::sync::Arc; @@ -375,7 +376,13 @@ where let msg = match msg { RelayMsg::Extended2(e) => e, - _ => return Err(Error::InternalError("Body didn't match cmd".into())), + _ => { + return Err(Error::Internal(internal!( + "Message body {:?} didn't match cmd {:?}", + msg, + msg.cmd() + ))) + } }; let relay_handshake = msg.into_body(); @@ -1033,9 +1040,9 @@ impl Reactor { self.meta_handler = Some((handler, done)); Ok(()) } else { - Err(Error::InternalError( - "Tried to install a meta-cell handler before the old one was gone.".into(), - )) + Err(Error::Internal(internal!( + "Tried to install a meta-cell handler before the old one was gone." + ))) } } @@ -1113,9 +1120,10 @@ impl Reactor { let _ = done.send(if let Some(hop) = self.hop_mut(hop) { Ok(hop.sendwindow.window_and_expected_tags()) } else { - Err(Error::InternalError( - "received QuerySendWindow for unknown hop".into(), - )) + Err(Error::Internal(internal!( + "received QuerySendWindow for unknown hop {:?}", + hop + ))) }); } #[cfg(test)] @@ -1138,7 +1146,7 @@ impl Reactor { ) -> Result { let hop = self .hop_mut(hopnum) - .ok_or_else(|| Error::InternalError(format!("No such hop {:?}", hopnum)))?; + .ok_or_else(|| Error::Internal(internal!("No such hop {:?}", hopnum)))?; let send_window = StreamSendWindow::new(SEND_WINDOW_INIT); let r = hop.map.add_ent(sender, rx, send_window)?; let cell = RelayCell::new(r, message); @@ -1153,7 +1161,10 @@ impl Reactor { fn close_stream(&mut self, cx: &mut Context<'_>, hopnum: HopNum, id: StreamId) -> Result<()> { // Mark the stream as closing. let hop = self.hop_mut(hopnum).ok_or_else(|| { - Error::InternalError("Tried to close a stream on a hop that wasn't there?".into()) + Error::Internal(internal!( + "Tried to close a stream on a hop {:?} that wasn't there?", + hopnum + )) })?; let should_send_end = hop.map.terminate(id)?; @@ -1238,7 +1249,10 @@ impl Reactor { self.send_relay_cell(cx, hopnum, false, cell)?; self.hop_mut(hopnum) .ok_or_else(|| { - Error::InternalError("Trying to send SENDME to nonexistent hop".to_string()) + Error::Internal(internal!( + "Trying to send SENDME to nonexistent hop {:?}", + hopnum + )) })? .recvwindow .put(); diff --git a/crates/tor-proto/src/circuit/sendme.rs b/crates/tor-proto/src/circuit/sendme.rs index 8b3538704..e0d2001e5 100644 --- a/crates/tor-proto/src/circuit/sendme.rs +++ b/crates/tor-proto/src/circuit/sendme.rs @@ -14,6 +14,7 @@ use std::collections::VecDeque; use tor_cell::relaycell::msg::RelayMsg; use tor_cell::relaycell::RelayCell; +use tor_error::internal; use crate::{Error, Result}; @@ -187,7 +188,7 @@ where let v = self .window .checked_add(P::increment()) - .ok_or_else(|| Error::InternalError("Overflow on SENDME window".into()))?; + .ok_or_else(|| Error::Internal(internal!("Overflow on SENDME window")))?; self.window = v; Ok(v) } diff --git a/crates/tor-proto/src/circuit/streammap.rs b/crates/tor-proto/src/circuit/streammap.rs index ef10cf978..7cef082d5 100644 --- a/crates/tor-proto/src/circuit/streammap.rs +++ b/crates/tor-proto/src/circuit/streammap.rs @@ -11,6 +11,7 @@ use tor_cell::relaycell::{msg::RelayMsg, StreamId}; use futures::channel::mpsc; use std::collections::hash_map::Entry; use std::collections::HashMap; +use tor_error::internal; use rand::Rng; @@ -180,7 +181,7 @@ impl StreamMap { pub(super) fn terminate(&mut self, id: StreamId) -> Result { // Progress the stream's state machine accordingly match self.m.remove(&id).ok_or_else(|| { - Error::InternalError("Somehow we terminated a nonexistent connection‽".into()) + Error::Internal(internal!("Somehow we terminated a nonexistent stream?")) })? { StreamEnt::EndReceived => Ok(ShouldSendEnd::DontSend), StreamEnt::Open { diff --git a/crates/tor-proto/src/crypto/cell.rs b/crates/tor-proto/src/crypto/cell.rs index a7902cfa8..20bf0fb3a 100644 --- a/crates/tor-proto/src/crypto/cell.rs +++ b/crates/tor-proto/src/crypto/cell.rs @@ -10,6 +10,7 @@ use crate::{Error, Result}; use std::convert::TryInto; use tor_cell::chancell::RawCellBody; +use tor_error::internal; use generic_array::GenericArray; @@ -258,7 +259,10 @@ pub(crate) mod tor1 { } fn initialize(seed: &[u8]) -> Result { if seed.len() != Self::seed_len() { - return Err(Error::InternalError("length is invalid".to_string())); + return Err(Error::Internal(internal!( + "seed length {} was invalid", + seed.len() + ))); } let keylen = SC::KeySize::to_usize(); let dlen = D::OutputSize::to_usize(); diff --git a/crates/tor-proto/src/crypto/handshake/hs_ntor.rs b/crates/tor-proto/src/crypto/handshake/hs_ntor.rs index 7ca01dfd6..ffbd179a3 100644 --- a/crates/tor-proto/src/crypto/handshake/hs_ntor.rs +++ b/crates/tor-proto/src/crypto/handshake/hs_ntor.rs @@ -38,6 +38,7 @@ use digest::Digest; use generic_array::GenericArray; use rand_core::{CryptoRng, RngCore}; use std::convert::TryInto; +use tor_error::into_internal; use tor_llcrypto::cipher::aes::Aes256Ctr; use zeroize::Zeroizing; @@ -371,7 +372,8 @@ fn hs_ntor_mac(key: &[u8], message: &[u8]) -> Result { let result = d.finalize(); result .try_into() - .map_err(|_| Error::InternalError("failed MAC computation".into())) + .map_err(into_internal!("failed MAC computation")) + .map_err(Error::from) } /// Helper function: Compute the part of the HS ntor handshake that generates @@ -418,10 +420,12 @@ fn get_introduce1_key_material( // Extract the keys into arrays let enc_key = hs_keys[0..32] .try_into() - .map_err(|_| Error::InternalError("converting enc_key".into()))?; + .map_err(into_internal!("converting enc_key")) + .map_err(Error::from)?; let mac_key = hs_keys[32..64] .try_into() - .map_err(|_| Error::InternalError("converting mac_key".into()))?; + .map_err(into_internal!("converting mac_key")) + .map_err(Error::from)?; Ok((enc_key, mac_key)) } diff --git a/crates/tor-proto/src/stream/data.rs b/crates/tor-proto/src/stream/data.rs index 79e6e06a8..54f6a20ab 100644 --- a/crates/tor-proto/src/stream/data.rs +++ b/crates/tor-proto/src/stream/data.rs @@ -21,6 +21,7 @@ use std::pin::Pin; use crate::circuit::StreamTarget; use crate::stream::StreamReader; use tor_cell::relaycell::msg::{Data, RelayMsg}; +use tor_error::internal; /// An anonymized stream over the Tor network. /// @@ -188,9 +189,10 @@ impl DataStream { self.r.state = Some(DataReaderState::Ready(imp)); result } else { - Err(Error::InternalError( - "Expected ready state of a new stream.".to_owned(), - )) + Err(Error::Internal(internal!( + "Expected ready state, got {:?}", + state + ))) } } } @@ -451,6 +453,16 @@ enum DataReaderState { ReadingCell(Pin)> + Send>>), } +impl std::fmt::Debug for DataReaderState { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Closed => write!(f, "Closed"), + Self::Ready(_) => write!(f, "Ready"), + Self::ReadingCell(_) => write!(f, "ReadingCell"), + } + } +} + /// Wrapper for the read part of a DataStream struct DataReaderImpl { /// The underlying StreamReader object. diff --git a/crates/tor-proto/src/util/err.rs b/crates/tor-proto/src/util/err.rs index cdad2a6d8..343d49e4f 100644 --- a/crates/tor-proto/src/util/err.rs +++ b/crates/tor-proto/src/util/err.rs @@ -2,6 +2,7 @@ use std::sync::Arc; use thiserror::Error; use tor_cell::relaycell::msg::EndReason; +use tor_error::InternalError; /// An error type for the tor-proto crate. /// @@ -32,7 +33,7 @@ pub enum Error { NoSuchHop, /// There was a programming error somewhere in the code. #[error("Internal programming error: {0}")] - InternalError(String), + Internal(#[from] InternalError), /// The authentication information on this cell was completely wrong, /// or the cell was corrupted. #[error("bad relay cell authentication")] @@ -125,7 +126,7 @@ impl From for std::io::Error { BytesErr(_) | MissingKey | BadCellAuth | BadHandshake | ChanProto(_) | CircProto(_) | CellErr(_) | ChanMismatch(_) | StreamProto(_) => ErrorKind::InvalidData, - InternalError(_) | IdRangeFull | CircExtend(_) | BadConfig(_) | ResolveError(_) => { + Internal(_) | IdRangeFull | CircExtend(_) | BadConfig(_) | ResolveError(_) => { ErrorKind::Other } };