proto: small err cleanups

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"

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<std::io::Error> for Error {
-    fn from(e: std::io::Error) -> Error {
-        Error::Io(e)
-    }
-}
-impl From<tor_bytes::Error> 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 }))
     }

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<Self> {
         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<Option<IpAddr>> {
             (&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<Self> {
         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()?;

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;

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<tor_bytes::Error> for Error {
@@ -35,22 +46,8 @@ impl From<tor_bytes::Error> 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<std::io::Error> for Error {
+    fn from(e: std::io::Error) -> Self {
+        Error::IoErr(e)
     }
 }
-
-impl std::error::Error for Error {}