Skip to content

Commit 7c9abd3

Browse files
anugrahsinghalSequeI
anugrahsinghal
and
SequeI
authored
fix(proxy): keep connection open for reactive proxy auth on CONNECT (#1151)
* fix(proxy): keep connection open for reactive proxy auth on CONNECT The intercept CONNECT path replied 407 then returned, dropping the socket, so clients that authenticate reactively (CONNECT -> 407 -> retry with Proxy-Authorization on the same connection) failed on the retry. Keep the connection open across the 407 and re-read the retried CONNECT, re-validating on the same socket. The retry must target the same authority; client disconnects and oversized heads still close the connection. Signed-off-by: Anugrah Singhal <anugrahsinghal1@gmail.com> * fix(proxy): normalize retried CONNECT authority before comparison Hostnames are case-insensitive and the CONNECT port may be implicit (RFC 9110), so comparing the retried authority raw against the original raw authority wrongly rejected legitimate retries that differed only in hostname casing or explicit/implicit :443 (e.g. API.OPENAI.COM:443 vs api.openai.com). Normalize the retried authority via the shared normalize_authority helper and compare against the already-normalized host_port, matching the initial parse. Addresses the review comment on PR #1151. Signed-off-by: Anugrah Singhal <anugrahsinghal1@gmail.com> --------- Signed-off-by: Anugrah Singhal <anugrahsinghal1@gmail.com> Co-authored-by: Aleks <121458075+SequeI@users.noreply.github.com>
1 parent a793fe2 commit 7c9abd3

1 file changed

Lines changed: 209 additions & 43 deletions

File tree

‎crates/nono-proxy/src/server.rs‎

Lines changed: 209 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,23 @@ async fn accept_loop(
614614
}
615615
}
616616

