diff --git a/crates/nono-proxy/src/config.rs b/crates/nono-proxy/src/config.rs index 8c704b4ee..55d05ea3b 100644 --- a/crates/nono-proxy/src/config.rs +++ b/crates/nono-proxy/src/config.rs @@ -467,7 +467,12 @@ pub struct ExternalProxyConfig { /// Authentication for an external proxy. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ExternalProxyAuth { - /// Keystore account name for proxy credentials. + /// Proxy username (not secret; stored in config, not keyring). + pub username: String, + + /// Keyring reference for the proxy password only. + /// Supports the same URI schemes as route `credential_key` + /// (plain account name, `op://`, `file://`, `env://`, etc.). pub keyring_account: String, /// Authentication scheme (only "basic" supported). @@ -546,6 +551,22 @@ mod tests { assert_eq!(ext.bypass_hosts[1], "*.private.net"); } + #[test] + fn test_external_proxy_auth_deserializes_with_username() { + let json = r#"{ + "address": "proxy:3128", + "auth": { + "username": "alice", + "keyring_account": "corp-proxy-password" + } + }"#; + let ext: ExternalProxyConfig = serde_json::from_str(json).unwrap(); + let auth = ext.auth.unwrap(); + assert_eq!(auth.username, "alice"); + assert_eq!(auth.keyring_account, "corp-proxy-password"); + assert_eq!(auth.scheme, "basic"); + } + #[test] fn test_external_proxy_config_bypass_hosts_default_empty() { let json = r#"{"address": "proxy:3128", "auth": null}"#; diff --git a/crates/nono-proxy/src/credential.rs b/crates/nono-proxy/src/credential.rs index 84f071b66..a97a006aa 100644 --- a/crates/nono-proxy/src/credential.rs +++ b/crates/nono-proxy/src/credential.rs @@ -333,7 +333,7 @@ const KEYRING_SERVICE: &str = nono::keystore::DEFAULT_SERVICE; /// /// Delegates to the appropriate URI-specific redaction helper so that /// secrets (account names, file paths, field names) are never echoed raw. -fn redact_credential_ref(key: &str) -> String { +pub(crate) fn redact_credential_ref(key: &str) -> String { if nono::keystore::is_op_uri(key) { nono::keystore::redact_op_uri(key) } else if nono::keystore::is_apple_password_uri(key) { diff --git a/crates/nono-proxy/src/external.rs b/crates/nono-proxy/src/external.rs index 88d6ca51f..cbaa2f2c9 100644 --- a/crates/nono-proxy/src/external.rs +++ b/crates/nono-proxy/src/external.rs @@ -8,10 +8,12 @@ //! [`connect_via_proxy`] so the TLS-intercept upstream leg can reuse it. use crate::audit; -use crate::config::ExternalProxyConfig; +use crate::config::{ExternalProxyAuth, ExternalProxyConfig}; +use crate::credential::redact_credential_ref; use crate::error::{ProxyError, Result}; use crate::filter::ProxyFilter; use crate::token; +use base64::Engine; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; use tokio::net::TcpStream; use tracing::debug; @@ -163,6 +165,31 @@ impl BypassMatcher { } } +const KEYRING_SERVICE: &str = nono::keystore::DEFAULT_SERVICE; + +/// Build a `Basic ` Proxy-Authorization header value for `auth`. +pub(crate) fn build_basic_proxy_auth_header(auth: &ExternalProxyAuth) -> Result> { + if auth.scheme != "basic" { + return Err(ProxyError::ExternalProxy(format!( + "unsupported external proxy auth scheme '{}'; only 'basic' is supported", + auth.scheme + ))); + } + let redacted_ref = redact_credential_ref(&auth.keyring_account); + let password = nono::keystore::load_secret_by_ref(KEYRING_SERVICE, &auth.keyring_account) + .map_err(|e| { + ProxyError::ExternalProxy(format!( + "proxy auth credential '{}' unavailable: {}", + redacted_ref, e + )) + })?; + let plaintext = Zeroizing::new(format!("{}:{}", auth.username, password.as_str())); + let mut encoded = base64::engine::general_purpose::STANDARD.encode(plaintext.as_bytes()); + let header = Zeroizing::new(format!("Basic {}", encoded)); + zeroize::Zeroize::zeroize(&mut encoded); + Ok(header) +} + /// Handle a CONNECT request by chaining it to an external proxy. /// /// 1. Validate session token @@ -209,22 +236,42 @@ pub async fn handle_external_proxy( return Err(ProxyError::HostDenied { host, reason }); } - // External proxy authentication is not yet implemented. If auth is - // configured, fail loudly rather than silently sending unauthenticated - // requests that the enterprise proxy will reject. - if external_config.auth.is_some() { - return Err(ProxyError::ExternalProxy( - "external proxy authentication is configured but not yet implemented; \ - remove the auth section from the external proxy config or wait for \ - a future release" - .to_string(), - )); - } + let proxy_auth = if let Some(ref auth) = external_config.auth { + match build_basic_proxy_auth_header(auth) { + Ok(h) => Some(h), + Err(e) => { + audit::log_denied( + audit_log, + audit::ProxyMode::External, + &audit::EventContext { + auth_mechanism: Some( + nono::undo::NetworkAuditAuthMechanism::ProxyAuthorization, + ), + auth_outcome: Some(nono::undo::NetworkAuditAuthOutcome::Failed), + denial_category: Some( + nono::undo::NetworkAuditDenialCategory::AuthenticationFailed, + ), + ..audit::EventContext::default() + }, + &host, + port, + &e.to_string(), + ); + send_response(stream, 502, "Bad Gateway").await?; + return Err(e); + } + } + } else { + None + }; - // Connect to enterprise proxy and CONNECT through it to the upstream. - // Auth is gated above; pass None until configurable proxy auth lands. - let mut proxy_stream = match connect_via_proxy(&external_config.address, &host, port, None) - .await + let mut proxy_stream = match connect_via_proxy( + &external_config.address, + &host, + port, + proxy_auth.as_ref().map(|s| s.as_str()), + ) + .await { Ok(s) => s, Err(ProxyError::ExternalProxy(msg)) if msg.contains("rejected CONNECT") => { @@ -339,7 +386,7 @@ fn parse_status_code(line: &str) -> Result { } /// Send an HTTP response line. -async fn send_response(stream: &mut TcpStream, status: u16, reason: &str) -> Result<()> { +pub(crate) async fn send_response(stream: &mut TcpStream, status: u16, reason: &str) -> Result<()> { let response = format!("HTTP/1.1 {} {}\r\n\r\n", status, reason); stream.write_all(response.as_bytes()).await?; stream.flush().await?; @@ -351,6 +398,29 @@ async fn send_response(stream: &mut TcpStream, status: u16, reason: &str) -> Res mod tests { use super::*; + static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + struct EnvGuard { + _lock: std::sync::MutexGuard<'static, ()>, + key: &'static str, + } + + impl EnvGuard { + #[allow(clippy::disallowed_methods)] + fn set(key: &'static str, value: &str) -> Self { + let lock = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + unsafe { std::env::set_var(key, value) }; + Self { _lock: lock, key } + } + } + + #[allow(clippy::disallowed_methods)] + impl Drop for EnvGuard { + fn drop(&mut self) { + unsafe { std::env::remove_var(self.key) }; + } + } + #[test] fn test_parse_connect_target() { let (host, port) = parse_connect_target("CONNECT api.openai.com:443 HTTP/1.1").unwrap(); @@ -460,4 +530,27 @@ mod tests { assert!(matcher.is_empty()); assert!(!matcher.matches("anything.com")); } + + #[test] + fn build_basic_proxy_auth_header_encodes_correctly() { + let _guard = EnvGuard::set("NONO_TEST_PROXY_PASS", "s3cr3t"); + let auth = ExternalProxyAuth { + username: "alice".to_string(), + keyring_account: "env://NONO_TEST_PROXY_PASS".to_string(), + scheme: "basic".to_string(), + }; + let header = build_basic_proxy_auth_header(&auth).unwrap(); + // "alice:s3cr3t" base64-encoded is "YWxpY2U6czNjcjN0" + assert_eq!(header.as_str(), "Basic YWxpY2U6czNjcjN0"); + } + + #[test] + fn build_basic_proxy_auth_header_missing_credential_returns_error() { + let auth = ExternalProxyAuth { + username: "alice".to_string(), + keyring_account: "env://NONO_TEST_PROXY_PASS_MISSING_XYZ".to_string(), + scheme: "basic".to_string(), + }; + assert!(build_basic_proxy_auth_header(&auth).is_err()); + } } diff --git a/crates/nono-proxy/src/server.rs b/crates/nono-proxy/src/server.rs index 493d92b53..39e730c1b 100644 --- a/crates/nono-proxy/src/server.rs +++ b/crates/nono-proxy/src/server.rs @@ -824,6 +824,47 @@ async fn handle_connection(mut stream: tokio::net::TcpStream, state: &ProxyState // Decide whether the upstream leg should chain through // the corporate proxy. Mirrors the bypass logic used for // transparent CONNECT below. + let intercept_proxy_auth: Option> = if let Some( + ref ext_config, + ) = + state.config.external_proxy + { + let bypassed = !state.bypass_matcher.is_empty() + && state.bypass_matcher.matches(&host); + if bypassed { + None + } else { + match ext_config + .auth + .as_ref() + .map(external::build_basic_proxy_auth_header) + .transpose() + { + Ok(h) => h, + Err(e) => { + audit::log_denied( + Some(&state.audit_log), + audit::ProxyMode::ConnectIntercept, + &audit::EventContext { + route_id, + auth_mechanism: Some(nono::undo::NetworkAuditAuthMechanism::ProxyAuthorization), + auth_outcome: Some(nono::undo::NetworkAuditAuthOutcome::Failed), + denial_category: Some(nono::undo::NetworkAuditDenialCategory::AuthenticationFailed), + ..audit::EventContext::default() + }, + &host, + port, + &e.to_string(), + ); + external::send_response(&mut stream, 502, "Bad Gateway") + .await?; + return Err(e); + } + } + } + } else { + None + }; let upstream_proxy = if let Some(ref ext_config) = state.config.external_proxy { let bypassed = !state.bypass_matcher.is_empty() @@ -831,34 +872,12 @@ async fn handle_connection(mut stream: tokio::net::TcpStream, state: &ProxyState if bypassed { debug!("tls_intercept: bypassing upstream proxy for {}", host); None - } else if ext_config.auth.is_some() { - // Auth is configured but not yet implemented. - // Fail loudly rather than silently connecting - // without auth — the corporate proxy would - // reject anyway. - let msg = "external proxy authentication is configured \ - but not yet implemented; remove the auth \ - section from the external proxy config or \ - wait for a future release"; - audit::log_denied( - Some(&state.audit_log), - audit::ProxyMode::ConnectIntercept, - &audit::EventContext { - route_id, - ..audit::EventContext::default() - }, - &host, - port, - msg, - ); - let response = - "HTTP/1.1 502 Bad Gateway\r\nContent-Length: 0\r\n\r\n"; - stream.write_all(response.as_bytes()).await?; - return Err(ProxyError::ExternalProxy(msg.to_string())); } else { Some(tls_intercept::InterceptUpstreamProxy { proxy_addr: &ext_config.address, - proxy_auth_header: None, + proxy_auth_header: intercept_proxy_auth + .as_ref() + .map(|s| s.as_str()), }) } } else { diff --git a/crates/nono-proxy/tests/proxy_auth_integration.rs b/crates/nono-proxy/tests/proxy_auth_integration.rs new file mode 100644 index 000000000..374af1d26 --- /dev/null +++ b/crates/nono-proxy/tests/proxy_auth_integration.rs @@ -0,0 +1,193 @@ +#![allow(clippy::unwrap_used)] + +use std::sync::Arc; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::net::TcpListener; +use tokio::sync::Mutex; + +use nono_proxy::config::{ExternalProxyAuth, ExternalProxyConfig, ProxyConfig}; + +static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + +struct EnvGuard { + _lock: std::sync::MutexGuard<'static, ()>, + key: &'static str, +} + +impl EnvGuard { + #[allow(clippy::disallowed_methods)] + fn set(key: &'static str, value: &str) -> Self { + let lock = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + unsafe { std::env::set_var(key, value) }; + Self { _lock: lock, key } + } +} + +#[allow(clippy::disallowed_methods)] +impl Drop for EnvGuard { + fn drop(&mut self) { + unsafe { std::env::remove_var(self.key) }; + } +} + +/// TCP listener that records the first CONNECT request and replies 200. +async fn fake_proxy() -> (String, Arc>>) { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = format!("127.0.0.1:{}", listener.local_addr().unwrap().port()); + let captured: Arc>> = Arc::new(Mutex::new(Vec::new())); + let captured_clone = Arc::clone(&captured); + tokio::spawn(async move { + if let Ok((mut stream, _)) = listener.accept().await { + let mut buf = vec![0u8; 4096]; + let n = stream.read(&mut buf).await.unwrap_or(0); + *captured_clone.lock().await = buf[..n].to_vec(); + let _ = stream + .write_all(b"HTTP/1.1 200 Connection Established\r\n\r\n") + .await; + } + }); + (addr, captured) +} + +async fn connect_through_proxy(proxy_port: u16, proxy_token: &str, target: &str) -> Vec { + let mut stream = tokio::net::TcpStream::connect(format!("127.0.0.1:{}", proxy_port)) + .await + .unwrap(); + let request = format!( + "CONNECT {} HTTP/1.1\r\nHost: {}\r\nProxy-Authorization: Bearer {}\r\n\r\n", + target, target, proxy_token + ); + stream.write_all(request.as_bytes()).await.unwrap(); + let mut resp = vec![0u8; 1024]; + let _ = tokio::time::timeout( + std::time::Duration::from_millis(500), + stream.read(&mut resp), + ) + .await; + resp +} + +#[tokio::test] +async fn external_connect_sends_basic_auth_header() { + let _guard = EnvGuard::set("NONO_TEST_CORP_PASS", "s3cr3t"); + + let (fake_addr, captured) = fake_proxy().await; + let config = ProxyConfig { + external_proxy: Some(ExternalProxyConfig { + address: fake_addr, + auth: Some(ExternalProxyAuth { + username: "alice".to_string(), + keyring_account: "env://NONO_TEST_CORP_PASS".to_string(), + scheme: "basic".to_string(), + }), + bypass_hosts: vec![], + }), + ..Default::default() + }; + + let handle = nono_proxy::start(config).await.unwrap(); + connect_through_proxy(handle.port, handle.token.as_str(), "example.com:443").await; + handle.shutdown(); + + let raw = captured.lock().await; + let request = String::from_utf8_lossy(&raw); + println!("Fake proxy received:\n{}", request); + + // "alice:s3cr3t" base64 = "YWxpY2U6czNjcjN0" + assert!( + request.contains("Proxy-Authorization: Basic YWxpY2U6czNjcjN0"), + "expected Basic auth header, got:\n{}", + request + ); + assert!(request.contains("CONNECT example.com:443")); +} + +#[tokio::test] +async fn external_connect_missing_credential_returns_502() { + let (fake_addr, _) = fake_proxy().await; + let config = ProxyConfig { + external_proxy: Some(ExternalProxyConfig { + address: fake_addr, + auth: Some(ExternalProxyAuth { + username: "alice".to_string(), + keyring_account: "env://NONO_TEST_CORP_PASS_MISSING_XYZ_123".to_string(), + scheme: "basic".to_string(), + }), + bypass_hosts: vec![], + }), + ..Default::default() + }; + + let handle = nono_proxy::start(config).await.unwrap(); + let mut stream = tokio::net::TcpStream::connect(format!("127.0.0.1:{}", handle.port)) + .await + .unwrap(); + let request = format!( + "CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\nProxy-Authorization: Bearer {}\r\n\r\n", + handle.token.as_str() + ); + stream.write_all(request.as_bytes()).await.unwrap(); + let mut resp = vec![0u8; 1024]; + let n = tokio::time::timeout( + std::time::Duration::from_millis(500), + stream.read(&mut resp), + ) + .await + .unwrap() + .unwrap_or(0); + let response = String::from_utf8_lossy(&resp[..n]); + println!("Response for missing credential:\n{}", response); + handle.shutdown(); + + assert!(response.contains("502"), "expected 502, got:\n{}", response); + assert!( + !response.contains("s3cr3t"), + "password must not appear in response" + ); +} + +#[tokio::test] +async fn external_connect_unsupported_scheme_returns_502() { + let _guard = EnvGuard::set("NONO_TEST_CORP_PASS2", "s3cr3t"); + + let (fake_addr, _) = fake_proxy().await; + let config = ProxyConfig { + external_proxy: Some(ExternalProxyConfig { + address: fake_addr, + auth: Some(ExternalProxyAuth { + username: "alice".to_string(), + keyring_account: "env://NONO_TEST_CORP_PASS2".to_string(), + scheme: "ntlm".to_string(), + }), + bypass_hosts: vec![], + }), + ..Default::default() + }; + + let handle = nono_proxy::start(config).await.unwrap(); + let mut stream = tokio::net::TcpStream::connect(format!("127.0.0.1:{}", handle.port)) + .await + .unwrap(); + let request = format!( + "CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\nProxy-Authorization: Bearer {}\r\n\r\n", + handle.token.as_str() + ); + stream.write_all(request.as_bytes()).await.unwrap(); + let mut resp = vec![0u8; 1024]; + let n = tokio::time::timeout( + std::time::Duration::from_millis(500), + stream.read(&mut resp), + ) + .await + .unwrap() + .unwrap_or(0); + let response = String::from_utf8_lossy(&resp[..n]); + println!("Response for unsupported scheme:\n{}", response); + handle.shutdown(); + + assert!(response.contains("502"), "expected 502, got:\n{}", response); + assert!( + !response.contains("s3cr3t"), + "password must not appear in response" + ); +}