vincenzopalazzo
/
arti
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge branch 'error_cleanup_2' into 'main' 

Error refactoring: bytes, cert, proto.

See merge request tpo/core/arti!604
This commit is contained in:
eta 2022-06-24 13:01:42 +00:00
parent ea3160737c 38004a4f4d
commit ec3e1f2ada
24 changed files with 303 additions and 162 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Cargo.lock generated
View File

@ -3492,6 +3492,7 @@ dependencies = [
"digest 0.10.3",
"hex-literal",
"signature",
"thiserror",
"tor-bytes",
"tor-checkable",
"tor-llcrypto",

16
crates/tor-bytes/src/err.rs
View File

@ -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,
}

2
crates/tor-bytes/src/lib.rs
View File

@ -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;

8
crates/tor-bytes/src/writer.rs
View File

@ -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);
}
}

1
crates/tor-cert/Cargo.toml
View File

@ -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" }

1
crates/tor-cert/semver.md Normal file
View File

@ -0,0 +1 @@
BREAKING: No longer using tor_bytes::Error for non-decoding errors

24
crates/tor-cert/src/err.rs Normal file
View File

@ -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,
}

54
crates/tor-cert/src/lib.rs
View File

@ -83,16 +83,22 @@
#![allow(clippy::let_unit_value)] // This can reasonably be done for explicitness
//! <!-- @@ end lint list maintained by maint/add_warning @@ -->
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<T> = std::result::Result<T, CertError>;
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<Self> {
fn from_reader(key_type: KeyType, r: &mut Reader<'_>) -> BytesResult<Self> {
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<Self> {
fn take_from(b: &mut Reader<'_>) -> BytesResult<Self> {
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<KeyUnknownCert> {
pub fn decode(cert: &[u8]) -> BytesResult<KeyUnknownCert> {
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<ed25519::PublicKey>) -> Result<UncheckedCert> {
pub fn check_key(self, pkey: &Option<ed25519::PublicKey>) -> CertResult<UncheckedCert> {
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<SigCheckedCert> 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<CertExt> = r.extract();
let e: Result<CertExt, BytesError> = 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);

5
crates/tor-cert/tests/invalid_certs.rs
View File

@ -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
);
}

20
crates/tor-netdoc/src/doc/routerdesc.rs
View File

@ -405,7 +405,8 @@ impl RouterDesc {
.parse_obj::<UnvalidatedEdCert>("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<Box<dyn ll::pk::ValidatableSignature>> = vec![
Box::new(rsa_signature),
Box::new(ed_signature),

3
crates/tor-netdoc/src/err.rs
View File

@ -291,6 +291,9 @@ pub(crate) enum ParseErrorSource {
/// An error when validating a signature.
#[error("Invalid signature")]
Signature(#[source] Arc<signature::Error>),
/// 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),

15
crates/tor-proto/src/channel/codec.rs
View File

@ -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<Option<Self::Item>, Self::Error> {
Ok(self.0.decode_cell(src)?)
self.0.decode_cell(src).map_err(CodecError::DecCell)
}
}

53
crates/tor-proto/src/channel/handshake.rs
View File

@ -112,7 +112,10 @@ pub struct VerifiedChannel<T: AsyncRead + AsyncWrite + Send + Unpin + 'static, S
fn codec_err_to_handshake(err: CodecError) -> 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<T: AsyncRead + AsyncWrite + Send + Unpin + 'static, S: SleepProvider>
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<T: AsyncRead + AsyncWrite + Send + Unpin + 'static, S: SleepProvider>
.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<T: AsyncRead + AsyncWrite + Send + Unpin + 'static, S: SleepProvider> 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<T: AsyncRead + AsyncWrite + Send + Unpin + 'static, S: SleepProvider> 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<T: AsyncRead + AsyncWrite + Send + Unpin + 'static, S: SleepProvider> 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"
);
}

25
crates/tor-proto/src/channel/reactor.rs
View File

@ -47,7 +47,8 @@ pub(super) type ReactorResultChannel<T> = oneshot::Sender<Result<T>>;
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"
);
});
}

5
crates/tor-proto/src/circuit.rs
View File

@ -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));
});
}

8
crates/tor-proto/src/circuit/halfstream.rs
View File

@ -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!"
);
}
}

8
crates/tor-proto/src/circuit/reactor.rs
View File

@ -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);

12
crates/tor-proto/src/crypto/handshake.rs
View File

@ -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),
}

6
crates/tor-proto/src/crypto/handshake/fast.rs
View File

@ -39,7 +39,7 @@ impl super::ClientHandshake for CreateFastClient {
fn client2<T: AsRef<[u8]>>(state: Self::StateType, msg: T) -> Result<Self::KeyGen> {
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<u8>)> {
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]);

25
crates/tor-proto/src/crypto/handshake/hs_ntor.rs
View File

@ -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

12
crates/tor-proto/src/crypto/handshake/ntor.rs
View File

@ -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)
}
}

12
crates/tor-proto/src/crypto/handshake/ntor_v3.rs
View File

@ -468,7 +468,7 @@ fn server_handshake_ntor_v3_no_keygen<REPLY: MsgReply>(
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<u8>, 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)
}
}

4
crates/tor-proto/src/crypto/ll/kdf.rs
View File

@ -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)
}
}

145
crates/tor-proto/src/util/err.rs
View File

@ -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<std::io::Error>),
/// 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<std::io::Error>),
/// 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<tor_cell::Error> 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<Error> for std::io::Error {
@ -140,7 +182,7 @@ impl From<Error> 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<Error> 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,