From 8ddef08c72b0047dc8a001e2072cfbec88d0ac3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jon=20H=C3=A4ggblad?= Date: Thu, 6 Mar 2025 10:09:15 +0100 Subject: [PATCH] Tweak surb management to be more conservative (#5570) To reduce the risk of the IPR DoS the client: - Lower the timeout until the IPR will disconnect a client - Reduce fewer surbs at a time. Large surb requests increases the latency until all fragments in the response have been delivered. The efficiency gains of having large surb requests dimishes quickly for large sizes as well --- common/client-core/config-types/src/lib.rs | 2 +- .../src/client/real_messages_control/message_handler.rs | 6 ++++-- .../client-core/src/client/replies/reply_controller/mod.rs | 2 +- service-providers/ip-packet-router/src/constants.rs | 4 ++-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/common/client-core/config-types/src/lib.rs b/common/client-core/config-types/src/lib.rs index 8eb95d62f1..82e93f9b09 100644 --- a/common/client-core/config-types/src/lib.rs +++ b/common/client-core/config-types/src/lib.rs @@ -50,7 +50,7 @@ const DEFAULT_MINIMUM_REPLY_SURB_THRESHOLD_BUFFER: usize = 0; // define how much to request at once // clients/client-core/src/client/replies/reply_controller.rs const DEFAULT_MINIMUM_REPLY_SURB_REQUEST_SIZE: u32 = 10; -const DEFAULT_MAXIMUM_REPLY_SURB_REQUEST_SIZE: u32 = 100; +const DEFAULT_MAXIMUM_REPLY_SURB_REQUEST_SIZE: u32 = 30; const DEFAULT_MAXIMUM_ALLOWED_SURB_REQUEST_SIZE: u32 = 500; diff --git a/common/client-core/src/client/real_messages_control/message_handler.rs b/common/client-core/src/client/real_messages_control/message_handler.rs index b8417cead9..e0d0884e96 100644 --- a/common/client-core/src/client/real_messages_control/message_handler.rs +++ b/common/client-core/src/client/real_messages_control/message_handler.rs @@ -33,10 +33,12 @@ pub enum PreparationError { #[error(transparent)] NymTopologyError(#[from] NymTopologyError), - #[error("The received message cannot be sent using a single reply surb. It ended up getting split into {fragments} fragments.")] + #[error("message too long for a single SURB, splitting into {fragments} fragments.")] MessageTooLongForSingleSurb { fragments: usize }, - #[error("Not enough reply SURBs to send the message. We have {available} available and require at least {required}.")] + #[error( + "not enough reply SURBs to send the message, available: {available} required: {required}." + )] NotEnoughSurbs { available: usize, required: usize }, } diff --git a/common/client-core/src/client/replies/reply_controller/mod.rs b/common/client-core/src/client/replies/reply_controller/mod.rs index fb8cdc468b..7b59359407 100644 --- a/common/client-core/src/client/replies/reply_controller/mod.rs +++ b/common/client-core/src/client/replies/reply_controller/mod.rs @@ -746,7 +746,7 @@ where .request_additional_reply_surbs(target, request_size) .await { - warn!("failed to request additional surbs... - {err}") + info!("{err}") } } diff --git a/service-providers/ip-packet-router/src/constants.rs b/service-providers/ip-packet-router/src/constants.rs index 128afa271b..a707199977 100644 --- a/service-providers/ip-packet-router/src/constants.rs +++ b/service-providers/ip-packet-router/src/constants.rs @@ -7,8 +7,8 @@ use std::time::Duration; pub(crate) const DISCONNECT_TIMER_INTERVAL: Duration = Duration::from_secs(10); // We consider a client inactive if it hasn't sent any mixnet packets in this duration -pub(crate) const CLIENT_MIXNET_INACTIVITY_TIMEOUT: Duration = Duration::from_secs(5 * 60); +pub(crate) const CLIENT_MIXNET_INACTIVITY_TIMEOUT: Duration = Duration::from_secs(60); // We consider a client handler inactive if it hasn't received any packets from the tun device in // this duration -pub(crate) const CLIENT_HANDLER_ACTIVITY_TIMEOUT: Duration = Duration::from_secs(10 * 60); +pub(crate) const CLIENT_HANDLER_ACTIVITY_TIMEOUT: Duration = Duration::from_secs(5 * 60);