617+
/// Normalise a CONNECT authority to lowercase `host:port`, defaulting the port
618+
/// to 443 when absent. Handles IPv6 brackets: `[::1]:443` already has a port,
619+
/// `[::1]` needs the default, `host:443` has a port.
620+
fn normalize_authority(authority: &str) -> String {
621+
if authority.starts_with('[') {
622+
if authority.contains("]:") {
623+
authority.to_lowercase()
624+
} else {
625+
format!("{}:443", authority.to_lowercase())
626+
}
627+
} else if authority.contains(':') {
628+
authority.to_lowercase()
629+
} else {
630+
format!("{}:443", authority.to_lowercase())
631+
}
632+
}
633+
617634
/// Handle a single client connection.
618635
///
619636
/// Reads the first HTTP line to determine the proxy mode:
@@ -677,19 +694,7 @@ async fn handle_connection(mut stream: tokio::net::TcpStream, state: &ProxyState
677694
if !state.route_store.is_empty()
678695
&& let Some(authority) = first_line.split_whitespace().nth(1)
679696
{
680-
// Normalise authority to host:port. Handle IPv6 brackets:
681-
// "[::1]:443" already has port, "[::1]" needs default, "host:443" has port.
682-
let host_port = if authority.starts_with('[') {
683-
if authority.contains("]:") {
684-
authority.to_lowercase()
685-
} else {
686-
format!("{}:443", authority.to_lowercase())
687-
}
688-
} else if authority.contains(':') {
689-
authority.to_lowercase()
690-
} else {
691-
format!("{}:443", authority.to_lowercase())
692-
};
697+
let host_port = normalize_authority(authority);
693698

694699
if state.route_store.is_route_upstream(&host_port) {
695700
let route_id = state
@@ -710,36 +715,89 @@ async fn handle_connection(mut stream: tokio::net::TcpStream, state: &ProxyState
710715
// (we mint a leaf cert and decrypt traffic), so
711716
// unlike the lenient transparent-tunnel path we
712717
// require Proxy-Authorization here.
713-
if let Err(e) =
714-
token::validate_proxy_auth(&header_bytes, &state.session_token)
715-
{
716-
debug!(
717-
"tls_intercept: rejecting CONNECT to {}:{} — {}",
718-
host, port, e
719-
);
720-
audit::log_denied(
721-
Some(&state.audit_log),
722-
audit::ProxyMode::ConnectIntercept,
723-
&audit::EventContext {
724-
route_id,
725-
auth_mechanism: Some(
726-
nono::undo::NetworkAuditAuthMechanism::ProxyAuthorization,
727-
),
728-
auth_outcome: Some(
729-
nono::undo::NetworkAuditAuthOutcome::Failed,
730-
),
731-
denial_category: Some(
732-
nono::undo::NetworkAuditDenialCategory::AuthenticationFailed,
733-
),
734-
..audit::EventContext::default()
735-
},
736-
&host,
737-
port,
738-
"proxy auth missing or invalid",
739-
);
740-
let response = "HTTP/1.1 407 Proxy Authentication Required\r\nProxy-Authenticate: Basic realm=\"nono\"\r\nContent-Length: 0\r\n\r\n";
741-
stream.write_all(response.as_bytes()).await?;
742-
return Ok(());
718+
// Reactive proxy auth (RFC 7235 / RFC 9110 §15.5.8): a
719+
// client may send the first CONNECT without credentials,
720+
// receive the 407 challenge, then retry the CONNECT with
721+
// Proxy-Authorization on the SAME connection. Keep the
722+
// connection open across the 407 and re-read the retried
723+
// request head rather than dropping the socket — closing
724+
// it breaks reactive clients (Apache HttpClient, Java's
725+
// HttpClient, Maven's native resolver).
726+
let mut current_headers = header_bytes;
727+
loop {
728+
match token::validate_proxy_auth(&current_headers, &state.session_token)
729+
{
730+
Ok(()) => break,
731+
Err(e) => {
732+
debug!(
733+
"tls_intercept: CONNECT to {}:{} missing/invalid proxy auth — {}",
734+
host, port, e
735+
);
736+
audit::log_denied(
737+
Some(&state.audit_log),
738+
audit::ProxyMode::ConnectIntercept,
739+
&audit::EventContext {
740+
route_id,
741+
auth_mechanism: Some(
742+
nono::undo::NetworkAuditAuthMechanism::ProxyAuthorization,
743+
),
744+
auth_outcome: Some(
745+
nono::undo::NetworkAuditAuthOutcome::Failed,
746+
),
747+
denial_category: Some(
748+
nono::undo::NetworkAuditDenialCategory::AuthenticationFailed,
749+
),
750+
..audit::EventContext::default()
751+
},
752+
&host,
753+
port,
754+
"proxy auth missing or invalid",
755+
);
756+
let response = "HTTP/1.1 407 Proxy Authentication Required\r\nProxy-Authenticate: Basic realm=\"nono\"\r\nContent-Length: 0\r\n\r\n";
757+
stream.write_all(response.as_bytes()).await?;
758+
759+
// Read the client's retried request head on
760+
// the same connection.
761+
let mut buf_reader = BufReader::new(&mut stream);
762+
let mut retry_line = String::new();
763+
buf_reader.read_line(&mut retry_line).await?;
764+
if retry_line.is_empty() {
765+
return Ok(()); // client disconnected
766+
}
767+
let mut retry_headers = Vec::new();
768+
loop {
769+
let mut line = String::new();
770+
let n = buf_reader.read_line(&mut line).await?;
771+
if n == 0 || line.trim().is_empty() {
772+
break;
773+
}
774+
retry_headers.extend_from_slice(line.as_bytes());
775+
if retry_headers.len() > MAX_HEADER_SIZE {
776+
drop(buf_reader);
777+
let too_large = "HTTP/1.1 431 Request Header Fields Too Large\r\n\r\n";
778+
stream.write_all(too_large.as_bytes()).await?;
779+
return Ok(());
780+
}
781+
}
782+
drop(buf_reader);
783+
784+
// host/port/route are reused from the first
785+
// CONNECT, so the retry must target the same
786+
// authority; anything else (or a non-CONNECT
787+
// request) would desync routing.
788+
let same_authority = retry_line
789+
.trim_end()
790+
.strip_prefix("CONNECT ")
791+
.and_then(|rest| rest.split_whitespace().next())
792+
.map(normalize_authority)
793+
.as_deref()
794+
== Some(host_port.as_str());
795+
if !same_authority {
796+
return Ok(());
797+
}
798+
current_headers = retry_headers;
799+
}
800+
}
743801
}
744802

745803
let ctx = tls_intercept::InterceptCtx {
@@ -874,6 +932,26 @@ async fn handle_connection(mut stream: tokio::net::TcpStream, state: &ProxyState
874932
mod tests {
875933
use super::*;
876934

935+
#[test]
936+
fn normalize_authority_normalises_case_and_default_port() {
937+
assert_eq!(normalize_authority("API.OpenAI.com"), "api.openai.com:443");
938+
assert_eq!(
939+
normalize_authority("api.openai.com:443"),
940+
"api.openai.com:443"
941+
);
942+
assert_eq!(
943+
normalize_authority("api.openai.com:8443"),
944+
"api.openai.com:8443"
945+
);
946+
assert_eq!(normalize_authority("[::1]"), "[::1]:443");
947+
assert_eq!(normalize_authority("[::1]:8443"), "[::1]:8443");
948+
// case- and port-insensitive equality is the point of the retry guard
949+
assert_eq!(
950+
normalize_authority("API.OPENAI.COM:443"),
951+
normalize_authority("api.openai.com")
952+
);
953+
}
954+
877955
#[tokio::test]
878956
async fn test_proxy_starts_and_binds() {
879957
let config = ProxyConfig::default();
@@ -1691,4 +1769,92 @@ mod tests {
16911769

16921770
handle.shutdown();
16931771
}
1772+
1773+
/// Regression test for reactive proxy auth on the intercept CONNECT path.
1774+
/// After a 407 the proxy must keep the connection open and answer the
1775+
/// client's credentialed retry on the same socket, rather than closing it
1776+
/// (which breaks reactive clients such as Apache HttpClient / Maven's
1777+
/// native resolver).
1778+
#[tokio::test]
1779+
async fn reactive_proxy_auth_retry_answered_after_407() {
1780+
use base64::Engine;
1781+
use std::time::Duration;
1782+
use tokio::io::{AsyncReadExt, AsyncWriteExt};
1783+
use tokio::net::TcpStream;
1784+
1785+
let dir = tempfile::tempdir().unwrap();
1786+
let config = ProxyConfig {
1787+
routes: vec![crate::config::RouteConfig {
1788+
prefix: "openai".to_string(),
1789+
upstream: "https://api.openai.com".to_string(),
1790+
credential_key: Some("env://NONO_TEST_TOTALLY_MISSING".to_string()),
1791+
inject_mode: Default::default(),
1792+
inject_header: "Authorization".to_string(),
1793+
credential_format: Some("Bearer {}".to_string()),
1794+
path_pattern: None,
1795+
path_replacement: None,
1796+
query_param_name: None,
1797+
proxy: None,
1798+
env_var: None,
1799+
endpoint_rules: vec![],
1800+
tls_ca: None,
1801+
tls_client_cert: None,
1802+
tls_client_key: None,
1803+
oauth2: None,
1804+
}],
1805+
intercept_ca_dir: Some(dir.path().to_path_buf()),
1806+
..Default::default()
1807+
};
1808+
let handle = start(config).await.unwrap();
1809+
assert!(
1810+
handle.intercept_ca_path().is_some(),
1811+
"precondition: interception must be active so the 407 path is reached"
1812+
);
1813+
let port = handle.port;
1814+
let token = handle.token.to_string();
1815+
1816+
let mut sock = TcpStream::connect(("127.0.0.1", port)).await.unwrap();
1817+
1818+
// 1) Unauthenticated CONNECT -> expect a 407 challenge.
1819+
sock.write_all(b"CONNECT api.openai.com:443 HTTP/1.1\r\nHost: api.openai.com:443\r\n\r\n")
1820+
.await
1821+
.unwrap();
1822+
sock.flush().await.unwrap();
1823+
1824+
let mut buf = [0u8; 4096];
1825+
let n = sock.read(&mut buf).await.unwrap();
1826+
let response = String::from_utf8_lossy(&buf[..n]);
1827+
assert!(
1828+
response.starts_with("HTTP/1.1 407 "),
1829+
"expected 407 challenge, got: {:?}",
1830+
response
1831+
);
1832+
1833+
// 2) Reactive retry WITH valid credentials on the SAME socket.
1834+
let creds = base64::engine::general_purpose::STANDARD.encode(format!("nono:{}", token));
1835+
let retry = format!(
1836+
"CONNECT api.openai.com:443 HTTP/1.1\r\nHost: api.openai.com:443\r\nProxy-Authorization: Basic {}\r\n\r\n",
1837+
creds
1838+
);
1839+
sock.write_all(retry.as_bytes()).await.unwrap();
1840+
sock.flush().await.unwrap();
1841+
1842+
// 3) The proxy must answer the retried CONNECT on the same socket
1843+
// instead of returning EOF. (The upstream connect to api.openai.com
1844+
// may fail in the test env, so we require a response, not a 200.)
1845+
let mut retry_buf = [0u8; 4096];
1846+
let read_result =
1847+
tokio::time::timeout(Duration::from_secs(5), sock.read(&mut retry_buf)).await;
1848+
match read_result {
1849+
Ok(Ok(0)) => panic!(
1850+
"regression: proxy closed the socket after the 407 instead of \
1851+
answering the reactive retry"
1852+
),
1853+
Ok(Ok(_)) => {} // answered -> reactive auth handled
1854+
Ok(Err(e)) => panic!("retry read errored: {e}"),
1855+
Err(_) => panic!("retry read timed out — proxy did not answer the retry"),
1856+
}
1857+
1858+
handle.shutdown();
1859+
}
16941860
}

0 commit comments

Comments
 (0)