From 667d8d34eb3e65071e9f0bbc93229331fc8cb42d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Fri, 27 Oct 2023 11:56:32 +0100 Subject: [PATCH] an option to set whether mixnode should enforce forward travel policy --- .../mixnode-common/src/forward_travel/mod.rs | 45 +++++++++++++------ mixnode/src/commands/init.rs | 20 ++++++--- mixnode/src/commands/mod.rs | 5 +++ mixnode/src/commands/run.rs | 22 +++++---- mixnode/src/config/mod.rs | 18 ++++++++ mixnode/src/config/old_config_v1_1_21.rs | 1 + mixnode/src/config/template.rs | 6 +++ mixnode/src/node/mod.rs | 9 +++- 8 files changed, 96 insertions(+), 30 deletions(-) diff --git a/common/mixnode-common/src/forward_travel/mod.rs b/common/mixnode-common/src/forward_travel/mod.rs index 9a805732f1..610591b4b0 100644 --- a/common/mixnode-common/src/forward_travel/mod.rs +++ b/common/mixnode-common/src/forward_travel/mod.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 use crate::forward_travel::error::ForwardTravelError; -use log::{debug, error, trace, warn}; +use log::{debug, error, info, trace, warn}; use nym_mixnet_contract_common::{EpochId, GatewayBond, Layer, MixNodeBond}; use nym_network_defaults::NymNetworkDetails; use nym_task::TaskClient; @@ -44,6 +44,7 @@ impl AllowedAddressesProvider { pub async fn new( identity: IdentityKey, nyxd_endpoints: Vec, + allow_all: bool, network_details: Option, ) -> Result { let network = network_details.unwrap_or(NymNetworkDetails::new_mainnet()); @@ -52,13 +53,15 @@ impl AllowedAddressesProvider { identity, client_config: nyxd::Config::try_from_nym_network_details(&network)?, nyxd_endpoints, - ingress: AllowedPaths::new(), - egress: AllowedPaths::new(), + ingress: AllowedPaths::new(allow_all), + egress: AllowedPaths::new(allow_all), }; - // set initial values for ingress/egress - let client = provider.ephemeral_nyxd_client()?; - provider.update_state(client).await?; + if !allow_all { + // set initial values for ingress/egress + let client = provider.ephemeral_nyxd_client()?; + provider.update_state(client).await?; + } Ok(provider) } @@ -89,15 +92,11 @@ impl AllowedAddressesProvider { } pub fn ingress(&self) -> AllowedIngress { - AllowedIngress { - inner: Arc::clone(&self.ingress.inner), - } + self.ingress.clone() } pub fn egress(&self) -> AllowedEgress { - AllowedEgress { - inner: Arc::clone(&self.egress.inner), - } + self.egress.clone() } fn add_node_ips(raw_host: &str, identity: &str, set: &mut HashSet) { @@ -269,6 +268,14 @@ impl AllowedAddressesProvider { } pub async fn run(&mut self, mut task_client: TaskClient) { + if self.ingress.allow_all { + debug_assert!(self.egress.allow_all); + + info!("the forward travel is currently disabled - there's no point in starting the route refresher"); + task_client.mark_as_success(); + return; + } + debug!("Started ValidAddressesProvider with graceful shutdown support"); while !task_client.is_shutdown() { tokio::select! { @@ -293,12 +300,16 @@ impl AllowedAddressesProvider { #[derive(Clone)] pub struct AllowedPaths { + // this is fine that this value is not wrapped in an Arc and is not atomic given + // it's not expected to be modified at runtime + allow_all: bool, inner: Arc>, } impl AllowedPaths { - fn new() -> Self { + fn new(allow_all: bool) -> Self { AllowedPaths { + allow_all, inner: Arc::new(RwLock::new(AllowedPathsInner { previous_epoch: HashSet::new(), current_epoch: HashSet::new(), @@ -307,11 +318,19 @@ impl AllowedPaths { } pub fn is_allowed(&self, address: IpAddr) -> bool { + if self.allow_all { + return true; + } + let guard = self.inner.read(); guard.current_epoch.contains(&address) || guard.previous_epoch.contains(&address) } fn advance_epoch(&self, current_epoch: HashSet, reset_previous: bool) { + // if this is triggered, it's an implementation bug; + // we shouldn't be updating data if we're allowing everything regardless + debug_assert!(!self.allow_all); + let mut guard = self.inner.write(); let old_current = mem::replace(&mut guard.current_epoch, current_epoch); diff --git a/mixnode/src/commands/init.rs b/mixnode/src/commands/init.rs index 3e678d3d4a..ac2b8a5b45 100644 --- a/mixnode/src/commands/init.rs +++ b/mixnode/src/commands/init.rs @@ -16,31 +16,36 @@ use std::{fs, io}; #[derive(Args, Clone)] pub(crate) struct Init { /// Id of the mixnode we want to create config for - #[clap(long)] + #[arg(long)] id: String, /// The host on which the mixnode will be running - #[clap(long)] + #[arg(long)] host: IpAddr, /// The port on which the mixnode will be listening for mix packets - #[clap(long)] + #[arg(long)] mix_port: Option, /// The port on which the mixnode will be listening for verloc packets - #[clap(long)] + #[arg(long)] verloc_port: Option, /// The port on which the mixnode will be listening for http requests - #[clap(long)] + #[arg(long)] http_api_port: Option, /// Comma separated list of nym-api endpoints of the validators // the alias here is included for backwards compatibility (1.1.4 and before) - #[clap(long, alias = "validators", value_delimiter = ',')] + #[arg(long, alias = "validators", value_delimiter = ',')] nym_apis: Option>, - #[clap(short, long, default_value_t = OutputFormat::default())] + /// Specifies whether this node should accepts and send out packets that would only go to nodes + /// on the next mix layer + #[arg(long)] + enforce_forward_travel: bool, + + #[arg(short, long, default_value_t = OutputFormat::default())] output: OutputFormat, } @@ -52,6 +57,7 @@ impl From for OverrideConfig { mix_port: init_config.mix_port, verloc_port: init_config.verloc_port, http_api_port: init_config.http_api_port, + enforce_forward_travel: Some(init_config.enforce_forward_travel), nym_apis: init_config.nym_apis, } } diff --git a/mixnode/src/commands/mod.rs b/mixnode/src/commands/mod.rs index dfa1fcbd42..17bbeb7739 100644 --- a/mixnode/src/commands/mod.rs +++ b/mixnode/src/commands/mod.rs @@ -58,6 +58,7 @@ struct OverrideConfig { mix_port: Option, verloc_port: Option, http_api_port: Option, + enforce_forward_travel: Option, nym_apis: Option>, } @@ -83,6 +84,10 @@ fn override_config(config: Config, args: OverrideConfig) -> Config { .with_optional(Config::with_mix_port, args.mix_port) .with_optional(Config::with_verloc_port, args.verloc_port) .with_optional(Config::with_http_api_port, args.http_api_port) + .with_optional( + Config::with_enforce_forward_travel, + args.enforce_forward_travel, + ) .with_optional_custom_env( Config::with_custom_nym_apis, args.nym_apis, diff --git a/mixnode/src/commands/run.rs b/mixnode/src/commands/run.rs index c4ab5125fb..e3d583e044 100644 --- a/mixnode/src/commands/run.rs +++ b/mixnode/src/commands/run.rs @@ -14,35 +14,40 @@ use std::net::IpAddr; #[derive(Args, Clone)] pub(crate) struct Run { /// Id of the nym-mixnode we want to run - #[clap(long)] + #[arg(long)] id: String, /// The custom host on which the mixnode will be running - #[clap(long)] + #[arg(long)] host: Option, /// The wallet address you will use to bond this mixnode, e.g. nymt1z9egw0knv47nmur0p8vk4rcx59h9gg4zuxrrr9 - #[clap(long)] + #[arg(long)] wallet_address: Option, /// The port on which the mixnode will be listening for mix packets - #[clap(long)] + #[arg(long)] mix_port: Option, /// The port on which the mixnode will be listening for verloc packets - #[clap(long)] + #[arg(long)] verloc_port: Option, /// The port on which the mixnode will be listening for http requests - #[clap(long)] + #[arg(long)] http_api_port: Option, /// Comma separated list of nym-api endpoints of the validators // the alias here is included for backwards compatibility (1.1.4 and before) - #[clap(long, alias = "validators", value_delimiter = ',')] + #[arg(long, alias = "validators", value_delimiter = ',')] nym_apis: Option>, - #[clap(short, long, default_value_t = OutputFormat::default())] + /// Specifies whether this node should accepts and send out packets that would only go to nodes + /// on the next mix layer + #[arg(long)] + enforce_forward_travel: Option, + + #[arg(short, long, default_value_t = OutputFormat::default())] output: OutputFormat, } @@ -54,6 +59,7 @@ impl From for OverrideConfig { mix_port: run_config.mix_port, verloc_port: run_config.verloc_port, http_api_port: run_config.http_api_port, + enforce_forward_travel: run_config.enforce_forward_travel, nym_apis: run_config.nym_apis, } } diff --git a/mixnode/src/config/mod.rs b/mixnode/src/config/mod.rs index b593d07745..ee892e9fb9 100644 --- a/mixnode/src/config/mod.rs +++ b/mixnode/src/config/mod.rs @@ -175,11 +175,13 @@ impl Config { } // builder methods + #[must_use] pub fn with_custom_nym_apis(mut self, nym_api_urls: Vec) -> Self { self.mixnode.nym_api_urls = nym_api_urls; self } + #[must_use] pub fn with_listening_address(mut self, listening_address: IpAddr) -> Self { self.mixnode.listening_address = listening_address; @@ -189,22 +191,31 @@ impl Config { self } + #[must_use] pub fn with_mix_port(mut self, port: u16) -> Self { self.mixnode.mix_port = port; self } + #[must_use] pub fn with_verloc_port(mut self, port: u16) -> Self { self.mixnode.verloc_port = port; self } + #[must_use] pub fn with_http_api_port(mut self, port: u16) -> Self { let http_ip = self.http.bind_address.ip(); self.http.bind_address = SocketAddr::new(http_ip, port); self } + #[must_use] + pub fn with_enforce_forward_travel(mut self, forward_travel: bool) -> Self { + self.debug.enforce_forward_travel = forward_travel; + self + } + pub fn get_nym_api_endpoints(&self) -> Vec { self.mixnode.nym_api_urls.clone() } @@ -323,6 +334,10 @@ pub struct Debug { /// Maximum number of packets that can be stored waiting to get sent to a particular connection. pub maximum_connection_buffer_size: usize, + /// Specifies whether this node should accepts and send out packets that would only go to nodes + /// on the next mix layer. + pub enforce_forward_travel: bool, + /// Specifies whether the mixnode should be using the legacy framing for the sphinx packets. // it's set to true by default. The reason for that decision is to preserve compatibility with the // existing nodes whilst everyone else is upgrading and getting the code for handling the new field. @@ -339,6 +354,9 @@ impl Default for Debug { packet_forwarding_maximum_backoff: DEFAULT_PACKET_FORWARDING_MAXIMUM_BACKOFF, initial_connection_timeout: DEFAULT_INITIAL_CONNECTION_TIMEOUT, maximum_connection_buffer_size: DEFAULT_MAXIMUM_CONNECTION_BUFFER_SIZE, + + // let's keep it disabled for now to not surprise operators/users + enforce_forward_travel: false, use_legacy_framed_packet_version: false, } } diff --git a/mixnode/src/config/old_config_v1_1_21.rs b/mixnode/src/config/old_config_v1_1_21.rs index b8f5252893..bbd84d4839 100644 --- a/mixnode/src/config/old_config_v1_1_21.rs +++ b/mixnode/src/config/old_config_v1_1_21.rs @@ -232,6 +232,7 @@ impl From for DebugV1_1_32 { initial_connection_timeout: value.initial_connection_timeout, maximum_connection_buffer_size: value.maximum_connection_buffer_size, use_legacy_framed_packet_version: value.use_legacy_framed_packet_version, + ..Default::default() } } } diff --git a/mixnode/src/config/template.rs b/mixnode/src/config/template.rs index 3a89329f8c..e1d8515c03 100644 --- a/mixnode/src/config/template.rs +++ b/mixnode/src/config/template.rs @@ -87,4 +87,10 @@ node_description = '{{ storage_paths.node_description }}' # TODO +[debug] + +# Specifies whether this node should accepts and send out packets that would only go to nodes +# on the next mix layer. +enforce_forward_travel = {{ debug.enforce_forward_travel }} + "#; diff --git a/mixnode/src/node/mod.rs b/mixnode/src/node/mod.rs index a728ef3fd6..ab19c66bc9 100644 --- a/mixnode/src/node/mod.rs +++ b/mixnode/src/node/mod.rs @@ -131,8 +131,13 @@ impl MixNode { let nyxd_endpoints = self.config.mixnode.nyxd_urls.clone(); let network = NymNetworkDetails::new_from_env(); - let mut provider = - AllowedAddressesProvider::new(identity, nyxd_endpoints, Some(network)).await?; + let mut provider = AllowedAddressesProvider::new( + identity, + nyxd_endpoints, + !self.config.debug.enforce_forward_travel, + Some(network), + ) + .await?; let filters = (provider.ingress(), provider.egress());