From 7d559a9523f20d501ff745f3e75998307e840bfc Mon Sep 17 00:00:00 2001 From: 0xOse <0xosepatrick@gmail.com> Date: Fri, 3 Jul 2026 17:56:36 +0100 Subject: [PATCH 1/2] fix(logging): redact sensitive credentials from log output - Use Secret in keymanager::Client so Debug prints [REDACTED] - Manual Debug for KeymanagerConfig that redacts auth_token - redact_url() helper in bootnode strips userinfo before log fields - Unit tests for each redaction site --- Cargo.lock | 10 +++++++ Cargo.toml | 1 + crates/dkg/src/dkg.rs | 25 +++++++++++++++-- crates/eth2util/Cargo.toml | 1 + crates/eth2util/src/keymanager.rs | 15 ++++++++-- crates/p2p/src/bootnode.rs | 46 +++++++++++++++++++++++++++++-- 6 files changed, 91 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ac06d7cc..5177fb4c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5367,6 +5367,7 @@ dependencies = [ "regex", "reqwest 0.13.4", "scrypt", + "secrecy", "serde", "serde_json", "serde_with", @@ -6687,6 +6688,15 @@ dependencies = [ "cc", ] +[[package]] +name = "secrecy" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9bd1c54ea06cfd2f6b63219704de0b9b4f72dcc2b8fdef820be6cd799780e91e" +dependencies = [ + "zeroize", +] + [[package]] name = "security-framework" version = "3.7.0" diff --git a/Cargo.toml b/Cargo.toml index fdc5a056..af4e4ae4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -100,6 +100,7 @@ scrypt = "0.11.0" subtle = "2.6" unicode-normalization = "0.1.25" zeroize = "1.8.2" +secrecy = "0.8" uuid = { version = "1.19", features = ["serde", "v4"] } unsigned-varint = { version = "0.8", features = ["futures"] } serde_with = { version = "3.16", features = ["hex", "base64"] } diff --git a/crates/dkg/src/dkg.rs b/crates/dkg/src/dkg.rs index 08e0abc1..67616285 100644 --- a/crates/dkg/src/dkg.rs +++ b/crates/dkg/src/dkg.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, ffi::OsStr, num::TryFromIntError, path, time::Duration}; +use std::{collections::HashMap, ffi::OsStr, fmt, num::TryFromIntError, path, time::Duration}; use bon::Builder; use futures::StreamExt; @@ -219,7 +219,7 @@ pub enum DkgError { } /// Keymanager configuration accepted by the entrypoint. -#[derive(Debug, Clone, Default, Builder)] +#[derive(Clone, Default, Builder)] pub struct KeymanagerConfig { /// The keymanager URL. pub address: String, @@ -227,6 +227,15 @@ pub struct KeymanagerConfig { pub auth_token: String, } +impl fmt::Debug for KeymanagerConfig { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("KeymanagerConfig") + .field("address", &self.address) + .field("auth_token", &"") + .finish() + } +} + /// Publish configuration accepted by the entrypoint. #[derive(Debug, Clone, Builder)] pub struct PublishConfig { @@ -1286,4 +1295,16 @@ mod tests { DkgError::Disk(crate::disk::DiskError::MissingRequiredFiles { .. }) )); } + + #[test] + fn keymanager_config_debug_redacts_auth_token() { + let cfg = KeymanagerConfig::builder() + .address("https://keymanager.example".to_string()) + .auth_token("super-secret-token".to_string()) + .build(); + let rendered = format!("{cfg:?}"); + assert!(!rendered.contains("super-secret-token"), "token leaked in Debug: {rendered}"); + assert!(rendered.contains(""), "expected marker in Debug: {rendered}"); + assert!(rendered.contains("https://keymanager.example"), "address should be visible"); + } } diff --git a/crates/eth2util/Cargo.toml b/crates/eth2util/Cargo.toml index 691bd3c8..b51289b0 100644 --- a/crates/eth2util/Cargo.toml +++ b/crates/eth2util/Cargo.toml @@ -21,6 +21,7 @@ pbkdf2.workspace = true scrypt.workspace = true unicode-normalization.workspace = true zeroize.workspace = true +secrecy.workspace = true pluto-k1util.workspace = true chrono.workspace = true regex.workspace = true diff --git a/crates/eth2util/src/keymanager.rs b/crates/eth2util/src/keymanager.rs index bb1c842f..4a90268a 100644 --- a/crates/eth2util/src/keymanager.rs +++ b/crates/eth2util/src/keymanager.rs @@ -2,6 +2,7 @@ //! (https://ethereum.github.io/keymanager-APIs/) functionalities. use crate::keystore::Keystore; +use secrecy::{ExposeSecret, Secret}; use url::Url; /// Errors that can occur when using the keymanager client. @@ -63,7 +64,7 @@ pub type Result = std::result::Result; #[derive(Debug, Clone)] pub struct Client { base_url: Url, - auth_token: String, + auth_token: Secret, http_client: reqwest::Client, } @@ -80,7 +81,7 @@ impl Client { }; Ok(Self { base_url: Url::parse(&normalized)?, - auth_token: auth_token.as_ref().to_owned(), + auth_token: Secret::new(auth_token.as_ref().to_owned()), http_client: reqwest::Client::new(), }) } @@ -143,7 +144,7 @@ impl Client { .http_client .post(addr) .header("Content-Type", "application/json") - .header("Authorization", format!("Bearer {}", self.auth_token)) + .header("Authorization", format!("Bearer {}", self.auth_token.expose_secret())) .body(req_bytes) .timeout(timeout) .send() @@ -338,4 +339,12 @@ mod tests { let err = Client::new(input, AUTH_TOKEN).unwrap_err(); assert!(matches!(err, KeymanagerError::ParseUrl(_))); } + + #[test] + fn client_debug_redacts_auth_token() { + let client = Client::new("http://localhost:9999", "super-secret-token").unwrap(); + let rendered = format!("{client:?}"); + assert!(!rendered.contains("super-secret-token"), "token leaked in Debug: {rendered}"); + assert!(rendered.contains("REDACTED"), "expected REDACTED marker in Debug: {rendered}"); + } } diff --git a/crates/p2p/src/bootnode.rs b/crates/p2p/src/bootnode.rs index 2084083a..cd04ac40 100644 --- a/crates/p2p/src/bootnode.rs +++ b/crates/p2p/src/bootnode.rs @@ -7,6 +7,7 @@ use libp2p::Multiaddr; use pluto_eth2util::enr::Record; use tokio_util::sync::CancellationToken; use tracing::{info, warn}; +use url::Url; use crate::peer::{ AddrInfo, MutablePeer, Peer, PeerError, addr_infos_from_p2p_addrs, peer_id_from_key, @@ -159,6 +160,21 @@ pub async fn new_relays( Ok(resp) } +/// Strips embedded userinfo (basic-auth credentials) from a URL string before +/// it is passed to a log field so tokens and passwords cannot appear in logs. +fn redact_url(raw: &str) -> String { + let Ok(mut url) = Url::parse(raw) else { + return raw.to_owned(); + }; + if url.username().is_empty() && url.password().is_none() { + return raw.to_owned(); + } + if url.set_username("").is_err() || url.set_password(None).is_err() { + return "".to_owned(); + } + url.to_string() +} + /// Continuously resolves relay multiaddrs from an HTTP URL and updates the /// MutablePeer. /// @@ -184,7 +200,7 @@ async fn resolve_relay( { Ok(addrs) => addrs, Err(e) => { - tracing::error!(err = %e, url = %raw_url, "Failed resolving relay addresses from URL"); + tracing::error!(err = %e, url = %redact_url(&raw_url), "Failed resolving relay addresses from URL"); return; } }; @@ -208,7 +224,7 @@ async fn resolve_relay( let peer = Peer::new_relay_peer(&infos[0]); info!( peer = %peer.name, - url = %raw_url, + url = %redact_url(&raw_url), addrs = ?peer.addresses, "Resolved new relay" ); @@ -390,3 +406,29 @@ fn addr_info_from_p2p_addr(addr: &Multiaddr) -> std::result::Result Date: Tue, 7 Jul 2026 11:14:54 +0100 Subject: [PATCH 2/2] fix(logging): address reviewer feedback on #522 --- Cargo.lock | 4 +-- Cargo.toml | 2 +- crates/dkg/src/dkg.rs | 15 ++++++++-- crates/eth2util/src/keymanager.rs | 21 ++++++++++---- crates/p2p/src/bootnode.rs | 46 ++----------------------------- 5 files changed, 32 insertions(+), 56 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5177fb4c..456ec76c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6690,9 +6690,9 @@ dependencies = [ [[package]] name = "secrecy" -version = "0.8.0" +version = "0.10.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9bd1c54ea06cfd2f6b63219704de0b9b4f72dcc2b8fdef820be6cd799780e91e" +checksum = "e891af845473308773346dc847b2c23ee78fe442e0472ac50e22a18a93d3ae5a" dependencies = [ "zeroize", ] diff --git a/Cargo.toml b/Cargo.toml index af4e4ae4..56dce23d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -100,7 +100,7 @@ scrypt = "0.11.0" subtle = "2.6" unicode-normalization = "0.1.25" zeroize = "1.8.2" -secrecy = "0.8" +secrecy = "0.10" uuid = { version = "1.19", features = ["serde", "v4"] } unsigned-varint = { version = "0.8", features = ["futures"] } serde_with = { version = "3.16", features = ["hex", "base64"] } diff --git a/crates/dkg/src/dkg.rs b/crates/dkg/src/dkg.rs index 67616285..5e75daab 100644 --- a/crates/dkg/src/dkg.rs +++ b/crates/dkg/src/dkg.rs @@ -1303,8 +1303,17 @@ mod tests { .auth_token("super-secret-token".to_string()) .build(); let rendered = format!("{cfg:?}"); - assert!(!rendered.contains("super-secret-token"), "token leaked in Debug: {rendered}"); - assert!(rendered.contains(""), "expected marker in Debug: {rendered}"); - assert!(rendered.contains("https://keymanager.example"), "address should be visible"); + assert!( + !rendered.contains("super-secret-token"), + "token leaked in Debug: {rendered}" + ); + assert!( + rendered.contains(""), + "expected marker in Debug: {rendered}" + ); + assert!( + rendered.contains("https://keymanager.example"), + "address should be visible" + ); } } diff --git a/crates/eth2util/src/keymanager.rs b/crates/eth2util/src/keymanager.rs index 4a90268a..fb9a31ce 100644 --- a/crates/eth2util/src/keymanager.rs +++ b/crates/eth2util/src/keymanager.rs @@ -2,7 +2,7 @@ //! (https://ethereum.github.io/keymanager-APIs/) functionalities. use crate::keystore::Keystore; -use secrecy::{ExposeSecret, Secret}; +use secrecy::{ExposeSecret, SecretString}; use url::Url; /// Errors that can occur when using the keymanager client. @@ -64,7 +64,7 @@ pub type Result = std::result::Result; #[derive(Debug, Clone)] pub struct Client { base_url: Url, - auth_token: Secret, + auth_token: SecretString, http_client: reqwest::Client, } @@ -81,7 +81,7 @@ impl Client { }; Ok(Self { base_url: Url::parse(&normalized)?, - auth_token: Secret::new(auth_token.as_ref().to_owned()), + auth_token: SecretString::from(auth_token.as_ref().to_owned()), http_client: reqwest::Client::new(), }) } @@ -144,7 +144,10 @@ impl Client { .http_client .post(addr) .header("Content-Type", "application/json") - .header("Authorization", format!("Bearer {}", self.auth_token.expose_secret())) + .header( + "Authorization", + format!("Bearer {}", self.auth_token.expose_secret()), + ) .body(req_bytes) .timeout(timeout) .send() @@ -344,7 +347,13 @@ mod tests { fn client_debug_redacts_auth_token() { let client = Client::new("http://localhost:9999", "super-secret-token").unwrap(); let rendered = format!("{client:?}"); - assert!(!rendered.contains("super-secret-token"), "token leaked in Debug: {rendered}"); - assert!(rendered.contains("REDACTED"), "expected REDACTED marker in Debug: {rendered}"); + assert!( + !rendered.contains("super-secret-token"), + "token leaked in Debug: {rendered}" + ); + assert!( + rendered.contains("REDACTED"), + "expected REDACTED marker in Debug: {rendered}" + ); } } diff --git a/crates/p2p/src/bootnode.rs b/crates/p2p/src/bootnode.rs index cd04ac40..2084083a 100644 --- a/crates/p2p/src/bootnode.rs +++ b/crates/p2p/src/bootnode.rs @@ -7,7 +7,6 @@ use libp2p::Multiaddr; use pluto_eth2util::enr::Record; use tokio_util::sync::CancellationToken; use tracing::{info, warn}; -use url::Url; use crate::peer::{ AddrInfo, MutablePeer, Peer, PeerError, addr_infos_from_p2p_addrs, peer_id_from_key, @@ -160,21 +159,6 @@ pub async fn new_relays( Ok(resp) } -/// Strips embedded userinfo (basic-auth credentials) from a URL string before -/// it is passed to a log field so tokens and passwords cannot appear in logs. -fn redact_url(raw: &str) -> String { - let Ok(mut url) = Url::parse(raw) else { - return raw.to_owned(); - }; - if url.username().is_empty() && url.password().is_none() { - return raw.to_owned(); - } - if url.set_username("").is_err() || url.set_password(None).is_err() { - return "".to_owned(); - } - url.to_string() -} - /// Continuously resolves relay multiaddrs from an HTTP URL and updates the /// MutablePeer. /// @@ -200,7 +184,7 @@ async fn resolve_relay( { Ok(addrs) => addrs, Err(e) => { - tracing::error!(err = %e, url = %redact_url(&raw_url), "Failed resolving relay addresses from URL"); + tracing::error!(err = %e, url = %raw_url, "Failed resolving relay addresses from URL"); return; } }; @@ -224,7 +208,7 @@ async fn resolve_relay( let peer = Peer::new_relay_peer(&infos[0]); info!( peer = %peer.name, - url = %redact_url(&raw_url), + url = %raw_url, addrs = ?peer.addresses, "Resolved new relay" ); @@ -406,29 +390,3 @@ fn addr_info_from_p2p_addr(addr: &Multiaddr) -> std::result::Result