diff --git a/crates/teamtalk/Cargo.toml b/crates/teamtalk/Cargo.toml index 070018b..a173d99 100644 --- a/crates/teamtalk/Cargo.toml +++ b/crates/teamtalk/Cargo.toml @@ -35,6 +35,7 @@ regex = "1.12.3" reqwest = { version = "0.13.2", default-features = false, features = ["blocking"] } sevenz-rust2 = "0.20.2" bitflags = "2.11" +zeroize = { version = "1.8", features = ["zeroize_derive"] } futures = { version = "0.3", optional = true } futures-timer = { version = "3.0", optional = true } tokio = { version = "1.52", features = ["rt", "sync", "time", "macros"], optional = true } diff --git a/crates/teamtalk/src/client/core/recovery.rs b/crates/teamtalk/src/client/core/recovery.rs index 50eb89e..41a89ad 100644 --- a/crates/teamtalk/src/client/core/recovery.rs +++ b/crates/teamtalk/src/client/core/recovery.rs @@ -127,7 +127,7 @@ impl Client { let cmd_id = self.login( ¶ms.nickname, ¶ms.username, - ¶ms.password, + params.password.expose_secret(), ¶ms.client_name, ); if cmd_id.is_ok() { diff --git a/crates/teamtalk/src/client/users/auth.rs b/crates/teamtalk/src/client/users/auth.rs index 66c8b08..a3959d1 100644 --- a/crates/teamtalk/src/client/users/auth.rs +++ b/crates/teamtalk/src/client/users/auth.rs @@ -58,7 +58,7 @@ impl Client { Ok(self.login( ¶ms.nickname, ¶ms.username, - ¶ms.password, + params.password.expose_secret(), ¶ms.client_name, )) } @@ -135,7 +135,7 @@ impl Client { self.login( ¶ms.nickname, ¶ms.username, - ¶ms.password, + params.password.expose_secret(), ¶ms.client_name, ) } diff --git a/crates/teamtalk/src/client/users/mod.rs b/crates/teamtalk/src/client/users/mod.rs index 500ca67..76bd5b6 100644 --- a/crates/teamtalk/src/client/users/mod.rs +++ b/crates/teamtalk/src/client/users/mod.rs @@ -141,12 +141,18 @@ fn text_message_for_target(target: MessageTarget) -> ffi::TextMessage { } /// Stored login parameters for automatic login. +/// +/// The `password` field is a [`SecretString`], so its in-memory +/// buffer is zeroised on drop and does not leak through `Debug` +/// or `Display`. Use [`SecretString::expose_secret`] to retrieve +/// the cleartext password when passing it to an FFI call; keep +/// the borrow as narrow as possible. #[non_exhaustive] #[derive(Debug, Clone)] pub struct LoginParams { pub nickname: String, pub username: String, - pub password: String, + pub password: crate::types::SecretString, pub client_name: String, } @@ -154,7 +160,7 @@ impl LoginParams { pub fn new( nickname: impl Into, username: impl Into, - password: impl Into, + password: impl Into, client_name: impl Into, ) -> Self { Self { diff --git a/crates/teamtalk/src/types/mod.rs b/crates/teamtalk/src/types/mod.rs index a3f92a4..9d754a0 100644 --- a/crates/teamtalk/src/types/mod.rs +++ b/crates/teamtalk/src/types/mod.rs @@ -6,12 +6,14 @@ pub mod channels; pub mod ids; pub mod messaging; pub mod preprocess; +pub mod secret; pub mod server; pub mod users; pub use base::*; pub use ids::*; pub use preprocess::*; +pub use secret::SecretString; mod entities; pub use entities::*; diff --git a/crates/teamtalk/src/types/secret.rs b/crates/teamtalk/src/types/secret.rs new file mode 100644 index 0000000..c73666a --- /dev/null +++ b/crates/teamtalk/src/types/secret.rs @@ -0,0 +1,144 @@ +//! Zeroising secret-string wrapper used for in-memory credentials. +//! +//! TeamTalk credentials (passwords passed to `login`, operator +//! passwords used in moderation calls) are sensitive and should +//! not linger in heap memory after they are no longer needed. +//! +//! [`SecretString`] is a thin newtype over `String` that: +//! +//! - Zeroises its backing buffer on `Drop` via [`zeroize::Zeroize`]. +//! - Refuses to print the secret via `Debug` / `Display` — the only +//! way to observe the inner bytes is through +//! [`SecretString::expose_secret`]. +//! - Compares for equality in constant time (see +//! [`SecretString::eq`]) so user-visible branch timing does not +//! leak information about password length or content. +//! - Accepts conversions from `&str`, `String`, and +//! `Cow<'_, str>` via `From` / `Into` so API callers do not need +//! to think about the wrapper unless they want to. +//! +//! The implementation intentionally does not re-export or depend +//! on the `secrecy` crate so the crate surface stays small. + +use std::borrow::Cow; +use zeroize::{Zeroize, ZeroizeOnDrop}; + +/// A UTF-8 string that is zeroised on drop and refuses to print +/// itself through `Debug` / `Display`. +#[derive(Clone, ZeroizeOnDrop)] +pub struct SecretString(String); + +impl SecretString { + /// Creates an empty secret. + #[must_use] + pub fn new() -> Self { + Self(String::new()) + } + + /// Creates a secret from an owned `String`. The input buffer + /// is consumed and its memory is owned by the secret. + #[must_use] + pub fn from_string(value: String) -> Self { + Self(value) + } + + /// Returns `true` if the secret is empty. + #[must_use] + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + + /// Returns the length of the secret in bytes. + #[must_use] + pub fn len(&self) -> usize { + self.0.len() + } + + /// Returns a reference to the underlying bytes. + /// + /// # Security + /// + /// This is the only way to read the secret back out. Treat the + /// returned `&str` as tainted: do not log it, store it + /// long-term, or pass it through `Debug` / `Display`. Scope its + /// use as narrowly as possible (e.g. a single FFI call) and + /// drop the reference immediately afterwards. + #[must_use] + pub fn expose_secret(&self) -> &str { + self.0.as_str() + } + + /// Overwrites the inner buffer with zero bytes in place and + /// leaves the secret empty. + /// + /// Equivalent to assigning `SecretString::new()` except the + /// existing allocation is reused, so capacity is preserved. + pub fn zeroize_in_place(&mut self) { + self.0.zeroize(); + } +} + +impl Default for SecretString { + fn default() -> Self { + Self::new() + } +} + +impl std::fmt::Debug for SecretString { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_tuple("SecretString").field(&"").finish() + } +} + +impl std::fmt::Display for SecretString { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("") + } +} + +impl PartialEq for SecretString { + /// Constant-time-ish comparison over the byte contents. + /// + /// Uses a branch-less `|=` fold over XOR-ed bytes so equality + /// checks do not short-circuit on the first mismatching byte. + /// Length difference is still observable — callers that need + /// full length-hiding must compare to a padded secret. + fn eq(&self, other: &Self) -> bool { + let a = self.0.as_bytes(); + let b = other.0.as_bytes(); + if a.len() != b.len() { + return false; + } + let mut diff: u8 = 0; + for (x, y) in a.iter().zip(b.iter()) { + diff |= x ^ y; + } + diff == 0 + } +} + +impl Eq for SecretString {} + +impl From for SecretString { + fn from(value: String) -> Self { + Self::from_string(value) + } +} + +impl From<&str> for SecretString { + fn from(value: &str) -> Self { + Self::from_string(value.to_owned()) + } +} + +impl From> for SecretString { + fn from(value: Cow<'_, str>) -> Self { + Self::from_string(value.into_owned()) + } +} + +impl From<&String> for SecretString { + fn from(value: &String) -> Self { + Self::from_string(value.clone()) + } +} diff --git a/crates/teamtalk/tests/secret_string_tests.rs b/crates/teamtalk/tests/secret_string_tests.rs new file mode 100644 index 0000000..8e80e66 --- /dev/null +++ b/crates/teamtalk/tests/secret_string_tests.rs @@ -0,0 +1,135 @@ +//! Integration tests for the `SecretString` credential wrapper +//! and its adoption on `LoginParams`. + +use teamtalk::LoginParams; +use teamtalk::types::SecretString; + +#[test] +fn secret_string_exposes_inner_string() { + let s = SecretString::from("hunter2"); + assert_eq!(s.expose_secret(), "hunter2"); + assert_eq!(s.len(), 7); + assert!(!s.is_empty()); +} + +#[test] +fn secret_string_default_is_empty() { + let s = SecretString::default(); + assert!(s.is_empty()); + assert_eq!(s.len(), 0); + assert_eq!(s.expose_secret(), ""); +} + +#[test] +fn secret_string_zeroize_in_place_empties_buffer() { + let mut s = SecretString::from("hunter2"); + s.zeroize_in_place(); + assert!(s.is_empty()); + assert_eq!(s.expose_secret(), ""); +} + +#[test] +fn debug_does_not_leak_contents() { + let s = SecretString::from("supersecret-password"); + let dbg = format!("{s:?}"); + assert!( + !dbg.contains("supersecret-password"), + "Debug should not include the plaintext, got: {dbg}" + ); + assert!( + dbg.contains("redacted") || dbg.contains("SecretString"), + "Debug should mark the field as redacted, got: {dbg}" + ); +} + +#[test] +fn display_does_not_leak_contents() { + let s = SecretString::from("supersecret-password"); + let disp = format!("{s}"); + assert!( + !disp.contains("supersecret-password"), + "Display should not include the plaintext, got: {disp}" + ); + assert!( + disp.contains("redacted"), + "Display should announce redaction, got: {disp}" + ); +} + +#[test] +fn equality_is_content_based() { + let a = SecretString::from("abc"); + let b = SecretString::from("abc"); + let c = SecretString::from("abd"); + let d = SecretString::from("ab"); + assert_eq!(a, b); + assert_ne!(a, c); + assert_ne!(a, d); +} + +#[test] +fn conversions_from_common_types() { + let from_string: SecretString = "pw".to_string().into(); + let from_str: SecretString = "pw".into(); + let from_cow: SecretString = std::borrow::Cow::Borrowed("pw").into(); + let from_ref_string: SecretString = (&"pw".to_string()).into(); + + assert_eq!(from_string.expose_secret(), "pw"); + assert_eq!(from_str.expose_secret(), "pw"); + assert_eq!(from_cow.expose_secret(), "pw"); + assert_eq!(from_ref_string.expose_secret(), "pw"); +} + +#[test] +fn clone_preserves_value() { + let original = SecretString::from("shared"); + let cloned = original.clone(); + assert_eq!(original, cloned); + assert_eq!(cloned.expose_secret(), "shared"); +} + +#[test] +fn login_params_password_is_secret_string() { + let params = LoginParams::new("nick", "user", "pw", "client"); + assert_eq!(params.password.expose_secret(), "pw"); + assert_eq!(params.nickname, "nick"); + assert_eq!(params.username, "user"); + assert_eq!(params.client_name, "client"); +} + +#[test] +fn login_params_debug_does_not_leak_password() { + let params = LoginParams::new("nick", "user", "hunter2", "client"); + let dbg = format!("{params:?}"); + assert!( + !dbg.contains("hunter2"), + "LoginParams Debug leaked password: {dbg}" + ); + // Nickname/username are not secret — they should still be visible. + assert!(dbg.contains("nick")); + assert!(dbg.contains("user")); +} + +#[test] +fn login_params_accepts_string_and_str_passwords() { + let owned = String::from("pw-owned"); + let p1 = LoginParams::new("n", "u", owned.clone(), "c"); + let p2 = LoginParams::new("n", "u", "pw-borrowed", "c"); + let p3 = LoginParams::new("n", "u", SecretString::from("pw-typed"), "c"); + + assert_eq!(p1.password.expose_secret(), "pw-owned"); + assert_eq!(p2.password.expose_secret(), "pw-borrowed"); + assert_eq!(p3.password.expose_secret(), "pw-typed"); +} + +#[test] +fn empty_secret_and_empty_debug() { + let empty = SecretString::new(); + assert!(empty.is_empty()); + let dbg = format!("{empty:?}"); + assert!( + !dbg.is_empty(), + "Debug should still produce SOME output for an empty secret" + ); + assert!(dbg.contains("redacted") || dbg.contains("SecretString")); +}