Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion crates/nono-proxy/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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}"#;
Expand Down
2 changes: 1 addition & 1 deletion crates/nono-proxy/src/credential.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
127 changes: 110 additions & 17 deletions crates/nono-proxy/src/external.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -163,6 +165,31 @@ impl BypassMatcher {
}
}

const KEYRING_SERVICE: &str = nono::keystore::DEFAULT_SERVICE;

/// Build a `Basic <token>` Proxy-Authorization header value for `auth`.
pub(crate) fn build_basic_proxy_auth_header(auth: &ExternalProxyAuth) -> Result<Zeroizing<String>> {
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
Expand Down Expand Up @@ -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
};
Comment thread
SequeI marked this conversation as resolved.

// 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") => {
Expand Down Expand Up @@ -339,7 +386,7 @@ fn parse_status_code(line: &str) -> Result<u16> {
}

/// 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?;
Expand All @@ -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) };
}
}
Comment thread
SequeI marked this conversation as resolved.

#[test]
fn test_parse_connect_target() {
let (host, port) = parse_connect_target("CONNECT api.openai.com:443 HTTP/1.1").unwrap();
Expand Down Expand Up @@ -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());
}
}
69 changes: 44 additions & 25 deletions crates/nono-proxy/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -824,41 +824,60 @@ 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<Zeroizing<String>> = 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()
&& state.bypass_matcher.matches(&host);
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()),
})
}
Comment on lines +827 to 882

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could simplify this, merging both blocks for intercept_proxy_auth and upstream, and computing bypassed once.

Suggested change
let intercept_proxy_auth: Option<Zeroizing<String>> = 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()
&& state.bypass_matcher.matches(&host);
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()),
})
}
let upstream_proxy =
if let Some(ref ext_config) = state.config.external_proxy {
let bypassed = !state.bypass_matcher.is_empty()
&& state.bypass_matcher.matches(&host);
if bypassed {
debug!(
"tls_intercept: bypassing upstream proxy for {}",
host
);
None
} else {
let proxy_auth_header = 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);
}
};
Some(tls_intercept::InterceptUpstreamProxy {
proxy_addr: &ext_config.address,
proxy_auth_header: proxy_auth_header
.as_ref()
.map(|s| s.as_str()),
})
}
} else {
None
};

} else {
Expand Down
Loading