mirror of
https://github.com/openai/codex.git
synced 2026-04-29 02:41:12 +03:00
fix(network-proxy): fail closed on network-proxy DNS lookup errors (#15909)
## Summary Fail closed when the network proxy's local/private IP pre-check hits a DNS lookup error or timeout, instead of treating the hostname as public and allowing the request. ## Root cause `host_resolves_to_non_public_ip()` returned `false` on resolver failure, which created a fail-open path in the `allow_local_binding = false` boundary. The eventual connect path performs its own DNS resolution later, so a transient pre-check failure is not evidence that the destination is public. ## Changes - Treat DNS lookup errors/timeouts as local/private for blocking purposes - Add a regression test for an allowlisted hostname that fails DNS resolution ## Validation - `cargo test -p codex-network-proxy` - `cargo clippy -p codex-network-proxy --all-targets -- -D warnings` - `just fmt` - `just argument-comment-lint`
This commit is contained in:
@@ -698,12 +698,22 @@ async fn host_resolves_to_non_public_ip(host: &str, port: u16) -> bool {
|
||||
return is_non_public_ip(ip);
|
||||
}
|
||||
|
||||
// If DNS lookup fails, default to "not local/private" rather than blocking. In practice, the
|
||||
// subsequent connect attempt will fail anyway, and blocking on transient resolver issues would
|
||||
// make the proxy fragile. The allowlist/denylist remains the primary control plane.
|
||||
// Block the request if this DNS lookup fails. We resolve the hostname again when we connect,
|
||||
// so a failed check here does not prove the destination is public.
|
||||
let addrs = match timeout(DNS_LOOKUP_TIMEOUT, lookup_host((host, port))).await {
|
||||
Ok(Ok(addrs)) => addrs,
|
||||
Ok(Err(_)) | Err(_) => return false,
|
||||
Ok(Err(err)) => {
|
||||
debug!(
|
||||
"blocking host because DNS lookup failed during local/private IP check (host={host}, port={port}): {err}"
|
||||
);
|
||||
return true;
|
||||
}
|
||||
Err(_) => {
|
||||
debug!(
|
||||
"blocking host because DNS lookup timed out during local/private IP check (host={host}, port={port})"
|
||||
);
|
||||
return true;
|
||||
}
|
||||
};
|
||||
|
||||
for addr in addrs {
|
||||
@@ -903,17 +913,18 @@ mod tests {
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
state.host_blocked("evil.example", 80).await.unwrap(),
|
||||
// Use a public IP literal to avoid relying on ambient DNS behavior.
|
||||
state.host_blocked("8.8.8.8", 80).await.unwrap(),
|
||||
HostBlockDecision::Allowed
|
||||
);
|
||||
|
||||
state.add_denied_domain("evil.example").await.unwrap();
|
||||
state.add_denied_domain("8.8.8.8").await.unwrap();
|
||||
|
||||
let (allowed, denied) = state.current_patterns().await.unwrap();
|
||||
assert_eq!(allowed, vec!["*".to_string()]);
|
||||
assert_eq!(denied, vec!["evil.example".to_string()]);
|
||||
assert_eq!(denied, vec!["8.8.8.8".to_string()]);
|
||||
assert_eq!(
|
||||
state.host_blocked("evil.example", 80).await.unwrap(),
|
||||
state.host_blocked("8.8.8.8", 80).await.unwrap(),
|
||||
HostBlockDecision::Blocked(HostBlockReason::Denied)
|
||||
);
|
||||
}
|
||||
@@ -1237,6 +1248,23 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn host_blocked_rejects_allowlisted_hostname_when_dns_lookup_fails() {
|
||||
let state = network_proxy_state_for_policy(NetworkProxySettings {
|
||||
allowed_domains: vec!["does-not-resolve.invalid".to_string()],
|
||||
allow_local_binding: false,
|
||||
..NetworkProxySettings::default()
|
||||
});
|
||||
|
||||
assert_eq!(
|
||||
state
|
||||
.host_blocked("does-not-resolve.invalid", 80)
|
||||
.await
|
||||
.unwrap(),
|
||||
HostBlockDecision::Blocked(HostBlockReason::NotAllowedLocal)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_policy_against_constraints_disallows_widening_allowed_domains() {
|
||||
let constraints = NetworkProxyConstraints {
|
||||
|
||||
Reference in New Issue
Block a user