Skip to content
Merged
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
5 changes: 4 additions & 1 deletion codex-rs/network-proxy/src/http_proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,10 @@ mod tests {
#[tokio::test]
async fn http_connect_accept_allows_allowlisted_host_in_full_mode() {
let policy = {
let mut policy = NetworkProxySettings::default();
let mut policy = NetworkProxySettings {
allow_local_binding: true,
..NetworkProxySettings::default()
};
policy.set_allowed_domains(vec!["example.com".to_string()]);
policy
};
Expand Down
52 changes: 46 additions & 6 deletions codex-rs/network-proxy/src/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ impl Host {
/// Returns true if the host is a loopback hostname or IP literal.
pub fn is_loopback_host(host: &Host) -> bool {
let host = host.as_str();
let host = host.split_once('%').map(|(ip, _)| ip).unwrap_or(host);
let host = unscoped_ip_literal(host).unwrap_or(host);
if host == "localhost" {
return true;
}
Expand Down Expand Up @@ -103,24 +103,48 @@ pub fn normalize_host(host: &str) -> String {
if host.starts_with('[')
&& let Some(end) = host.find(']')
{
return normalize_dns_host(&host[1..end]);
return normalize_dns_host_or_ip_literal(&host[1..end]);
}

// The proxy stack should typically hand us a host without a port, but be
// defensive and strip `:port` when there is exactly one `:`.
if host.bytes().filter(|b| *b == b':').count() == 1 {
let host = host.split(':').next().unwrap_or_default();
return normalize_dns_host(host);
return normalize_dns_host_or_ip_literal(host);
}

// Avoid mangling unbracketed IPv6 literals, but strip trailing dots so fully qualified domain
// names are treated the same as their dotless variants.
normalize_dns_host(host)
normalize_dns_host_or_ip_literal(host)
}

fn normalize_dns_host(host: &str) -> String {
fn normalize_dns_host_or_ip_literal(host: &str) -> String {
let host = host.to_ascii_lowercase();
host.trim_end_matches('.').to_string()
let host = host.trim_end_matches('.');
if let Some(ip) = normalize_ip_literal(host) {
return ip;
}
host.to_string()
}

pub(crate) fn unscoped_ip_literal(host: &str) -> Option<&str> {
let (ip, _) = host.split_once('%')?;
ip.parse::<IpAddr>().ok()?;
Some(ip)
}

fn normalize_ip_literal(host: &str) -> Option<String> {
if host.parse::<IpAddr>().is_ok() {
return Some(host.to_string());
}
for delimiter in ["%25", "%"] {
if let Some((ip, scope)) = host.split_once(delimiter)
&& ip.parse::<IpAddr>().is_ok()
{
return Some(format!("{ip}%{scope}"));
}
}
None
}

fn normalize_pattern(pattern: &str) -> String {
Expand Down Expand Up @@ -392,6 +416,15 @@ mod tests {
assert_eq!(true, set.is_match("::1"));
}

#[test]
fn compile_globset_preserves_scoped_ipv6_literals() {
let set = compile_denylist_globset(&["[fe80::1%25lo0]".to_string()]).unwrap();

assert_eq!(true, set.is_match("fe80::1%lo0"));
assert_eq!(false, set.is_match("fe80::1%lo1"));
assert_eq!(false, set.is_match("fe80::1"));
}

#[test]
fn is_loopback_host_handles_localhost_variants() {
assert!(is_loopback_host(&Host::parse("localhost").unwrap()));
Expand Down Expand Up @@ -462,4 +495,11 @@ mod tests {
assert_eq!(normalize_host("[::1]"), "::1");
assert_eq!(normalize_host("[::1]:443"), "::1");
}

#[test]
fn normalize_host_preserves_ipv6_scope_ids() {
assert_eq!(normalize_host("fe80::1%lo0"), "fe80::1%lo0");
assert_eq!(normalize_host("[fe80::1%lo0]"), "fe80::1%lo0");
assert_eq!(normalize_host("[fe80::1%25lo0]"), "fe80::1%lo0");
}
}
21 changes: 11 additions & 10 deletions codex-rs/network-proxy/src/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -790,9 +790,16 @@ mod tests {

#[tokio::test]
async fn managed_proxy_builder_uses_loopback_ports() {
let http_listener = StdTcpListener::bind(SocketAddr::from(([127, 0, 0, 1], 0))).unwrap();
let http_addr = http_listener.local_addr().unwrap();
let socks_listener = StdTcpListener::bind(SocketAddr::from(([127, 0, 0, 1], 0))).unwrap();
let socks_addr = socks_listener.local_addr().unwrap();
drop(http_listener);
drop(socks_listener);

let state = Arc::new(network_proxy_state_for_policy(NetworkProxySettings {
proxy_url: "http://127.0.0.1:43128".to_string(),
socks_url: "http://127.0.0.1:48081".to_string(),
proxy_url: format!("http://{http_addr}"),
socks_url: format!("http://{socks_addr}"),
..NetworkProxySettings::default()
}));
let proxy = match NetworkProxy::builder().state(state).build().await {
Expand All @@ -812,14 +819,8 @@ mod tests {
assert!(proxy.socks_addr.ip().is_loopback());
#[cfg(target_os = "windows")]
{
assert_eq!(
proxy.http_addr,
"127.0.0.1:43128".parse::<SocketAddr>().unwrap()
);
assert_eq!(
proxy.socks_addr,
"127.0.0.1:48081".parse::<SocketAddr>().unwrap()
);
assert_eq!(proxy.http_addr, http_addr);
assert_eq!(proxy.socks_addr, socks_addr);
}
#[cfg(not(target_os = "windows"))]
{
Expand Down
83 changes: 75 additions & 8 deletions codex-rs/network-proxy/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::policy::Host;
use crate::policy::is_loopback_host;
use crate::policy::is_non_public_ip;
use crate::policy::normalize_host;
use crate::policy::unscoped_ip_literal;
use crate::reasons::REASON_DENIED;
use crate::reasons::REASON_NOT_ALLOWED;
use crate::reasons::REASON_NOT_ALLOWED_LOCAL;
Expand Down Expand Up @@ -371,11 +372,11 @@ impl NetworkProxyState {
// 1) explicit deny always wins
// 2) local/private networking is opt-in (defense-in-depth)
// 3) allowlist is enforced when configured
if deny_set.is_match(host_str) {
if globset_matches_host_or_unscoped(&deny_set, host_str) {
return Ok(HostBlockDecision::Blocked(HostBlockReason::Denied));
}

let is_allowlisted = allow_set.is_match(host_str);
let is_allowlisted = globset_matches_host_or_unscoped(&allow_set, host_str);
if !allow_local_binding {
// If the intent is "prevent access to local/internal networks", we must not rely solely
// on string checks like `localhost` / `127.0.0.1`. Attackers can use DNS rebinding or
Expand All @@ -386,10 +387,7 @@ impl NetworkProxyState {
// allowlisted; hostnames that resolve to local/private IPs are blocked even if
// allowlisted.
let local_literal = {
let host_no_scope = host_str
.split_once('%')
.map(|(ip, _)| ip)
.unwrap_or(host_str);
let host_no_scope = unscoped_ip_literal(host_str).unwrap_or(host_str);
if is_loopback_host(&host) {
true
} else if let Ok(ip) = host_no_scope.parse::<IpAddr>() {
Expand Down Expand Up @@ -797,8 +795,13 @@ fn log_domain_list_changes(list_name: &str, previous: &[String], next: &[String]
}
}

fn globset_matches_host_or_unscoped(set: &GlobSet, host: &str) -> bool {
set.is_match(host) || unscoped_ip_literal(host).is_some_and(|ip| set.is_match(ip))
}

fn is_explicit_local_allowlisted(allowed_domains: &[String], host: &Host) -> bool {
let normalized_host = host.as_str();
let unscoped_host = unscoped_ip_literal(normalized_host);
allowed_domains.iter().any(|pattern| {
let pattern = pattern.trim();
if pattern == "*" || pattern.starts_with("*.") || pattern.starts_with("**.") {
Expand All @@ -807,7 +810,9 @@ fn is_explicit_local_allowlisted(allowed_domains: &[String], host: &Host) -> boo
if pattern.contains('*') || pattern.contains('?') {
return false;
}
normalize_host(pattern) == normalized_host
let normalized_pattern = normalize_host(pattern);
normalized_pattern == normalized_host
|| unscoped_host.is_some_and(|ip| normalized_pattern == ip)
})
}

Expand Down Expand Up @@ -1247,7 +1252,7 @@ mod tests {

#[tokio::test]
async fn host_blocked_allows_scoped_ipv6_literal_when_explicitly_allowlisted() {
let state = network_proxy_state_for_policy(network_settings(&["fe80::1%lo0"], &[]));
let state = network_proxy_state_for_policy(network_settings(&["fe80::1"], &[]));

assert_eq!(
state
Expand All @@ -1258,6 +1263,68 @@ mod tests {
);
}

#[tokio::test]
async fn host_blocked_requires_exact_scoped_ipv6_allowlist_match() {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allow_local_binding: true,
..network_settings(&["fe80::1%eth0"], &[])
});

assert_eq!(
state
.host_blocked("fe80::1%eth0", /*port*/ 80)
.await
.unwrap(),
HostBlockDecision::Allowed
);
assert_eq!(
state
.host_blocked("fe80::1%eth1", /*port*/ 80)
.await
.unwrap(),
HostBlockDecision::Blocked(HostBlockReason::NotAllowed)
);
}

#[tokio::test]
async fn host_blocked_denies_scoped_ipv6_literal_before_local_binding() {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allow_local_binding: true,
..network_settings(&["*"], &["fd00::1"])
});

for host in ["fd00::1%eth0", "[fd00::1%eth0]", "[fd00::1%25eth0]"] {
assert_eq!(
state.host_blocked(host, /*port*/ 80).await.unwrap(),
HostBlockDecision::Blocked(HostBlockReason::Denied),
"host should be denied after normalization: {host}"
);
}
}

#[tokio::test]
async fn host_blocked_requires_exact_scoped_ipv6_denylist_match() {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allow_local_binding: true,
..network_settings(&["*"], &["fd00::1%eth0"])
});

assert_eq!(
state
.host_blocked("fd00::1%eth0", /*port*/ 80)
.await
.unwrap(),
HostBlockDecision::Blocked(HostBlockReason::Denied)
);
assert_eq!(
state
.host_blocked("fd00::1%eth1", /*port*/ 80)
.await
.unwrap(),
HostBlockDecision::Allowed
);
}

#[tokio::test]
async fn host_blocked_rejects_private_ip_literals_when_local_binding_disabled() {
let state = network_proxy_state_for_policy(network_settings(&["example.com"], &[]));
Expand Down
Loading