diff --git a/Cargo.lock b/Cargo.lock index 1ce4499c3..b45e2b7cc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3476,6 +3476,7 @@ dependencies = [ "serde", "thiserror", "tor-basic-utils", + "tor-cell", "tor-config", "tor-error", "tor-linkspec", diff --git a/crates/tor-chanmgr/Cargo.toml b/crates/tor-chanmgr/Cargo.toml index 1baa87d1e..9a06821ab 100644 --- a/crates/tor-chanmgr/Cargo.toml +++ b/crates/tor-chanmgr/Cargo.toml @@ -25,6 +25,7 @@ rand = "0.8" serde = { version = "1.0.103", features = ["derive"] } thiserror = "1" tor-basic-utils = { path = "../tor-basic-utils", version = "0.3.3" } +tor-cell = { path = "../tor-cell", version = "0.5.0" } tor-config = { path = "../tor-config", version = "0.5.0" } tor-error = { path = "../tor-error", version = "0.3.2" } tor-linkspec = { path = "../tor-linkspec", version = "0.4.0" } diff --git a/crates/tor-chanmgr/src/mgr/map.rs b/crates/tor-chanmgr/src/mgr/map.rs index aa24ffc73..b87c93cbe 100644 --- a/crates/tor-chanmgr/src/mgr/map.rs +++ b/crates/tor-chanmgr/src/mgr/map.rs @@ -8,6 +8,7 @@ use crate::{ChannelConfig, Dormancy, Error, Result}; use std::collections::{hash_map, HashMap}; use std::result::Result as StdResult; use std::sync::Arc; +use tor_cell::chancell::msg::PaddingNegotiate; use tor_config::PaddingLevel; use tor_error::{internal, into_internal}; use tor_netdir::{params::CHANNEL_PADDING_TIMEOUT_UPPER_BOUND, NetDir}; @@ -337,8 +338,6 @@ impl ChannelMap { netdir: tor_netdir::Result>, ) -> StdResult<(), tor_error::Bug> { use ChannelState as CS; - // TODO when entering/leaving dormant mode, send CELL_PADDING_NEGOTIATE to peers - // TODO with reduced padding, send CELL_PADDING_NEGOTIATE // TODO when we support operation as a relay, inter-relay channels ought // not to get padding. @@ -423,16 +422,61 @@ fn parameterize( dormancy: Dormancy, netdir: StdResult<&NetDirExtract, &()>, ) -> StdResult, tor_error::Bug> { - let padding_parameters = padding_parameters(config.padding, netdir)?; + // Everything in this calculation applies to *all* channels, disregarding + // channel usage. Usage is handled downstream, in the channel frontend. + // See the module doc in `crates/tor-proto/src/channel/padding.rs`. + + let padding_of_level = |level| padding_parameters(level, netdir); + let padding_parameters = padding_of_level(config.padding)?; + let padding_default = padding_of_level(PaddingLevel::default())?; let send_padding = (match dormancy { Dormancy::Active => true, Dormancy::Dormant => false, }) && padding_parameters != PaddingParameters::all_zeroes(); + let recv_padding = match config.padding { + PaddingLevel::Reduced => false, + PaddingLevel::Normal => send_padding, + PaddingLevel::None => false, + }; + + // Whether the inbound padding approach we are to use, is the same as the default + // derived from the netdir (disregarding our config and dormancy). + // + // Ie, whether the parameters we want are precisely those that a peer would + // use by default (assuming they have the same view of the netdir as us). + let recv_equals_default = if padding_default == PaddingParameters::all_zeroes() { + // The netdir has padding disabled by setting the parameters to zero. + // That means our peers won't do padding unless we ask it to. + // Or to put it another way, our desired peer padding approach is the same as our + // peers' default iff we also think padding should be disabled. + !recv_padding + } else { + // Peers are going to do padding. That is the same approach as we want + // if we want to receive padding, with the same parameters. + recv_padding && padding_parameters == padding_default + }; + + let padding_negotiate = if recv_equals_default { + // Our padding approach is the same as peers' defaults. So the PADDING_NEGOTIATE + // message we need to send is the START(0,0). (The channel frontend elides an + // initial message of this form, - see crates/tor-proto/src/channel.rs::note_usage.) + //p + // If the netdir default is no padding, and we previously negotiated + // padding being enabled, and now want to disable it, we would send + // START(0,0) rather than STOP. That is OK (even, arguably, right). + PaddingNegotiate::start_default() + } else if !recv_padding { + PaddingNegotiate::stop() + } else { + padding_parameters.padding_negotiate_cell()? + }; + let mut update = channels_params .start_update() - .padding_enable(send_padding); + .padding_enable(send_padding) + .padding_negotiate(padding_negotiate); if send_padding { update = update.padding_parameters(padding_parameters); } @@ -721,7 +765,8 @@ mod test { "ChannelsParamsUpdates { padding_enable: None, \ padding_parameters: Some(Parameters { \ low_ms: IntegerMilliseconds { value: 1500 }, \ - high_ms: IntegerMilliseconds { value: 9500 } }) }" + high_ms: IntegerMilliseconds { value: 9500 } }), \ + padding_negotiate: None }" ); }); eprintln!(); diff --git a/crates/tor-proto/src/channel.rs b/crates/tor-proto/src/channel.rs index df5033ad5..968ed99c9 100644 --- a/crates/tor-proto/src/channel.rs +++ b/crates/tor-proto/src/channel.rs @@ -75,7 +75,7 @@ use crate::{Error, Result}; use std::pin::Pin; use std::sync::{Mutex, MutexGuard}; use std::time::Duration; -use tor_cell::chancell::{msg, ChanCell, CircId}; +use tor_cell::chancell::{msg, msg::PaddingNegotiate, ChanCell, CircId}; use tor_error::internal; use tor_linkspec::{HasRelayIds, OwnedChanTarget}; use tor_rtcompat::SleepProvider; @@ -212,8 +212,6 @@ enum PaddingControlState { /// (which is fine since this timer is not enabled). /// * We don't send any PADDING_NEGOTIATE cells. The peer is supposed to come to the /// same conclusions as us, based on channel usage: it should also not send padding. - /// (Note: sending negotiation cells is not yet done at this point in the branch, - /// but it will be organised via `ChannelsParamsUpdates`.) #[educe(Default)] UsageDoesNotImplyPadding { /// The last padding parameters (from reparameterize) @@ -448,7 +446,13 @@ impl Channel { // Well, apparently the channel usage *does* imply padding now, // so we need to (belatedly) enable the timer, // send the padding negotiation cell, etc. - let params = params.clone(); + let mut params = params.clone(); + + // Except, maybe the padding we would be requesting is precisely default, + // so we wouldn't actually want to send that cell. + if params.padding_negotiate == Some(PaddingNegotiate::start_default()) { + params.padding_negotiate = None; + } match self.send_control(CtrlMsg::ConfigUpdate(Arc::new(params))) { Ok(()) => {} diff --git a/crates/tor-proto/src/channel/params.rs b/crates/tor-proto/src/channel/params.rs index 3987455ae..7177036e6 100644 --- a/crates/tor-proto/src/channel/params.rs +++ b/crates/tor-proto/src/channel/params.rs @@ -2,6 +2,8 @@ use educe::Educe; +use tor_cell::chancell::msg::PaddingNegotiate; + use super::padding; /// Generate most of the module: things which contain or process all params fields (or each one) @@ -133,6 +135,9 @@ define_channels_params_and_automatic_impls! { /// rather than the parameters changing, /// so the padding timer always keeps parameters, even when disabled. padding_parameters: padding::Parameters, + + /// Channel padding negotiation cell + padding_negotiate: PaddingNegotiate, } /// Placeholder function for saying whether to enable channel padding diff --git a/crates/tor-proto/src/channel/reactor.rs b/crates/tor-proto/src/channel/reactor.rs index cbd853432..bf9ed7cee 100644 --- a/crates/tor-proto/src/channel/reactor.rs +++ b/crates/tor-proto/src/channel/reactor.rs @@ -263,6 +263,7 @@ impl Reactor { // if one is added and we fail to handle it here. padding_enable, padding_parameters, + padding_negotiate, } = &*updates; if let Some(parameters) = padding_parameters { self.padding_timer.as_mut().reconfigure(parameters); @@ -274,6 +275,9 @@ impl Reactor { self.padding_timer.as_mut().disable(); } } + if let Some(padding_negotiate) = padding_negotiate { + self.special_outgoing.padding_negotiate = Some(padding_negotiate.clone()); + } } } Ok(())