openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-08-23 13:09:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

Harden network proxy MITM authorization (#37211)

Browse Source 
## Why

MITM hooks authorize requests before the upstream server parses them. Paths that
can be decoded or normalized to a different resource must not match an allowed
path, and hosts that require MITM inspection must not bypass it through the
plain HTTP proxy path.

## What changed

- Reject ambiguous hook paths, including traversal segments, backslashes,
  malformed percent encodings, and encoded separators or percent signs.
- Block plain HTTP proxy requests for hosts whose policy always requires MITM,
  recording the decision as `mitm_required`.

## Testing

- Cover safe and ambiguous path forms, encoded traversal through repository
  allowlists, and absolute-form HTTPS requests sent to the HTTP proxy.

GitOrigin-RevId: 8812a980ac64a97cbac3a237376d29be5ded9220
This commit is contained in:
kevinlin-openai
2026-08-06 03:48:54 +00:00
committed by copyberry
parent 1ae82ce6a5
commit 7a0e974e08
6 changed files with 353 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

64
codex-rs/network-proxy/src/authorization_path.rs Normal file
View File

@@ -0,0 +1,64 @@
/// Returns whether `path` has an unambiguous interpretation for authorization.
///
/// MITM hooks authorize the request before the upstream server parses it. Reject
/// path forms that common upstreams may decode or normalize into a different
/// resource after a hook has matched.
pub(crate) fn is_safe_for_authorization(path: &str) -> bool {
path.split('/').all(is_safe_segment_for_authorization)
}
fn is_safe_segment_for_authorization(segment: &str) -> bool {
let bytes = segment.as_bytes();
let mut index = 0;
let mut decoded_dots = 0;
let mut has_non_dot = false;
while index < bytes.len() {
match bytes[index] {
b'.' => {
decoded_dots += 1;
index += 1;
}
b'\\' => return false,
b'%' => {
let Some(high) = bytes
.get(index + 1)
.and_then(|byte| decode_hex_digit(*byte))
else {
return false;
};
let Some(low) = bytes
.get(index + 2)
.and_then(|byte| decode_hex_digit(*byte))
else {
return false;
};
let decoded = high << 4 | low;
match decoded {
b'%' | b'/' | b'\\' => return false,
b'.' => decoded_dots += 1,
_ => has_non_dot = true,
}
index += 3;
}
_ => {
has_non_dot = true;
index += 1;
}
}
}
has_non_dot || !matches!(decoded_dots, 1 | 2)
}
fn decode_hex_digit(byte: u8) -> Option<u8> {
match byte {
b'0'..=b'9' => Some(byte - b'0'),
b'a'..=b'f' => Some(byte - b'a' + 10),
b'A'..=b'F' => Some(byte - b'A' + 10),
_ => None,
}
}
#[cfg(test)]
#[path = "authorization_path_tests.rs"]
mod tests;

46
codex-rs/network-proxy/src/authorization_path_tests.rs Normal file
View File

@@ -0,0 +1,46 @@
use super::is_safe_for_authorization;
use pretty_assertions::assert_eq;
#[test]
fn accepts_unambiguous_paths() {
let paths = [
"/openai/openai",
"/openai/openai/issues/123",
"/openai/openai/a..b",
"/openai/openai/%20space",
"/openai/openai/%E2%9C%93",
"/openai/openai/contents/%2Egitignore",
"/openai/openai/contents/.%2Egithub",
"/openai/openai/%2e%2efoo",
"/openai/openai/%2e%2e%2e",
];
assert_eq!(
paths.map(is_safe_for_authorization),
[true, true, true, true, true, true, true, true, true]
);
}
#[test]
fn rejects_paths_with_ambiguous_segments_or_encodings() {
let paths = [
"/openai/openai/../codex",
"/openai/openai/./issues",
"/openai/openai\\..\\codex",
"/openai/openai/%2e%2e/codex",
"/openai/openai/%2E%2E/codex",
"/openai/openai/%2f..%2fcodex",
"/openai/openai/%5c..%5ccodex",
"/openai/openai/%252e%252e/codex",
"/openai/openai/%",
"/openai/openai/%2",
"/openai/openai/%zz",
];
assert_eq!(
paths.map(is_safe_for_authorization),
[
false, false, false, false, false, false, false, false, false, false, false
]
);
}

129
codex-rs/network-proxy/src/http_proxy.rs
View File

@@ -758,6 +758,54 @@ async fn http_plain_proxy(
}
}
let host_mitm_requirement = match app_state.host_mitm_requirement(&host).await {
Ok(requirement) => requirement,
Err(err) => {
return Ok(internal_error("failed to inspect MITM requirements", err));
}
};
if host_mitm_requirement == HostMitmRequirement::Always {
emit_http_block_decision_audit_event(
&app_state,
BlockDecisionAuditEventArgs {
source: NetworkDecisionSource::ModeGuard,
reason: REASON_MITM_REQUIRED,
protocol: NetworkProtocol::Http,
server_address: host.as_str(),
server_port: port,
method: Some(req.method().as_str()),
client_addr: client.as_deref(),
},
);
let details = PolicyDecisionDetails {
decision: NetworkPolicyDecision::Deny,
reason: REASON_MITM_REQUIRED,
source: NetworkDecisionSource::ModeGuard,
protocol: NetworkProtocol::Http,
host: &host,
port,
};
let _ = app_state
.record_blocked(BlockedRequest::new(BlockedRequestArgs {
host: host.clone(),
reason: REASON_MITM_REQUIRED.to_string(),
client: client.clone(),
method: Some(req.method().as_str().to_string()),
mode: None,
protocol: "http".to_string(),
decision: Some(details.decision.as_str().to_string()),
source: Some(details.source.as_str().to_string()),
port: Some(port),
}))
.await;
let client = client.as_deref().unwrap_or_default();
warn!(
"request blocked; MITM required to enforce host policy (client={client}, host={host}, method={})",
req.method()
);
return Ok(json_blocked(&host, REASON_MITM_REQUIRED, Some(&details)));
}
if !method_allowed {
emit_http_block_decision_audit_event(
&app_state,
@@ -1418,6 +1466,87 @@ mod tests {
target_task.await.expect("target task should finish");
}
#[tokio::test]
async fn http_proxy_blocks_absolute_form_https_for_hooked_host() {
let target_listener = TokioTcpListener::bind((Ipv4Addr::LOCALHOST, 0))
.await
.expect("target listener should bind");
let target_addr = target_listener
.local_addr()
.expect("target listener should expose local addr");
let target_task = tokio::spawn(async move {
timeout(Duration::from_secs(1), target_listener.accept())
.await
.is_ok()
});
let state = Arc::new(network_proxy_state_for_policy({
let mut network = NetworkProxyConfig {
allow_local_binding: true,
mitm: true,
mitm_hooks: vec![crate::mitm_hook::MitmHookConfig {
host: "127.0.0.1".to_string(),
matcher: crate::mitm_hook::MitmHookMatchConfig {
methods: vec!["GET".to_string()],
path_prefixes: vec!["/repos/openai/ALLOWED".to_string()],
..crate::mitm_hook::MitmHookMatchConfig::default()
},
actions: crate::mitm_hook::MitmHookActionsConfig::default(),
}],
..NetworkProxyConfig::default()
};
network.set_allowed_domains(vec!["127.0.0.1".to_string()]);
network
}));
let listener =
StdTcpListener::bind((Ipv4Addr::LOCALHOST, 0)).expect("proxy listener should bind");
let proxy_addr = listener
.local_addr()
.expect("proxy listener should expose local addr");
let proxy_task = tokio::spawn(run_http_proxy_with_std_listener(
state.clone(),
listener,
/*policy_decider*/ None,
/*environment_id*/ None,
));
let mut stream = tokio::net::TcpStream::connect(proxy_addr)
.await
.expect("client should connect to proxy");
let request = format!(
"GET https://127.0.0.1:{port}/repos/openai/UNAUTHORIZED HTTP/1.1\r\nHost: 127.0.0.1:{port}\r\nConnection: close\r\n\r\n",
port = target_addr.port()
);
stream
.write_all(request.as_bytes())
.await
.expect("client should write absolute-form HTTPS request");
let mut buf = [0_u8; 512];
let bytes_read = timeout(Duration::from_secs(2), stream.read(&mut buf))
.await
.expect("proxy should respond before timeout")
.expect("client should read proxy response");
let response = String::from_utf8_lossy(&buf[..bytes_read]);
assert!(
response.starts_with("HTTP/1.1 403 Forbidden\r\n"),
"unexpected proxy response: {response:?}"
);
assert!(response.contains("x-proxy-error: blocked-by-mitm-required\r\n"));
assert!(
!target_task.await.expect("target task should finish"),
"blocked request must not reach upstream"
);
let blocked = state.drain_blocked().await.unwrap();
assert_eq!(blocked.len(), 1);
assert_eq!(blocked[0].reason, REASON_MITM_REQUIRED);
drop(stream);
proxy_task.abort();
let _ = proxy_task.await;
}
#[tokio::test(flavor = "current_thread")]
async fn http_plain_proxy_blocks_unix_socket_when_method_not_allowed() {
let state = Arc::new(network_proxy_state_for_policy(NetworkProxyConfig::default()));

1
codex-rs/network-proxy/src/lib.rs
View File

@@ -1,6 +1,7 @@
#![deny(clippy::print_stdout, clippy::print_stderr)]
mod attribution;
mod authorization_path;
mod certs;
mod config;
mod connect_policy;

43
codex-rs/network-proxy/src/mitm_hook.rs
View File

@@ -1,5 +1,6 @@
#![cfg_attr(not(test), allow(dead_code))]
use crate::authorization_path::is_safe_for_authorization;
use crate::config::NetworkProxyConfig;
use crate::policy::normalize_host;
use anyhow::Context as _;
@@ -392,7 +393,7 @@ fn hook_matches(hook: &MitmHook, req: &Request) -> bool {
}
let path = req.uri().path();
if !path_matches(&hook.matcher.path_prefixes, path) {
if !is_safe_for_authorization(path) || !path_matches(&hook.matcher.path_prefixes, path) {
return false;
}
@@ -918,6 +919,46 @@ mod tests {
);
}
#[test]
fn evaluate_rejects_paths_that_upstream_may_normalize() {
let mut config = base_config();
let mut hook = github_hook();
hook.matcher.methods = vec!["GET".to_string()];
hook.matcher.path_prefixes = vec!["pattern:/openai/openai/**".to_string()];
config.mitm_hooks = vec![hook];
let hooks = compile_mitm_hooks_with_resolvers(
&config,
|_| Some("abc".to_string()),
|_| Err(anyhow!("unexpected file lookup")),
)
.unwrap();
let paths = [
"/openai/openai/../codex",
"/openai/openai/%2e%2e/codex",
"/openai/openai/%2E%2E/codex",
"/openai/openai/.%2e/codex",
"/openai/openai/%2e./codex",
"/openai/openai/%252e%252e/codex",
"/openai/openai/%2f..%2fcodex",
"/openai/openai/%5c..%5ccodex",
"/openai/openai/%2e%2e/%2e%2e/microsoft/vscode",
];
let actual = paths
.iter()
.map(|path| {
let req = Request::builder()
.method(Method::GET)
.uri(*path)
.body(Body::empty())
.unwrap();
evaluate_mitm_hooks(&hooks, "api.github.com", &req)
})
.collect::<Vec<_>>();
assert_eq!(actual, vec![HookEvaluation::HookedHostNoMatch; paths.len()]);
}
#[test]
fn evaluate_treats_glob_metacharacters_as_literal_without_glob_prefix() {
let mut config = base_config();

71
codex-rs/network-proxy/src/mitm_tests.rs
View File

@@ -277,6 +277,77 @@ async fn mitm_policy_allows_matching_hooked_write_in_full_mode() {
assert_eq!(app_state.blocked_snapshot().await.unwrap().len(), 0);
}
#[tokio::test]
async fn mitm_policy_blocks_encoded_path_traversal_for_repository_allowlist() {
let mut hook = github_write_hook();
hook.host = "github.com".to_string();
hook.matcher.methods = vec!["GET".to_string()];
hook.matcher.path_prefixes = vec!["pattern:/openai/openai/**".to_string()];
hook.actions.inject_request_headers.clear();
let mut network = NetworkProxyConfig {
mitm: true,
mitm_hooks: vec![hook],
mode: NetworkMode::Full,
..NetworkProxyConfig::default()
};
network.set_allowed_domains(vec!["github.com".to_string()]);
let app_state = Arc::new(network_proxy_state_for_policy(network));
let ctx = policy_ctx(
app_state.clone(),
NetworkMode::Full,
"github.com",
/*target_port*/ 443,
);
let paths = [
"/openai/openai/issues",
"/openai/codex",
"/openai/openai/%2e%2e/codex",
"/openai/openai/%2e%2e/%2e%2e/microsoft/vscode",
];
let mut actual = Vec::with_capacity(paths.len());
for path in paths {
let req = Request::builder()
.method(Method::GET)
.uri(path)
.header(HOST, "github.com")
.body(Body::empty())
.unwrap();
let response = mitm_blocking_response(&req, &ctx).await.unwrap();
actual.push(response.map(|response| {
(
response.status(),
response.headers().get("x-proxy-error").cloned(),
)
}));
}
assert_eq!(
actual,
vec![
None,
Some((
StatusCode::FORBIDDEN,
Some(HeaderValue::from_static("blocked-by-mitm-hook")),
)),
Some((
StatusCode::FORBIDDEN,
Some(HeaderValue::from_static("blocked-by-mitm-hook")),
)),
Some((
StatusCode::FORBIDDEN,
Some(HeaderValue::from_static("blocked-by-mitm-hook")),
)),
]
);
let blocked = app_state.drain_blocked().await.unwrap();
assert_eq!(blocked.len(), 3);
assert!(
blocked
.iter()
.all(|request| request.reason == REASON_MITM_HOOK_DENIED)
);
}
#[tokio::test]
async fn mitm_policy_blocks_matching_hooked_write_in_limited_mode() {
let mut hook = github_write_hook();