diff --git a/codex-rs/network-proxy/src/authorization_path.rs b/codex-rs/network-proxy/src/authorization_path.rs new file mode 100644 index 0000000000..7a692b8c2c --- /dev/null +++ b/codex-rs/network-proxy/src/authorization_path.rs @@ -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 { + 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; diff --git a/codex-rs/network-proxy/src/authorization_path_tests.rs b/codex-rs/network-proxy/src/authorization_path_tests.rs new file mode 100644 index 0000000000..d76da4d416 --- /dev/null +++ b/codex-rs/network-proxy/src/authorization_path_tests.rs @@ -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 + ] + ); +} diff --git a/codex-rs/network-proxy/src/http_proxy.rs b/codex-rs/network-proxy/src/http_proxy.rs index e8a77d8567..ad7884707b 100644 --- a/codex-rs/network-proxy/src/http_proxy.rs +++ b/codex-rs/network-proxy/src/http_proxy.rs @@ -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())); diff --git a/codex-rs/network-proxy/src/lib.rs b/codex-rs/network-proxy/src/lib.rs index 2ad9a21c8c..99862b32e9 100644 --- a/codex-rs/network-proxy/src/lib.rs +++ b/codex-rs/network-proxy/src/lib.rs @@ -1,6 +1,7 @@ #![deny(clippy::print_stdout, clippy::print_stderr)] mod attribution; +mod authorization_path; mod certs; mod config; mod connect_policy; diff --git a/codex-rs/network-proxy/src/mitm_hook.rs b/codex-rs/network-proxy/src/mitm_hook.rs index 7ad370aaff..6e2ee5cf8a 100644 --- a/codex-rs/network-proxy/src/mitm_hook.rs +++ b/codex-rs/network-proxy/src/mitm_hook.rs @@ -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::>(); + + assert_eq!(actual, vec![HookEvaluation::HookedHostNoMatch; paths.len()]); + } + #[test] fn evaluate_treats_glob_metacharacters_as_literal_without_glob_prefix() { let mut config = base_config(); diff --git a/codex-rs/network-proxy/src/mitm_tests.rs b/codex-rs/network-proxy/src/mitm_tests.rs index 31e9fc8aaf..a449ce32a2 100644 --- a/codex-rs/network-proxy/src/mitm_tests.rs +++ b/codex-rs/network-proxy/src/mitm_tests.rs @@ -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();