From e84fc998eed46d417f7d27b4fdf23935babccce9 Mon Sep 17 00:00:00 2001 From: CodeGhost21 <164498022+CodeGhost21@users.noreply.github.com> Date: Wed, 20 May 2026 08:21:08 +0530 Subject: [PATCH] fix(security): fail closed in pairing.rs::is_authenticated when token is unset (#1919) (#2108) Co-authored-by: Cursor --- src/openhuman/security/mod.rs | 5 +- src/openhuman/security/pairing.rs | 138 +++++++++++++++++++++++- src/openhuman/security/pairing_tests.rs | 73 ++++++++++++- 3 files changed, 207 insertions(+), 9 deletions(-) diff --git a/src/openhuman/security/mod.rs b/src/openhuman/security/mod.rs index 969e2047d..aef2a6723 100644 --- a/src/openhuman/security/mod.rs +++ b/src/openhuman/security/mod.rs @@ -21,7 +21,10 @@ pub use detect::create_sandbox; pub use ops as rpc; pub use ops::*; #[allow(unused_imports)] -pub use pairing::PairingGuard; +pub use pairing::{ + ensure_core_rpc_token_for_bind, is_public_bind, CoreBindTokenError, PairingGuard, + CORE_TOKEN_ENV_VAR, +}; pub use policy::validate_path_within_root; #[allow(unused_imports)] pub use policy::AutonomyLevel; diff --git a/src/openhuman/security/pairing.rs b/src/openhuman/security/pairing.rs index f040e1fc9..e86c041aa 100644 --- a/src/openhuman/security/pairing.rs +++ b/src/openhuman/security/pairing.rs @@ -7,9 +7,17 @@ use parking_lot::Mutex; use sha2::{Digest, Sha256}; use std::collections::HashSet; +use std::io::Write as _; +use std::path::Path; use std::sync::Arc; use std::time::Instant; +#[cfg(unix)] +use std::os::unix::fs::{OpenOptionsExt as _, PermissionsExt as _}; + +/// Environment variable for the core JSON-RPC bearer token (see `crate::core::auth`). +pub const CORE_TOKEN_ENV_VAR: &str = "OPENHUMAN_CORE_TOKEN"; + /// Maximum failed pairing attempts before lockout. const MAX_PAIR_ATTEMPTS: u32 = 5; /// Lockout duration after too many failed pairing attempts. @@ -156,13 +164,31 @@ impl PairingGuard { } /// Check if a bearer token is valid (compares against stored hashes). + /// + /// Always fails closed on empty/whitespace tokens. When pairing is not required, + /// configured tokens are still honored if present; with no tokens configured, + /// every request is rejected. pub fn is_authenticated(&self, token: &str) -> bool { - if !self.require_pairing { - return true; + if token.trim().is_empty() { + log::debug!("[openhuman:pairing] is_authenticated: rejected empty bearer token"); + return false; } - let hashed = hash_token(token); + let tokens = self.paired_tokens.lock(); - tokens.contains(&hashed) + if tokens.is_empty() { + log::debug!( + "[openhuman:pairing] is_authenticated: no paired tokens configured (require_pairing={})", + self.require_pairing + ); + return false; + } + + let hashed = hash_token(token); + let ok = tokens.contains(&hashed); + if !ok { + log::debug!("[openhuman:pairing] is_authenticated: bearer token not in paired set"); + } + ok } /// Returns true if pairing is satisfied (has at least one token). @@ -252,11 +278,113 @@ pub fn constant_time_eq(a: &str, b: &str) -> bool { /// Check if a host string represents a non-localhost bind address. pub fn is_public_bind(host: &str) -> bool { !matches!( - host, + host.trim(), "127.0.0.1" | "localhost" | "::1" | "[::1]" | "0:0:0:0:0:0:0:1" ) } +/// Error while resolving or persisting a core RPC token for a bind address. +#[derive(Debug, thiserror::Error)] +pub enum CoreBindTokenError { + #[error( + "{CORE_TOKEN_ENV_VAR} must not be empty when binding on a non-loopback address ({host})" + )] + EmptyEnvToken { host: String }, + #[error("failed to persist core RPC token at {path}: {source}")] + Persist { + path: String, + #[source] + source: std::io::Error, + }, +} + +/// Ensure a non-empty core RPC bearer token exists before binding on `host`. +/// +/// **Loopback** (`127.0.0.1`, `localhost`, `::1`, …): returns `Ok(None)` when +/// `env_token` is unset/empty so local dev can rely on other startup paths. +/// +/// **Non-loopback** (`0.0.0.0`, LAN IPs, …): returns a usable token — either the +/// trimmed `env_token` or a freshly generated 256-bit value written to +/// `{workspace_dir}/core.token` (owner-only on Unix), matching the standalone CLI +/// path in `crate::core::auth::init_rpc_token`. +pub fn ensure_core_rpc_token_for_bind( + host: &str, + workspace_dir: &Path, + env_token: Option<&str>, +) -> Result, CoreBindTokenError> { + let host = host.trim(); + if let Some(raw) = env_token { + let trimmed = raw.trim(); + if !trimmed.is_empty() { + log::info!( + "[openhuman:pairing] core RPC token supplied via {CORE_TOKEN_ENV_VAR} for bind host={host}" + ); + return Ok(Some(trimmed.to_string())); + } + if is_public_bind(host) { + log::error!( + "[openhuman:pairing] {CORE_TOKEN_ENV_VAR} is set but empty on public bind host={host}" + ); + return Err(CoreBindTokenError::EmptyEnvToken { + host: host.to_string(), + }); + } + } + + if !is_public_bind(host) { + log::debug!( + "[openhuman:pairing] loopback bind host={host}: no {CORE_TOKEN_ENV_VAR} configured" + ); + return Ok(None); + } + + let token = generate_core_rpc_token(); + let token_path = workspace_dir.join("core.token"); + write_core_token_file(&token_path, &token).map_err(|source| CoreBindTokenError::Persist { + path: token_path.display().to_string(), + source, + })?; + log::warn!( + "[openhuman:pairing] Public bind on {host} without {CORE_TOKEN_ENV_VAR}: \ + generated token at {} — set {CORE_TOKEN_ENV_VAR} explicitly for stable deployments", + token_path.display() + ); + Ok(Some(token)) +} + +/// Generate a 256-bit core RPC bearer token (lowercase hex, no `zc_` prefix). +fn generate_core_rpc_token() -> String { + use rand::RngExt as _; + let mut bytes = [0u8; 32]; + rand::rng().fill(&mut bytes); + hex::encode(bytes) +} + +/// Write `token` to `path` with owner-only permissions on Unix (`0o600`). +fn write_core_token_file(path: &Path, token: &str) -> std::io::Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + + #[cfg(unix)] + { + let mut file = std::fs::OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .mode(0o600) + .open(path)?; + file.write_all(token.as_bytes())?; + } + + #[cfg(not(unix))] + { + std::fs::write(path, token)?; + } + + Ok(()) +} + #[cfg(test)] #[path = "pairing_tests.rs"] mod tests; diff --git a/src/openhuman/security/pairing_tests.rs b/src/openhuman/security/pairing_tests.rs index f8eb4b79f..d402788bf 100644 --- a/src/openhuman/security/pairing_tests.rs +++ b/src/openhuman/security/pairing_tests.rs @@ -70,10 +70,29 @@ async fn is_authenticated_with_invalid_token() { } #[test] -async fn is_authenticated_when_pairing_disabled() { +async fn empty_token_rejected_when_require_pairing_false() { let (guard, _) = PairingGuard::new(false, &[]); - assert!(guard.is_authenticated("anything")); - assert!(guard.is_authenticated("")); + assert!(!guard.is_authenticated("")); + assert!(!guard.is_authenticated(" ")); +} + +#[test] +async fn empty_token_rejected_when_require_pairing_true() { + let (guard, _) = PairingGuard::new(true, &["zc_valid".into()]); + assert!(!guard.is_authenticated("")); +} + +#[test] +async fn is_authenticated_rejects_unknown_when_pairing_disabled_and_no_tokens() { + let (guard, _) = PairingGuard::new(false, &[]); + assert!(!guard.is_authenticated("anything")); +} + +#[test] +async fn is_authenticated_honors_configured_tokens_when_pairing_disabled() { + let (guard, _) = PairingGuard::new(false, &["zc_dev".into()]); + assert!(guard.is_authenticated("zc_dev")); + assert!(!guard.is_authenticated("zc_other")); } #[test] @@ -146,6 +165,54 @@ async fn real_ip_is_public() { assert!(is_public_bind("10.0.0.1")); } +// ── Core RPC bind token ────────────────────────────────── + +#[test] +async fn public_bind_without_env_token_auto_generates() { + let tmp = std::env::temp_dir().join(format!("pairing-bind-test-{}", std::process::id())); + std::fs::create_dir_all(&tmp).unwrap(); + let token = ensure_core_rpc_token_for_bind("0.0.0.0", &tmp, None) + .expect("public bind should auto-generate a token") + .expect("public bind should return Some(token)"); + assert_eq!(token.len(), 64); + assert!(token.chars().all(|c| c.is_ascii_hexdigit())); + let from_disk = std::fs::read_to_string(tmp.join("core.token")).unwrap(); + assert_eq!(from_disk, token); + std::fs::remove_dir_all(&tmp).ok(); +} + +#[test] +async fn public_bind_rejects_explicit_empty_env_token() { + let tmp = std::env::temp_dir().join(format!("pairing-bind-empty-{}", std::process::id())); + std::fs::create_dir_all(&tmp).unwrap(); + let err = ensure_core_rpc_token_for_bind("0.0.0.0", &tmp, Some(" ")) + .expect_err("empty env token on public bind must fail"); + assert!(matches!(err, CoreBindTokenError::EmptyEnvToken { .. })); + std::fs::remove_dir_all(&tmp).ok(); +} + +#[test] +async fn loopback_bind_without_env_token_returns_none() { + let tmp = std::env::temp_dir().join(format!("pairing-bind-loopback-{}", std::process::id())); + std::fs::create_dir_all(&tmp).unwrap(); + let token = ensure_core_rpc_token_for_bind("127.0.0.1", &tmp, None).unwrap(); + assert!(token.is_none()); + assert!(!tmp.join("core.token").exists()); + std::fs::remove_dir_all(&tmp).ok(); +} + +#[test] +async fn public_bind_uses_nonempty_env_token_without_writing_file() { + let tmp = std::env::temp_dir().join(format!("pairing-bind-env-token-{}", std::process::id())); + std::fs::create_dir_all(&tmp).unwrap(); + let token = ensure_core_rpc_token_for_bind("0.0.0.0", &tmp, Some(" abc123 ")) + .unwrap() + .expect("env token should be returned"); + assert_eq!(token, "abc123"); + assert!(!tmp.join("core.token").exists()); + std::fs::remove_dir_all(&tmp).ok(); +} + // ── constant_time_eq ───────────────────────────────────── #[test]