From 7095192063432893c891300d24855014c2b83099 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 6 Sep 2020 17:50:06 -0400 Subject: [PATCH] proto: small err cleanups --- tor-proto/Cargo.toml | 1 + tor-proto/src/chancell/codec.rs | 27 +++++---------------- tor-proto/src/chancell/msg.rs | 12 +++++---- tor-proto/src/lib.rs | 1 + tor-proto/src/util/err.rs | 43 +++++++++++++++------------------ 5 files changed, 35 insertions(+), 49 deletions(-) diff --git a/tor-proto/Cargo.toml b/tor-proto/Cargo.toml index 6cc3854f8..92adff549 100644 --- a/tor-proto/Cargo.toml +++ b/tor-proto/Cargo.toml @@ -27,6 +27,7 @@ stream-cipher = "0.7.1" sha2 = "0.9.1" futures_codec = "*" bytes = "*" +thiserror = "*" [dev-dependencies] hex-literal = "0.3.1" diff --git a/tor-proto/src/chancell/codec.rs b/tor-proto/src/chancell/codec.rs index 65fbf69be..e9558d32d 100644 --- a/tor-proto/src/chancell/codec.rs +++ b/tor-proto/src/chancell/codec.rs @@ -1,28 +1,11 @@ use crate::chancell::{msg::ChanMsg, ChanCell, ChanCmd, CircID}; use crate::crypto::cell::CELL_BODY_LEN; +use crate::Error; use arrayref::{array_mut_ref, array_ref}; use bytes; use futures_codec; use tor_bytes::{self, Reader, Writer}; -// XXXX make a crate-level error type -pub enum Error { - Io(std::io::Error), - Bytes(tor_bytes::Error), - Misc(), -} - -impl From for Error { - fn from(e: std::io::Error) -> Error { - Error::Io(e) - } -} -impl From for Error { - fn from(e: tor_bytes::Error) -> Error { - Error::Bytes(e) - } -} - // Note: only link versions 3 and higher are supported. Versions cell // is not supported via coder/decoder ,since it always uses a two-byte // circuit-ID. @@ -47,7 +30,7 @@ impl futures_codec::Encoder for ChannelCodec { msg.write_body_onto(dst); let len = dst.len() - pos - 2; if len > std::u16::MAX as usize { - return Err(Error::Misc()); + return Err(Error::InternalError("ran out of space for varcell".into())); } // go back and set the length. *(array_mut_ref![&mut dst[pos..pos + 2], 0, 2]) = (len as u16).to_be_bytes(); @@ -56,7 +39,7 @@ impl futures_codec::Encoder for ChannelCodec { msg.write_body_onto(dst); let len = dst.len() - pos; if len > CELL_BODY_LEN { - return Err(Error::Misc()); + return Err(Error::InternalError("ran out of space for cell".into())); } // pad to end of fixed-length cell dst.write_zeros(CELL_BODY_LEN - len); @@ -93,7 +76,9 @@ impl futures_codec::Decoder for ChannelCodec { let msg = r.extract()?; if !cmd.accepts_circid_val(circid) { - return Err(Error::Misc()); + return Err(Error::ChanProto( + "Invalid circuit ID for cell command".into(), + )); } Ok(Some(ChanCell { circid, msg })) } diff --git a/tor-proto/src/chancell/msg.rs b/tor-proto/src/chancell/msg.rs index 70463172f..1560906fd 100644 --- a/tor-proto/src/chancell/msg.rs +++ b/tor-proto/src/chancell/msg.rs @@ -1,6 +1,6 @@ /// A channel message is a decoded channel cell. use crate::crypto::cell::{RawCellBody, CELL_BODY_LEN}; -use tor_bytes::{Error, Readable, Reader, Result, Writer}; +use tor_bytes::{self, Error, Readable, Reader, Result, Writer}; use super::ChanCmd; @@ -149,7 +149,7 @@ impl ChanMsg for VPadding { impl Readable for VPadding { fn take_from(r: &mut Reader<'_>) -> Result { if r.remaining() > std::u16::MAX as usize { - return Err(Error::BadMessage("XX")); + return Err(Error::BadMessage("Too many bytes in VPADDING cell".into())); } Ok(VPadding { len: r.remaining() as u16, @@ -319,8 +319,8 @@ fn take_one_netinfo_addr(r: &mut Reader<'_>) -> Result> { (&mut bytes[..]).copy_from_slice(abody); Ok(Some(IpAddr::V6(bytes.into()))) } - (0x04, _) => Err(Error::BadMessage("XX")), - (0x06, _) => Err(Error::BadMessage("XX")), + (0x04, _) => Ok(None), // ignore + (0x06, _) => Ok(None), // ignore (_, _) => Ok(None), } } @@ -401,7 +401,9 @@ impl Readable for PaddingNegotiate { fn take_from(r: &mut Reader<'_>) -> Result { let v = r.take_u8()?; if v != 0 { - return Err(Error::BadMessage("XX")); + return Err(Error::BadMessage( + "Unrecognized padding negotiation version", + )); } let command = r.take_u8()?; let ito_low_ms = r.take_u16()?; diff --git a/tor-proto/src/lib.rs b/tor-proto/src/lib.rs index 8c04ef752..c17c6b7bc 100644 --- a/tor-proto/src/lib.rs +++ b/tor-proto/src/lib.rs @@ -27,6 +27,7 @@ pub mod chancell; mod crypto; pub mod proto; mod util; + pub use util::err::Error; use zeroize::Zeroizing; diff --git a/tor-proto/src/util/err.rs b/tor-proto/src/util/err.rs index 4589a2979..812ff199a 100644 --- a/tor-proto/src/util/err.rs +++ b/tor-proto/src/util/err.rs @@ -1,32 +1,43 @@ //! Define an error type for the tor-proto crate. -use std::fmt; +use thiserror::Error; /// An error type for the tor-proto crate. /// /// This type should probably be split into several. There's more /// than one kind of error that can occur while doing something with /// the Tor protocol. -/// -/// TODO: convert this to use thiserror. -#[derive(Debug, PartialEq, Eq)] +#[derive(Error, Debug)] #[non_exhaustive] pub enum Error { /// An error that occurred in the tor_bytes crate while decoding an /// object. - BytesErr(tor_bytes::Error), + #[error("parsing error: {0}")] + BytesErr(#[source] tor_bytes::Error), + /// An error that occurred from the io system. + #[error("io error: {0}")] + IoErr(#[source] std::io::Error), /// Somebody asked for a key that we didn't have. + #[error("specified key was missing")] MissingKey, /// We tried to produce too much output for some function. + #[error("couldn't produce that much output")] InvalidOutputLength, /// We tried to encrypt a message to a hop that wasn't there. + #[error("tried to encrypt to nonexistent hop")] NoSuchHop, /// There was a programming error somewhere in the code. - InternalError, + #[error("Internal programming error: {0}")] + InternalError(String), /// The authentication information on this cell was completely wrong, /// or the cell was corrupted. + #[error("bad relay cell authentication")] BadCellAuth, /// A circuit-extension handshake failed. + #[error("handshake failed")] BadHandshake, + /// Protocol violation at the channel level + #[error("channel protocol violation")] + ChanProto(String), } impl From for Error { @@ -35,22 +46,8 @@ impl From for Error { } } -impl fmt::Display for Error { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - use Error::*; - match self { - BytesErr(e) => { - return e.fmt(f); - } - MissingKey => "Request that would need a key I don't have", - InvalidOutputLength => "Tried to extract too much data from a KDF", - InternalError => "Ran into an internal programming error", - NoSuchHop => "Tried to send a cell to a hop that wasn't there", - BadCellAuth => "Cell wasn't for me, or hash was bad", - BadHandshake => "Incorrect handshake.", - } - .fmt(f) +impl From for Error { + fn from(e: std::io::Error) -> Self { + Error::IoErr(e) } } - -impl std::error::Error for Error {}