diff --git a/Cargo.lock b/Cargo.lock index c9c8eca3d..74a347d56 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3492,6 +3492,7 @@ dependencies = [ "digest 0.10.3", "hex-literal", "signature", + "thiserror", "tor-bytes", "tor-checkable", "tor-llcrypto", diff --git a/crates/tor-bytes/src/err.rs b/crates/tor-bytes/src/err.rs index e42a20b6d..d0744d283 100644 --- a/crates/tor-bytes/src/err.rs +++ b/crates/tor-bytes/src/err.rs @@ -2,7 +2,7 @@ use thiserror::Error; -/// Error type for decoding and encoding Tor objects from and to bytes. +/// Error type for decoding Tor objects from bytes. // // TODO(nickm): This error type could use a redesign: it doesn't do a good job // of preserving context. At the least it should say what kind of object it @@ -20,8 +20,8 @@ pub enum Error { /// Called Reader::should_be_exhausted(), but found bytes anyway. #[error("Extra bytes at end of object")] ExtraneousBytes, - /// Invalid length value (eg, overflow) - #[error("Object length out of bounds")] + /// Invalid length value + #[error("Object length too large to represent as usize")] BadLengthValue, /// An attempt to parse an object failed for some reason related to its /// contents. @@ -48,3 +48,13 @@ impl PartialEq for Error { } } } + +/// Error type for encoding Tor objects to bytes. +#[derive(Error, Debug, Clone, Eq, PartialEq)] +#[non_exhaustive] +pub enum EncodeError { + /// We tried to encode an object with an attached length, but the length was + /// too large to encode in the available space. + #[error("Object length too large to encode")] + BadLengthValue, +} diff --git a/crates/tor-bytes/src/lib.rs b/crates/tor-bytes/src/lib.rs index 5491d6479..450e613dd 100644 --- a/crates/tor-bytes/src/lib.rs +++ b/crates/tor-bytes/src/lib.rs @@ -85,7 +85,7 @@ mod impls; mod reader; mod writer; -pub use err::Error; +pub use err::{EncodeError, Error}; pub use reader::Reader; pub use writer::Writer; diff --git a/crates/tor-bytes/src/writer.rs b/crates/tor-bytes/src/writer.rs index b7985888d..b28df33d5 100644 --- a/crates/tor-bytes/src/writer.rs +++ b/crates/tor-bytes/src/writer.rs @@ -4,9 +4,9 @@ use std::marker::PhantomData; use educe::Educe; +use crate::EncodeError; use crate::Writeable; use crate::WriteableOnce; -use crate::{Error, Result}; /// A byte-oriented trait for writing to small arrays. /// @@ -166,9 +166,9 @@ where /// In these cases you should have ensured, somehow, that overflow cannot happen. /// Ideally, by making your `Writeable` type incapable of holding values /// whose encoded length doesn't fit in the length field. - pub fn finish(self) -> Result<()> { + pub fn finish(self) -> Result<(), EncodeError> { let length = self.inner.len(); - let length: L = length.try_into().map_err(|_| Error::BadLengthValue)?; + let length: L = length.try_into().map_err(|_| EncodeError::BadLengthValue)?; self.outer.write(&length); self.outer.write(&self.inner); Ok(()) @@ -245,6 +245,6 @@ mod tests { let mut w = v.write_nested_u8len(); w.write_zeros(256); - assert_eq!(w.finish().err().unwrap(), Error::BadLengthValue); + assert_eq!(w.finish().err().unwrap(), EncodeError::BadLengthValue); } } diff --git a/crates/tor-cert/Cargo.toml b/crates/tor-cert/Cargo.toml index 2c320266c..0cc9ea69d 100644 --- a/crates/tor-cert/Cargo.toml +++ b/crates/tor-cert/Cargo.toml @@ -15,6 +15,7 @@ repository = "https://gitlab.torproject.org/tpo/core/arti.git/" caret = { path = "../caret", version = "0.2.0" } digest = "0.10.0" signature = "1" +thiserror = "1" tor-bytes = { path = "../tor-bytes", version = "0.3.0" } tor-checkable = { path = "../tor-checkable", version = "0.3.0" } tor-llcrypto = { path = "../tor-llcrypto", version = "0.3.0" } diff --git a/crates/tor-cert/semver.md b/crates/tor-cert/semver.md new file mode 100644 index 000000000..05b44c67a --- /dev/null +++ b/crates/tor-cert/semver.md @@ -0,0 +1 @@ +BREAKING: No longer using tor_bytes::Error for non-decoding errors diff --git a/crates/tor-cert/src/err.rs b/crates/tor-cert/src/err.rs new file mode 100644 index 000000000..12e88d65f --- /dev/null +++ b/crates/tor-cert/src/err.rs @@ -0,0 +1,24 @@ +//! Define error types for the tor-cert crate. +//! +//! Most of the encoding/decoding functions here return [`tor_bytes::Error`], +//! but many of them (related to certificate-specific operations) do not. + +use thiserror::Error; + +/// An error related to checking or validating a certificate +#[derive(Clone, Debug, Error, Eq, PartialEq)] +#[non_exhaustive] +pub enum CertError { + /// The key on a certificate was not as expected. + #[error("Key on certificate was not as expected")] + KeyMismatch, + + /// We tried to get the signing key from a certificate that didn't include + /// one. + #[error("Missing signing key on certificate")] + MissingPubKey, + + /// We tried to validate a signature, and found that it was wrong. + #[error("Signature on certificate was invalid")] + BadSignature, +} diff --git a/crates/tor-cert/src/lib.rs b/crates/tor-cert/src/lib.rs index e42ddcb9e..afe45211b 100644 --- a/crates/tor-cert/src/lib.rs +++ b/crates/tor-cert/src/lib.rs @@ -83,16 +83,22 @@ #![allow(clippy::let_unit_value)] // This can reasonably be done for explicitness //! +mod err; pub mod rsa; use caret::caret_int; use signature::Verifier; -use tor_bytes::{Error, Result}; +use tor_bytes::{Error as BytesError, Result as BytesResult}; use tor_bytes::{Readable, Reader}; use tor_llcrypto::pk::*; use std::time; +pub use err::CertError; + +/// A Result defined to use CertError +type CertResult = std::result::Result; + caret_int! { /// Recognized values for Tor's certificate type field. /// @@ -241,7 +247,7 @@ impl CertifiedKey { } /// Try to extract a CertifiedKey from a Reader, given that we have /// already read its type as `key_type`. - fn from_reader(key_type: KeyType, r: &mut Reader<'_>) -> Result { + fn from_reader(key_type: KeyType, r: &mut Reader<'_>) -> BytesResult { Ok(match key_type { KeyType::ED25519_KEY => CertifiedKey::Ed25519(r.extract()?), KeyType::SHA256_OF_RSA => CertifiedKey::RsaSha256Digest(r.extract()?), @@ -343,7 +349,7 @@ impl Writeable for UnrecognizedExt { */ impl Readable for CertExt { - fn take_from(b: &mut Reader<'_>) -> Result { + fn take_from(b: &mut Reader<'_>) -> BytesResult { let len = b.take_u16()?; let ext_type: ExtType = b.take_u8()?.into(); let flags = b.take_u8()?; @@ -352,16 +358,16 @@ impl Readable for CertExt { Ok(match ext_type { ExtType::SIGNED_WITH_ED25519_KEY => { if body.len() != 32 { - return Err(Error::BadMessage("wrong length on Ed25519 key")); + return Err(BytesError::BadMessage("wrong length on Ed25519 key")); } CertExt::SignedWithEd25519(SignedWithEd25519Ext { pk: ed25519::PublicKey::from_bytes(body) - .map_err(|_| Error::BadMessage("invalid Ed25519 public key"))?, + .map_err(|_| BytesError::BadMessage("invalid Ed25519 public key"))?, }) } _ => { if (flags & 1) != 0 { - return Err(Error::BadMessage( + return Err(BytesError::BadMessage( "unrecognized certificate extension, with 'affects_validation' flag set.", )); } @@ -412,13 +418,13 @@ impl Ed25519Cert { /// Note that the resulting KeyUnknownCertificate is not checked /// for validity at all: you will need to provide it with an expected /// signing key, then check it for timeliness and well-signedness. - pub fn decode(cert: &[u8]) -> Result { + pub fn decode(cert: &[u8]) -> BytesResult { let mut r = Reader::from_slice(cert); let v = r.take_u8()?; if v != 1 { // This would be something other than a "v1" certificate. We don't // understand those. - return Err(Error::BadMessage("Unrecognized certificate version")); + return Err(BytesError::BadMessage("Unrecognized certificate version")); } let cert_type = r.take_u8()?.into(); let exp_hours = r.take_u32()?; @@ -519,13 +525,13 @@ impl KeyUnknownCert { /// /// On success, we can check whether the certificate is well-signed; /// otherwise, we can't check the certificate. - pub fn check_key(self, pkey: &Option) -> Result { + pub fn check_key(self, pkey: &Option) -> CertResult { let real_key = match (pkey, self.cert.cert.signed_with) { (Some(a), Some(b)) if a == &b => b, - (Some(_), Some(_)) => return Err(Error::BadMessage("Mismatched public key on cert")), + (Some(_), Some(_)) => return Err(CertError::KeyMismatch), (Some(a), None) => *a, (None, Some(b)) => b, - (None, None) => return Err(Error::BadMessage("Missing public key on cert")), + (None, None) => return Err(CertError::MissingPubKey), }; Ok(UncheckedCert { cert: Ed25519Cert { @@ -567,12 +573,9 @@ impl UncheckedCert { /// been checked, and a signature to validate. pub fn dangerously_split( self, - ) -> Result<(SigCheckedCert, ed25519::ValidatableEd25519Signature)> { + ) -> CertResult<(SigCheckedCert, ed25519::ValidatableEd25519Signature)> { use tor_checkable::SelfSigned; - let signing_key = self - .cert - .signed_with - .ok_or(Error::BadMessage("Missing public key on cert"))?; + let signing_key = self.cert.signed_with.ok_or(CertError::MissingPubKey)?; let signature = ed25519::ValidatableEd25519Signature::new(signing_key, self.signature, &self.text[..]); Ok((self.dangerously_assume_wellsigned(), signature)) @@ -592,17 +595,14 @@ impl UncheckedCert { } impl tor_checkable::SelfSigned for UncheckedCert { - type Error = Error; + type Error = CertError; - fn is_well_signed(&self) -> Result<()> { - let pubkey = &self - .cert - .signed_with - .ok_or(Error::BadMessage("Certificate was not in fact self-signed"))?; + fn is_well_signed(&self) -> CertResult<()> { + let pubkey = &self.cert.signed_with.ok_or(CertError::MissingPubKey)?; pubkey .verify(&self.text[..], &self.signature) - .map_err(|_| Error::BadMessage("Invalid certificate signature"))?; + .map_err(|_| CertError::BadSignature)?; Ok(()) } @@ -638,7 +638,7 @@ mod test { use hex_literal::hex; #[test] - fn parse_unrecognized_ext() -> Result<()> { + fn parse_unrecognized_ext() -> BytesResult<()> { // case one: a flag is set but we don't know it let b = hex!("0009 99 10 657874656e73696f6e"); let mut r = Reader::from_slice(&b); @@ -651,11 +651,11 @@ mod test { // handle the extension. let b = hex!("0009 99 11 657874656e73696f6e"); let mut r = Reader::from_slice(&b); - let e: Result = r.extract(); + let e: Result = r.extract(); assert!(e.is_err()); assert_eq!( e.err().unwrap(), - Error::BadMessage( + BytesError::BadMessage( "unrecognized certificate extension, with 'affects_validation' flag set." ) ); @@ -664,7 +664,7 @@ mod test { } #[test] - fn certified_key() -> Result<()> { + fn certified_key() -> BytesResult<()> { let b = hex!("4c27616d6f757220756e6974206365757820717527656e636861c3ae6e616974206c6520666572"); let mut r = Reader::from_slice(&b); diff --git a/crates/tor-cert/tests/invalid_certs.rs b/crates/tor-cert/tests/invalid_certs.rs index d15f63e90..47b669ccc 100644 --- a/crates/tor-cert/tests/invalid_certs.rs +++ b/crates/tor-cert/tests/invalid_certs.rs @@ -1,4 +1,5 @@ use tor_bytes::Error; +use tor_cert::CertError; //use tor_cert::rsa::RsaCrosscert; use tor_cert::Ed25519Cert; use tor_llcrypto::pk::ed25519; @@ -69,7 +70,7 @@ fn mismatched_signing_key() { // wasn't what the cert contained. assert_eq!( cert.check_key(&Some(not_that_key)).err().unwrap(), - Error::BadMessage("Mismatched public key on cert") + CertError::KeyMismatch ); // from testvec_certs. @@ -86,6 +87,6 @@ fn mismatched_signing_key() { // a signing-key extension in the cert. assert_eq!( cert.check_key(&None).err().unwrap(), - Error::BadMessage("Missing public key on cert") + CertError::MissingPubKey ); } diff --git a/crates/tor-netdoc/src/doc/routerdesc.rs b/crates/tor-netdoc/src/doc/routerdesc.rs index 1727c377b..c3b290985 100644 --- a/crates/tor-netdoc/src/doc/routerdesc.rs +++ b/crates/tor-netdoc/src/doc/routerdesc.rs @@ -405,7 +405,8 @@ impl RouterDesc { .parse_obj::("ED25519 CERT")? .check_cert_type(tor_cert::CertType::IDENTITY_V_SIGNING)? .into_unchecked() - .check_key(&None)?; + .check_key(&None) + .map_err(|err| EK::BadSignature.err().with_source(err))?; let sk = cert.peek_subject_key().as_ed25519().ok_or_else(|| { EK::BadObjectVal .at_pos(cert_tok.pos()) @@ -509,7 +510,7 @@ impl RouterDesc { if sign != 0 && sign != 1 { return Err(EK::BadArgument.at_pos(cc.arg_pos(0)).with_msg("not 0 or 1")); } - let ntor_as_ed = + let ntor_as_ed: ll::pk::ed25519::PublicKey = ll::pk::keymanip::convert_curve25519_to_ed25519_public(&ntor_onion_key, sign) .ok_or_else(|| { EK::BadArgument @@ -521,7 +522,8 @@ impl RouterDesc { .check_cert_type(tor_cert::CertType::NTOR_CC_IDENTITY)? .check_subject_key_is(identity_cert.peek_signing_key())? .into_unchecked() - .check_key(&Some(ntor_as_ed))? + .check_key(&Some(ntor_as_ed)) + .map_err(|err| EK::BadSignature.err().with_source(err))? }; // TAP key @@ -636,8 +638,16 @@ impl RouterDesc { }; // Now we're going to collect signatures and expiration times. - let (identity_cert, identity_sig) = identity_cert.dangerously_split()?; - let (crosscert_cert, cc_sig) = crosscert_cert.dangerously_split()?; + let (identity_cert, identity_sig) = identity_cert.dangerously_split().map_err(|err| { + EK::BadObjectVal + .with_msg("missing public key") + .with_source(err) + })?; + let (crosscert_cert, cc_sig) = crosscert_cert.dangerously_split().map_err(|err| { + EK::BadObjectVal + .with_msg("missing public key") + .with_source(err) + })?; let signatures: Vec> = vec![ Box::new(rsa_signature), Box::new(ed_signature), diff --git a/crates/tor-netdoc/src/err.rs b/crates/tor-netdoc/src/err.rs index 140ff5c11..7684d4e68 100644 --- a/crates/tor-netdoc/src/err.rs +++ b/crates/tor-netdoc/src/err.rs @@ -291,6 +291,9 @@ pub(crate) enum ParseErrorSource { /// An error when validating a signature. #[error("Invalid signature")] Signature(#[source] Arc), + /// An error when validating a signature on an embedded binary certificate. + #[error("Invalid certificate")] + CertSignature(#[from] tor_cert::CertError), /// Invalid protocol versions. #[error("Protocol versions")] Protovers(#[from] tor_protover::ParseError), diff --git a/crates/tor-proto/src/channel/codec.rs b/crates/tor-proto/src/channel/codec.rs index 54a706586..d381ffc82 100644 --- a/crates/tor-proto/src/channel/codec.rs +++ b/crates/tor-proto/src/channel/codec.rs @@ -17,11 +17,14 @@ pub(crate) enum CodecError { /// /// (This isn't wrapped in an Arc, because we don't need this type to be /// clone; it's crate-internal.) - #[error("Io error")] + #[error("Io error reading or writing a channel cell")] Io(#[from] IoError), - /// An error from the cell encoding/decoding logic. - #[error("encoding/decoding error")] - Cell(#[from] tor_cell::Error), + /// An error from the cell decoding logic. + #[error("Error decoding an incoming channel cell")] + DecCell(#[source] tor_cell::Error), + /// An error from the cell encoding logic. + #[error("Error encoding an outgoing channel cell")] + EncCell(#[source] tor_cell::Error), } /// Asynchronous wrapper around ChannelCodec in tor_cell, with implementation @@ -44,7 +47,7 @@ impl futures_codec::Encoder for ChannelCodec { type Error = CodecError; fn encode(&mut self, item: Self::Item, dst: &mut BytesMut) -> Result<(), Self::Error> { - self.0.write_cell(item, dst)?; + self.0.write_cell(item, dst).map_err(CodecError::EncCell)?; Ok(()) } } @@ -54,7 +57,7 @@ impl futures_codec::Decoder for ChannelCodec { type Error = CodecError; fn decode(&mut self, src: &mut BytesMut) -> Result, Self::Error> { - Ok(self.0.decode_cell(src)?) + self.0.decode_cell(src).map_err(CodecError::DecCell) } } diff --git a/crates/tor-proto/src/channel/handshake.rs b/crates/tor-proto/src/channel/handshake.rs index 351afd1e0..e8d26ddc5 100644 --- a/crates/tor-proto/src/channel/handshake.rs +++ b/crates/tor-proto/src/channel/handshake.rs @@ -112,7 +112,10 @@ pub struct VerifiedChannel Error { match err { CodecError::Io(e) => Error::HandshakeIoErr(Arc::new(e)), - CodecError::Cell(e) => Error::HandshakeProto(format!("Invalid cell on handshake: {}", e)), + CodecError::DecCell(e) => { + Error::HandshakeProto(format!("Invalid cell on handshake: {}", e)) + } + CodecError::EncCell(e) => Error::from_cell_enc(e, "cell on handshake"), } } @@ -150,7 +153,8 @@ impl trace!("{}: sending versions", self.unique_id); // Send versions cell { - let my_versions = msg::Versions::new(LINK_PROTOCOLS)?; + let my_versions = msg::Versions::new(LINK_PROTOCOLS) + .map_err(|e| Error::from_cell_enc(e, "versions message"))?; self.tls .write_all(&my_versions.encode_for_handshake()) .await @@ -185,7 +189,9 @@ impl .await .map_err(io_err_to_handshake)?; let mut reader = Reader::from_slice(&msg); - reader.extract()? + reader + .extract() + .map_err(|e| Error::from_bytes_err(e, "versions cell"))? }; trace!("{}: received {:?}", self.unique_id, their_versions); @@ -400,7 +406,11 @@ impl Unver // clock skew.) // Check the identity->signing cert - let (id_sk, id_sk_sig) = id_sk.check_key(&None)?.dangerously_split()?; + let (id_sk, id_sk_sig) = id_sk + .check_key(&None) + .map_err(Error::HandshakeCertErr)? + .dangerously_split() + .map_err(Error::HandshakeCertErr)?; sigs.push(&id_sk_sig); let (id_sk_timeliness, id_sk) = check_timeliness(id_sk, now, self.clock_skew); @@ -417,8 +427,10 @@ impl Unver // Now look at the signing->TLS cert and check it against the // peer certificate. let (sk_tls, sk_tls_sig) = sk_tls - .check_key(&Some(*signing_key))? // TODO(nickm): this is a bad interface - .dangerously_split()?; + .check_key(&Some(*signing_key)) + .map_err(Error::HandshakeCertErr)? + .dangerously_split() + .map_err(Error::HandshakeCertErr)?; sigs.push(&sk_tls_sig); let (sk_tls_timeliness, sk_tls) = check_timeliness(sk_tls, now, self.clock_skew); @@ -461,7 +473,8 @@ impl Unver let rsa_cert = c .cert_body(CertType::RSA_ID_V_IDENTITY) .ok_or_else(|| Error::HandshakeProto("No RSA->Ed crosscert".into()))?; - let rsa_cert = tor_cert::rsa::RsaCrosscert::decode(rsa_cert)? + let rsa_cert = tor_cert::rsa::RsaCrosscert::decode(rsa_cert) + .map_err(|e| Error::from_bytes_err(e, "RSA identity cross-certificate"))? .check_signature(&pkrsa) .map_err(|_| Error::HandshakeProto("Bad RSA->Ed crosscert signature".into()))?; let (rsa_cert_timeliness, rsa_cert) = check_timeliness(rsa_cert, now, self.clock_skew); @@ -685,14 +698,14 @@ pub(super) mod test { assert!(matches!(err, Error::HandshakeProto(_))); assert_eq!( format!("{}", err), - "handshake protocol violation: Doesn't seem to be a tor relay" + "Handshake protocol violation: Doesn't seem to be a tor relay" ); let err = connect_err(&hex!("0000 07 0004 1234 ffff")[..], rt.clone()).await; assert!(matches!(err, Error::HandshakeProto(_))); assert_eq!( format!("{}", err), - "handshake protocol violation: No shared link protocols" + "Handshake protocol violation: No shared link protocols" ); }); } @@ -721,7 +734,7 @@ pub(super) mod test { assert!(matches!(err, Error::HandshakeProto(_))); assert_eq!( format!("{}", err), - "handshake protocol violation: Duplicate certs cell" + "Handshake protocol violation: Duplicate certs cell" ); let mut buf = Vec::new(); @@ -734,7 +747,7 @@ pub(super) mod test { assert!(matches!(err, Error::HandshakeProto(_))); assert_eq!( format!("{}", err), - "handshake protocol violation: Duplicate authchallenge cell" + "Handshake protocol violation: Duplicate authchallenge cell" ); }); } @@ -749,7 +762,7 @@ pub(super) mod test { assert!(matches!(err, Error::HandshakeProto(_))); assert_eq!( format!("{}", err), - "handshake protocol violation: Missing certs cell" + "Handshake protocol violation: Missing certs cell" ); }); } @@ -765,7 +778,7 @@ pub(super) mod test { assert!(matches!(err, Error::HandshakeProto(_))); assert_eq!( format!("{}", err), - "handshake protocol violation: Unexpected cell type CREATE" + "Handshake protocol violation: Unexpected cell type CREATE" ); }); } @@ -844,7 +857,7 @@ pub(super) mod test { .unwrap(); assert_eq!( format!("{}", err), - "handshake protocol violation: Missing IDENTITY_V_SIGNING certificate" + "Handshake protocol violation: Missing IDENTITY_V_SIGNING certificate" ); } @@ -903,7 +916,7 @@ pub(super) mod test { assert_eq!( format!("{}", res), - format!("handshake protocol violation: {}", expect_err.unwrap()) + format!("Handshake protocol violation: {}", expect_err.unwrap()) ); } } @@ -929,7 +942,7 @@ pub(super) mod test { assert_eq!( format!("{}", err), - "handshake protocol violation: Peer ed25519 id not as expected" + "Handshake protocol violation: Peer ed25519 id not as expected" ); let err = certs_test( @@ -945,7 +958,7 @@ pub(super) mod test { assert_eq!( format!("{}", err), - "handshake protocol violation: Peer RSA id not as expected" + "Handshake protocol violation: Peer RSA id not as expected" ); let err = certs_test( @@ -961,7 +974,7 @@ pub(super) mod test { assert_eq!( format!("{}", err), - "handshake protocol violation: Peer cert did not authenticate TLS cert" + "Handshake protocol violation: Peer cert did not authenticate TLS cert" ); } @@ -991,7 +1004,7 @@ pub(super) mod test { assert_eq!( format!("{}", res), - "handshake protocol violation: Invalid ed25519 signature in handshake" + "Handshake protocol violation: Invalid ed25519 signature in handshake" ); let mut certs = msg::Certs::new_empty(); @@ -1012,7 +1025,7 @@ pub(super) mod test { assert_eq!( format!("{}", res), - "handshake protocol violation: Bad RSA->Ed crosscert signature" + "Handshake protocol violation: Bad RSA->Ed crosscert signature" ); } diff --git a/crates/tor-proto/src/channel/reactor.rs b/crates/tor-proto/src/channel/reactor.rs index b71278f0e..bd810c961 100644 --- a/crates/tor-proto/src/channel/reactor.rs +++ b/crates/tor-proto/src/channel/reactor.rs @@ -47,7 +47,8 @@ pub(super) type ReactorResultChannel = oneshot::Sender>; fn codec_err_to_chan(err: CodecError) -> Error { match err { CodecError::Io(e) => crate::Error::ChanIoErr(Arc::new(e)), - CodecError::Cell(e) => e.into(), + CodecError::EncCell(err) => Error::from_cell_enc(err, "channel cell"), + CodecError::DecCell(err) => Error::from_cell_dec(err, "channel cell"), } } @@ -438,7 +439,7 @@ pub(crate) mod test { let dummy_target = OwnedChanTarget::new(vec![], [6; 32].into(), [10; 20].into()); let send1 = send1.sink_map_err(|e| { trace!("got sink error: {}", e); - CodecError::Cell(tor_cell::Error::ChanProto("dummy message".into())) + CodecError::DecCell(tor_cell::Error::ChanProto("dummy message".into())) }); let (chan, reactor) = crate::channel::Channel::new( link_protocol, @@ -566,7 +567,7 @@ pub(crate) mod test { let (circ, _) = futures::join!(pending.create_firsthop_fast(&circparams), send_response); // Make sure statuses are as expected. - assert!(matches!(circ.err().unwrap(), Error::BadCircHandshake)); + assert!(matches!(circ.err().unwrap(), Error::BadCircHandshakeAuth)); reactor.run_once().await.unwrap(); @@ -604,13 +605,13 @@ pub(crate) mod test { let e = reactor.run_once().await.unwrap_err().unwrap_err(); assert_eq!( format!("{}", e), - "channel protocol violation: CREATE2 cell on client channel" + "Channel protocol violation: CREATE2 cell on client channel" ); let e = reactor.run_once().await.unwrap_err().unwrap_err(); assert_eq!( format!("{}", e), - "channel protocol violation: Unexpected CREATED* cell not on opening circuit" + "Channel protocol violation: Unexpected CREATED* cell not on opening circuit" ); // Can't get a relay cell on a circuit we've never heard of. @@ -622,7 +623,7 @@ pub(crate) mod test { let e = reactor.run_once().await.unwrap_err().unwrap_err(); assert_eq!( format!("{}", e), - "channel protocol violation: Relay cell on nonexistent circuit" + "Channel protocol violation: Relay cell on nonexistent circuit" ); // Can't get handshaking cells while channel is open. @@ -634,7 +635,7 @@ pub(crate) mod test { let e = reactor.run_once().await.unwrap_err().unwrap_err(); assert_eq!( format!("{}", e), - "channel protocol violation: VERSIONS cell after handshake is done" + "Channel protocol violation: VERSIONS cell after handshake is done" ); // We don't accept CREATED. @@ -646,7 +647,7 @@ pub(crate) mod test { let e = reactor.run_once().await.unwrap_err().unwrap_err(); assert_eq!( format!("{}", e), - "channel protocol violation: CREATED cell received, but we never send CREATEs" + "Channel protocol violation: CREATED cell received, but we never send CREATEs" ); }); } @@ -695,7 +696,7 @@ pub(crate) mod test { let e = reactor.run_once().await.unwrap_err().unwrap_err(); assert_eq!( format!("{}", e), - "channel protocol violation: Relay cell on pending circuit before CREATED* received" + "Channel protocol violation: Relay cell on pending circuit before CREATED* received" ); // If a relay cell is sent on a non-existent channel, that's an error. @@ -706,7 +707,7 @@ pub(crate) mod test { let e = reactor.run_once().await.unwrap_err().unwrap_err(); assert_eq!( format!("{}", e), - "channel protocol violation: Relay cell on nonexistent circuit" + "Channel protocol violation: Relay cell on nonexistent circuit" ); // It's fine to get a relay cell on a DestroySent channel: that happens @@ -729,7 +730,7 @@ pub(crate) mod test { let e = reactor.run_once().await.unwrap_err().unwrap_err(); assert_eq!( format!("{}", e), - "channel protocol violation: Too many cells received on destroyed circuit" + "Channel protocol violation: Too many cells received on destroyed circuit" ); }); } @@ -793,7 +794,7 @@ pub(crate) mod test { let e = reactor.run_once().await.unwrap_err().unwrap_err(); assert_eq!( format!("{}", e), - "channel protocol violation: Destroy for nonexistent circuit" + "Channel protocol violation: Destroy for nonexistent circuit" ); }); } diff --git a/crates/tor-proto/src/circuit.rs b/crates/tor-proto/src/circuit.rs index f01b7c6df..a27994a42 100644 --- a/crates/tor-proto/src/circuit.rs +++ b/crates/tor-proto/src/circuit.rs @@ -342,7 +342,8 @@ impl ClientCirc { let parameters = parameters.unwrap_or_default(); let begin_flags = parameters.begin_flags(); let optimistic = parameters.is_optimistic(); - let beginmsg = Begin::new(target, port, begin_flags)?; + let beginmsg = Begin::new(target, port, begin_flags) + .map_err(|e| Error::from_cell_enc(e, "begin message"))?; self.begin_data_stream(beginmsg.into(), optimistic).await } @@ -1151,7 +1152,7 @@ mod test { let extended2 = relaymsg::Extended2::new(vec![99; 256]).into(); let cc = rmsg_to_ccmsg(0, extended2); let error = bad_extend_test_impl(&rt, 2.into(), cc).await; - assert!(matches!(error, Error::BadCircHandshake)); + assert!(matches!(error, Error::BadCircHandshakeAuth)); }); } diff --git a/crates/tor-proto/src/circuit/halfstream.rs b/crates/tor-proto/src/circuit/halfstream.rs index 838357d75..7c70c947e 100644 --- a/crates/tor-proto/src/circuit/halfstream.rs +++ b/crates/tor-proto/src/circuit/halfstream.rs @@ -99,7 +99,7 @@ mod test { let e = hs.handle_msg(&m).err().unwrap(); assert_eq!( format!("{}", e), - "circuit protocol violation: Received a SENDME when none was expected" + "Circuit protocol violation: Received a SENDME when none was expected" ); Ok(()) } @@ -124,7 +124,7 @@ mod test { let e = hs.handle_msg(&m).err().unwrap(); assert_eq!( format!("{}", e), - "circuit protocol violation: Received a data cell in violation of a window" + "Circuit protocol violation: Received a data cell in violation of a window" ); } @@ -143,7 +143,7 @@ mod test { let e = hs.handle_msg(&m).err().unwrap(); assert_eq!( format!("{}", e), - "circuit protocol violation: Bad CONNECTED cell on a closed stream!" + "Circuit protocol violation: Bad CONNECTED cell on a closed stream!" ); } @@ -154,7 +154,7 @@ mod test { let e = hs.handle_msg(&m).err().unwrap(); assert_eq!( format!("{}", e), - "circuit protocol violation: Bad EXTENDED2 cell on a closed stream!" + "Circuit protocol violation: Bad EXTENDED2 cell on a closed stream!" ); } } diff --git a/crates/tor-proto/src/circuit/reactor.rs b/crates/tor-proto/src/circuit/reactor.rs index c5da96f8f..905441ec4 100644 --- a/crates/tor-proto/src/circuit/reactor.rs +++ b/crates/tor-proto/src/circuit/reactor.rs @@ -1006,7 +1006,10 @@ impl Reactor { return Ok(()); } } - let mut body: RelayCellBody = cell.encode(&mut rand::thread_rng())?.into(); + let mut body: RelayCellBody = cell + .encode(&mut rand::thread_rng()) + .map_err(|e| Error::from_cell_enc(e, "relay cell body"))? + .into(); let tag = self.crypto_out.encrypt(&mut body, hop)?; // NOTE(eta): Now that we've encrypted the cell, we *must* either send it or abort // the whole circuit (e.g. by returning an error). @@ -1242,7 +1245,8 @@ impl Reactor { tag_copy }; // Decode the cell. - let msg = RelayCell::decode(body.into())?; + let msg = + RelayCell::decode(body.into()).map_err(|e| Error::from_bytes_err(e, "relay cell"))?; let c_t_w = sendme::cell_counts_towards_windows(&msg); diff --git a/crates/tor-proto/src/crypto/handshake.rs b/crates/tor-proto/src/crypto/handshake.rs index 0af659efb..d8e1b926b 100644 --- a/crates/tor-proto/src/crypto/handshake.rs +++ b/crates/tor-proto/src/crypto/handshake.rs @@ -122,17 +122,17 @@ impl KeyGenerator for ShakeKeyGenerator { /// An error produced by a Relay's attempt to handle a client's onion handshake. #[derive(Clone, Debug, thiserror::Error)] pub(crate) enum RelayHandshakeError { - /// An error in parsing or encoding a handshake message. - #[error("problem in handshake format.")] + /// An error in parsing a handshake message. + #[error("Problem decoding onion handshake")] Fmt(#[from] tor_bytes::Error), /// The client asked for a key we didn't have. - #[error("unrecognized key or identity in handshake")] + #[error("Client asked for a key or ID that we don't have")] MissingKey, /// The client did something wrong with their handshake or cryptography. - #[error("bad handshake from client")] - BadHandshake, + #[error("Bad handshake from client")] + BadClientHandshake, /// An internal error. - #[error("internal error")] + #[error("Internal error")] Internal(#[from] tor_error::Bug), } diff --git a/crates/tor-proto/src/crypto/handshake/fast.rs b/crates/tor-proto/src/crypto/handshake/fast.rs index 07c31f052..aaaabef21 100644 --- a/crates/tor-proto/src/crypto/handshake/fast.rs +++ b/crates/tor-proto/src/crypto/handshake/fast.rs @@ -39,7 +39,7 @@ impl super::ClientHandshake for CreateFastClient { fn client2>(state: Self::StateType, msg: T) -> Result { let msg = msg.as_ref(); if msg.len() != FAST_S_HANDSHAKE_LEN { - return Err(Error::BadCircHandshake); + return Err(Error::BadCircHandshakeAuth); } let mut inp = Vec::new(); inp.extend(&state.0[..]); @@ -48,7 +48,7 @@ impl super::ClientHandshake for CreateFastClient { let kh_expect = LegacyKdf::new(0).derive(&inp[..], 20)?; if !bytes_eq(&kh_expect, &msg[20..40]) { - return Err(Error::BadCircHandshake); + return Err(Error::BadCircHandshakeAuth); } Ok(super::TapKeyGenerator::new(inp.into())) @@ -71,7 +71,7 @@ impl super::ServerHandshake for CreateFastServer { ) -> RelayHandshakeResult<(Self::KeyGen, Vec)> { let msg = msg.as_ref(); if msg.len() != FAST_C_HANDSHAKE_LEN { - return Err(RelayHandshakeError::BadHandshake); + return Err(RelayHandshakeError::BadClientHandshake); } let mut reply = vec![0_u8; FAST_S_HANDSHAKE_LEN]; rng.fill_bytes(&mut reply[0..20]); diff --git a/crates/tor-proto/src/crypto/handshake/hs_ntor.rs b/crates/tor-proto/src/crypto/handshake/hs_ntor.rs index 533c3ebeb..9ff583b4a 100644 --- a/crates/tor-proto/src/crypto/handshake/hs_ntor.rs +++ b/crates/tor-proto/src/crypto/handshake/hs_ntor.rs @@ -222,8 +222,12 @@ where { // Extract the public key of the service from the message let mut cur = Reader::from_slice(msg.as_ref()); - let Y: curve25519::PublicKey = cur.extract()?; - let mac_tag: MacTag = cur.extract()?; + let Y: curve25519::PublicKey = cur + .extract() + .map_err(|e| Error::from_bytes_err(e, "hs_ntor handshake"))?; + let mac_tag: MacTag = cur + .extract() + .map_err(|e| Error::from_bytes_err(e, "hs_ntor handshake"))?; // Get EXP(Y,x) and EXP(B,x) let xy = state.x.diffie_hellman(&Y); @@ -240,7 +244,7 @@ where // Validate the MAC! if my_mac_tag != mac_tag { - return Err(Error::BadCircHandshake); + return Err(Error::BadCircHandshakeAuth); } Ok(keygen) @@ -305,10 +309,17 @@ where { // Extract all the useful pieces from the message let mut cur = Reader::from_slice(msg.as_ref()); - let X: curve25519::PublicKey = cur.extract()?; + let X: curve25519::PublicKey = cur + .extract() + .map_err(|e| Error::from_bytes_err(e, "hs ntor handshake"))?; let remaining_bytes = cur.remaining(); - let ciphertext = &mut cur.take(remaining_bytes - 32)?.to_vec(); - let mac_tag: MacTag = cur.extract()?; + let ciphertext = &mut cur + .take(remaining_bytes - 32) + .map_err(|e| Error::from_bytes_err(e, "hs ntor handshake"))? + .to_vec(); + let mac_tag: MacTag = cur + .extract() + .map_err(|e| Error::from_bytes_err(e, "hs ntor handshake"))?; // Now derive keys needed for handling the INTRO1 cell let bx = proto_input.b.diffie_hellman(&X); @@ -328,7 +339,7 @@ where let my_mac_tag = hs_ntor_mac(&mac_body, &mac_key)?; if my_mac_tag != mac_tag { - return Err(Error::BadCircHandshake); + return Err(Error::BadCircHandshakeAuth); } // Decrypt the ENCRYPTED_DATA from the intro cell diff --git a/crates/tor-proto/src/crypto/handshake/ntor.rs b/crates/tor-proto/src/crypto/handshake/ntor.rs index b33be1658..74e14f332 100644 --- a/crates/tor-proto/src/crypto/handshake/ntor.rs +++ b/crates/tor-proto/src/crypto/handshake/ntor.rs @@ -169,8 +169,12 @@ where T: AsRef<[u8]>, { let mut cur = Reader::from_slice(msg.as_ref()); - let their_pk: PublicKey = cur.extract()?; - let auth: Authcode = cur.extract()?; + let their_pk: PublicKey = cur + .extract() + .map_err(|e| Error::from_bytes_err(e, "v3 ntor handshake"))?; + let auth: Authcode = cur + .extract() + .map_err(|e| Error::from_bytes_err(e, "v3 ntor handshake"))?; let xy = state.my_sk.diffie_hellman(&their_pk); let xb = state.my_sk.diffie_hellman(&state.relay_public.pk); @@ -185,7 +189,7 @@ where if okay.into() { Ok(keygen) } else { - Err(Error::BadCircHandshake) + Err(Error::BadCircHandshakeAuth) } } @@ -305,7 +309,7 @@ where if okay.into() { Ok((keygen, reply)) } else { - Err(RelayHandshakeError::BadHandshake) + Err(RelayHandshakeError::BadClientHandshake) } } diff --git a/crates/tor-proto/src/crypto/handshake/ntor_v3.rs b/crates/tor-proto/src/crypto/handshake/ntor_v3.rs index 9dc13dae8..4608038ac 100644 --- a/crates/tor-proto/src/crypto/handshake/ntor_v3.rs +++ b/crates/tor-proto/src/crypto/handshake/ntor_v3.rs @@ -468,7 +468,7 @@ fn server_handshake_ntor_v3_no_keygen( if okay.into() { Ok((reply, keystream)) } else { - Err(RelayHandshakeError::BadHandshake) + Err(RelayHandshakeError::BadClientHandshake) } } @@ -485,8 +485,12 @@ fn client_handshake_ntor_v3_part2( verification: &[u8], ) -> Result<(Vec, impl digest::XofReader)> { let mut reader = Reader::from_slice(relay_handshake); - let y_pk: curve25519::PublicKey = reader.extract()?; - let auth: DigestVal = reader.extract()?; + let y_pk: curve25519::PublicKey = reader + .extract() + .map_err(|e| Error::from_bytes_err(e, "v3 ntor handshake"))?; + let auth: DigestVal = reader + .extract() + .map_err(|e| Error::from_bytes_err(e, "v3 ntor handshake"))?; let encrypted_msg = reader.into_rest(); let yx = state.my_sk.diffie_hellman(&y_pk); @@ -540,7 +544,7 @@ fn client_handshake_ntor_v3_part2( if okay.into() { Ok((server_reply, keystream)) } else { - Err(Error::BadCircHandshake) + Err(Error::BadCircHandshakeAuth) } } diff --git a/crates/tor-proto/src/crypto/ll/kdf.rs b/crates/tor-proto/src/crypto/ll/kdf.rs index 70c10809e..98102ba8a 100644 --- a/crates/tor-proto/src/crypto/ll/kdf.rs +++ b/crates/tor-proto/src/crypto/ll/kdf.rs @@ -64,7 +64,7 @@ impl Kdf for LegacyKdf { let mut result = Zeroizing::new(Vec::with_capacity(n_bytes + Sha1::output_size())); let mut k = self.idx; if n_bytes > Sha1::output_size() * (256 - (k as usize)) { - return Err(Error::InvalidOutputLength); + return Err(Error::InvalidKDFOutputLength); } while result.len() < n_bytes { @@ -93,7 +93,7 @@ impl Kdf for Ntor1Kdf<'_, '_> { let mut result = Zeroizing::new(vec![0; n_bytes]); hkdf.expand(self.m_expand, &mut result[..]) - .map_err(|_| Error::InvalidOutputLength)?; + .map_err(|_| Error::InvalidKDFOutputLength)?; Ok(result) } } diff --git a/crates/tor-proto/src/util/err.rs b/crates/tor-proto/src/util/err.rs index f24834bac..9004cf5d1 100644 --- a/crates/tor-proto/src/util/err.rs +++ b/crates/tor-proto/src/util/err.rs @@ -14,90 +14,118 @@ use tor_error::{ErrorKind, HasKind}; pub enum Error { /// An error that occurred in the tor_bytes crate while decoding an /// object. - #[error("parsing error")] - BytesErr(#[from] tor_bytes::Error), + #[error("Unable to parse {object}")] + BytesErr { + /// What we were trying to parse. + object: &'static str, + /// The error that occurred while parsing it. + #[source] + err: tor_bytes::Error, + }, /// An error that occurred from the io system when using a /// channel. - #[error("io error on channel")] + #[error("IO error on channel with peer")] ChanIoErr(#[source] Arc), /// An error from the io system that occurred when trying to connect a channel. - #[error("io error in handshake")] + #[error("IO error while handshaking with peer")] HandshakeIoErr(#[source] Arc), - /// An error occurred in the cell-handling layer. - #[error("cell encoding error")] - CellErr(#[source] tor_cell::Error), - /// We tried to produce too much output for some function. - #[error("couldn't produce that much output")] - InvalidOutputLength, + /// An error occurred while trying to create or encode a cell. + #[error("Unable to generate or encode {object}")] + CellEncodeErr { + /// The object we were trying to create or encode. + object: &'static str, + /// The error that occurred. + #[source] + err: tor_cell::Error, + }, + /// An error occurred while trying to decode or parse a cell. + #[error("Error while parsing {object}")] + CellDecodeErr { + /// The object we were trying to decode. + object: &'static str, + /// The error that occurred. + #[source] + err: tor_cell::Error, + }, + /// We found a problem with one of the certificates in the channel + /// handshake. + #[error("Problem with certificate on handshake")] + HandshakeCertErr(#[source] tor_cert::CertError), + /// We tried to produce too much output for a key derivation function. + #[error("Tried to extract too many bytes from a KDF")] + InvalidKDFOutputLength, /// We tried to encrypt a message to a hop that wasn't there. - #[error("tried to encrypt to nonexistent hop")] + #[error("Tried to encrypt a cell for a nonexistent hop")] NoSuchHop, /// The authentication information on this cell was completely wrong, /// or the cell was corrupted. - #[error("bad relay cell authentication")] + #[error("Bad relay cell authentication")] BadCellAuth, - /// A circuit-extension handshake failed. - #[error("handshake failed")] - BadCircHandshake, + /// A circuit-extension handshake failed due to a mismatched authentication + /// value. + #[error("Circuit-extension handshake authentication failed")] + BadCircHandshakeAuth, /// Handshake protocol violation. - #[error("handshake protocol violation: {0}")] + #[error("Handshake protocol violation: {0}")] HandshakeProto(String), /// Handshake broken, maybe due to clock skew. /// /// (If the problem can't be due to clock skew, we return HandshakeProto /// instead.) - #[error("handshake failed due to expired certificates (possible clock skew)")] + #[error("Handshake failed due to expired certificates (possible clock skew)")] HandshakeCertsExpired { /// For how long has the circuit been expired? expired_by: Duration, }, /// Protocol violation at the channel level, other than at the handshake /// stage. - #[error("channel protocol violation: {0}")] + #[error("Channel protocol violation: {0}")] ChanProto(String), /// Protocol violation at the circuit level - #[error("circuit protocol violation: {0}")] + #[error("Circuit protocol violation: {0}")] CircProto(String), - /// Channel is closed. - #[error("{0}")] + /// Channel is closed, or became closed while we were trying to do some + /// operation. + #[error("Channel closed")] ChannelClosed(#[from] ChannelClosed), - /// Circuit is closed. - #[error("circuit closed")] + /// Circuit is closed, or became closed while we were trying to so some + /// operation. + #[error("Circuit closed")] CircuitClosed, /// Can't allocate any more circuit or stream IDs on a channel. - #[error("too many entries in map: can't allocate ID")] + #[error("Too many entries in map: can't allocate ID")] IdRangeFull, /// Couldn't extend a circuit because the extending relay or the /// target relay refused our request. - #[error("circuit extension handshake error: {0}")] + #[error("Circuit extension refused: {0}")] CircRefused(&'static str), /// Tried to make or use a stream to an invalid destination address. - #[error("invalid stream target address")] + #[error("Invalid stream target address")] BadStreamAddress, /// Received an End cell from the other end of a stream. - #[error("Received an End cell with reason {0}")] + #[error("Received an END cell with reason {0}")] EndReceived(EndReason), /// Stream was already closed when we tried to use it. - #[error("stream not connected")] + #[error("Stream not connected")] NotConnected, /// Stream protocol violation - #[error("stream protocol violation: {0}")] + #[error("Stream protocol violation: {0}")] StreamProto(String), /// Channel does not match target - #[error("channel mismatch: {0}")] + #[error("Peer identity mismatch: {0}")] ChanMismatch(String), /// There was a programming error somewhere in our code, or the calling code. #[error("Programming error")] Bug(#[from] tor_error::Bug), /// Remote DNS lookup failed. - #[error("remote resolve failed")] + #[error("Remote resolve failed")] ResolveError(#[source] ResolveError), } -/// Error which indicates that the channel was closed +/// Error which indicates that the channel was closed. #[derive(Error, Debug, Clone)] -#[error("channel closed")] +#[error("Channel closed")] pub struct ChannelClosed; impl HasKind for ChannelClosed { @@ -111,23 +139,37 @@ impl HasKind for ChannelClosed { #[non_exhaustive] pub enum ResolveError { /// A transient error which can be retried - #[error("received retriable transient error")] + #[error("Received retriable transient error")] Transient, /// A non transient error, which shouldn't be retried - #[error("received not retriable error")] + #[error("Received non-retriable error")] Nontransient, /// Could not parse the response properly - #[error("received unrecognized result")] + #[error("Received unrecognized result")] Unrecognized, } -impl From for Error { - fn from(err: tor_cell::Error) -> Error { +impl Error { + /// Create an error from a tor_cell error that has occurred while trying to + /// encode or create something of type `object` + pub(crate) fn from_cell_enc(err: tor_cell::Error, object: &'static str) -> Error { + Error::CellEncodeErr { object, err } + } + + /// Create an error from a tor_cell error that has occurred while trying to + /// decode something of type `object` + pub(crate) fn from_cell_dec(err: tor_cell::Error, object: &'static str) -> Error { match err { tor_cell::Error::ChanProto(msg) => Error::ChanProto(msg), - _ => Error::CellErr(err), + _ => Error::CellDecodeErr { err, object }, } } + + /// Create an error for a tor_bytes error that occurred while parsing + /// something of type `object`. + pub(crate) fn from_bytes_err(err: tor_bytes::Error, object: &'static str) -> Error { + Error::BytesErr { err, object } + } } impl From for std::io::Error { @@ -140,7 +182,7 @@ impl From for std::io::Error { Err(arc) => return std::io::Error::new(arc.kind(), arc), }, - InvalidOutputLength | NoSuchHop | BadStreamAddress => ErrorKind::InvalidInput, + InvalidKDFOutputLength | NoSuchHop | BadStreamAddress => ErrorKind::InvalidInput, NotConnected => ErrorKind::NotConnected, @@ -148,15 +190,17 @@ impl From for std::io::Error { CircuitClosed => ErrorKind::ConnectionReset, - BytesErr(_) + BytesErr { .. } | BadCellAuth - | BadCircHandshake + | BadCircHandshakeAuth | HandshakeProto(_) + | HandshakeCertErr(_) | ChanProto(_) | HandshakeCertsExpired { .. } | ChannelClosed(_) | CircProto(_) - | CellErr(_) + | CellDecodeErr { .. } + | CellEncodeErr { .. } | ChanMismatch(_) | StreamProto(_) => ErrorKind::InvalidData, @@ -174,15 +218,20 @@ impl HasKind for Error { use Error as E; use ErrorKind as EK; match self { - E::BytesErr(BytesError::Bug(e)) => e.kind(), - E::BytesErr(_) => EK::TorProtocolViolation, + E::BytesErr { + err: BytesError::Bug(e), + .. + } => e.kind(), + E::BytesErr { .. } => EK::TorProtocolViolation, E::ChanIoErr(_) => EK::LocalNetworkError, E::HandshakeIoErr(_) => EK::TorAccessFailed, - E::CellErr(e) => e.kind(), - E::InvalidOutputLength => EK::Internal, + E::HandshakeCertErr(_) => EK::TorProtocolViolation, + E::CellEncodeErr { err, .. } => err.kind(), + E::CellDecodeErr { err, .. } => err.kind(), + E::InvalidKDFOutputLength => EK::Internal, E::NoSuchHop => EK::BadApiUsage, E::BadCellAuth => EK::TorProtocolViolation, - E::BadCircHandshake => EK::TorProtocolViolation, + E::BadCircHandshakeAuth => EK::TorProtocolViolation, E::HandshakeProto(_) => EK::TorAccessFailed, E::HandshakeCertsExpired { .. } => EK::ClockSkew, E::ChanProto(_) => EK::TorProtocolViolation,