diff --git a/Cargo.lock b/Cargo.lock index a77fa9b3..b143643f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5841,6 +5841,7 @@ dependencies = [ "regex", "reqwest 0.13.4", "scrypt", + "secrecy", "serde", "serde_json", "serde_with", @@ -7219,6 +7220,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 4264df4f..e3b14e25 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -84,6 +84,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 f62385f1..101ad687 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, @@ -147,6 +148,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. /// @@ -169,7 +185,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; } }; @@ -193,7 +209,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" ); @@ -348,3 +364,29 @@ fn addr_info_from_p2p_addr(addr: &Multiaddr) -> std::result::Result