From 18a8e307fbb323fd2f8182decbb17bd0a67006cb Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Wed, 21 Oct 2026 01:19:21 +0700 Subject: [PATCH] feat(app): allow hexadecimal HMAC key specification and redact keys from debug output --- Cargo.lock | 1 + crates/irtt-app/Cargo.toml | 1 + crates/irtt-app/src/cmd/client/args.rs | 2 +- crates/irtt-app/src/cmd/server/args.rs | 40 ++++++++++++-------- crates/irtt-app/src/cmd/server/run.rs | 2 +- crates/irtt-app/src/cmd/tui/args.rs | 2 +- crates/irtt-app/src/shared/client/args.rs | 12 ++---- crates/irtt-app/src/shared/client/config.rs | 12 +++--- crates/irtt-app/src/shared/client/prepare.rs | 2 +- crates/irtt-app/src/shared/client/targets.rs | 9 ++++- crates/irtt-app/src/shared/hmac.rs | 14 +++++++ crates/irtt-app/src/shared/mod.rs | 5 +++ docs/irtt-client.md | 5 ++- docs/irtt-server.md | 5 ++- 14 files changed, 72 insertions(+), 40 deletions(-) create mode 100644 crates/irtt-app/src/shared/hmac.rs diff --git a/Cargo.lock b/Cargo.lock index cc459e3..1f40985 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -832,6 +832,7 @@ dependencies = [ "crossterm", "ctrlc", "futures-core", + "hex", "irtt-client", "irtt-proto", "irtt-server", diff --git a/crates/irtt-app/Cargo.toml b/crates/irtt-app/Cargo.toml index 485647f..df09532 100644 --- a/crates/irtt-app/Cargo.toml +++ b/crates/irtt-app/Cargo.toml @@ -52,6 +52,7 @@ clap = { version = "4.6.7", features = ["derive", "env"] } clap-num = "=1.2.0" crossterm = { version = "0.29.0", features = ["event-stream"], optional = true } futures-core = { version = "0.3.34", default-features = false, optional = true } +hex = "0.4.3" ctrlc = { version = "3.5.2", features = ["termination"] } irtt-client = { workspace = true, optional = true } irtt-proto = { workspace = true, optional = true } diff --git a/crates/irtt-app/src/cmd/client/args.rs b/crates/irtt-app/src/cmd/client/args.rs index 846cc2a..71e121b 100644 --- a/crates/irtt-app/src/cmd/client/args.rs +++ b/crates/irtt-app/src/cmd/client/args.rs @@ -16,7 +16,7 @@ pub struct ClientArgs { value_name = "TARGET", num_args = 0.., value_parser = parse_target, - long_help = "Server address/host, optionally prefixed with LABEL= and suffixed with @hmac=KEY. Repeat for multi-target mode.\n\nExamples:\n irtt-client host-a:2112 host-b:2112\n irtt-client eu=host-a:2112 us=host-b:2112\n irtt-client ams=host-a:2112@hmac=secret public=host-b:2112@hmac=\n\nA target without @hmac= inherits --hmac. @hmac= explicitly disables it for that target." + long_help = "Server address/host, optionally prefixed with LABEL= and suffixed with @hmac=KEY. Repeat for multi-target mode. Keys use literal UTF-8 or lowercase 0x followed by hexadecimal bytes.\n\nExamples:\n irtt-client host-a:2112 host-b:2112\n irtt-client eu=host-a:2112 us=host-b:2112\n irtt-client ams=host-a:2112@hmac=secret public=host-b:2112@hmac=\n\nA target without @hmac= inherits --hmac. @hmac= explicitly disables it for that target." )] pub targets: Vec, diff --git a/crates/irtt-app/src/cmd/server/args.rs b/crates/irtt-app/src/cmd/server/args.rs index 3e16a5b..55fc52d 100644 --- a/crates/irtt-app/src/cmd/server/args.rs +++ b/crates/irtt-app/src/cmd/server/args.rs @@ -12,7 +12,7 @@ const DEFAULT_PORT: u16 = 2112; // Every policy option is optional. An omitted option leaves the corresponding // `ServerConfig` default in place rather than restating it here, so library and // CLI defaults cannot drift apart. -#[derive(Debug, Clone, Parser)] +#[derive(Clone, Parser)] #[command( name = "irtt-server", about = "Minimal IRTT-compatible UDP server", @@ -28,13 +28,14 @@ pub struct ServerArgs { )] pub bind: Vec, + // Decode during configuration preparation so Clap cannot echo rejected keys. /// HMAC key; every request must then carry a valid MAC. #[arg( long, value_name = "KEY", - value_parser = parse_hmac_key, env = "IRTT_SERVER_HMAC", - long_help = "HMAC key, taken as the UTF-8 bytes of this argument.\n\nAuthentication is global: with a key configured, every request must carry a valid MAC and every reply is authenticated. Without one, authenticated requests are dropped. The key is visible in the process arguments, or in the environment as IRTT_SERVER_HMAC." + hide_env_values = true, + long_help = "HMAC key: literal UTF-8 or lowercase 0x followed by hexadecimal bytes (digits may use either case). Empty keys and invalid hex are rejected.\n\nAuthentication is global: with a key configured, every request must carry a valid MAC and every reply is authenticated. Without one, authenticated requests are dropped. The key is visible in the process arguments, or in the environment as IRTT_SERVER_HMAC." )] pub hmac: Option, @@ -126,6 +127,23 @@ pub struct ServerArgs { pub no_dscp: bool, } +impl std::fmt::Debug for ServerArgs { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ServerArgs") + .field("hmac", &self.hmac.as_ref().map(|_| "[REDACTED]")) + .field("bind", &self.bind) + .field("max_sessions", &self.max_sessions) + .field("max_packet_length", &self.max_packet_length) + .field("min_interval", &self.min_interval) + .field("burst", &self.burst) + .field("idle_timeout", &self.idle_timeout) + .field("max_duration", &self.max_duration) + .field("timestamp_allowance", &self.timestamp_allowance) + .field("no_dscp", &self.no_dscp) + .finish() + } +} + /// The command-line spelling of [`TimestampAllowance`]. /// /// A CLI-local enum rather than a `clap` derive on the library type: `irtt-server` @@ -166,11 +184,10 @@ impl ServerArgs { } /// Builds the server configuration this invocation asks for. - #[must_use] - pub fn server_config(&self) -> ServerConfig { + pub fn server_config(&self) -> Result { let mut config = ServerConfig::default(); if let Some(key) = self.hmac.as_ref() { - config = config.with_hmac_key(key.as_bytes()); + config = config.with_hmac_key(crate::shared::decode_hmac_key(key)?); } if let Some(value) = self.max_sessions { config = config.with_max_sessions(value); @@ -198,19 +215,10 @@ impl ServerArgs { if self.no_dscp { config = config.with_dscp_allowed(false); } - config + Ok(config) } } -/// Rejects an empty HMAC key so a blank shell expansion cannot silently -/// enable authentication with predictable, empty key material. -fn parse_hmac_key(input: &str) -> Result { - if input.is_empty() { - return Err("HMAC key must not be empty".to_owned()); - } - Ok(input.to_owned()) -} - /// Parses a duration whose zero is meaningful server policy. fn parse_duration_allow_zero(input: &str) -> Result { if input == "0" { diff --git a/crates/irtt-app/src/cmd/server/run.rs b/crates/irtt-app/src/cmd/server/run.rs index 442330e..55f9541 100644 --- a/crates/irtt-app/src/cmd/server/run.rs +++ b/crates/irtt-app/src/cmd/server/run.rs @@ -36,7 +36,7 @@ async fn serve( args: ServerArgs, shutdown_requested: &AtomicBool, ) -> Result<(), Box> { - let config = args.server_config(); + let config = args.server_config()?; let binds = args.resolve_binds(); let is_implicit_default = args.bind.is_empty(); let set = match ServerSet::bind(binds.clone(), config.clone()).await { diff --git a/crates/irtt-app/src/cmd/tui/args.rs b/crates/irtt-app/src/cmd/tui/args.rs index cccc668..a99675f 100644 --- a/crates/irtt-app/src/cmd/tui/args.rs +++ b/crates/irtt-app/src/cmd/tui/args.rs @@ -18,7 +18,7 @@ pub struct TuiArgs { num_args = 1.., required = true, value_parser = parse_target, - long_help = "Server address/host, optionally prefixed with LABEL= and suffixed with @hmac=KEY. Repeat for multi-target mode.\n\nExplicit labels are used in the legend and status table. A target without @hmac= inherits --hmac; @hmac= disables it for that target.\n\nExamples:\n irtt-tui host.example\n irtt-tui eu=host.example\n irtt-tui eu=host-a.example@hmac=secret us=host-b.example" + long_help = "Server address/host, optionally prefixed with LABEL= and suffixed with @hmac=KEY. Repeat for multi-target mode. Keys use literal UTF-8 or lowercase 0x followed by hexadecimal bytes.\n\nExplicit labels are used in the legend and status table. A target without @hmac= inherits --hmac; @hmac= disables it for that target.\n\nExamples:\n irtt-tui host.example\n irtt-tui eu=host.example\n irtt-tui eu=host-a.example@hmac=secret us=host-b.example" )] pub targets: Vec, diff --git a/crates/irtt-app/src/shared/client/args.rs b/crates/irtt-app/src/shared/client/args.rs index 77de651..cbdc8fe 100644 --- a/crates/irtt-app/src/shared/client/args.rs +++ b/crates/irtt-app/src/shared/client/args.rs @@ -26,8 +26,9 @@ pub struct CommonClientArgs { #[arg(long, default_value_t = 0, value_parser = parse_length)] pub length: u32, - /// HMAC key. - #[arg(long, value_parser = parse_hmac_key)] + // Decode during configuration preparation so Clap cannot echo rejected keys. + /// HMAC key: literal UTF-8 or lowercase 0x followed by hexadecimal bytes. + #[arg(long)] pub hmac: Option, /// Clock mode to request. @@ -267,13 +268,6 @@ pub fn parse_length(input: &str) -> Result { Ok(length) } -pub fn parse_hmac_key(input: &str) -> Result { - if input.is_empty() { - return Err("HMAC key must not be empty".to_owned()); - } - Ok(input.to_owned()) -} - pub fn parse_server_fill(input: &str) -> Result { if input.is_empty() { return Err("server fill must not be empty".to_owned()); diff --git a/crates/irtt-app/src/shared/client/config.rs b/crates/irtt-app/src/shared/client/config.rs index 62b7532..be01f26 100644 --- a/crates/irtt-app/src/shared/client/config.rs +++ b/crates/irtt-app/src/shared/client/config.rs @@ -21,8 +21,8 @@ impl CommonClientArgs { /// Build the shared client configuration template for a managed run. /// /// The config is endpoint-independent; managed targets own their addresses. - pub fn to_client_config(&self, duration: Duration) -> ClientConfig { - ClientConfig { + pub fn to_client_config(&self, duration: Duration) -> Result { + Ok(ClientConfig { address_family: self.address_family(), open: OpenPolicy { negotiation: if self.loose { @@ -45,9 +45,11 @@ impl CommonClientArgs { }, auth: self .hmac - .as_ref() + .as_deref() + .map(crate::shared::decode_hmac_key) + .transpose()? .map_or(Authentication::Unauthenticated, |key| { - Authentication::Hmac(HmacKey::new(key.as_bytes())) + Authentication::Hmac(HmacKey::new(key)) }), socket: SocketConfig { bind_addr: self.bind_addr, @@ -61,7 +63,7 @@ impl CommonClientArgs { ..SocketConfig::default() }, ..ClientConfig::default() - } + }) } } diff --git a/crates/irtt-app/src/shared/client/prepare.rs b/crates/irtt-app/src/shared/client/prepare.rs index 1d598b0..7474207 100644 --- a/crates/irtt-app/src/shared/client/prepare.rs +++ b/crates/irtt-app/src/shared/client/prepare.rs @@ -125,7 +125,7 @@ pub async fn prepare_managed_run( .await?; Ok(ManagedRunSetup { targets, - client: common.to_client_config(duration), + client: common.to_client_config(duration)?, pacing: selection.pacing.into(), stdin_controlled: selection.stdin_controlled, preparation, diff --git a/crates/irtt-app/src/shared/client/targets.rs b/crates/irtt-app/src/shared/client/targets.rs index eaa9675..377e7b5 100644 --- a/crates/irtt-app/src/shared/client/targets.rs +++ b/crates/irtt-app/src/shared/client/targets.rs @@ -88,12 +88,13 @@ impl From for ManagedPacing { } } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] enum TargetParseError { EmptyLabel, EmptyAddress, InvalidEscape, EmptyTarget, + InvalidHmac(String), } impl fmt::Display for TargetParseError { @@ -103,6 +104,7 @@ impl fmt::Display for TargetParseError { Self::EmptyAddress => "target address must not be empty", Self::InvalidEscape => "invalid target escape sequence", Self::EmptyTarget => "target must not be empty", + Self::InvalidHmac(message) => message, }; f.write_str(message) } @@ -197,7 +199,10 @@ fn parse_target_syntax( let auth = if value.is_empty() { TargetAuth::Override(Authentication::Unauthenticated) } else { - TargetAuth::Override(Authentication::Hmac(HmacKey::new(value.into_bytes()))) + TargetAuth::Override(Authentication::Hmac(HmacKey::new( + crate::shared::decode_hmac_key(&value) + .map_err(TargetParseError::InvalidHmac)?, + ))) }; (&input[..index], auth) } diff --git a/crates/irtt-app/src/shared/hmac.rs b/crates/irtt-app/src/shared/hmac.rs new file mode 100644 index 0000000..017d276 --- /dev/null +++ b/crates/irtt-app/src/shared/hmac.rs @@ -0,0 +1,14 @@ +/// Decode CLI key syntax without ever including key material in diagnostics. +pub(crate) fn decode_hmac_key(input: &str) -> Result, String> { + let key = match input.strip_prefix("0x") { + Some(hex) => hex::decode(hex).map_err(|_| { + "HMAC hexadecimal key must contain an even number of valid hexadecimal digits" + .to_owned() + })?, + None => input.as_bytes().to_vec(), + }; + if key.is_empty() { + return Err("HMAC key must not be empty".to_owned()); + } + Ok(key) +} diff --git a/crates/irtt-app/src/shared/mod.rs b/crates/irtt-app/src/shared/mod.rs index f08b072..a3bb17c 100644 --- a/crates/irtt-app/src/shared/mod.rs +++ b/crates/irtt-app/src/shared/mod.rs @@ -1,2 +1,7 @@ #[cfg(any(feature = "client", feature = "tui"))] pub mod client; + +#[cfg(any(feature = "client", feature = "tui", feature = "server"))] +mod hmac; +#[cfg(any(feature = "client", feature = "tui", feature = "server"))] +pub(crate) use hmac::decode_hmac_key; diff --git a/docs/irtt-client.md b/docs/irtt-client.md index fdf9ac8..917f68d 100644 --- a/docs/irtt-client.md +++ b/docs/irtt-client.md @@ -37,12 +37,13 @@ overrides it for that target, and `@hmac=` explicitly disables HMAC for that target. The modifier uses `@`, not a shell command separator; quote an argument if its key needs shell quoting. +Global `--hmac KEY` and per-target `@hmac=KEY` accept plain-text keys or hexadecimal keys prefixed with `0x`. For example, `secret` and `0x736563726574` are equivalent. + Target syntax supports these backslash escapes in labels, addresses, and HMAC values: `\\` for a literal backslash, `\@` for a literal `@`, `\=` for a literal `=`, and `\;` for a literal semicolon. Escaping `@` is necessary in a label or address when it would otherwise begin `@hmac=`; escaping the first -`=` is how an unlabeled target or a label can contain that character. An `=` in -an HMAC value, including Base64 padding, does not need escaping. Positional +`=` is how an unlabeled target or a label can contain that character. An `=` in an HMAC value does not need escaping. Positional target arguments may contain a raw comma; stdin target sets have the additional `\,` escape described below. diff --git a/docs/irtt-server.md b/docs/irtt-server.md index f69be58..1bffa66 100644 --- a/docs/irtt-server.md +++ b/docs/irtt-server.md @@ -74,8 +74,9 @@ IPv6 listener. ### `--hmac KEY` -HMAC key, taken as the UTF-8 bytes of this argument. With a key configured, -every request must carry a valid MAC and every reply is authenticated; +Accepts a plain-text key or a hexadecimal key prefixed with `0x`. For example, `secret` and `0x736563726574` are equivalent. The `IRTT_SERVER_HMAC` environment variable uses the same syntax. + +With a key configured, every request must carry a valid MAC and every reply is authenticated; without one, authenticated requests are dropped. The key is visible in the process arguments.