Commit 41a1872f authored by opara's avatar opara 🙃
Browse files

Merge branch 'destroy' into 'main' 

Always use destroy reason NONE in circuit handshake code

Closes #2466

See merge request !4088
parents f4054647 f26f6521

crates/tor-cell/src/chancell/msg.rs

+7 −0
Original line number Diff line number Diff line
@@ -475,6 +475,13 @@ pub struct Destroy { 
}

impl Destroy {

    /// Create a new destroy cell.

    ///

    /// If generating a new destroy cell,

    /// you should always create it with [`DestroyReason::NONE`].

    ///

    /// `tor-spec/tearing-down-circuits.md`:

    ///

    /// > Implementations SHOULD always use the NONE reason to avoid side channels: [...]

    pub fn new(reason: DestroyReason) -> Self {

        Destroy { reason }

    }

crates/tor-proto/src/crypto/handshake.rs

+0 −14
Original line number Diff line number Diff line
@@ -22,7 +22,6 @@ use crate::Result; 
//use zeroize::Zeroizing;

use rand_core::{CryptoRng, Rng};

use tor_bytes::SecretBuf;

use tor_cell::chancell::msg::DestroyReason;

use tor_error::{ErrorKind, HasKind};



/// A ClientHandshake is used to generate a client onionskin and
@@ -185,19 +184,6 @@ pub(crate) enum RelayHandshakeError { 
    Internal(#[from] tor_error::Bug),

}



impl RelayHandshakeError {

    /// The reason to use in a DESTROY message for this failure.

    pub(crate) fn destroy_reason(&self) -> DestroyReason {

        match self {

            Self::Fmt(_) => DestroyReason::PROTOCOL,

            // TODO(relay): Is this right?

            Self::MissingKey => DestroyReason::OR_IDENTITY,

            Self::BadClientHandshake => DestroyReason::PROTOCOL,

            Self::Internal(_) => DestroyReason::INTERNAL,

        }

    }

}



impl HasKind for RelayHandshakeError {

    fn kind(&self) -> ErrorKind {

        match self {

crates/tor-proto/src/relay/channel/create_handler.rs

+5 −15
Original line number Diff line number Diff line
@@ -114,7 +114,11 @@ impl CreateRequestHandler { 
                // TODO(relay): The log messages throughout could be very noisy, so should have rate limiting.

                let cmd = msg.cmd();

                debug_report!(&e, %cmd, "Failed to handle circuit create request");

                Err(Destroy::new(e.destroy_reason()))



                // `tor-spec/tearing-down-circuits.md`:

                //

                // > Implementations SHOULD always use the NONE reason to avoid side channels: [...]

                Err(Destroy::new(DestroyReason::NONE))

            }

        }

    }
@@ -268,20 +272,6 @@ enum HandleCreateError { 
    Internal(#[from] tor_error::Bug),

}



impl HandleCreateError {

    /// The reason to use in a DESTROY message for this failure.

    fn destroy_reason(&self) -> DestroyReason {

        // Note that this may return an INTERNAL destroy reason even when

        // the inner error is not `ErrorKind::Internal`.

        match self {

            Self::Handshake(e) => e.destroy_reason(),

            Self::Memquota(_) => DestroyReason::INTERNAL,

            Self::Spawn(_) => DestroyReason::INTERNAL,

            Self::Internal(_) => DestroyReason::INTERNAL,

        }

    }

}



impl HasKind for HandleCreateError {

    fn kind(&self) -> ErrorKind {

        match self {