From 4b3a9a65f3a4d6918010976208c853fe6e80704b Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Thu, 9 Jul 2026 22:09:06 +0900 Subject: [PATCH 01/23] Add upstream proxy feature --- Cargo.lock | 1 + Cargo.toml | 1 + README.md | 22 ++ docs/SUMMARY.md | 1 + docs/advanced/upstream-proxy.md | 61 ++++ docs/guide/configuration.md | 13 +- src/lib.rs | 1 + src/main.rs | 29 +- src/proxy.rs | 253 ++++++++++--- src/proxy_tls.rs | 2 +- src/upstream.rs | 611 ++++++++++++++++++++++++++++++++ 11 files changed, 941 insertions(+), 54 deletions(-) create mode 100644 docs/advanced/upstream-proxy.md create mode 100644 src/upstream.rs diff --git a/Cargo.lock b/Cargo.lock index d8dbd4f8..29e88d90 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1034,6 +1034,7 @@ dependencies = [ "tls-parser", "tokio", "tokio-rustls", + "tower-service", "tracing", "tracing-subscriber", "url", diff --git a/Cargo.toml b/Cargo.toml index b04dfbbc..068f5dfb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ tracing-subscriber = { version = "0.3", features = ["env-filter", "chrono"] } chrono = "0.4" dirs = "6.0.0" hyper-rustls = "0.27.7" +tower-service = "0.3" tls-parser = "0.12.2" camino = "1.1.11" filetime = "0.2" diff --git a/README.md b/README.md index 619c87ec..33b5651f 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ Or download a pre-built binary from the [releases page](https://github.com/coder - 🌐 **HTTP/HTTPS interception** - Transparent proxy with TLS certificate injection - 🛡️ **DNS exfiltration protection** - Prevents data leakage through DNS queries - 🔧 **Multiple evaluation approaches** - JS expressions or custom programs +- 🏢 **Upstream proxy support** - Chain httpjail's egress through a corporate proxy - 🖥️ **Cross-platform** - Native support for Linux and macOS ## Quick Start @@ -61,8 +62,28 @@ httpjail --server --js "true" # Run Docker containers with network isolation (Linux only) httpjail --js "r.host === 'api.github.com'" --docker-run -- --rm alpine:latest wget -qO- https://api.github.com + +# Route httpjail's own egress through an upstream (corporate) proxy +httpjail --upstream-proxy http://proxy.corp:3128 --js "true" -- curl https://api.github.com +# Credentials and HTTPS proxies are supported: http://user:pass@proxy.corp:3128, https://proxy.corp:8443 +# May also be set via the HTTPJAIL_UPSTREAM_PROXY environment variable ``` +### Upstream (corporate) proxy + +When httpjail itself runs in an environment with no direct internet access, use +`--upstream-proxy ` (or the `HTTPJAIL_UPSTREAM_PROXY` environment variable) +to route httpjail's outbound requests through an upstream proxy. Rule evaluation +still happens locally on the intercepted traffic; only the re-originated request +is forwarded through the proxy. + +- `http://`, `https://` and bare `host:port` (http assumed) forms are accepted. +- Basic authentication is supported via `http://user:pass@host:port`. +- HTTPS destinations are reached via a `CONNECT` tunnel through the proxy, while + plain HTTP destinations are forwarded in absolute-form. +- This is independent of the `HTTP_PROXY`/`HTTPS_PROXY` variables that httpjail + sets *inside* the jail to point sandboxed processes at itself. + ## Documentation Docs are stored in the `docs/` directory and served @@ -82,6 +103,7 @@ Table of Contents: - [TLS Interception](https://coder.github.io/httpjail/advanced/tls-interception.html) - [DNS Exfiltration](https://coder.github.io/httpjail/advanced/dns-exfiltration.html) - [Server Mode](https://coder.github.io/httpjail/advanced/server-mode.html) +- [Upstream Proxy](https://coder.github.io/httpjail/advanced/upstream-proxy.html) ## License diff --git a/docs/SUMMARY.md b/docs/SUMMARY.md index 92a3d246..86ce2d77 100644 --- a/docs/SUMMARY.md +++ b/docs/SUMMARY.md @@ -20,6 +20,7 @@ - [TLS Interception](./advanced/tls-interception.md) - [DNS Exfiltration](./advanced/dns-exfiltration.md) - [Server Mode](./advanced/server-mode.md) +- [Upstream Proxy](./advanced/upstream-proxy.md) --- diff --git a/docs/advanced/upstream-proxy.md b/docs/advanced/upstream-proxy.md new file mode 100644 index 00000000..35061ff2 --- /dev/null +++ b/docs/advanced/upstream-proxy.md @@ -0,0 +1,61 @@ +# Upstream Proxy + +By default httpjail contacts destination servers directly. When httpjail itself +runs in an environment that has no direct internet access — for example behind a +corporate proxy — you can route httpjail's own outbound requests through an +upstream proxy with `--upstream-proxy` (or the `HTTPJAIL_UPSTREAM_PROXY` +environment variable). + +Rule evaluation still happens locally on the intercepted traffic. Only the +request that httpjail re-originates towards the real destination is forwarded +through the upstream proxy. + +```bash +# Route httpjail's egress through a corporate proxy +httpjail --upstream-proxy http://proxy.corp:3128 --js "true" -- curl https://api.github.com + +# With Basic authentication +httpjail --upstream-proxy http://user:pass@proxy.corp:3128 --js "true" -- ./my-app + +# Through an HTTPS proxy +httpjail --upstream-proxy https://proxy.corp:8443 --js "true" -- ./my-app + +# Via the environment variable (equivalent to --upstream-proxy) +HTTPJAIL_UPSTREAM_PROXY=http://proxy.corp:3128 httpjail --js "true" -- ./my-app +``` + +## Accepted formats + +| Form | Example | Notes | +| --- | --- | --- | +| `http://host:port` | `http://proxy.corp:3128` | Plain HTTP proxy | +| `https://host:port` | `https://proxy.corp:8443` | Connection to the proxy is wrapped in TLS | +| `host:port` | `proxy.corp:3128` | Bare authority, `http` scheme assumed | +| With credentials | `http://user:pass@proxy.corp:3128` | Sends `Proxy-Authorization: Basic ...` | + +The command-line flag takes precedence over the environment variable. Credentials +are never written to the logs. + +## How it works + +- **HTTPS destinations** are reached by issuing a `CONNECT` to the upstream + proxy to obtain a raw TCP tunnel; httpjail then performs the destination TLS + handshake over that tunnel. TLS is validated against Mozilla's webpki roots + plus the httpjail CA, exactly as for a direct connection. +- **Plain HTTP destinations** are forwarded to the proxy in absolute-form, with + the `Proxy-Authorization` header attached when credentials are configured. +- Only connection setup (TCP connect, optional TLS to the proxy, and the + `CONNECT` exchange) is bounded by a timeout. The established tunnel carries no + timeout, so long-running connections such as WebSocket and gRPC keep working. + +## Relationship to `HTTP_PROXY` / `HTTPS_PROXY` + +This feature is independent of the `HTTP_PROXY` and `HTTPS_PROXY` variables that +httpjail sets *inside* the jail to point sandboxed processes at httpjail itself. + +``` +[ jailed process ] --HTTP_PROXY/HTTPS_PROXY--> [ httpjail ] --upstream-proxy--> [ corporate proxy ] --> internet +``` + +The jailed process always talks to httpjail; `--upstream-proxy` only affects the +hop from httpjail to the outside world. diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 7338e46b..527d3655 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -88,10 +88,15 @@ These are automatically set in the jailed process: These affect httpjail's behavior: -| Variable | Description | Example | -| ------------------ | -------------------------- | -------------------------------- | -| `RUST_LOG` | Logging level | `debug`, `info`, `warn`, `error` | -| `HTTPJAIL_CA_CERT` | Custom CA certificate path | `/etc/pki/custom-ca.pem` | +| Variable | Description | Example | +| ------------------------ | -------------------------------------- | -------------------------------- | +| `RUST_LOG` | Logging level | `debug`, `info`, `warn`, `error` | +| `HTTPJAIL_CA_CERT` | Custom CA certificate path | `/etc/pki/custom-ca.pem` | +| `HTTPJAIL_UPSTREAM_PROXY`| Upstream proxy for httpjail's egress | `http://proxy.corp:3128` | + +The `--upstream-proxy` command-line flag takes precedence over +`HTTPJAIL_UPSTREAM_PROXY`. See [Upstream Proxy](../advanced/upstream-proxy.md) +for details. ## Platform-Specific Configuration diff --git a/src/lib.rs b/src/lib.rs index 40c97467..764f41d7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,5 +9,6 @@ pub mod proxy_tls; pub mod rules; pub mod sys_resource; pub mod tls; +pub mod upstream; pub mod test_utils; diff --git a/src/main.rs b/src/main.rs index 3fc1fb92..625ef95d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -85,6 +85,13 @@ struct RunArgs { #[arg(long = "request-log", value_name = "FILE")] request_log: Option, + /// Route httpjail's own upstream requests through an upstream (corporate) proxy. + /// Accepts http://host:port, https://host:port, or host:port (http assumed), + /// optionally with credentials: http://user:pass@host:port. + /// Falls back to the HTTPJAIL_UPSTREAM_PROXY environment variable. + #[arg(long = "upstream-proxy", value_name = "URL")] + upstream_proxy: Option, + /// Use weak mode (environment variables only, no system isolation) #[arg(long = "weak")] weak: bool, @@ -590,7 +597,27 @@ async fn main() -> Result<()> { } }; - let mut proxy = ProxyServer::new(http_bind, https_bind, rule_engine); + // Resolve the optional upstream (corporate) proxy from the flag or env var. + // This is independent of the HTTP_PROXY/HTTPS_PROXY variables httpjail sets + // *inside* the jail to point sandboxed processes at itself. + let upstream_proxy_spec = args + .run_args + .upstream_proxy + .clone() + .or_else(|| std::env::var("HTTPJAIL_UPSTREAM_PROXY").ok()); + let upstream_proxy = match upstream_proxy_spec { + Some(spec) => { + let proxy = httpjail::upstream::UpstreamProxy::parse(&spec) + .with_context(|| format!("Failed to parse upstream proxy: {}", spec))?; + // Avoid logging the spec verbatim as it may contain credentials. + info!("Routing httpjail upstream requests through the configured upstream proxy"); + Some(proxy) + } + None => None, + }; + + let mut proxy = + ProxyServer::new_with_upstream_proxy(http_bind, https_bind, rule_engine, upstream_proxy); // Start proxy in background if running as server; otherwise start with random ports let (actual_http_port, actual_https_port) = proxy.start().await?; diff --git a/src/proxy.rs b/src/proxy.rs index 373251eb..bebb5beb 100644 --- a/src/proxy.rs +++ b/src/proxy.rs @@ -3,17 +3,21 @@ use crate::dangerous_verifier::create_dangerous_client_config; use crate::rules::{Action, RuleEngine}; #[allow(unused_imports)] use crate::tls::CertificateManager; +use crate::upstream::{ProxyConnector, UpstreamProxy}; use anyhow::Result; use bytes::Bytes; use http_body_util::{BodyExt, Full, combinators::BoxBody}; use hyper::body::Incoming; +use hyper::header::{HeaderValue, PROXY_AUTHORIZATION}; use hyper::server::conn::http1; use hyper::service::service_fn; use hyper::{Error as HyperError, Request, Response, StatusCode, Uri}; use hyper_rustls::HttpsConnectorBuilder; use hyper_util::client::legacy::Client; +use hyper_util::client::legacy::connect::HttpConnector; use hyper_util::rt::{TokioExecutor, TokioIo}; use rand::Rng; +use rustls::pki_types::CertificateDer; #[cfg(target_os = "linux")] use std::os::fd::AsRawFd; @@ -158,13 +162,70 @@ pub fn apply_request_byte_limit( ))) } +/// Direct (no upstream proxy) upstream client type. +type DirectClient = Client, BoxBody>; + +/// Upstream client that routes every re-originated request through an upstream +/// (corporate) proxy via a [`ProxyConnector`]. +type ProxiedClient = + Client, BoxBody>; + +/// Upstream client: either contacts destinations directly or routes through a +/// configured upstream proxy. Both variants are high-level pooled clients. +pub enum UpstreamClient { + Direct(DirectClient), + Proxied { + client: ProxiedClient, + /// Attached to plain-HTTP requests forwarded through the proxy in + /// absolute-form (HTTPS carries credentials on the CONNECT instead). + http_auth: Option, + }, +} + +impl UpstreamClient { + /// Forward a prepared request upstream. No timeout is applied here so that + /// long-running connections (WebSocket, gRPC, ...) keep working. + pub async fn request( + &self, + mut req: Request>, + ) -> Result> { + match self { + UpstreamClient::Direct(client) => client.request(req).await.map_err(Into::into), + UpstreamClient::Proxied { client, http_auth } => { + if req.uri().scheme_str() == Some("http") + && let Some(auth) = http_auth + { + req.headers_mut().insert(PROXY_AUTHORIZATION, auth.clone()); + } + client.request(req).await.map_err(Into::into) + } + } + } +} + // Shared HTTP/HTTPS client for upstream requests -static HTTPS_CLIENT: OnceLock< - Client< - hyper_rustls::HttpsConnector, - BoxBody, - >, -> = OnceLock::new(); +static HTTPS_CLIENT: OnceLock = OnceLock::new(); + +/// Build a pooled hyper client over the given connector with the shared tuning. +fn build_pooled_client(connector: C) -> Client> +where + C: tower_service::Service + Clone + Send + Sync + 'static, + C::Response: hyper_util::client::legacy::connect::Connection + + hyper::rt::Read + + hyper::rt::Write + + Unpin + + Send + + 'static, + C::Future: Send + Unpin + 'static, + C::Error: Into>, +{ + Client::builder(TokioExecutor::new()) + .pool_idle_timeout(Duration::from_secs(5)) + .pool_max_idle_per_host(1) + .http1_title_case_headers(false) + .http1_preserve_header_case(true) + .build(connector) +} /// Prepare a request for forwarding to upstream server /// Removes proxy-specific headers and converts body to BoxBody @@ -250,45 +311,73 @@ fn create_client_config_with_ca( .with_no_client_auth() } -/// Initialize the HTTP client with the httpjail CA certificate -pub fn init_client_with_ca(ca_cert_der: rustls::pki_types::CertificateDer<'static>) { +/// Build the direct (no upstream proxy) HTTPS connector: webpki roots plus the +/// httpjail CA, with fast IPv6->IPv4 fallback (or the dangerous no-verification +/// config for testing). +fn build_direct_connector( + ca_cert_der: CertificateDer<'static>, + dangerous: bool, +) -> hyper_rustls::HttpsConnector { + if dangerous { + let config = create_dangerous_client_config(); + HttpsConnectorBuilder::new() + .with_tls_config(config) + .https_or_http() + .enable_http1() + .build() + } else { + let config = create_client_config_with_ca(ca_cert_der); + // Build an HttpConnector with fast IPv6->IPv4 fallback + let mut http = HttpConnector::new(); + http.enforce_http(false); + http.set_happy_eyeballs_timeout(Some(Duration::from_millis(250))); + hyper_rustls::HttpsConnector::from((http, config)) + } +} + +/// Initialize the shared upstream client with the httpjail CA certificate and an +/// optional upstream proxy. When a proxy is configured, all re-originated +/// requests are routed through it; otherwise destinations are contacted directly. +pub fn init_client_with_ca( + ca_cert_der: CertificateDer<'static>, + upstream_proxy: Option, +) { HTTPS_CLIENT.get_or_init(|| { // Check if we should dangerously disable cert validation (TESTING ONLY!) - let https = if std::env::var("HTTPJAIL_DANGER_DISABLE_CERT_VALIDATION").is_ok() { - let config = create_dangerous_client_config(); - - hyper_rustls::HttpsConnectorBuilder::new() - .with_tls_config(config) - .https_or_http() - .enable_http1() - .build() - } else { - // Normal path - use webpki roots + httpjail CA - let config = create_client_config_with_ca(ca_cert_der); - // Build an HttpConnector with fast IPv6->IPv4 fallback - let mut http = hyper_util::client::legacy::connect::HttpConnector::new(); - http.enforce_http(false); - http.set_happy_eyeballs_timeout(Some(Duration::from_millis(250))); - let https = hyper_rustls::HttpsConnector::from((http, config)); - info!("HTTPS connector initialized with webpki roots and httpjail CA"); - https - }; + let dangerous = std::env::var("HTTPJAIL_DANGER_DISABLE_CERT_VALIDATION").is_ok(); - Client::builder(TokioExecutor::new()) - // Keep minimal pooling but with shorter timeouts - .pool_idle_timeout(Duration::from_secs(5)) - .pool_max_idle_per_host(1) - .http1_title_case_headers(false) - .http1_preserve_header_case(true) - .build(https) + match upstream_proxy { + None => { + let https = build_direct_connector(ca_cert_der, dangerous); + info!("HTTPS connector initialized with webpki roots and httpjail CA"); + UpstreamClient::Direct(build_pooled_client(https)) + } + Some(proxy) => { + // Both the destination TLS (layered over the CONNECT tunnel by + // the HttpsConnector) and the optional https:// proxy TLS trust + // the same roots as the direct client. + let make_config = || { + if dangerous { + create_dangerous_client_config() + } else { + create_client_config_with_ca(ca_cert_der.clone()) + } + }; + let http_auth = proxy.http_auth(); + let connector = ProxyConnector::new(proxy, Arc::new(make_config())); + let https = hyper_rustls::HttpsConnector::from((connector, make_config())); + info!("Upstream client initialized to route through the upstream proxy"); + UpstreamClient::Proxied { + client: build_pooled_client(https), + http_auth, + } + } + } }); } -/// Get or create the shared HTTP/HTTPS client -pub fn get_client() -> &'static Client< - hyper_rustls::HttpsConnector, - BoxBody, -> { +/// Get or create the shared upstream client +pub fn get_client() -> &'static UpstreamClient { HTTPS_CLIENT.get_or_init(|| { // Fallback initialization if not already initialized with CA // This should not happen in normal operation @@ -301,13 +390,7 @@ pub fn get_client() -> &'static Client< .enable_http1() .build(); - Client::builder(TokioExecutor::new()) - // Keep minimal pooling but with shorter timeouts - .pool_idle_timeout(Duration::from_secs(5)) - .pool_max_idle_per_host(1) - .http1_title_case_headers(false) - .http1_preserve_header_case(true) - .build(https) + UpstreamClient::Direct(build_pooled_client(https)) }) } @@ -400,12 +483,23 @@ impl ProxyServer { http_bind: Option, https_bind: Option, rule_engine: RuleEngine, + ) -> Self { + Self::new_with_upstream_proxy(http_bind, https_bind, rule_engine, None) + } + + /// Like [`ProxyServer::new`], but routes httpjail's own re-originated + /// requests through the given upstream (corporate) proxy when set. + pub fn new_with_upstream_proxy( + http_bind: Option, + https_bind: Option, + rule_engine: RuleEngine, + upstream_proxy: Option, ) -> Self { let cert_manager = CertificateManager::new().expect("Failed to create certificate manager"); // Initialize the HTTP client with our CA certificate let ca_cert_der = cert_manager.get_ca_cert_der(); - init_client_with_ca(ca_cert_der); + init_client_with_ca(ca_cert_der, upstream_proxy); // Generate a unique nonce for loop detection (Issue #84) // Use 16 random hex characters for a reasonably short but collision-resistant ID @@ -671,7 +765,7 @@ async fn proxy_request( elapsed.as_millis(), e ); - return Err(e.into()); + return Err(e); } }; @@ -752,4 +846,67 @@ mod tests { assert!((8000..=8999).contains(&https_port)); assert_ne!(http_port, https_port); } + + /// A plain-HTTP request routed through an upstream proxy must be forwarded in + /// absolute-form with the configured `Proxy-Authorization` header. + #[tokio::test] + async fn proxied_http_uses_absolute_form_with_auth() { + use http_body_util::Empty; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::TcpListener; + + // Fake upstream proxy: capture the forwarded request, then reply 200. + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let server = tokio::spawn(async move { + let (mut sock, _) = listener.accept().await.unwrap(); + let mut buf = Vec::new(); + let mut byte = [0u8; 1]; + while sock.read(&mut byte).await.unwrap() != 0 { + buf.push(byte[0]); + if buf.ends_with(b"\r\n\r\n") { + break; + } + } + sock.write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\n\r\n") + .await + .unwrap(); + sock.flush().await.unwrap(); + String::from_utf8_lossy(&buf).into_owned() + }); + + let proxy = UpstreamProxy::parse(&format!("http://user:pass@{}", addr)).unwrap(); + let http_auth = proxy.http_auth(); + let connector = ProxyConnector::new(proxy, Arc::new(create_dangerous_client_config())); + let https = + hyper_rustls::HttpsConnector::from((connector, create_dangerous_client_config())); + let client = UpstreamClient::Proxied { + client: build_pooled_client(https), + http_auth, + }; + + let body = Empty::::new() + .map_err(|never| match never {}) + .boxed(); + let req = Request::builder() + .uri("http://target.example/path") + .body(body) + .unwrap(); + let resp = client.request(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + + let forwarded = server.await.unwrap(); + assert!( + forwarded.starts_with("GET http://target.example/path HTTP/1.1\r\n"), + "expected absolute-form request line, got: {forwarded}" + ); + // Header names are case-insensitive; base64("user:pass") == dXNlcjpwYXNz + assert!( + forwarded.lines().any(|l| { + l.to_ascii_lowercase().starts_with("proxy-authorization:") + && l.contains("Basic dXNlcjpwYXNz") + }), + "missing proxy auth, got: {forwarded}" + ); + } } diff --git a/src/proxy_tls.rs b/src/proxy_tls.rs index f7c1da56..005dec17 100644 --- a/src/proxy_tls.rs +++ b/src/proxy_tls.rs @@ -595,7 +595,7 @@ async fn proxy_https_request( // The hyper_util error doesn't expose underlying IO errors directly - return Err(e.into()); + return Err(e); } }; diff --git a/src/upstream.rs b/src/upstream.rs new file mode 100644 index 00000000..dcb9eb6a --- /dev/null +++ b/src/upstream.rs @@ -0,0 +1,611 @@ +//! Forward httpjail's own re-originated requests through an upstream +//! (e.g. corporate) HTTP proxy. +//! +//! httpjail terminates the jailed process's traffic and then re-originates the +//! request towards the real destination. When httpjail itself has no direct +//! egress, that re-originated request must instead traverse an upstream proxy. +//! +//! This is implemented as a hyper *connector* ([`ProxyConnector`]) so that the +//! same high-level `hyper_util` `Client` used for direct egress can be reused +//! unchanged: connection pooling, request serialization and (for HTTPS) the +//! destination TLS handshake are all handled by hyper's own machinery. The +//! connector only decides how the underlying byte stream is obtained: +//! +//! * for `https://` destinations it issues a `CONNECT` to the proxy to obtain a +//! raw TCP tunnel; the surrounding `hyper_rustls::HttpsConnector` then performs +//! the destination TLS handshake over that tunnel, and +//! * for `http://` destinations it returns the proxy connection marked as +//! proxied, so hyper emits the request in absolute-form for the proxy to +//! forward. +//! +//! Following the streaming style used elsewhere in httpjail (see `proxy_tls.rs`) +//! every bounded read during setup is guarded by a timeout, while the +//! established tunnel itself is left unbounded so long-running connections +//! (WebSocket, gRPC, ...) keep working. + +use anyhow::{Context as _, Result, anyhow, bail}; +use hyper::Uri; +use hyper::header::HeaderValue; +use hyper::rt::{Read, ReadBufCursor, Write}; +use hyper_util::client::legacy::connect::{Connected, Connection, HttpConnector}; +use hyper_util::rt::TokioIo; +use rustls::pki_types::ServerName; +use std::future::Future; +use std::io; +use std::pin::Pin; +use std::sync::Arc; +use std::task::{Context, Poll}; +use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; +use tokio::time::{Duration, timeout}; +use tokio_rustls::TlsConnector; +use tower_service::Service; +use tracing::debug; + +type BoxError = Box; + +/// Timeout for establishing the tunnel through the upstream proxy (TCP connect, +/// optional TLS to the proxy and the `CONNECT` exchange). This bounds setup +/// only; the resulting tunnel carries no timeout so long-running connections +/// keep working. +const PROXY_SETUP_TIMEOUT: Duration = Duration::from_secs(30); + +/// Upper bound on the size of the upstream proxy's `CONNECT` response headers. +/// A well-behaved proxy answers with a short status line and a few headers. +const MAX_CONNECT_RESPONSE_BYTES: usize = 16 * 1024; + +/// Object-safe combination of the async byte-stream traits we erase over so the +/// connector can hold either a plain TCP stream or a TLS stream (when the proxy +/// itself is reached over `https://`) behind a single type. +trait IoStream: AsyncRead + AsyncWrite + Unpin + Send {} +impl IoStream for T {} + +/// A heap-erased byte stream carrying the connection to the proxy. +type BoxedIo = Box; + +/// Parsed configuration for an upstream proxy. +#[derive(Clone, Debug)] +pub struct UpstreamProxy { + /// Proxy host (DNS name or IP literal) to dial. + host: String, + /// Proxy port. + port: u16, + /// Whether the connection to the proxy itself is wrapped in TLS (an + /// `https://` proxy URL). + tls: bool, + /// Pre-built `Proxy-Authorization` header value when credentials are given. + auth: Option, +} + +impl UpstreamProxy { + /// Parse an upstream proxy specification such as `http://proxy.corp:3128`, + /// `http://user:pass@proxy.corp:3128`, `https://proxy.corp:8443` or a bare + /// `proxy.corp:3128` (the `http` scheme is then assumed). + pub fn parse(spec: &str) -> Result { + let spec = spec.trim(); + if spec.is_empty() { + bail!("Upstream proxy specification is empty"); + } + + // Accept a bare `host:port` by assuming the http scheme. + let (scheme, rest) = match spec.split_once("://") { + Some((scheme, rest)) => (scheme.to_ascii_lowercase(), rest), + None => ("http".to_string(), spec), + }; + + let tls = match scheme.as_str() { + "http" => false, + "https" => true, + other => bail!("Unsupported upstream proxy scheme '{}': {}", other, spec), + }; + + // Drop any path/query/fragment component; only the authority is used. + let authority = rest.split(['/', '?', '#']).next().unwrap_or(rest); + + // Split optional `userinfo@` from the `host:port` authority. + let (userinfo, host_port) = match authority.rsplit_once('@') { + Some((userinfo, host_port)) => (Some(userinfo), host_port), + None => (None, authority), + }; + + let default_port = if tls { 443 } else { 80 }; + let (host, port) = parse_host_port(host_port, default_port) + .with_context(|| format!("Invalid upstream proxy authority: {}", spec))?; + + let auth = match userinfo { + Some(userinfo) => Some(build_basic_auth(userinfo)?), + None => None, + }; + + Ok(UpstreamProxy { + host, + port, + tls, + auth, + }) + } + + /// The `Proxy-Authorization` header value, if credentials were supplied. + /// + /// Needed by the plain-HTTP forwarding path, where the header travels on the + /// forwarded request itself rather than on a `CONNECT`. + pub fn http_auth(&self) -> Option { + self.auth.clone() + } +} + +/// Split a `host:port` authority into its parts, handling bracketed IPv6 +/// literals (`[::1]:3128`). Falls back to `default_port` when no port is given. +fn parse_host_port(authority: &str, default_port: u16) -> Result<(String, u16)> { + if let Some(rest) = authority.strip_prefix('[') { + // Bracketed IPv6 literal: `[addr]` or `[addr]:port`. + let (addr, after) = rest + .split_once(']') + .ok_or_else(|| anyhow!("unterminated IPv6 literal: {}", authority))?; + let port = match after.strip_prefix(':') { + Some(port) => port.parse().context("invalid port")?, + None if after.is_empty() => default_port, + None => bail!("unexpected characters after IPv6 literal: {}", authority), + }; + return Ok((addr.to_string(), port)); + } + + let (host, port) = match authority.rsplit_once(':') { + Some((host, port)) => (host, port.parse().context("invalid port")?), + None => (authority, default_port), + }; + + if host.is_empty() { + bail!("missing host: {}", authority); + } + Ok((host.to_string(), port)) +} + +/// Build a `Proxy-Authorization: Basic ...` header value from `user:pass` +/// userinfo, percent-decoding each component first. +fn build_basic_auth(userinfo: &str) -> Result { + let (user, pass) = match userinfo.split_once(':') { + Some((user, pass)) => (user, pass), + None => (userinfo, ""), + }; + let token = + base64_encode(format!("{}:{}", percent_decode(user), percent_decode(pass)).as_bytes()); + HeaderValue::from_str(&format!("Basic {}", token)) + .context("Invalid characters in upstream proxy credentials") +} + +/// A hyper connector that routes outbound connections through an +/// [`UpstreamProxy`]. +/// +/// It is intended to be used as the inner connector of a +/// `hyper_rustls::HttpsConnector`: this connector yields a raw byte stream (the +/// proxy connection for HTTP, or a `CONNECT` tunnel for HTTPS) and the +/// surrounding HTTPS connector layers the destination TLS on top when needed. +#[derive(Clone)] +pub struct ProxyConnector { + /// Used solely to dial the proxy's `host:port` (never the destination). + http: HttpConnector, + proxy: Arc, + /// TLS configuration used only when the proxy itself is `https://`. + proxy_tls: Arc, +} + +impl ProxyConnector { + pub fn new(proxy: UpstreamProxy, proxy_tls: Arc) -> Self { + let mut http = HttpConnector::new(); + // The proxy is addressed via an http(s) URL; allow non-http schemes so + // the connector does not reject the dial target. + http.enforce_http(false); + http.set_happy_eyeballs_timeout(Some(Duration::from_millis(250))); + ProxyConnector { + http, + proxy: Arc::new(proxy), + proxy_tls, + } + } +} + +impl Service for ProxyConnector { + type Response = ProxyStream; + type Error = BoxError; + type Future = Pin> + Send>>; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.http.poll_ready(cx).map_err(Into::into) + } + + fn call(&mut self, dst: Uri) -> Self::Future { + let mut http = self.http.clone(); + let proxy = Arc::clone(&self.proxy); + let proxy_tls = Arc::clone(&self.proxy_tls); + + Box::pin(async move { + // Dial the proxy (TCP). The destination scheme is irrelevant here; + // we always connect to the proxy's host:port. + let proxy_uri: Uri = format!("http://{}:{}", proxy.host, proxy.port).parse()?; + let tcp = http.call(proxy_uri).await?.into_inner(); + let _ = tcp.set_nodelay(true); + + // Optionally negotiate TLS with the proxy itself. + let mut stream: BoxedIo = if proxy.tls { + let name = ServerName::try_from(proxy.host.clone()).map_err(|_| { + BoxError::from(format!("Invalid proxy host for TLS SNI: {}", proxy.host)) + })?; + let connector = TlsConnector::from(Arc::clone(&proxy_tls)); + let tls = match timeout(PROXY_SETUP_TIMEOUT, connector.connect(name, tcp)).await { + Ok(result) => result?, + Err(_) => return Err(timed_out("during TLS handshake with upstream proxy")), + }; + Box::new(tls) + } else { + Box::new(tcp) + }; + + let proxied = if dst.scheme_str() == Some("https") { + let host = dst.host().ok_or_else(|| { + BoxError::from(format!("CONNECT target has no host: {}", dst)) + })?; + let port = dst.port_u16().unwrap_or(443); + establish_connect_tunnel(&mut stream, host, port, proxy.auth.as_ref()) + .await + .map_err(|e| -> BoxError { e.into() })?; + // The tunnel is transparent end-to-end; destination TLS is + // layered on top by the surrounding HttpsConnector and the + // request is sent in origin-form, so do not mark it proxied. + false + } else { + // Plain HTTP: the proxy forwards absolute-form requests. Mark the + // connection proxied so hyper emits absolute-form request lines. + true + }; + + Ok(ProxyStream::new(stream, proxied)) + }) + } +} + +/// Build a timeout error for the upstream proxy setup phase. +fn timed_out(phase: &str) -> BoxError { + format!("Timeout {} with upstream proxy", phase).into() +} + +/// The connector's response: a byte stream plus the proxied flag that hyper +/// consults to decide between absolute-form and origin-form request lines. +pub struct ProxyStream { + io: TokioIo, + proxied: bool, +} + +impl ProxyStream { + fn new(io: BoxedIo, proxied: bool) -> Self { + ProxyStream { + io: TokioIo::new(io), + proxied, + } + } +} + +impl Connection for ProxyStream { + fn connected(&self) -> Connected { + Connected::new().proxy(self.proxied) + } +} + +impl Read for ProxyStream { + fn poll_read( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: ReadBufCursor<'_>, + ) -> Poll> { + Pin::new(&mut self.get_mut().io).poll_read(cx, buf) + } +} + +impl Write for ProxyStream { + fn poll_write( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &[u8], + ) -> Poll> { + Pin::new(&mut self.get_mut().io).poll_write(cx, buf) + } + + fn poll_flush(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + Pin::new(&mut self.get_mut().io).poll_flush(cx) + } + + fn poll_shutdown(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + Pin::new(&mut self.get_mut().io).poll_shutdown(cx) + } + + fn is_write_vectored(&self) -> bool { + self.io.is_write_vectored() + } + + fn poll_write_vectored( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + bufs: &[io::IoSlice<'_>], + ) -> Poll> { + Pin::new(&mut self.get_mut().io).poll_write_vectored(cx, bufs) + } +} + +/// Send a `CONNECT` request to the upstream proxy and validate its response, +/// leaving `stream` positioned at the start of the tunnel payload on success. +async fn establish_connect_tunnel( + stream: &mut S, + host: &str, + port: u16, + auth: Option<&HeaderValue>, +) -> Result<()> +where + S: AsyncRead + AsyncWrite + Unpin, +{ + // Bracket IPv6 literals in the request-target and Host header. + let target = if host.contains(':') { + format!("[{host}]:{port}") + } else { + format!("{host}:{port}") + }; + + let mut request = format!("CONNECT {target} HTTP/1.1\r\nHost: {target}\r\n"); + if let Some(value) = auth { + let value = value + .to_str() + .context("Proxy-Authorization contains non-ASCII bytes")?; + request.push_str("Proxy-Authorization: "); + request.push_str(value); + request.push_str("\r\n"); + } + request.push_str("\r\n"); + + match timeout(PROXY_SETUP_TIMEOUT, stream.write_all(request.as_bytes())).await { + Ok(result) => result.context("Failed to write CONNECT request")?, + Err(_) => bail!("Timeout writing CONNECT request to upstream proxy"), + } + match timeout(PROXY_SETUP_TIMEOUT, stream.flush()).await { + Ok(result) => result.context("Failed to flush CONNECT request")?, + Err(_) => bail!("Timeout flushing CONNECT request to upstream proxy"), + } + + let status = match timeout(PROXY_SETUP_TIMEOUT, read_connect_status(stream)).await { + Ok(result) => result?, + Err(_) => bail!("Timeout reading CONNECT response from upstream proxy"), + }; + + if !(200..300).contains(&status) { + bail!( + "Upstream proxy refused CONNECT to {}:{} with status {}", + host, + port, + status + ); + } + + debug!( + "Established CONNECT tunnel to {}:{} via upstream proxy", + host, port + ); + Ok(()) +} + +/// Read the proxy's `CONNECT` response up to the end of its headers and return +/// the HTTP status code. Reads are bounded by [`MAX_CONNECT_RESPONSE_BYTES`] to +/// avoid consuming tunnel payload and to bound memory. +async fn read_connect_status(stream: &mut S) -> Result +where + S: AsyncRead + Unpin, +{ + let mut buf = Vec::with_capacity(128); + let mut byte = [0u8; 1]; + loop { + let n = stream.read(&mut byte).await?; + if n == 0 { + bail!("Upstream proxy closed connection during CONNECT"); + } + buf.push(byte[0]); + if buf.ends_with(b"\r\n\r\n") { + break; + } + if buf.len() > MAX_CONNECT_RESPONSE_BYTES { + bail!("Upstream proxy CONNECT response exceeded size limit"); + } + } + + // Parse the status code from the first line, e.g. + // `HTTP/1.1 200 Connection established`. + let head = std::str::from_utf8(&buf).context("Non-UTF8 CONNECT response")?; + let first_line = head.lines().next().unwrap_or(""); + first_line + .split_whitespace() + .nth(1) + .and_then(|code| code.parse::().ok()) + .ok_or_else(|| anyhow!("Malformed CONNECT status line: {:?}", first_line)) +} + +/// Minimal RFC 4648 base64 encoder (standard alphabet, with padding). Used only +/// to build the `Proxy-Authorization: Basic ...` credential token, avoiding a +/// dedicated base64 dependency. +fn base64_encode(input: &[u8]) -> String { + const ALPHABET: &[u8; 64] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; + let mut out = String::with_capacity(input.len().div_ceil(3) * 4); + for chunk in input.chunks(3) { + let b0 = chunk[0] as u32; + let b1 = *chunk.get(1).unwrap_or(&0) as u32; + let b2 = *chunk.get(2).unwrap_or(&0) as u32; + let triple = (b0 << 16) | (b1 << 8) | b2; + out.push(ALPHABET[((triple >> 18) & 0x3f) as usize] as char); + out.push(ALPHABET[((triple >> 12) & 0x3f) as usize] as char); + out.push(if chunk.len() > 1 { + ALPHABET[((triple >> 6) & 0x3f) as usize] as char + } else { + '=' + }); + out.push(if chunk.len() > 2 { + ALPHABET[(triple & 0x3f) as usize] as char + } else { + '=' + }); + } + out +} + +/// Decode `%XX` percent-escapes in the userinfo portion of a proxy URL. Any +/// malformed escape is left verbatim. +fn percent_decode(input: &str) -> String { + let bytes = input.as_bytes(); + let mut out = Vec::with_capacity(bytes.len()); + let mut i = 0; + while i < bytes.len() { + if bytes[i] == b'%' && i + 2 < bytes.len() { + let hi = (bytes[i + 1] as char).to_digit(16); + let lo = (bytes[i + 2] as char).to_digit(16); + if let (Some(hi), Some(lo)) = (hi, lo) { + out.push((hi * 16 + lo) as u8); + i += 3; + continue; + } + } + out.push(bytes[i]); + i += 1; + } + String::from_utf8_lossy(&out).into_owned() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn base64_matches_known_vectors() { + assert_eq!(base64_encode(b""), ""); + assert_eq!(base64_encode(b"f"), "Zg=="); + assert_eq!(base64_encode(b"fo"), "Zm8="); + assert_eq!(base64_encode(b"foo"), "Zm9v"); + assert_eq!(base64_encode(b"user:pass"), "dXNlcjpwYXNz"); + } + + #[test] + fn parse_plain_proxy() { + let p = UpstreamProxy::parse("http://proxy.corp:3128").unwrap(); + assert_eq!(p.host, "proxy.corp"); + assert_eq!(p.port, 3128); + assert!(!p.tls); + assert!(p.auth.is_none()); + } + + #[test] + fn parse_bare_hostport_defaults_to_http() { + let p = UpstreamProxy::parse("proxy.corp:8080").unwrap(); + assert_eq!(p.host, "proxy.corp"); + assert_eq!(p.port, 8080); + assert!(!p.tls); + } + + #[test] + fn parse_https_proxy_default_port() { + let p = UpstreamProxy::parse("https://proxy.corp").unwrap(); + assert!(p.tls); + assert_eq!(p.port, 443); + } + + #[test] + fn parse_ipv6_literal_with_port() { + let p = UpstreamProxy::parse("http://[::1]:3128").unwrap(); + assert_eq!(p.host, "::1"); + assert_eq!(p.port, 3128); + } + + #[test] + fn parse_proxy_with_credentials() { + let p = UpstreamProxy::parse("http://alice:s3cr3t@proxy.corp:3128").unwrap(); + // base64("alice:s3cr3t") + assert_eq!(p.auth.unwrap().to_str().unwrap(), "Basic YWxpY2U6czNjcjN0"); + } + + #[test] + fn parse_credentials_are_percent_decoded() { + // "p@ss:word" encoded in the userinfo. + let p = UpstreamProxy::parse("http://user:p%40ss%3Aword@proxy.corp:3128").unwrap(); + assert_eq!( + p.auth.unwrap().to_str().unwrap(), + format!("Basic {}", base64_encode(b"user:p@ss:word")) + ); + } + + #[test] + fn reject_unknown_scheme() { + assert!(UpstreamProxy::parse("ftp://proxy.corp:21").is_err()); + } + + #[test] + fn reject_empty_spec() { + assert!(UpstreamProxy::parse(" ").is_err()); + } + + /// Drive the proxy side of an in-memory duplex: read request headers up to + /// the blank line, then reply with `response`. Returns the request text. + async fn fake_proxy(mut end: tokio::io::DuplexStream, response: &'static [u8]) -> String { + let mut buf = Vec::new(); + let mut byte = [0u8; 1]; + loop { + let n = end.read(&mut byte).await.unwrap(); + if n == 0 { + break; + } + buf.push(byte[0]); + if buf.ends_with(b"\r\n\r\n") { + break; + } + } + end.write_all(response).await.unwrap(); + end.flush().await.unwrap(); + String::from_utf8_lossy(&buf).into_owned() + } + + #[tokio::test] + async fn connect_tunnel_sends_request_and_accepts_2xx() { + let (mut client_end, proxy_end) = tokio::io::duplex(1024); + let proxy = tokio::spawn(fake_proxy( + proxy_end, + b"HTTP/1.1 200 Connection established\r\n\r\n", + )); + + let auth = HeaderValue::from_static("Basic dXNlcjpwYXNz"); + establish_connect_tunnel(&mut client_end, "example.com", 443, Some(&auth)) + .await + .unwrap(); + + let request = proxy.await.unwrap(); + assert!(request.starts_with("CONNECT example.com:443 HTTP/1.1\r\n")); + assert!(request.contains("Host: example.com:443\r\n")); + assert!(request.contains("Proxy-Authorization: Basic dXNlcjpwYXNz\r\n")); + } + + #[tokio::test] + async fn connect_tunnel_brackets_ipv6_literal() { + let (mut client_end, proxy_end) = tokio::io::duplex(1024); + let proxy = tokio::spawn(fake_proxy( + proxy_end, + b"HTTP/1.1 200 Connection established\r\n\r\n", + )); + + establish_connect_tunnel(&mut client_end, "::1", 443, None) + .await + .unwrap(); + + let request = proxy.await.unwrap(); + assert!(request.starts_with("CONNECT [::1]:443 HTTP/1.1\r\n")); + } + + #[tokio::test] + async fn connect_tunnel_rejects_non_2xx() { + let (mut client_end, proxy_end) = tokio::io::duplex(1024); + tokio::spawn(fake_proxy(proxy_end, b"HTTP/1.1 403 Forbidden\r\n\r\n")); + + let err = establish_connect_tunnel(&mut client_end, "blocked.test", 443, None) + .await + .unwrap_err(); + assert!(err.to_string().contains("403"), "unexpected error: {}", err); + } +} From a28754e55aebb26997a8b4cc711738faffc87321 Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Mon, 13 Jul 2026 22:59:21 +0900 Subject: [PATCH 02/23] fix: redact upstream proxy credentials --- src/main.rs | 29 ++++++++++++++++++++++++++++- src/upstream.rs | 41 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/src/main.rs b/src/main.rs index 625ef95d..6fd6dce0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -7,6 +7,7 @@ use httpjail::rules::shell::ShellRuleEngine; use httpjail::rules::v8_js::V8JsRuleEngine; use httpjail::rules::{Action, RuleEngine}; use hyper::Method; +use std::fmt; use std::fs::OpenOptions; use std::os::unix::process::ExitStatusExt; use std::sync::atomic::{AtomicBool, Ordering}; @@ -40,7 +41,7 @@ enum Command { }, } -#[derive(Parser, Debug)] +#[derive(Parser)] struct RunArgs { /// Use shell script for evaluating requests /// The script receives environment variables: @@ -146,6 +147,32 @@ struct RunArgs { exec_command: Vec, } +impl fmt::Debug for RunArgs { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let upstream_proxy = self + .upstream_proxy + .as_deref() + .map(httpjail::upstream::redact_proxy_spec); + f.debug_struct("RunArgs") + .field("sh", &self.sh) + .field("proc", &self.proc) + .field("js", &self.js) + .field("js_file", &self.js_file) + .field("request_log", &self.request_log) + .field("upstream_proxy", &upstream_proxy) + .field("weak", &self.weak) + .field("verbose", &self.verbose) + .field("timeout", &self.timeout) + .field("no_jail_cleanup", &self.no_jail_cleanup) + .field("cleanup", &self.cleanup) + .field("server", &self.server) + .field("test", &self.test) + .field("docker_run", &self.docker_run) + .field("exec_command", &self.exec_command) + .finish() + } +} + fn setup_logging(verbosity: u8) { use tracing_subscriber::fmt::time::FormatTime; diff --git a/src/upstream.rs b/src/upstream.rs index dcb9eb6a..437fd213 100644 --- a/src/upstream.rs +++ b/src/upstream.rs @@ -85,6 +85,7 @@ impl UpstreamProxy { if spec.is_empty() { bail!("Upstream proxy specification is empty"); } + let redacted_spec = redact_proxy_spec(spec); // Accept a bare `host:port` by assuming the http scheme. let (scheme, rest) = match spec.split_once("://") { @@ -95,7 +96,11 @@ impl UpstreamProxy { let tls = match scheme.as_str() { "http" => false, "https" => true, - other => bail!("Unsupported upstream proxy scheme '{}': {}", other, spec), + other => bail!( + "Unsupported upstream proxy scheme '{}': {}", + other, + redacted_spec + ), }; // Drop any path/query/fragment component; only the authority is used. @@ -109,7 +114,7 @@ impl UpstreamProxy { let default_port = if tls { 443 } else { 80 }; let (host, port) = parse_host_port(host_port, default_port) - .with_context(|| format!("Invalid upstream proxy authority: {}", spec))?; + .with_context(|| format!("Invalid upstream proxy authority: {}", redacted_spec))?; let auth = match userinfo { Some(userinfo) => Some(build_basic_auth(userinfo)?), @@ -133,6 +138,22 @@ impl UpstreamProxy { } } +/// Redact userinfo from a proxy URL-like string before it is logged or included +/// in an error message. +pub fn redact_proxy_spec(spec: &str) -> String { + let spec = spec.trim(); + let (prefix, rest) = match spec.split_once("://") { + Some((scheme, rest)) => (format!("{scheme}://"), rest), + None => (String::new(), spec), + }; + let authority_end = rest.find(['/', '?', '#']).unwrap_or(rest.len()); + let (authority, suffix) = rest.split_at(authority_end); + let Some((_, host_port)) = authority.rsplit_once('@') else { + return spec.to_string(); + }; + format!("{prefix}@{host_port}{suffix}") +} + /// Split a `host:port` authority into its parts, handling bracketed IPv6 /// literals (`[::1]:3128`). Falls back to `default_port` when no port is given. fn parse_host_port(authority: &str, default_port: u16) -> Result<(String, u16)> { @@ -533,6 +554,22 @@ mod tests { ); } + #[test] + fn redact_proxy_spec_removes_userinfo() { + assert_eq!( + redact_proxy_spec("http://user:secret@proxy.corp:3128/path"), + "http://@proxy.corp:3128/path" + ); + assert_eq!( + redact_proxy_spec("user:secret@proxy.corp:3128"), + "@proxy.corp:3128" + ); + assert_eq!( + redact_proxy_spec("http://proxy.corp:3128"), + "http://proxy.corp:3128" + ); + } + #[test] fn reject_unknown_scheme() { assert!(UpstreamProxy::parse("ftp://proxy.corp:21").is_err()); From 1bd3203d1af7b88285491a690d8191c185efd189 Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Mon, 13 Jul 2026 23:01:57 +0900 Subject: [PATCH 03/23] fix: time out upstream proxy TCP connects --- src/upstream.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/upstream.rs b/src/upstream.rs index 437fd213..10d5d00d 100644 --- a/src/upstream.rs +++ b/src/upstream.rs @@ -243,7 +243,10 @@ impl Service for ProxyConnector { // Dial the proxy (TCP). The destination scheme is irrelevant here; // we always connect to the proxy's host:port. let proxy_uri: Uri = format!("http://{}:{}", proxy.host, proxy.port).parse()?; - let tcp = http.call(proxy_uri).await?.into_inner(); + let tcp = match timeout(PROXY_SETUP_TIMEOUT, http.call(proxy_uri)).await { + Ok(result) => result?.into_inner(), + Err(_) => return Err(timed_out("connecting to upstream proxy")), + }; let _ = tcp.set_nodelay(true); // Optionally negotiate TLS with the proxy itself. From d08e61c37235491137d1e3f4e66a395df4d0877d Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Mon, 13 Jul 2026 23:03:57 +0900 Subject: [PATCH 04/23] fix: support IPv6 upstream proxy addresses --- src/upstream.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/upstream.rs b/src/upstream.rs index 10d5d00d..15c96449 100644 --- a/src/upstream.rs +++ b/src/upstream.rs @@ -242,7 +242,8 @@ impl Service for ProxyConnector { Box::pin(async move { // Dial the proxy (TCP). The destination scheme is irrelevant here; // we always connect to the proxy's host:port. - let proxy_uri: Uri = format!("http://{}:{}", proxy.host, proxy.port).parse()?; + let proxy_uri: Uri = format!("http://{}", host_port_authority(&proxy.host, proxy.port)) + .parse()?; let tcp = match timeout(PROXY_SETUP_TIMEOUT, http.call(proxy_uri)).await { Ok(result) => result?.into_inner(), Err(_) => return Err(timed_out("connecting to upstream proxy")), @@ -366,11 +367,7 @@ where S: AsyncRead + AsyncWrite + Unpin, { // Bracket IPv6 literals in the request-target and Host header. - let target = if host.contains(':') { - format!("[{host}]:{port}") - } else { - format!("{host}:{port}") - }; + let target = host_port_authority(host, port); let mut request = format!("CONNECT {target} HTTP/1.1\r\nHost: {target}\r\n"); if let Some(value) = auth { @@ -413,6 +410,16 @@ where Ok(()) } +/// Format a host and port for use as an HTTP authority, bracketing IPv6 +/// literals as required by URI syntax. +fn host_port_authority(host: &str, port: u16) -> String { + if host.contains(':') { + format!("[{host}]:{port}") + } else { + format!("{host}:{port}") + } +} + /// Read the proxy's `CONNECT` response up to the end of its headers and return /// the HTTP status code. Reads are bounded by [`MAX_CONNECT_RESPONSE_BYTES`] to /// avoid consuming tunnel payload and to bound memory. @@ -540,6 +547,12 @@ mod tests { assert_eq!(p.port, 3128); } + #[test] + fn host_port_authority_brackets_ipv6_literal() { + assert_eq!(host_port_authority("::1", 3128), "[::1]:3128"); + assert_eq!(host_port_authority("proxy.corp", 3128), "proxy.corp:3128"); + } + #[test] fn parse_proxy_with_credentials() { let p = UpstreamProxy::parse("http://alice:s3cr3t@proxy.corp:3128").unwrap(); From 3d11be88d68da70766655546326789ccf62c527f Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Mon, 13 Jul 2026 23:15:05 +0900 Subject: [PATCH 05/23] refactor: use libraries for proxy credential encoding --- Cargo.lock | 2 ++ Cargo.toml | 2 ++ src/upstream.rs | 67 +++++-------------------------------------------- 3 files changed, 10 insertions(+), 61 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 29e88d90..007ef279 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1005,6 +1005,7 @@ dependencies = [ "assert_cmd", "async-trait", "atty", + "base64", "bytes", "camino", "chrono", @@ -1020,6 +1021,7 @@ dependencies = [ "hyper-util", "libc", "lru", + "percent-encoding", "pprof", "predicates", "rand", diff --git a/Cargo.toml b/Cargo.toml index 068f5dfb..b2ffec42 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ webpki-roots = "0.26" lru = "0.12" rand = "0.8" anyhow = "1.0" +base64 = "0.22" tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter", "chrono"] } chrono = "0.4" @@ -37,6 +38,7 @@ tls-parser = "0.12.2" camino = "1.1.11" filetime = "0.2" ctrlc = "3.4" +percent-encoding = "2.3" url = "2.5" v8 = "129" serde = { version = "1.0", features = ["derive"] } diff --git a/src/upstream.rs b/src/upstream.rs index 15c96449..a996de39 100644 --- a/src/upstream.rs +++ b/src/upstream.rs @@ -24,11 +24,13 @@ //! (WebSocket, gRPC, ...) keep working. use anyhow::{Context as _, Result, anyhow, bail}; +use base64::{Engine as _, engine::general_purpose::STANDARD}; use hyper::Uri; use hyper::header::HeaderValue; use hyper::rt::{Read, ReadBufCursor, Write}; use hyper_util::client::legacy::connect::{Connected, Connection, HttpConnector}; use hyper_util::rt::TokioIo; +use percent_encoding::percent_decode_str; use rustls::pki_types::ServerName; use std::future::Future; use std::io; @@ -188,8 +190,9 @@ fn build_basic_auth(userinfo: &str) -> Result { Some((user, pass)) => (user, pass), None => (userinfo, ""), }; - let token = - base64_encode(format!("{}:{}", percent_decode(user), percent_decode(pass)).as_bytes()); + let user = percent_decode_str(user).decode_utf8_lossy(); + let pass = percent_decode_str(pass).decode_utf8_lossy(); + let token = STANDARD.encode(format!("{user}:{pass}")); HeaderValue::from_str(&format!("Basic {}", token)) .context("Invalid characters in upstream proxy credentials") } @@ -454,68 +457,10 @@ where .ok_or_else(|| anyhow!("Malformed CONNECT status line: {:?}", first_line)) } -/// Minimal RFC 4648 base64 encoder (standard alphabet, with padding). Used only -/// to build the `Proxy-Authorization: Basic ...` credential token, avoiding a -/// dedicated base64 dependency. -fn base64_encode(input: &[u8]) -> String { - const ALPHABET: &[u8; 64] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/"; - let mut out = String::with_capacity(input.len().div_ceil(3) * 4); - for chunk in input.chunks(3) { - let b0 = chunk[0] as u32; - let b1 = *chunk.get(1).unwrap_or(&0) as u32; - let b2 = *chunk.get(2).unwrap_or(&0) as u32; - let triple = (b0 << 16) | (b1 << 8) | b2; - out.push(ALPHABET[((triple >> 18) & 0x3f) as usize] as char); - out.push(ALPHABET[((triple >> 12) & 0x3f) as usize] as char); - out.push(if chunk.len() > 1 { - ALPHABET[((triple >> 6) & 0x3f) as usize] as char - } else { - '=' - }); - out.push(if chunk.len() > 2 { - ALPHABET[(triple & 0x3f) as usize] as char - } else { - '=' - }); - } - out -} - -/// Decode `%XX` percent-escapes in the userinfo portion of a proxy URL. Any -/// malformed escape is left verbatim. -fn percent_decode(input: &str) -> String { - let bytes = input.as_bytes(); - let mut out = Vec::with_capacity(bytes.len()); - let mut i = 0; - while i < bytes.len() { - if bytes[i] == b'%' && i + 2 < bytes.len() { - let hi = (bytes[i + 1] as char).to_digit(16); - let lo = (bytes[i + 2] as char).to_digit(16); - if let (Some(hi), Some(lo)) = (hi, lo) { - out.push((hi * 16 + lo) as u8); - i += 3; - continue; - } - } - out.push(bytes[i]); - i += 1; - } - String::from_utf8_lossy(&out).into_owned() -} - #[cfg(test)] mod tests { use super::*; - #[test] - fn base64_matches_known_vectors() { - assert_eq!(base64_encode(b""), ""); - assert_eq!(base64_encode(b"f"), "Zg=="); - assert_eq!(base64_encode(b"fo"), "Zm8="); - assert_eq!(base64_encode(b"foo"), "Zm9v"); - assert_eq!(base64_encode(b"user:pass"), "dXNlcjpwYXNz"); - } - #[test] fn parse_plain_proxy() { let p = UpstreamProxy::parse("http://proxy.corp:3128").unwrap(); @@ -566,7 +511,7 @@ mod tests { let p = UpstreamProxy::parse("http://user:p%40ss%3Aword@proxy.corp:3128").unwrap(); assert_eq!( p.auth.unwrap().to_str().unwrap(), - format!("Basic {}", base64_encode(b"user:p@ss:word")) + "Basic dXNlcjpwQHNzOndvcmQ=" ); } From 7301b5a8246d4b1b9a2e8d0058adf9eb69820bf4 Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Mon, 13 Jul 2026 23:20:58 +0900 Subject: [PATCH 06/23] refactor: parse upstream proxy URLs with url crate --- src/upstream.rs | 83 ++++++++++++++++++------------------------------- 1 file changed, 31 insertions(+), 52 deletions(-) diff --git a/src/upstream.rs b/src/upstream.rs index a996de39..ced970e2 100644 --- a/src/upstream.rs +++ b/src/upstream.rs @@ -42,6 +42,7 @@ use tokio::time::{Duration, timeout}; use tokio_rustls::TlsConnector; use tower_service::Service; use tracing::debug; +use url::{Host, Url}; type BoxError = Box; @@ -88,14 +89,16 @@ impl UpstreamProxy { bail!("Upstream proxy specification is empty"); } let redacted_spec = redact_proxy_spec(spec); - - // Accept a bare `host:port` by assuming the http scheme. - let (scheme, rest) = match spec.split_once("://") { - Some((scheme, rest)) => (scheme.to_ascii_lowercase(), rest), - None => ("http".to_string(), spec), + let normalized = if spec.contains("://") { + spec.to_string() + } else { + format!("http://{spec}") }; - let tls = match scheme.as_str() { + let url = Url::parse(&normalized) + .with_context(|| format!("Invalid upstream proxy URL: {}", redacted_spec))?; + + let tls = match url.scheme() { "http" => false, "https" => true, other => bail!( @@ -105,22 +108,21 @@ impl UpstreamProxy { ), }; - // Drop any path/query/fragment component; only the authority is used. - let authority = rest.split(['/', '?', '#']).next().unwrap_or(rest); - - // Split optional `userinfo@` from the `host:port` authority. - let (userinfo, host_port) = match authority.rsplit_once('@') { - Some((userinfo, host_port)) => (Some(userinfo), host_port), - None => (None, authority), + let host = match url.host() { + Some(Host::Domain(host)) => host.to_string(), + Some(Host::Ipv4(host)) => host.to_string(), + Some(Host::Ipv6(host)) => host.to_string(), + None => bail!("Invalid upstream proxy authority: {}", redacted_spec), }; - let default_port = if tls { 443 } else { 80 }; - let (host, port) = parse_host_port(host_port, default_port) - .with_context(|| format!("Invalid upstream proxy authority: {}", redacted_spec))?; + let port = url + .port_or_known_default() + .ok_or_else(|| anyhow!("Invalid upstream proxy authority: {}", redacted_spec))?; - let auth = match userinfo { - Some(userinfo) => Some(build_basic_auth(userinfo)?), - None => None, + let auth = if !url.username().is_empty() || url.password().is_some() { + Some(build_basic_auth(url.username(), url.password())?) + } else { + None }; Ok(UpstreamProxy { @@ -156,42 +158,11 @@ pub fn redact_proxy_spec(spec: &str) -> String { format!("{prefix}@{host_port}{suffix}") } -/// Split a `host:port` authority into its parts, handling bracketed IPv6 -/// literals (`[::1]:3128`). Falls back to `default_port` when no port is given. -fn parse_host_port(authority: &str, default_port: u16) -> Result<(String, u16)> { - if let Some(rest) = authority.strip_prefix('[') { - // Bracketed IPv6 literal: `[addr]` or `[addr]:port`. - let (addr, after) = rest - .split_once(']') - .ok_or_else(|| anyhow!("unterminated IPv6 literal: {}", authority))?; - let port = match after.strip_prefix(':') { - Some(port) => port.parse().context("invalid port")?, - None if after.is_empty() => default_port, - None => bail!("unexpected characters after IPv6 literal: {}", authority), - }; - return Ok((addr.to_string(), port)); - } - - let (host, port) = match authority.rsplit_once(':') { - Some((host, port)) => (host, port.parse().context("invalid port")?), - None => (authority, default_port), - }; - - if host.is_empty() { - bail!("missing host: {}", authority); - } - Ok((host.to_string(), port)) -} - /// Build a `Proxy-Authorization: Basic ...` header value from `user:pass` /// userinfo, percent-decoding each component first. -fn build_basic_auth(userinfo: &str) -> Result { - let (user, pass) = match userinfo.split_once(':') { - Some((user, pass)) => (user, pass), - None => (userinfo, ""), - }; +fn build_basic_auth(user: &str, pass: Option<&str>) -> Result { let user = percent_decode_str(user).decode_utf8_lossy(); - let pass = percent_decode_str(pass).decode_utf8_lossy(); + let pass = percent_decode_str(pass.unwrap_or("")).decode_utf8_lossy(); let token = STANDARD.encode(format!("{user}:{pass}")); HeaderValue::from_str(&format!("Basic {}", token)) .context("Invalid characters in upstream proxy credentials") @@ -470,6 +441,14 @@ mod tests { assert!(p.auth.is_none()); } + #[test] + fn parse_proxy_ignores_path_query_and_fragment() { + let p = UpstreamProxy::parse("http://proxy.corp:3128/path?ignored=true#frag").unwrap(); + assert_eq!(p.host, "proxy.corp"); + assert_eq!(p.port, 3128); + assert!(!p.tls); + } + #[test] fn parse_bare_hostport_defaults_to_http() { let p = UpstreamProxy::parse("proxy.corp:8080").unwrap(); From 701d116d3a9e5875a54229871f61b4f67879f564 Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Tue, 21 Jul 2026 21:24:22 +0900 Subject: [PATCH 07/23] fix: use proxy environment for upstream egress Remove the upstream proxy CLI option and httpjail-specific environment variable, and resolve httpjail's own upstream proxy from HTTP_PROXY and HTTPS_PROXY instead. Also keep the new upstream proxy initialization logs at debug level so normal CLI output is not affected. --- README.md | 17 ++--- docs/advanced/upstream-proxy.md | 35 ++++----- docs/guide/configuration.md | 17 +++-- src/jail/linux/docker.rs | 7 ++ src/jail/linux/mod.rs | 7 ++ src/main.rs | 43 +++-------- src/proxy.rs | 50 ++++++------- src/upstream.rs | 125 ++++++++++++++++++++++++++++++-- 8 files changed, 205 insertions(+), 96 deletions(-) diff --git a/README.md b/README.md index 33b5651f..8be65841 100644 --- a/README.md +++ b/README.md @@ -64,25 +64,24 @@ httpjail --server --js "true" httpjail --js "r.host === 'api.github.com'" --docker-run -- --rm alpine:latest wget -qO- https://api.github.com # Route httpjail's own egress through an upstream (corporate) proxy -httpjail --upstream-proxy http://proxy.corp:3128 --js "true" -- curl https://api.github.com +HTTPS_PROXY=http://proxy.corp:3128 httpjail --js "true" -- curl https://api.github.com # Credentials and HTTPS proxies are supported: http://user:pass@proxy.corp:3128, https://proxy.corp:8443 -# May also be set via the HTTPJAIL_UPSTREAM_PROXY environment variable ``` ### Upstream (corporate) proxy -When httpjail itself runs in an environment with no direct internet access, use -`--upstream-proxy ` (or the `HTTPJAIL_UPSTREAM_PROXY` environment variable) -to route httpjail's outbound requests through an upstream proxy. Rule evaluation -still happens locally on the intercepted traffic; only the re-originated request -is forwarded through the proxy. +When httpjail itself runs in an environment with no direct internet access, set +the `HTTP_PROXY` and/or `HTTPS_PROXY` environment variables to route httpjail's +outbound requests through an upstream proxy. Rule evaluation still happens +locally on the intercepted traffic; only the re-originated request is forwarded +through the proxy. - `http://`, `https://` and bare `host:port` (http assumed) forms are accepted. - Basic authentication is supported via `http://user:pass@host:port`. - HTTPS destinations are reached via a `CONNECT` tunnel through the proxy, while plain HTTP destinations are forwarded in absolute-form. -- This is independent of the `HTTP_PROXY`/`HTTPS_PROXY` variables that httpjail - sets *inside* the jail to point sandboxed processes at itself. +- In weak mode, httpjail overwrites proxy env vars inside the jailed process to + point sandboxed processes at httpjail itself. ## Documentation diff --git a/docs/advanced/upstream-proxy.md b/docs/advanced/upstream-proxy.md index 35061ff2..c1faac7f 100644 --- a/docs/advanced/upstream-proxy.md +++ b/docs/advanced/upstream-proxy.md @@ -3,25 +3,25 @@ By default httpjail contacts destination servers directly. When httpjail itself runs in an environment that has no direct internet access — for example behind a corporate proxy — you can route httpjail's own outbound requests through an -upstream proxy with `--upstream-proxy` (or the `HTTPJAIL_UPSTREAM_PROXY` -environment variable). +upstream proxy with the `HTTP_PROXY` and/or `HTTPS_PROXY` environment variables. Rule evaluation still happens locally on the intercepted traffic. Only the request that httpjail re-originates towards the real destination is forwarded through the upstream proxy. ```bash -# Route httpjail's egress through a corporate proxy -httpjail --upstream-proxy http://proxy.corp:3128 --js "true" -- curl https://api.github.com +# Route httpjail's HTTPS egress through a corporate proxy +HTTPS_PROXY=http://proxy.corp:3128 httpjail --js "true" -- curl https://api.github.com + +# Route both HTTP and HTTPS egress through the same proxy +HTTP_PROXY=http://proxy.corp:3128 HTTPS_PROXY=http://proxy.corp:3128 \ + httpjail --js "true" -- ./my-app # With Basic authentication -httpjail --upstream-proxy http://user:pass@proxy.corp:3128 --js "true" -- ./my-app +HTTPS_PROXY=http://user:pass@proxy.corp:3128 httpjail --js "true" -- ./my-app # Through an HTTPS proxy -httpjail --upstream-proxy https://proxy.corp:8443 --js "true" -- ./my-app - -# Via the environment variable (equivalent to --upstream-proxy) -HTTPJAIL_UPSTREAM_PROXY=http://proxy.corp:3128 httpjail --js "true" -- ./my-app +HTTPS_PROXY=https://proxy.corp:8443 httpjail --js "true" -- ./my-app ``` ## Accepted formats @@ -33,8 +33,8 @@ HTTPJAIL_UPSTREAM_PROXY=http://proxy.corp:3128 httpjail --js "true" -- ./my-app | `host:port` | `proxy.corp:3128` | Bare authority, `http` scheme assumed | | With credentials | `http://user:pass@proxy.corp:3128` | Sends `Proxy-Authorization: Basic ...` | -The command-line flag takes precedence over the environment variable. Credentials -are never written to the logs. +`HTTP_PROXY` is used for `http://` destinations. `HTTPS_PROXY` is used for +`https://` destinations. Credentials are never written to the logs. ## How it works @@ -48,14 +48,15 @@ are never written to the logs. `CONNECT` exchange) is bounded by a timeout. The established tunnel carries no timeout, so long-running connections such as WebSocket and gRPC keep working. -## Relationship to `HTTP_PROXY` / `HTTPS_PROXY` +## Relationship to jailed process proxy variables -This feature is independent of the `HTTP_PROXY` and `HTTPS_PROXY` variables that -httpjail sets *inside* the jail to point sandboxed processes at httpjail itself. +The proxy environment variables configure httpjail's own egress. In weak mode, +httpjail overwrites `HTTP_PROXY` and `HTTPS_PROXY` inside the jailed process to +point sandboxed processes at httpjail itself. ``` -[ jailed process ] --HTTP_PROXY/HTTPS_PROXY--> [ httpjail ] --upstream-proxy--> [ corporate proxy ] --> internet +[ jailed process ] --> [ httpjail ] --HTTP_PROXY/HTTPS_PROXY--> [ corporate proxy ] --> internet ``` -The jailed process always talks to httpjail; `--upstream-proxy` only affects the -hop from httpjail to the outside world. +The jailed process talks to httpjail; the proxy env vars only affect the hop +from httpjail to the outside world. diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 527d3655..5993fa0e 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -7,7 +7,7 @@ httpjail's behavior can be configured through command-line options, environment httpjail follows a simple configuration hierarchy: 1. **Command-line options** - Highest priority, override everything -2. **Environment variables** - Set by httpjail for the jailed process +2. **Environment variables** - Configure httpjail and the jailed process ## Key Configuration Areas @@ -72,9 +72,11 @@ httpjail --proc ./rate-limiter.py \ ## Environment Variables -### Set by httpjail +### Set for the jailed process -These are automatically set in the jailed process: +These are set in the jailed process where applicable. In weak mode, httpjail +sets proxy variables so applications talk to httpjail. On Linux strong mode, +traffic is redirected transparently without setting proxy variables. | Variable | Description | Example | | --------------- | ---------------------------- | ------------------------ | @@ -84,7 +86,7 @@ These are automatically set in the jailed process: | `SSL_CERT_DIR` | CA certificate directory | `/tmp/httpjail-certs/` | | `NO_PROXY` | Bypass proxy for these hosts | `localhost,127.0.0.1` | -### Controlling httpjail +### Consumed by httpjail These affect httpjail's behavior: @@ -92,11 +94,10 @@ These affect httpjail's behavior: | ------------------------ | -------------------------------------- | -------------------------------- | | `RUST_LOG` | Logging level | `debug`, `info`, `warn`, `error` | | `HTTPJAIL_CA_CERT` | Custom CA certificate path | `/etc/pki/custom-ca.pem` | -| `HTTPJAIL_UPSTREAM_PROXY`| Upstream proxy for httpjail's egress | `http://proxy.corp:3128` | +| `HTTP_PROXY` | Upstream proxy for httpjail HTTP egress | `http://proxy.corp:3128` | +| `HTTPS_PROXY` | Upstream proxy for httpjail HTTPS egress | `http://proxy.corp:3128` | -The `--upstream-proxy` command-line flag takes precedence over -`HTTPJAIL_UPSTREAM_PROXY`. See [Upstream Proxy](../advanced/upstream-proxy.md) -for details. +See [Upstream Proxy](../advanced/upstream-proxy.md) for details. ## Platform-Specific Configuration diff --git a/src/jail/linux/docker.rs b/src/jail/linux/docker.rs index 5282cc65..68bb7efa 100644 --- a/src/jail/linux/docker.rs +++ b/src/jail/linux/docker.rs @@ -311,6 +311,13 @@ impl DockerLinux { let mut cmd = Command::new("docker"); cmd.arg("run"); + // The parent process may use proxy env vars for httpjail's own egress. + // Do not leak those credentials or settings into the Docker CLI process; + // Docker network isolation routes container traffic through httpjail. + for key in ["HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"] { + cmd.env_remove(key); + } + // Use our isolated Docker network cmd.args(["--network", &network_name]); diff --git a/src/jail/linux/mod.rs b/src/jail/linux/mod.rs index 7d6165f8..8d7810cd 100644 --- a/src/jail/linux/mod.rs +++ b/src/jail/linux/mod.rs @@ -544,6 +544,13 @@ impl Jail for LinuxJail { cmd.env(key, value); } + // The parent process may use proxy env vars for httpjail's own egress. + // Do not leak those credentials or settings into the jailed command; + // native Linux isolation redirects traffic transparently. + for key in ["HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"] { + cmd.env_remove(key); + } + // Preserve SUDO environment variables for consistency with macOS if let Ok(sudo_user) = std::env::var("SUDO_USER") { cmd.env("SUDO_USER", sudo_user); diff --git a/src/main.rs b/src/main.rs index 6fd6dce0..db25a455 100644 --- a/src/main.rs +++ b/src/main.rs @@ -86,13 +86,6 @@ struct RunArgs { #[arg(long = "request-log", value_name = "FILE")] request_log: Option, - /// Route httpjail's own upstream requests through an upstream (corporate) proxy. - /// Accepts http://host:port, https://host:port, or host:port (http assumed), - /// optionally with credentials: http://user:pass@host:port. - /// Falls back to the HTTPJAIL_UPSTREAM_PROXY environment variable. - #[arg(long = "upstream-proxy", value_name = "URL")] - upstream_proxy: Option, - /// Use weak mode (environment variables only, no system isolation) #[arg(long = "weak")] weak: bool, @@ -149,17 +142,12 @@ struct RunArgs { impl fmt::Debug for RunArgs { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let upstream_proxy = self - .upstream_proxy - .as_deref() - .map(httpjail::upstream::redact_proxy_spec); f.debug_struct("RunArgs") .field("sh", &self.sh) .field("proc", &self.proc) .field("js", &self.js) .field("js_file", &self.js_file) .field("request_log", &self.request_log) - .field("upstream_proxy", &upstream_proxy) .field("weak", &self.weak) .field("verbose", &self.verbose) .field("timeout", &self.timeout) @@ -624,27 +612,18 @@ async fn main() -> Result<()> { } }; - // Resolve the optional upstream (corporate) proxy from the flag or env var. - // This is independent of the HTTP_PROXY/HTTPS_PROXY variables httpjail sets - // *inside* the jail to point sandboxed processes at itself. - let upstream_proxy_spec = args - .run_args - .upstream_proxy - .clone() - .or_else(|| std::env::var("HTTPJAIL_UPSTREAM_PROXY").ok()); - let upstream_proxy = match upstream_proxy_spec { - Some(spec) => { - let proxy = httpjail::upstream::UpstreamProxy::parse(&spec) - .with_context(|| format!("Failed to parse upstream proxy: {}", spec))?; - // Avoid logging the spec verbatim as it may contain credentials. - info!("Routing httpjail upstream requests through the configured upstream proxy"); - Some(proxy) - } - None => None, - }; + let upstream_proxies = httpjail::upstream::UpstreamProxies::from_env() + .context("Failed to configure upstream proxy from environment")?; + if upstream_proxies.is_some() { + debug!("Routing httpjail upstream requests through the proxy environment"); + } - let mut proxy = - ProxyServer::new_with_upstream_proxy(http_bind, https_bind, rule_engine, upstream_proxy); + let mut proxy = ProxyServer::new_with_upstream_proxies( + http_bind, + https_bind, + rule_engine, + upstream_proxies, + ); // Start proxy in background if running as server; otherwise start with random ports let (actual_http_port, actual_https_port) = proxy.start().await?; diff --git a/src/proxy.rs b/src/proxy.rs index bebb5beb..da7315a3 100644 --- a/src/proxy.rs +++ b/src/proxy.rs @@ -3,12 +3,12 @@ use crate::dangerous_verifier::create_dangerous_client_config; use crate::rules::{Action, RuleEngine}; #[allow(unused_imports)] use crate::tls::CertificateManager; -use crate::upstream::{ProxyConnector, UpstreamProxy}; +use crate::upstream::{ProxyConnector, UpstreamProxies}; use anyhow::Result; use bytes::Bytes; use http_body_util::{BodyExt, Full, combinators::BoxBody}; use hyper::body::Incoming; -use hyper::header::{HeaderValue, PROXY_AUTHORIZATION}; +use hyper::header::PROXY_AUTHORIZATION; use hyper::server::conn::http1; use hyper::service::service_fn; use hyper::{Error as HyperError, Request, Response, StatusCode, Uri}; @@ -176,9 +176,7 @@ pub enum UpstreamClient { Direct(DirectClient), Proxied { client: ProxiedClient, - /// Attached to plain-HTTP requests forwarded through the proxy in - /// absolute-form (HTTPS carries credentials on the CONNECT instead). - http_auth: Option, + proxies: UpstreamProxies, }, } @@ -191,9 +189,9 @@ impl UpstreamClient { ) -> Result> { match self { UpstreamClient::Direct(client) => client.request(req).await.map_err(Into::into), - UpstreamClient::Proxied { client, http_auth } => { + UpstreamClient::Proxied { client, proxies } => { if req.uri().scheme_str() == Some("http") - && let Some(auth) = http_auth + && let Some(auth) = proxies.http_auth() { req.headers_mut().insert(PROXY_AUTHORIZATION, auth.clone()); } @@ -336,23 +334,24 @@ fn build_direct_connector( } /// Initialize the shared upstream client with the httpjail CA certificate and an -/// optional upstream proxy. When a proxy is configured, all re-originated -/// requests are routed through it; otherwise destinations are contacted directly. +/// optional upstream proxies. When a proxy is configured for a destination +/// scheme, matching re-originated requests are routed through it; otherwise +/// destinations are contacted directly. pub fn init_client_with_ca( ca_cert_der: CertificateDer<'static>, - upstream_proxy: Option, + upstream_proxies: Option, ) { HTTPS_CLIENT.get_or_init(|| { // Check if we should dangerously disable cert validation (TESTING ONLY!) let dangerous = std::env::var("HTTPJAIL_DANGER_DISABLE_CERT_VALIDATION").is_ok(); - match upstream_proxy { + match upstream_proxies { None => { let https = build_direct_connector(ca_cert_der, dangerous); - info!("HTTPS connector initialized with webpki roots and httpjail CA"); + debug!("HTTPS connector initialized with webpki roots and httpjail CA"); UpstreamClient::Direct(build_pooled_client(https)) } - Some(proxy) => { + Some(proxies) => { // Both the destination TLS (layered over the CONNECT tunnel by // the HttpsConnector) and the optional https:// proxy TLS trust // the same roots as the direct client. @@ -363,13 +362,13 @@ pub fn init_client_with_ca( create_client_config_with_ca(ca_cert_der.clone()) } }; - let http_auth = proxy.http_auth(); - let connector = ProxyConnector::new(proxy, Arc::new(make_config())); + let connector = + ProxyConnector::with_config(proxies.clone(), Arc::new(make_config())); let https = hyper_rustls::HttpsConnector::from((connector, make_config())); - info!("Upstream client initialized to route through the upstream proxy"); + debug!("Upstream client initialized to route through the upstream proxy"); UpstreamClient::Proxied { client: build_pooled_client(https), - http_auth, + proxies, } } } @@ -484,22 +483,22 @@ impl ProxyServer { https_bind: Option, rule_engine: RuleEngine, ) -> Self { - Self::new_with_upstream_proxy(http_bind, https_bind, rule_engine, None) + Self::new_with_upstream_proxies(http_bind, https_bind, rule_engine, None) } /// Like [`ProxyServer::new`], but routes httpjail's own re-originated - /// requests through the given upstream (corporate) proxy when set. - pub fn new_with_upstream_proxy( + /// requests through the configured upstream proxies when set. + pub fn new_with_upstream_proxies( http_bind: Option, https_bind: Option, rule_engine: RuleEngine, - upstream_proxy: Option, + upstream_proxies: Option, ) -> Self { let cert_manager = CertificateManager::new().expect("Failed to create certificate manager"); // Initialize the HTTP client with our CA certificate let ca_cert_der = cert_manager.get_ca_cert_der(); - init_client_with_ca(ca_cert_der, upstream_proxy); + init_client_with_ca(ca_cert_der, upstream_proxies); // Generate a unique nonce for loop detection (Issue #84) // Use 16 random hex characters for a reasonably short but collision-resistant ID @@ -875,14 +874,15 @@ mod tests { String::from_utf8_lossy(&buf).into_owned() }); - let proxy = UpstreamProxy::parse(&format!("http://user:pass@{}", addr)).unwrap(); - let http_auth = proxy.http_auth(); + let proxy = + crate::upstream::UpstreamProxy::parse(&format!("http://user:pass@{}", addr)).unwrap(); + let proxies = UpstreamProxies::all(proxy.clone()); let connector = ProxyConnector::new(proxy, Arc::new(create_dangerous_client_config())); let https = hyper_rustls::HttpsConnector::from((connector, create_dangerous_client_config())); let client = UpstreamClient::Proxied { client: build_pooled_client(https), - http_auth, + proxies, }; let body = Empty::::new() diff --git a/src/upstream.rs b/src/upstream.rs index ced970e2..ad98cc40 100644 --- a/src/upstream.rs +++ b/src/upstream.rs @@ -79,6 +79,76 @@ pub struct UpstreamProxy { auth: Option, } +/// Upstream proxy configuration resolved from the proxy environment. +#[derive(Clone, Debug)] +pub struct UpstreamProxies { + http: Option, + https: Option, +} + +impl UpstreamProxies { + /// Resolve httpjail's own egress proxy settings from the proxy environment. + pub fn from_env() -> Result> { + let http = proxy_from_env("HTTP_PROXY", "http_proxy")?; + let https = proxy_from_env("HTTPS_PROXY", "https_proxy")?; + Ok(Self::from_proxies(http, https)) + } + + fn proxy_for_uri(&self, uri: &Uri) -> Option<&UpstreamProxy> { + match uri.scheme_str() { + Some("http") => self.http.as_ref(), + Some("https") => self.https.as_ref(), + _ => None, + } + } + + pub(crate) fn http_auth(&self) -> Option { + self.http.as_ref().and_then(UpstreamProxy::http_auth) + } + + pub(crate) fn all(proxy: UpstreamProxy) -> Self { + Self { + http: Some(proxy.clone()), + https: Some(proxy), + } + } + + fn from_proxies(http: Option, https: Option) -> Option { + if http.is_none() && https.is_none() { + return None; + } + Some(Self { http, https }) + } + + #[cfg(test)] + fn from_specs(http: Option<&str>, https: Option<&str>) -> Result> { + let http = parse_optional_proxy_spec("HTTP_PROXY", http)?; + let https = parse_optional_proxy_spec("HTTPS_PROXY", https)?; + Ok(Self::from_proxies(http, https)) + } +} + +fn proxy_from_env(primary: &str, fallback: &str) -> Result> { + for name in [primary, fallback] { + if let Ok(value) = std::env::var(name) { + let proxy = parse_optional_proxy_spec(name, Some(&value))?; + if proxy.is_some() { + return Ok(proxy); + } + } + } + Ok(None) +} + +fn parse_optional_proxy_spec(name: &str, spec: Option<&str>) -> Result> { + let Some(spec) = spec.map(str::trim).filter(|spec| !spec.is_empty()) else { + return Ok(None); + }; + UpstreamProxy::parse(spec) + .map(Some) + .with_context(|| format!("Failed to parse {name}")) +} + impl UpstreamProxy { /// Parse an upstream proxy specification such as `http://proxy.corp:3128`, /// `http://user:pass@proxy.corp:3128`, `https://proxy.corp:8443` or a bare @@ -179,13 +249,17 @@ fn build_basic_auth(user: &str, pass: Option<&str>) -> Result { pub struct ProxyConnector { /// Used solely to dial the proxy's `host:port` (never the destination). http: HttpConnector, - proxy: Arc, + proxies: Arc, /// TLS configuration used only when the proxy itself is `https://`. proxy_tls: Arc, } impl ProxyConnector { pub fn new(proxy: UpstreamProxy, proxy_tls: Arc) -> Self { + Self::with_config(UpstreamProxies::all(proxy), proxy_tls) + } + + pub fn with_config(proxies: UpstreamProxies, proxy_tls: Arc) -> Self { let mut http = HttpConnector::new(); // The proxy is addressed via an http(s) URL; allow non-http schemes so // the connector does not reject the dial target. @@ -193,7 +267,7 @@ impl ProxyConnector { http.set_happy_eyeballs_timeout(Some(Duration::from_millis(250))); ProxyConnector { http, - proxy: Arc::new(proxy), + proxies: Arc::new(proxies), proxy_tls, } } @@ -210,14 +284,23 @@ impl Service for ProxyConnector { fn call(&mut self, dst: Uri) -> Self::Future { let mut http = self.http.clone(); - let proxy = Arc::clone(&self.proxy); + let proxy = self.proxies.proxy_for_uri(&dst).cloned(); let proxy_tls = Arc::clone(&self.proxy_tls); Box::pin(async move { + let Some(proxy) = proxy else { + let tcp = match timeout(PROXY_SETUP_TIMEOUT, http.call(dst.clone())).await { + Ok(result) => result?.into_inner(), + Err(_) => return Err(timed_out("connecting directly to destination")), + }; + let _ = tcp.set_nodelay(true); + return Ok(ProxyStream::new(Box::new(tcp), false)); + }; + // Dial the proxy (TCP). The destination scheme is irrelevant here; // we always connect to the proxy's host:port. - let proxy_uri: Uri = format!("http://{}", host_port_authority(&proxy.host, proxy.port)) - .parse()?; + let proxy_uri: Uri = + format!("http://{}", host_port_authority(&proxy.host, proxy.port)).parse()?; let tcp = match timeout(PROXY_SETUP_TIMEOUT, http.call(proxy_uri)).await { Ok(result) => result?.into_inner(), Err(_) => return Err(timed_out("connecting to upstream proxy")), @@ -520,6 +603,38 @@ mod tests { assert!(UpstreamProxy::parse(" ").is_err()); } + #[test] + fn proxy_config_uses_specs_by_scheme() { + let proxies = UpstreamProxies::from_specs( + Some("http://http-proxy.corp:3128"), + Some("http://https-proxy.corp:8443"), + ) + .unwrap() + .unwrap(); + + let http_uri: Uri = "http://example.com/".parse().unwrap(); + let https_uri: Uri = "https://example.com/".parse().unwrap(); + + assert_eq!( + proxies + .proxy_for_uri(&http_uri) + .map(|proxy| proxy.host.as_str()), + Some("http-proxy.corp") + ); + assert_eq!( + proxies + .proxy_for_uri(&https_uri) + .map(|proxy| proxy.host.as_str()), + Some("https-proxy.corp") + ); + } + + #[test] + fn proxy_config_ignores_empty_specs() { + let proxies = UpstreamProxies::from_specs(Some(" "), None).unwrap(); + assert!(proxies.is_none()); + } + /// Drive the proxy side of an in-memory duplex: read request headers up to /// the blank line, then reply with `response`. Returns the request text. async fn fake_proxy(mut end: tokio::io::DuplexStream, response: &'static [u8]) -> String { From 7180064d25f953a736bc1b8af7cda407dcfe1792 Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Fri, 7 Aug 2026 17:37:45 +0000 Subject: [PATCH 08/23] refactor: derive Debug for RunArgs The manual Debug implementation was needed while RunArgs carried a dedicated upstream proxy option whose value could contain credentials. That option was removed in 701d116 in favor of the standard HTTP_PROXY / HTTPS_PROXY environment variables, so the implementation now enumerates every field in declaration order and produces output identical to the derive. --- src/main.rs | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/src/main.rs b/src/main.rs index db25a455..082d013e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -7,7 +7,6 @@ use httpjail::rules::shell::ShellRuleEngine; use httpjail::rules::v8_js::V8JsRuleEngine; use httpjail::rules::{Action, RuleEngine}; use hyper::Method; -use std::fmt; use std::fs::OpenOptions; use std::os::unix::process::ExitStatusExt; use std::sync::atomic::{AtomicBool, Ordering}; @@ -41,7 +40,7 @@ enum Command { }, } -#[derive(Parser)] +#[derive(Parser, Debug)] struct RunArgs { /// Use shell script for evaluating requests /// The script receives environment variables: @@ -140,27 +139,6 @@ struct RunArgs { exec_command: Vec, } -impl fmt::Debug for RunArgs { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("RunArgs") - .field("sh", &self.sh) - .field("proc", &self.proc) - .field("js", &self.js) - .field("js_file", &self.js_file) - .field("request_log", &self.request_log) - .field("weak", &self.weak) - .field("verbose", &self.verbose) - .field("timeout", &self.timeout) - .field("no_jail_cleanup", &self.no_jail_cleanup) - .field("cleanup", &self.cleanup) - .field("server", &self.server) - .field("test", &self.test) - .field("docker_run", &self.docker_run) - .field("exec_command", &self.exec_command) - .finish() - } -} - fn setup_logging(verbosity: u8) { use tracing_subscriber::fmt::time::FormatTime; From 88c13dc80b17514611f1de8fe3527350b0aae646 Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Fri, 7 Aug 2026 17:37:45 +0000 Subject: [PATCH 09/23] refactor: drop TLS connections to the upstream proxy Reaching the upstream proxy itself over TLS (an `https://` proxy URL) was implemented but never a demonstrated requirement, and it was the only reason the connector had to erase the stream type behind `Box` and carry a second rustls ClientConfig. HTTPS destinations are unaffected: they are still tunneled through a plain HTTP proxy with CONNECT, which is the standard corporate proxy configuration. `https://` proxy URLs are now rejected with an error naming the limitation rather than silently treated as plain HTTP, and ProxyStream holds a concrete TcpStream so established tunnels no longer pay for dynamic dispatch on every read and write. --- README.md | 5 +- docs/advanced/upstream-proxy.md | 16 ++--- src/proxy.rs | 22 +++---- src/upstream.rs | 101 ++++++++++++-------------------- 4 files changed, 58 insertions(+), 86 deletions(-) diff --git a/README.md b/README.md index 8be65841..fd1a3a5a 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ httpjail --js "r.host === 'api.github.com'" --docker-run -- --rm alpine:latest w # Route httpjail's own egress through an upstream (corporate) proxy HTTPS_PROXY=http://proxy.corp:3128 httpjail --js "true" -- curl https://api.github.com -# Credentials and HTTPS proxies are supported: http://user:pass@proxy.corp:3128, https://proxy.corp:8443 +# Basic authentication is supported: http://user:pass@proxy.corp:3128 ``` ### Upstream (corporate) proxy @@ -76,7 +76,8 @@ outbound requests through an upstream proxy. Rule evaluation still happens locally on the intercepted traffic; only the re-originated request is forwarded through the proxy. -- `http://`, `https://` and bare `host:port` (http assumed) forms are accepted. +- `http://host:port` and bare `host:port` (http assumed) forms are accepted. + Reaching the proxy itself over TLS (`https://proxy`) is not supported. - Basic authentication is supported via `http://user:pass@host:port`. - HTTPS destinations are reached via a `CONNECT` tunnel through the proxy, while plain HTTP destinations are forwarded in absolute-form. diff --git a/docs/advanced/upstream-proxy.md b/docs/advanced/upstream-proxy.md index c1faac7f..67c744bb 100644 --- a/docs/advanced/upstream-proxy.md +++ b/docs/advanced/upstream-proxy.md @@ -19,9 +19,6 @@ HTTP_PROXY=http://proxy.corp:3128 HTTPS_PROXY=http://proxy.corp:3128 \ # With Basic authentication HTTPS_PROXY=http://user:pass@proxy.corp:3128 httpjail --js "true" -- ./my-app - -# Through an HTTPS proxy -HTTPS_PROXY=https://proxy.corp:8443 httpjail --js "true" -- ./my-app ``` ## Accepted formats @@ -29,13 +26,18 @@ HTTPS_PROXY=https://proxy.corp:8443 httpjail --js "true" -- ./my-app | Form | Example | Notes | | --- | --- | --- | | `http://host:port` | `http://proxy.corp:3128` | Plain HTTP proxy | -| `https://host:port` | `https://proxy.corp:8443` | Connection to the proxy is wrapped in TLS | | `host:port` | `proxy.corp:3128` | Bare authority, `http` scheme assumed | | With credentials | `http://user:pass@proxy.corp:3128` | Sends `Proxy-Authorization: Basic ...` | `HTTP_PROXY` is used for `http://` destinations. `HTTPS_PROXY` is used for `https://` destinations. Credentials are never written to the logs. +Note that the value describes how httpjail reaches the proxy, not the scheme of +the destinations it covers: `HTTPS_PROXY=http://proxy.corp:3128` is the normal +configuration and sends HTTPS destinations through a plain HTTP proxy. Reaching +the proxy itself over TLS (an `https://` proxy URL) is not supported and is +rejected with an error. + ## How it works - **HTTPS destinations** are reached by issuing a `CONNECT` to the upstream @@ -44,9 +46,9 @@ HTTPS_PROXY=https://proxy.corp:8443 httpjail --js "true" -- ./my-app plus the httpjail CA, exactly as for a direct connection. - **Plain HTTP destinations** are forwarded to the proxy in absolute-form, with the `Proxy-Authorization` header attached when credentials are configured. -- Only connection setup (TCP connect, optional TLS to the proxy, and the - `CONNECT` exchange) is bounded by a timeout. The established tunnel carries no - timeout, so long-running connections such as WebSocket and gRPC keep working. +- Only connection setup (the TCP connect and the `CONNECT` exchange) is bounded + by a timeout. The established tunnel carries no timeout, so long-running + connections such as WebSocket and gRPC keep working. ## Relationship to jailed process proxy variables diff --git a/src/proxy.rs b/src/proxy.rs index da7315a3..5e3fbe5d 100644 --- a/src/proxy.rs +++ b/src/proxy.rs @@ -352,19 +352,15 @@ pub fn init_client_with_ca( UpstreamClient::Direct(build_pooled_client(https)) } Some(proxies) => { - // Both the destination TLS (layered over the CONNECT tunnel by - // the HttpsConnector) and the optional https:// proxy TLS trust - // the same roots as the direct client. - let make_config = || { - if dangerous { - create_dangerous_client_config() - } else { - create_client_config_with_ca(ca_cert_der.clone()) - } + // The destination TLS, layered over the CONNECT tunnel by the + // HttpsConnector, trusts the same roots as the direct client. + let config = if dangerous { + create_dangerous_client_config() + } else { + create_client_config_with_ca(ca_cert_der) }; - let connector = - ProxyConnector::with_config(proxies.clone(), Arc::new(make_config())); - let https = hyper_rustls::HttpsConnector::from((connector, make_config())); + let connector = ProxyConnector::with_config(proxies.clone()); + let https = hyper_rustls::HttpsConnector::from((connector, config)); debug!("Upstream client initialized to route through the upstream proxy"); UpstreamClient::Proxied { client: build_pooled_client(https), @@ -877,7 +873,7 @@ mod tests { let proxy = crate::upstream::UpstreamProxy::parse(&format!("http://user:pass@{}", addr)).unwrap(); let proxies = UpstreamProxies::all(proxy.clone()); - let connector = ProxyConnector::new(proxy, Arc::new(create_dangerous_client_config())); + let connector = ProxyConnector::new(proxy); let https = hyper_rustls::HttpsConnector::from((connector, create_dangerous_client_config())); let client = UpstreamClient::Proxied { diff --git a/src/upstream.rs b/src/upstream.rs index ad98cc40..5d3ffa4c 100644 --- a/src/upstream.rs +++ b/src/upstream.rs @@ -31,40 +31,29 @@ use hyper::rt::{Read, ReadBufCursor, Write}; use hyper_util::client::legacy::connect::{Connected, Connection, HttpConnector}; use hyper_util::rt::TokioIo; use percent_encoding::percent_decode_str; -use rustls::pki_types::ServerName; use std::future::Future; use std::io; use std::pin::Pin; use std::sync::Arc; use std::task::{Context, Poll}; use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; +use tokio::net::TcpStream; use tokio::time::{Duration, timeout}; -use tokio_rustls::TlsConnector; use tower_service::Service; use tracing::debug; use url::{Host, Url}; type BoxError = Box; -/// Timeout for establishing the tunnel through the upstream proxy (TCP connect, -/// optional TLS to the proxy and the `CONNECT` exchange). This bounds setup -/// only; the resulting tunnel carries no timeout so long-running connections -/// keep working. +/// Timeout for establishing the tunnel through the upstream proxy (TCP connect +/// and the `CONNECT` exchange). This bounds setup only; the resulting tunnel +/// carries no timeout so long-running connections keep working. const PROXY_SETUP_TIMEOUT: Duration = Duration::from_secs(30); /// Upper bound on the size of the upstream proxy's `CONNECT` response headers. /// A well-behaved proxy answers with a short status line and a few headers. const MAX_CONNECT_RESPONSE_BYTES: usize = 16 * 1024; -/// Object-safe combination of the async byte-stream traits we erase over so the -/// connector can hold either a plain TCP stream or a TLS stream (when the proxy -/// itself is reached over `https://`) behind a single type. -trait IoStream: AsyncRead + AsyncWrite + Unpin + Send {} -impl IoStream for T {} - -/// A heap-erased byte stream carrying the connection to the proxy. -type BoxedIo = Box; - /// Parsed configuration for an upstream proxy. #[derive(Clone, Debug)] pub struct UpstreamProxy { @@ -72,9 +61,6 @@ pub struct UpstreamProxy { host: String, /// Proxy port. port: u16, - /// Whether the connection to the proxy itself is wrapped in TLS (an - /// `https://` proxy URL). - tls: bool, /// Pre-built `Proxy-Authorization` header value when credentials are given. auth: Option, } @@ -151,8 +137,12 @@ fn parse_optional_proxy_spec(name: &str, spec: Option<&str>) -> Result Result { let spec = spec.trim(); if spec.is_empty() { @@ -168,15 +158,20 @@ impl UpstreamProxy { let url = Url::parse(&normalized) .with_context(|| format!("Invalid upstream proxy URL: {}", redacted_spec))?; - let tls = match url.scheme() { - "http" => false, - "https" => true, + match url.scheme() { + "http" => {} + "https" => bail!( + "Connecting to an upstream proxy over TLS is not supported: {}. \ + Use an 'http://' proxy URL; HTTPS destinations are still \ + tunneled through it with CONNECT.", + redacted_spec + ), other => bail!( "Unsupported upstream proxy scheme '{}': {}", other, redacted_spec ), - }; + } let host = match url.host() { Some(Host::Domain(host)) => host.to_string(), @@ -195,12 +190,7 @@ impl UpstreamProxy { None }; - Ok(UpstreamProxy { - host, - port, - tls, - auth, - }) + Ok(UpstreamProxy { host, port, auth }) } /// The `Proxy-Authorization` header value, if credentials were supplied. @@ -250,16 +240,14 @@ pub struct ProxyConnector { /// Used solely to dial the proxy's `host:port` (never the destination). http: HttpConnector, proxies: Arc, - /// TLS configuration used only when the proxy itself is `https://`. - proxy_tls: Arc, } impl ProxyConnector { - pub fn new(proxy: UpstreamProxy, proxy_tls: Arc) -> Self { - Self::with_config(UpstreamProxies::all(proxy), proxy_tls) + pub fn new(proxy: UpstreamProxy) -> Self { + Self::with_config(UpstreamProxies::all(proxy)) } - pub fn with_config(proxies: UpstreamProxies, proxy_tls: Arc) -> Self { + pub fn with_config(proxies: UpstreamProxies) -> Self { let mut http = HttpConnector::new(); // The proxy is addressed via an http(s) URL; allow non-http schemes so // the connector does not reject the dial target. @@ -268,7 +256,6 @@ impl ProxyConnector { ProxyConnector { http, proxies: Arc::new(proxies), - proxy_tls, } } } @@ -285,7 +272,6 @@ impl Service for ProxyConnector { fn call(&mut self, dst: Uri) -> Self::Future { let mut http = self.http.clone(); let proxy = self.proxies.proxy_for_uri(&dst).cloned(); - let proxy_tls = Arc::clone(&self.proxy_tls); Box::pin(async move { let Some(proxy) = proxy else { @@ -294,33 +280,18 @@ impl Service for ProxyConnector { Err(_) => return Err(timed_out("connecting directly to destination")), }; let _ = tcp.set_nodelay(true); - return Ok(ProxyStream::new(Box::new(tcp), false)); + return Ok(ProxyStream::new(tcp, false)); }; // Dial the proxy (TCP). The destination scheme is irrelevant here; // we always connect to the proxy's host:port. let proxy_uri: Uri = format!("http://{}", host_port_authority(&proxy.host, proxy.port)).parse()?; - let tcp = match timeout(PROXY_SETUP_TIMEOUT, http.call(proxy_uri)).await { + let mut stream = match timeout(PROXY_SETUP_TIMEOUT, http.call(proxy_uri)).await { Ok(result) => result?.into_inner(), Err(_) => return Err(timed_out("connecting to upstream proxy")), }; - let _ = tcp.set_nodelay(true); - - // Optionally negotiate TLS with the proxy itself. - let mut stream: BoxedIo = if proxy.tls { - let name = ServerName::try_from(proxy.host.clone()).map_err(|_| { - BoxError::from(format!("Invalid proxy host for TLS SNI: {}", proxy.host)) - })?; - let connector = TlsConnector::from(Arc::clone(&proxy_tls)); - let tls = match timeout(PROXY_SETUP_TIMEOUT, connector.connect(name, tcp)).await { - Ok(result) => result?, - Err(_) => return Err(timed_out("during TLS handshake with upstream proxy")), - }; - Box::new(tls) - } else { - Box::new(tcp) - }; + let _ = stream.set_nodelay(true); let proxied = if dst.scheme_str() == Some("https") { let host = dst.host().ok_or_else(|| { @@ -353,12 +324,12 @@ fn timed_out(phase: &str) -> BoxError { /// The connector's response: a byte stream plus the proxied flag that hyper /// consults to decide between absolute-form and origin-form request lines. pub struct ProxyStream { - io: TokioIo, + io: TokioIo, proxied: bool, } impl ProxyStream { - fn new(io: BoxedIo, proxied: bool) -> Self { + fn new(io: TcpStream, proxied: bool) -> Self { ProxyStream { io: TokioIo::new(io), proxied, @@ -520,7 +491,6 @@ mod tests { let p = UpstreamProxy::parse("http://proxy.corp:3128").unwrap(); assert_eq!(p.host, "proxy.corp"); assert_eq!(p.port, 3128); - assert!(!p.tls); assert!(p.auth.is_none()); } @@ -529,7 +499,6 @@ mod tests { let p = UpstreamProxy::parse("http://proxy.corp:3128/path?ignored=true#frag").unwrap(); assert_eq!(p.host, "proxy.corp"); assert_eq!(p.port, 3128); - assert!(!p.tls); } #[test] @@ -537,14 +506,18 @@ mod tests { let p = UpstreamProxy::parse("proxy.corp:8080").unwrap(); assert_eq!(p.host, "proxy.corp"); assert_eq!(p.port, 8080); - assert!(!p.tls); } + /// Reaching the proxy itself over TLS is not supported; the error must say + /// so rather than silently treating the proxy as plain HTTP. #[test] - fn parse_https_proxy_default_port() { - let p = UpstreamProxy::parse("https://proxy.corp").unwrap(); - assert!(p.tls); - assert_eq!(p.port, 443); + fn reject_tls_proxy_scheme() { + let err = UpstreamProxy::parse("https://proxy.corp:8443").unwrap_err(); + assert!( + err.to_string().contains("over TLS is not supported"), + "unexpected error: {}", + err + ); } #[test] From fab4e099a969779e7f6492e0c6a00fb621b9f74b Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Fri, 7 Aug 2026 17:53:23 +0000 Subject: [PATCH 10/23] refactor: own the upstream client per ProxyServer The upstream client lived in a process-global OnceLock, so its configuration was decided by whichever ProxyServer was constructed first. A later call to new_with_upstream_proxies silently discarded its upstream_proxies argument, and get_client() could install a native-roots fallback client that no proxy configuration could ever replace. UpstreamClient::new now builds a client per server and ProxyContext carries it to the handlers, so what a caller passes is what its requests use. The public init_client_with_ca and get_client are gone along with the static, and proxy_request / proxy_https_request take the client from the context they already receive. This also removes the pre-initialization fallback path entirely: a context cannot exist without a client, so the case the warning covered is now unrepresentable. --- src/proxy.rs | 149 ++++++++++++++++++++++++++--------------------- src/proxy_tls.rs | 60 +++++++++---------- 2 files changed, 112 insertions(+), 97 deletions(-) diff --git a/src/proxy.rs b/src/proxy.rs index 5e3fbe5d..564d77f4 100644 --- a/src/proxy.rs +++ b/src/proxy.rs @@ -29,7 +29,7 @@ use std::net::{Ipv4Addr, SocketAddr}; #[cfg(target_os = "linux")] use std::net::Ipv6Addr; -use std::sync::{Arc, OnceLock}; +use std::sync::Arc; use std::time::Duration; use tokio::net::{TcpListener, TcpStream}; use tokio::time::Instant; @@ -181,6 +181,45 @@ pub enum UpstreamClient { } impl UpstreamClient { + /// Build the upstream client for one [`ProxyServer`]: either contacting + /// destinations directly, or routing every re-originated request whose + /// scheme has a configured proxy through that proxy. + /// + /// Each `ProxyServer` owns its own client, so the upstream configuration is + /// exactly what the caller asked for rather than whatever the first + /// initialization in the process happened to install. + pub fn new( + ca_cert_der: CertificateDer<'static>, + upstream_proxies: Option, + ) -> Self { + // Check if we should dangerously disable cert validation (TESTING ONLY!) + let dangerous = std::env::var("HTTPJAIL_DANGER_DISABLE_CERT_VALIDATION").is_ok(); + + match upstream_proxies { + None => { + let https = build_direct_connector(ca_cert_der, dangerous); + debug!("HTTPS connector initialized with webpki roots and httpjail CA"); + UpstreamClient::Direct(build_pooled_client(https)) + } + Some(proxies) => { + // The destination TLS, layered over the CONNECT tunnel by the + // HttpsConnector, trusts the same roots as the direct client. + let config = if dangerous { + create_dangerous_client_config() + } else { + create_client_config_with_ca(ca_cert_der) + }; + let connector = ProxyConnector::with_config(proxies.clone()); + let https = hyper_rustls::HttpsConnector::from((connector, config)); + debug!("Upstream client initialized to route through the upstream proxy"); + UpstreamClient::Proxied { + client: build_pooled_client(https), + proxies, + } + } + } + } + /// Forward a prepared request upstream. No timeout is applied here so that /// long-running connections (WebSocket, gRPC, ...) keep working. pub async fn request( @@ -201,9 +240,6 @@ impl UpstreamClient { } } -// Shared HTTP/HTTPS client for upstream requests -static HTTPS_CLIENT: OnceLock = OnceLock::new(); - /// Build a pooled hyper client over the given connector with the shared tuning. fn build_pooled_client(connector: C) -> Client> where @@ -333,62 +369,6 @@ fn build_direct_connector( } } -/// Initialize the shared upstream client with the httpjail CA certificate and an -/// optional upstream proxies. When a proxy is configured for a destination -/// scheme, matching re-originated requests are routed through it; otherwise -/// destinations are contacted directly. -pub fn init_client_with_ca( - ca_cert_der: CertificateDer<'static>, - upstream_proxies: Option, -) { - HTTPS_CLIENT.get_or_init(|| { - // Check if we should dangerously disable cert validation (TESTING ONLY!) - let dangerous = std::env::var("HTTPJAIL_DANGER_DISABLE_CERT_VALIDATION").is_ok(); - - match upstream_proxies { - None => { - let https = build_direct_connector(ca_cert_der, dangerous); - debug!("HTTPS connector initialized with webpki roots and httpjail CA"); - UpstreamClient::Direct(build_pooled_client(https)) - } - Some(proxies) => { - // The destination TLS, layered over the CONNECT tunnel by the - // HttpsConnector, trusts the same roots as the direct client. - let config = if dangerous { - create_dangerous_client_config() - } else { - create_client_config_with_ca(ca_cert_der) - }; - let connector = ProxyConnector::with_config(proxies.clone()); - let https = hyper_rustls::HttpsConnector::from((connector, config)); - debug!("Upstream client initialized to route through the upstream proxy"); - UpstreamClient::Proxied { - client: build_pooled_client(https), - proxies, - } - } - } - }); -} - -/// Get or create the shared upstream client -pub fn get_client() -> &'static UpstreamClient { - HTTPS_CLIENT.get_or_init(|| { - // Fallback initialization if not already initialized with CA - // This should not happen in normal operation - warn!("HTTP client accessed before CA initialization, using native roots only"); - - let https = HttpsConnectorBuilder::new() - .with_native_roots() - .expect("Failed to load native roots") - .https_or_http() - .enable_http1() - .build(); - - UpstreamClient::Direct(build_pooled_client(https)) - }) -} - /// Try to bind to an available port in the given range (up to 16 attempts) async fn bind_to_available_port(start: u16, end: u16, ip: std::net::IpAddr) -> Result { let mut rng = rand::thread_rng(); @@ -463,6 +443,9 @@ async fn bind_listener(addr: std::net::SocketAddr) -> Result { pub struct ProxyContext { pub rule_engine: Arc, pub cert_manager: Arc, + /// Client used to re-originate allowed requests towards the real + /// destination, either directly or through an upstream proxy. + pub upstream_client: Arc, /// Unique nonce for this proxy instance, used for loop detection (Issue #84) pub loop_nonce: Arc, } @@ -492,9 +475,8 @@ impl ProxyServer { ) -> Self { let cert_manager = CertificateManager::new().expect("Failed to create certificate manager"); - // Initialize the HTTP client with our CA certificate - let ca_cert_der = cert_manager.get_ca_cert_der(); - init_client_with_ca(ca_cert_der, upstream_proxies); + // Build this server's upstream client, trusting our own CA + let upstream_client = UpstreamClient::new(cert_manager.get_ca_cert_der(), upstream_proxies); // Generate a unique nonce for loop detection (Issue #84) // Use 16 random hex characters for a reasonably short but collision-resistant ID @@ -506,6 +488,7 @@ impl ProxyServer { let context = ProxyContext { rule_engine: Arc::new(rule_engine), cert_manager: Arc::new(cert_manager), + upstream_client: Arc::new(upstream_client), loop_nonce: Arc::new(loop_nonce), }; @@ -684,7 +667,14 @@ pub async fn handle_http_request( "Request allowed: {} (max_tx_bytes: {:?})", full_url, evaluation.max_tx_bytes ); - match proxy_request(req, &full_url, evaluation.max_tx_bytes, &context.loop_nonce).await + match proxy_request( + req, + &full_url, + evaluation.max_tx_bytes, + &context.loop_nonce, + &context.upstream_client, + ) + .await { Ok(resp) => Ok(resp), Err(e) => { @@ -705,6 +695,7 @@ async fn proxy_request( full_url: &str, max_tx_bytes: Option, loop_nonce: &str, + client: &UpstreamClient, ) -> Result>> { // Parse the target URL let target_uri = full_url.parse::()?; @@ -737,9 +728,6 @@ async fn proxy_request( Request::from_parts(parts, body.boxed()) }; - // Use the shared HTTP/HTTPS client - let client = get_client(); - // Forward the request - no timeout to support long-running connections debug!("Sending HTTP request to upstream server: {}", full_url); let start = Instant::now(); @@ -842,6 +830,33 @@ mod tests { assert_ne!(http_port, https_port); } + /// Each server must honor the upstream configuration it was constructed + /// with. Before the client moved onto ProxyContext, whichever server ran + /// first installed a process-global client and the second silently inherited + /// it, so a proxied server created after a direct one lost its proxy. + #[tokio::test] + async fn upstream_client_is_per_server() { + let rule_engine = || { + let engine = V8JsRuleEngine::new("true".to_string()).unwrap(); + RuleEngine::from_trait(Box::new(engine), None) + }; + + let direct = ProxyServer::new(None, None, rule_engine()); + let proxies = + UpstreamProxies::all(crate::upstream::UpstreamProxy::parse("proxy:3128").unwrap()); + let proxied = + ProxyServer::new_with_upstream_proxies(None, None, rule_engine(), Some(proxies)); + + assert!(matches!( + direct.context.upstream_client.as_ref(), + UpstreamClient::Direct(_) + )); + assert!(matches!( + proxied.context.upstream_client.as_ref(), + UpstreamClient::Proxied { .. } + )); + } + /// A plain-HTTP request routed through an upstream proxy must be forwarded in /// absolute-form with the configured `Proxy-Authorization` header. #[tokio::test] diff --git a/src/proxy_tls.rs b/src/proxy_tls.rs index 005dec17..e464d6d2 100644 --- a/src/proxy_tls.rs +++ b/src/proxy_tls.rs @@ -486,8 +486,14 @@ async fn handle_decrypted_https_request( match evaluation.action { Action::Allow => { debug!("Request allowed: {}", full_url); - match proxy_https_request(req, &host, evaluation.max_tx_bytes, &context.loop_nonce) - .await + match proxy_https_request( + req, + &host, + evaluation.max_tx_bytes, + &context.loop_nonce, + &context.upstream_client, + ) + .await { Ok(resp) => Ok(resp), Err(e) => { @@ -509,6 +515,7 @@ async fn proxy_https_request( host: &str, max_tx_bytes: Option, loop_nonce: &str, + client: &crate::proxy::UpstreamClient, ) -> Result>> { // Build the target URL let path = req @@ -549,9 +556,6 @@ async fn proxy_https_request( Request::from_parts(parts, body.boxed()) }; - // Use the shared HTTP/HTTPS client from proxy module - let client = crate::proxy::get_client(); - // Forward the request - no timeout to support long-running connections (WebSocket, gRPC, etc.) debug!("Sending HTTPS request to upstream server: {}", target_url); debug!( @@ -652,6 +656,22 @@ mod tests { Arc::new(RuleEngine::from_trait(Box::new(engine), None)) } + /// Assemble a ProxyContext for the handler under test, with a direct + /// (no upstream proxy) client trusting the test CA. + fn create_test_context( + rule_engine: Arc, + cert_manager: Arc, + ) -> ProxyContext { + let upstream_client = + crate::proxy::UpstreamClient::new(cert_manager.get_ca_cert_der(), None); + ProxyContext { + rule_engine, + cert_manager, + upstream_client: Arc::new(upstream_client), + loop_nonce: Arc::new("test-nonce".to_string()), + } + } + /// Create a TLS client config that trusts any certificate (for testing) fn create_insecure_tls_config() -> Arc { let mut config = ClientConfig::builder() @@ -724,11 +744,7 @@ mod tests { // Spawn proxy handler tokio::spawn(async move { let (stream, addr) = listener.accept().await.unwrap(); - let context = ProxyContext { - rule_engine, - cert_manager, - loop_nonce: Arc::new("test-nonce".to_string()), - }; + let context = create_test_context(rule_engine, cert_manager); let _ = handle_connect_tunnel(stream, context, addr).await; }); @@ -764,11 +780,7 @@ mod tests { // Spawn proxy handler tokio::spawn(async move { let (stream, addr) = listener.accept().await.unwrap(); - let context = ProxyContext { - rule_engine: rule_engine.clone(), - cert_manager: Arc::clone(&cert_manager), - loop_nonce: Arc::new("test-nonce".to_string()), - }; + let context = create_test_context(rule_engine.clone(), Arc::clone(&cert_manager)); let _ = handle_connect_tunnel(stream, context, addr).await; }); @@ -806,11 +818,7 @@ mod tests { // Spawn proxy handler tokio::spawn(async move { let (stream, addr) = listener.accept().await.unwrap(); - let context = ProxyContext { - rule_engine: rule_engine.clone(), - cert_manager: Arc::clone(&cert_manager), - loop_nonce: Arc::new("test-nonce".to_string()), - }; + let context = create_test_context(rule_engine.clone(), Arc::clone(&cert_manager)); let _ = handle_transparent_tls(stream, context, addr).await; }); @@ -883,11 +891,7 @@ mod tests { let rule_engine = rule_engine.clone(); tokio::spawn(async move { let (stream, addr) = listener.accept().await.unwrap(); - let context = ProxyContext { - rule_engine: rule_engine.clone(), - cert_manager: cert_manager.clone(), - loop_nonce: Arc::new("test-nonce".to_string()), - }; + let context = create_test_context(rule_engine.clone(), cert_manager.clone()); let _ = handle_https_connection(stream, context, addr).await; }); @@ -922,11 +926,7 @@ mod tests { tokio::spawn(async move { let (stream, addr) = listener.accept().await.unwrap(); // Use the actual transparent TLS handler (which will extract SNI, etc.) - let context = ProxyContext { - rule_engine, - cert_manager, - loop_nonce: Arc::new("test-nonce".to_string()), - }; + let context = create_test_context(rule_engine, cert_manager); let _ = handle_transparent_tls(stream, context, addr).await; }); From 7c34dc13e92fc9ff2000b19c5bc7110bcbc09796 Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Fri, 7 Aug 2026 18:32:49 +0000 Subject: [PATCH 11/23] fix: send CONNECT to IPv6 destinations without doubled brackets Uri::host() returns an IPv6 literal with the square brackets that URI syntax requires around it, so `https://[::1]/` yields `[::1]`. host_port_authority() then saw a colon in the host and bracketed it a second time, making the connector emit `CONNECT [[::1]]:443` with a matching Host header. The authority is malformed, so an upstream proxy rejects it and IPv6 literal HTTPS destinations are unreachable. d08e61c fixed the same class of problem for the proxy's own address, which is parsed by the url crate and therefore arrives unbracketed; the destination side comes from Uri::host() and was left doubled. The existing test passed the bare "::1" directly and so did not exercise the Uri path. uri_host() now strips the brackets Uri::host() keeps, leaving host_port_authority() responsible for adding them back in authority position. The test builds its host from a Uri the way the connector does. --- src/upstream.rs | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/src/upstream.rs b/src/upstream.rs index 5d3ffa4c..82c50f1c 100644 --- a/src/upstream.rs +++ b/src/upstream.rs @@ -294,7 +294,7 @@ impl Service for ProxyConnector { let _ = stream.set_nodelay(true); let proxied = if dst.scheme_str() == Some("https") { - let host = dst.host().ok_or_else(|| { + let host = uri_host(&dst).ok_or_else(|| { BoxError::from(format!("CONNECT target has no host: {}", dst)) })?; let port = dst.port_u16().unwrap_or(443); @@ -438,8 +438,23 @@ where Ok(()) } +/// The destination host as a bare host name or IP literal. +/// +/// [`Uri::host`] keeps the square brackets that URI syntax requires around an +/// IPv6 literal (`https://[::1]/` yields `[::1]`), so the brackets are stripped +/// here to obtain the host itself. [`host_port_authority`] adds them back when +/// the host is used in an authority position. +fn uri_host(uri: &Uri) -> Option<&str> { + uri.host().map(|host| { + host.strip_prefix('[') + .and_then(|host| host.strip_suffix(']')) + .unwrap_or(host) + }) +} + /// Format a host and port for use as an HTTP authority, bracketing IPv6 -/// literals as required by URI syntax. +/// literals as required by URI syntax. `host` must be a bare host (see +/// [`uri_host`]); an already-bracketed literal would be bracketed twice. fn host_port_authority(host: &str, port: u16) -> String { if host.contains(':') { format!("[{host}]:{port}") @@ -647,20 +662,32 @@ mod tests { assert!(request.contains("Proxy-Authorization: Basic dXNlcjpwYXNz\r\n")); } + /// An IPv6 literal destination must reach the proxy as `[::1]:443`, taking + /// the host from the destination `Uri` exactly as the connector does. + /// `Uri::host()` returns the literal already bracketed, so feeding it + /// straight into the authority would produce `[[::1]]:443`. #[tokio::test] - async fn connect_tunnel_brackets_ipv6_literal() { + async fn connect_tunnel_brackets_ipv6_literal_from_uri() { let (mut client_end, proxy_end) = tokio::io::duplex(1024); let proxy = tokio::spawn(fake_proxy( proxy_end, b"HTTP/1.1 200 Connection established\r\n\r\n", )); - establish_connect_tunnel(&mut client_end, "::1", 443, None) + let dst: Uri = "https://[::1]/".parse().unwrap(); + let host = uri_host(&dst).unwrap(); + assert_eq!(host, "::1"); + + establish_connect_tunnel(&mut client_end, host, dst.port_u16().unwrap_or(443), None) .await .unwrap(); let request = proxy.await.unwrap(); - assert!(request.starts_with("CONNECT [::1]:443 HTTP/1.1\r\n")); + assert!( + request.starts_with("CONNECT [::1]:443 HTTP/1.1\r\n"), + "unexpected request: {request}" + ); + assert!(request.contains("Host: [::1]:443\r\n")); } #[tokio::test] From 894f40dec145217d757c1d1f5e580662ab12e527 Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Fri, 7 Aug 2026 19:25:17 +0000 Subject: [PATCH 12/23] feat: honor NO_PROXY for httpjail's own egress With HTTP_PROXY or HTTPS_PROXY set, every destination of that scheme was sent to the upstream proxy, including the internal hosts a corporate proxy environment expects to be reached directly. Such a proxy refuses those CONNECTs, so httpjail did not work in the environment the feature exists for. NO_PROXY now selects destinations to contact directly, following curl 8.14.1: domain entries match the domain and its subdomains at a label boundary, one leading and one trailing dot are ignored on both sides, a list of exactly `*` disables proxying, and IP or CIDR entries apply to destinations written as an IP literal. Domain entries and address entries never cross-match, as the destination decides which kind can apply. Entries are parsed once at startup so the request path only compares. Deliberate divergences, all documented: whitespace separates entries instead of truncating the list, `/0` covers its address family, a mistyped CIDR is a startup error rather than silently ignored, non-byte-aligned IPv6 prefixes match correctly (curl fixed the same defect in 8.17.0), and the uppercase spelling keeps precedence for consistency with the other proxy variables. Two properties the implementation is shaped around: Proxy-Authorization is now decided by http_auth_for_uri(), which is the only path the request builder uses. Attaching the header from the caller would have leaked the proxy's credentials to any bypassed internal host, and to the origin server itself for HTTPS destinations, whose request travels inside the CONNECT tunnel. from_env() never parses an input that cannot affect the outcome: a wildcard bypass short-circuits before the proxy URLs are read, and the bypass list is left unparsed when no proxy is configured. Otherwise a leftover NO_PROXY in an environment with no proxy at all would newly refuse to start. Errors and logs name an entry by position and never echo its text, since a NO_PROXY value can hold a mistakenly pasted proxy URL with credentials and redact_proxy_spec does not cover text after a slash. --- Cargo.lock | 1 + Cargo.toml | 1 + README.md | 2 + docs/advanced/upstream-proxy.md | 50 +++- docs/guide/configuration.md | 8 + src/proxy.rs | 6 +- src/upstream.rs | 433 +++++++++++++++++++++++++++++--- 7 files changed, 466 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 007ef279..74a78fb2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1019,6 +1019,7 @@ dependencies = [ "hyper", "hyper-rustls", "hyper-util", + "ipnet", "libc", "lru", "percent-encoding", diff --git a/Cargo.toml b/Cargo.toml index b2ffec42..683d5429 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,7 @@ camino = "1.1.11" filetime = "0.2" ctrlc = "3.4" percent-encoding = "2.3" +ipnet = "2" url = "2.5" v8 = "129" serde = { version = "1.0", features = ["derive"] } diff --git a/README.md b/README.md index fd1a3a5a..bf39b8df 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,8 @@ through the proxy. - `http://host:port` and bare `host:port` (http assumed) forms are accepted. Reaching the proxy itself over TLS (`https://proxy`) is not supported. - Basic authentication is supported via `http://user:pass@host:port`. +- `NO_PROXY` lists destinations to contact directly, with curl-compatible + matching (domains and subdomains, `*`, IPv4/IPv6 CIDR). - HTTPS destinations are reached via a `CONNECT` tunnel through the proxy, while plain HTTP destinations are forwarded in absolute-form. - In weak mode, httpjail overwrites proxy env vars inside the jailed process to diff --git a/docs/advanced/upstream-proxy.md b/docs/advanced/upstream-proxy.md index 67c744bb..dd67bfbf 100644 --- a/docs/advanced/upstream-proxy.md +++ b/docs/advanced/upstream-proxy.md @@ -38,6 +38,52 @@ configuration and sends HTTPS destinations through a plain HTTP proxy. Reaching the proxy itself over TLS (an `https://` proxy URL) is not supported and is rejected with an error. +## Bypassing the proxy with `NO_PROXY` + +`NO_PROXY` lists destinations that httpjail contacts directly instead of through +the upstream proxy. The syntax follows curl 8.14.1. + +| Form | Example | Notes | +| --- | --- | --- | +| Domain | `example.com` | Matches the domain and its subdomains, not `notexample.com` | +| Leading dot | `.example.com` | One leading dot is ignored; same as above | +| Wildcard | `*` | Only when the whole list is exactly `*`: no proxy is used at all | +| IPv4 CIDR | `192.168.0.0/16` | Only for destinations written as an IP literal | +| IPv6 CIDR | `2001:db8::/32` | Only for destinations written as an IP literal | +| Address | `192.168.1.1` | Without a prefix length, an exact address match | + +Entries are separated by commas, matched case-insensitively, and one trailing dot +is ignored on both the entry and the destination. Rule evaluation is unaffected: +a bypassed request is still checked against your rules, it just reaches the +destination directly. + +Not supported: + +- **Ports in entries.** `example.com:8080` matches nothing, because entries are + compared against the destination's host name only. It does not fall back to + matching `example.com`. +- **Globs, schemes and paths.** `*.example.com` and `https://example.com` match + nothing. Use `example.com`, which already covers subdomains. +- **Matching resolved addresses.** A CIDR entry applies only when the destination + itself is an IP literal; host names are never resolved to check them. + +A mistyped CIDR (`10.0.0.0/8x`, `10.0.0.0/33`) is reported as a configuration +error at startup rather than silently ignored. Entries that simply cannot match, +such as the unsupported forms above, are ignored and logged at debug level. + +`NO_PROXY` is only read when at least one of `HTTP_PROXY` / `HTTPS_PROXY` is set. + +### Differences from curl + +| Item | curl 8.14.1 | httpjail | +| --- | --- | --- | +| Variable precedence | `no_proxy`, then `NO_PROXY` | `NO_PROXY`, then `no_proxy`, consistent with the other proxy variables | +| Whitespace-only value | Counts as set, so the other spelling is not consulted | Counts as unset, falling through to the other spelling | +| Whitespace between entries | Stops parsing the list, silently discarding the rest | Separates entries, like a comma | +| `/0` prefix | Treated as an exact address match | Matches the whole address family | +| Mistyped CIDR | Silently ignored | Configuration error at startup | +| IPv6 prefix not a multiple of 8 | Inverted before curl 8.17.0 | Matches correctly, as curl 8.17.0 and later do | + ## How it works - **HTTPS destinations** are reached by issuing a `CONNECT` to the upstream @@ -45,7 +91,9 @@ rejected with an error. handshake over that tunnel. TLS is validated against Mozilla's webpki roots plus the httpjail CA, exactly as for a direct connection. - **Plain HTTP destinations** are forwarded to the proxy in absolute-form, with - the `Proxy-Authorization` header attached when credentials are configured. + the `Proxy-Authorization` header attached when credentials are configured. The + header is never sent to a destination that `NO_PROXY` bypasses, nor to an HTTPS + destination, whose request travels inside the tunnel to the origin server. - Only connection setup (the TCP connect and the `CONNECT` exchange) is bounded by a timeout. The established tunnel carries no timeout, so long-running connections such as WebSocket and gRPC keep working. diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 5993fa0e..3d7abfde 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -96,6 +96,14 @@ These affect httpjail's behavior: | `HTTPJAIL_CA_CERT` | Custom CA certificate path | `/etc/pki/custom-ca.pem` | | `HTTP_PROXY` | Upstream proxy for httpjail HTTP egress | `http://proxy.corp:3128` | | `HTTPS_PROXY` | Upstream proxy for httpjail HTTPS egress | `http://proxy.corp:3128` | +| `NO_PROXY` | Destinations httpjail contacts directly | `internal.corp,10.0.0.0/8` | + +`NO_PROXY` appears in both tables and means two different things. In the table +above it is what httpjail *sets* for the jailed process, so that the process does +not send its localhost traffic to httpjail. Here it is what httpjail *reads* for +its own egress, to decide which destinations to reach without the upstream proxy. +The value you set is used only for httpjail's own egress; it is not passed on to +the jailed process, which would let that process bypass httpjail entirely. See [Upstream Proxy](../advanced/upstream-proxy.md) for details. diff --git a/src/proxy.rs b/src/proxy.rs index 564d77f4..f44b40ba 100644 --- a/src/proxy.rs +++ b/src/proxy.rs @@ -229,10 +229,8 @@ impl UpstreamClient { match self { UpstreamClient::Direct(client) => client.request(req).await.map_err(Into::into), UpstreamClient::Proxied { client, proxies } => { - if req.uri().scheme_str() == Some("http") - && let Some(auth) = proxies.http_auth() - { - req.headers_mut().insert(PROXY_AUTHORIZATION, auth.clone()); + if let Some(auth) = proxies.http_auth_for_uri(req.uri()) { + req.headers_mut().insert(PROXY_AUTHORIZATION, auth); } client.request(req).await.map_err(Into::into) } diff --git a/src/upstream.rs b/src/upstream.rs index 82c50f1c..e1c1081a 100644 --- a/src/upstream.rs +++ b/src/upstream.rs @@ -30,9 +30,11 @@ use hyper::header::HeaderValue; use hyper::rt::{Read, ReadBufCursor, Write}; use hyper_util::client::legacy::connect::{Connected, Connection, HttpConnector}; use hyper_util::rt::TokioIo; +use ipnet::IpNet; use percent_encoding::percent_decode_str; use std::future::Future; use std::io; +use std::net::IpAddr; use std::pin::Pin; use std::sync::Arc; use std::task::{Context, Poll}; @@ -54,6 +56,146 @@ const PROXY_SETUP_TIMEOUT: Duration = Duration::from_secs(30); /// A well-behaved proxy answers with a short status line and a few headers. const MAX_CONNECT_RESPONSE_BYTES: usize = 16 * 1024; +/// The only `NO_PROXY` value that acts as a wildcard. Compared against the whole +/// list verbatim, as curl does, so `" * "` is not a wildcard. +const NO_PROXY_WILDCARD: &str = "*"; + +/// One parsed `NO_PROXY` entry. +/// +/// The destination decides which variants can apply: an IP literal destination is +/// only ever compared against [`NoProxyRule::Ip`] and [`NoProxyRule::Net`], and a +/// host name only against [`NoProxyRule::Domain`]. Domain rules and address rules +/// never cross-match, matching curl. +#[derive(Clone, Debug)] +enum NoProxyRule { + /// Label-boundary suffix match. Already ASCII-lowercased with one leading + /// and one trailing dot removed. + Domain(String), + /// An entry without a prefix length: exact address match. + Ip(IpAddr), + /// An entry with a prefix length. + Net(IpNet), +} + +/// The parsed `NO_PROXY` bypass list. +/// +/// Entries are parsed once at startup so that the request path only compares. +/// A wildcard list is not represented here: [`UpstreamProxies::from_specs`] +/// turns it into "no upstream proxy at all" before this type is built. +#[derive(Clone, Debug, Default)] +struct NoProxy { + rules: Vec, +} + +impl NoProxy { + /// Parse a `NO_PROXY` list. + /// + /// Entries are separated by commas; unlike curl, whitespace separates too. + /// curl stops parsing the whole list at the first whitespace-separated + /// token, silently discarding the remainder, which loses configuration + /// without saying so. + fn parse(spec: Option<&str>) -> Result { + let Some(spec) = spec else { + return Ok(Self::default()); + }; + + let mut rules = Vec::new(); + let tokens = spec + .split(|c: char| c == ',' || c.is_whitespace()) + .filter(|token| !token.is_empty()); + for (index, token) in tokens.enumerate() { + if let Some(rule) = parse_no_proxy_rule(index + 1, token)? { + rules.push(rule); + } + } + Ok(Self { rules }) + } + + /// Whether `host` (bare, as returned by [`uri_host`]) bypasses the proxy. + fn matches(&self, host: &str) -> bool { + if let Ok(ip) = host.parse::() { + return self.rules.iter().any(|rule| match rule { + NoProxyRule::Ip(entry) => *entry == ip, + NoProxyRule::Net(entry) => entry.contains(&ip), + NoProxyRule::Domain(_) => false, + }); + } + + // A single trailing dot denotes the same name; ignore it as curl does. + let host = host.strip_suffix('.').unwrap_or(host); + self.rules.iter().any(|rule| match rule { + NoProxyRule::Domain(entry) => domain_matches(entry, host), + NoProxyRule::Ip(_) | NoProxyRule::Net(_) => false, + }) + } +} + +/// Parse one `NO_PROXY` entry. `Ok(None)` means the entry can never match and is +/// dropped; `Err` is a configuration error that aborts startup. +/// +/// Neither the returned error nor any log line may contain the entry itself: a +/// `NO_PROXY` value can hold a mistakenly pasted proxy URL with credentials, and +/// `redact_proxy_spec` does not cover text after a slash. Only `index` and the +/// address that already parsed successfully are reported. +fn parse_no_proxy_rule(index: usize, token: &str) -> Result> { + // Treat an entry as CIDR only when the part before the slash is an address. + // A URL-shaped entry (`https://internal.corp`) then stays a domain entry + // rather than failing the whole configuration, which would refuse to start + // in environments where curl works. + if let Some((addr, _)) = token.split_once('/') + && let Ok(addr) = addr.parse::() + { + let net = token + .parse::() + .map_err(|_| anyhow!("entry {} (\"{}/…\") is not a valid CIDR", index, addr))?; + return Ok(Some(NoProxyRule::Net(net))); + } + + if let Ok(addr) = token.parse::() { + return Ok(Some(NoProxyRule::Ip(addr))); + } + + // One leading and one trailing dot are ignored, trailing first, as curl + // does. An entry of "." or ".." therefore becomes empty and must be dropped: + // an empty domain rule would suffix-match every host and bypass everything. + let domain = token.strip_suffix('.').unwrap_or(token); + let domain = domain.strip_prefix('.').unwrap_or(domain); + if domain.is_empty() { + return Ok(None); + } + + // Host names contain neither of these, so such an entry cannot ever match. + // Report the position only, never the value. + for unmatchable in ['/', ':'] { + if domain.contains(unmatchable) { + debug!( + "NO_PROXY entry {} contains '{}' and can never match a host name; ignoring", + index, unmatchable + ); + return Ok(None); + } + } + + Ok(Some(NoProxyRule::Domain(domain.to_ascii_lowercase()))) +} + +/// Whether `host` is `entry` itself or a subdomain of it. +/// +/// `entry` is already lowercased; `host` is compared case-insensitively. The +/// character before a suffix match must be a dot, so `example.com` matches +/// `www.example.com` but not `notexample.com`. Comparison is on bytes to avoid +/// slicing a multi-byte character. +fn domain_matches(entry: &str, host: &str) -> bool { + let (entry, host) = (entry.as_bytes(), host.as_bytes()); + let Some(offset) = host.len().checked_sub(entry.len()) else { + return false; + }; + if !host[offset..].eq_ignore_ascii_case(entry) { + return false; + } + offset == 0 || host[offset - 1] == b'.' +} + /// Parsed configuration for an upstream proxy. #[derive(Clone, Debug)] pub struct UpstreamProxy { @@ -70,60 +212,120 @@ pub struct UpstreamProxy { pub struct UpstreamProxies { http: Option, https: Option, + no_proxy: NoProxy, } impl UpstreamProxies { /// Resolve httpjail's own egress proxy settings from the proxy environment. pub fn from_env() -> Result> { - let http = proxy_from_env("HTTP_PROXY", "http_proxy")?; - let https = proxy_from_env("HTTPS_PROXY", "https_proxy")?; - Ok(Self::from_proxies(http, https)) + let (no_proxy, no_proxy_lower) = (env_var("NO_PROXY"), env_var("no_proxy")); + let (http, http_lower) = (env_var("HTTP_PROXY"), env_var("http_proxy")); + let (https, https_lower) = (env_var("HTTPS_PROXY"), env_var("https_proxy")); + + Self::from_specs( + first_set(http.as_deref(), http_lower.as_deref()), + first_set(https.as_deref(), https_lower.as_deref()), + first_set(no_proxy.as_deref(), no_proxy_lower.as_deref()), + ) } + /// Resolve the configuration from already-selected values. + /// + /// The order of the steps below is deliberate: an input is never parsed + /// unless its value can actually affect the outcome. Parsing eagerly would + /// turn an irrelevant leftover variable into a startup failure. + fn from_specs( + http: Option<&str>, + https: Option<&str>, + no_proxy: Option<&str>, + ) -> Result> { + // A bare `*` disables proxying outright, so the proxy URLs are never + // used and must not be validated. + if no_proxy == Some(NO_PROXY_WILDCARD) { + debug!("NO_PROXY is '*': contacting all destinations directly"); + return Ok(None); + } + + let http = parse_optional_proxy_spec("HTTP_PROXY", http)?; + let https = parse_optional_proxy_spec("HTTPS_PROXY", https)?; + + // Without a proxy there is nothing to bypass, so the bypass list is + // irrelevant and is left unparsed. + if http.is_none() && https.is_none() { + return Ok(None); + } + + Ok(Some(Self { + http, + https, + no_proxy: NoProxy::parse(no_proxy).context("Failed to parse NO_PROXY")?, + })) + } + + /// The proxy to use for `uri`, or `None` when the destination is contacted + /// directly (no proxy for that scheme, or the destination is bypassed). fn proxy_for_uri(&self, uri: &Uri) -> Option<&UpstreamProxy> { - match uri.scheme_str() { + let proxy = match uri.scheme_str() { Some("http") => self.http.as_ref(), Some("https") => self.https.as_ref(), _ => None, + }?; + + if let Some(host) = uri_host(uri) + && self.no_proxy.matches(host) + { + debug!("Bypassing upstream proxy for {}", host); + return None; } + + Some(proxy) } - pub(crate) fn http_auth(&self) -> Option { - self.http.as_ref().and_then(UpstreamProxy::http_auth) + /// The `Proxy-Authorization` value to attach to a request that is forwarded + /// to the proxy in absolute-form. + /// + /// `None` for HTTPS destinations: those travel inside a `CONNECT` tunnel to + /// the origin server, so a header added here would deliver the proxy's + /// credentials to the destination site itself. The tunnel's own credentials + /// are written by [`establish_connect_tunnel`]. + /// + /// `None` for destinations that bypass the proxy, which would otherwise hand + /// the credentials to an arbitrary internal host. + pub(crate) fn http_auth_for_uri(&self, uri: &Uri) -> Option { + if uri.scheme_str() != Some("http") { + return None; + } + self.proxy_for_uri(uri).and_then(UpstreamProxy::http_auth) } pub(crate) fn all(proxy: UpstreamProxy) -> Self { Self { http: Some(proxy.clone()), https: Some(proxy), + no_proxy: NoProxy::default(), } } +} - fn from_proxies(http: Option, https: Option) -> Option { - if http.is_none() && https.is_none() { - return None; - } - Some(Self { http, https }) - } - - #[cfg(test)] - fn from_specs(http: Option<&str>, https: Option<&str>) -> Result> { - let http = parse_optional_proxy_spec("HTTP_PROXY", http)?; - let https = parse_optional_proxy_spec("HTTPS_PROXY", https)?; - Ok(Self::from_proxies(http, https)) - } +fn env_var(name: &str) -> Option { + std::env::var(name).ok() } -fn proxy_from_env(primary: &str, fallback: &str) -> Result> { - for name in [primary, fallback] { - if let Ok(value) = std::env::var(name) { - let proxy = parse_optional_proxy_spec(name, Some(&value))?; - if proxy.is_some() { - return Ok(proxy); - } - } - } - Ok(None) +/// The first of the two values that is actually set, using the uppercase-first +/// precedence httpjail applies to every proxy environment variable. +/// +/// A value counts as unset when it is empty or contains only whitespace, so +/// `NO_PROXY=" " no_proxy=example.com` falls through to the lowercase +/// spelling. curl treats only a truly empty value as absent; this is a +/// documented divergence (see docs/advanced/upstream-proxy.md). +/// +/// The value is returned verbatim. Callers compare it against a literal (the +/// strict `*` check), so trimming here would change what they see. +fn first_set<'a>(primary: Option<&'a str>, fallback: Option<&'a str>) -> Option<&'a str> { + [primary, fallback] + .into_iter() + .flatten() + .find(|value| !value.trim().is_empty()) } fn parse_optional_proxy_spec(name: &str, spec: Option<&str>) -> Result> { @@ -596,6 +798,7 @@ mod tests { let proxies = UpstreamProxies::from_specs( Some("http://http-proxy.corp:3128"), Some("http://https-proxy.corp:8443"), + None, ) .unwrap() .unwrap(); @@ -619,10 +822,180 @@ mod tests { #[test] fn proxy_config_ignores_empty_specs() { - let proxies = UpstreamProxies::from_specs(Some(" "), None).unwrap(); + let proxies = UpstreamProxies::from_specs(Some(" "), None, None).unwrap(); assert!(proxies.is_none()); } + fn proxies_with_no_proxy(no_proxy: &str) -> UpstreamProxies { + UpstreamProxies::from_specs( + Some("http://user:pass@proxy.corp:3128"), + Some("http://proxy.corp:3128"), + Some(no_proxy), + ) + .unwrap() + .unwrap() + } + + /// Bypass matching, asserted through `proxy_for_uri` rather than the matcher + /// itself: that is the entry point both the connector and the + /// `Proxy-Authorization` decision go through. + #[test] + fn no_proxy_bypasses_matching_destinations() { + // (NO_PROXY, destination, bypassed?) + let cases = [ + // Apex, subdomain and the non-boundary near-miss. + ("example.com", "http://example.com/", true), + ("example.com", "http://www.example.com/", true), + ("example.com", "http://notexample.com/", false), + ("EXAMPLE.COM", "http://ExAmPlE.cOm/", true), + // One leading and one trailing dot are ignored, on either side. + (".example.com", "http://www.example.com/", true), + ("example.com.", "http://example.com/", true), + ("example.com", "http://example.com./", true), + // Only a list that is exactly "*" is a wildcard. `" * "` and a list + // containing `*` leave the token as an unmatchable domain entry. + (" * ", "http://anything.test/", false), + ("*,example.com", "http://anything.test/", false), + ("*,example.com", "http://example.com/", true), + // Addresses: CIDR, bare address, and non-byte-aligned prefixes, + // which curl <= 8.16.0 got backwards. + ("192.168.0.0/16", "http://192.168.4.5/", true), + ("192.168.0.0/16", "http://192.169.4.5/", false), + ("192.168.1.1", "http://192.168.1.1/", true), + ("192.168.1.1", "http://192.168.1.2/", false), + ("2001:db8::/32", "http://[2001:db8::1]/", true), + ("2001:db8::/32", "http://[2001:db9::1]/", false), + ("2001:db8::/65", "http://[2001:db8::1]/", true), + ("2001:db8::/65", "http://[2001:db8:0:0:8000::1]/", false), + ("::1/127", "http://[::1]/", true), + // Domain entries never match addresses and vice versa. + ("example.com", "http://192.168.1.1/", false), + ("192.168.0.0/16", "http://example.com/", false), + // Nothing here may degenerate into matching every host. + ("", "http://example.com/", false), + (".", "http://example.com/", false), + ("..", "http://example.com/", false), + (",,example.com,", "http://example.com/", true), + (",,example.com,", "http://other.test/", false), + // Unlike curl, whitespace separates instead of truncating the list. + ("a.test b.test", "http://a.test/", true), + ("a.test b.test", "http://b.test/", true), + // Entries that cannot match a host name are dropped, not errors. + ("https://internal.corp", "http://internal.corp/", false), + ("example.com:8080", "http://example.com/", false), + // The bypass applies to both schemes. + ("example.com", "https://example.com/", true), + ]; + + for (no_proxy, destination, bypassed) in cases { + let proxies = proxies_with_no_proxy(no_proxy); + let uri: Uri = destination.parse().unwrap(); + assert_eq!( + proxies.proxy_for_uri(&uri).is_none(), + bypassed, + "NO_PROXY={:?} destination={}", + no_proxy, + destination + ); + } + } + + /// `Proxy-Authorization` must never leave the proxy it belongs to. Expected + /// values are written out rather than derived, so a wrong rule in + /// `http_auth_for_uri` cannot make the test agree with it. + #[test] + fn proxy_auth_only_for_proxied_http_destinations() { + let proxies = proxies_with_no_proxy("internal.corp"); + + // Forwarded in absolute-form to the proxy: the header belongs here. + assert!( + proxies + .http_auth_for_uri(&"http://proxied.example/".parse().unwrap()) + .is_some() + ); + // Connected to directly: the proxy's credentials must not be sent. + assert!( + proxies + .http_auth_for_uri(&"http://internal.corp/".parse().unwrap()) + .is_none() + ); + // Sent inside a CONNECT tunnel, i.e. to the origin server itself. + assert!( + proxies + .http_auth_for_uri(&"https://proxied.example/".parse().unwrap()) + .is_none() + ); + assert!( + proxies + .http_auth_for_uri(&"https://internal.corp/".parse().unwrap()) + .is_none() + ); + } + + /// Configuration errors, and the inputs that must *not* become errors. + #[test] + fn no_proxy_configuration_errors() { + let proxy = Some("http://proxy.corp:3128"); + let parse = |no_proxy: &str| UpstreamProxies::from_specs(proxy, proxy, Some(no_proxy)); + + // A mistyped CIDR is a typo worth reporting, not something to ignore. + for invalid in ["10.0.0.0/8x", "10.0.0.0/33", "2001:db8::/129"] { + assert!(parse(invalid).is_err(), "expected error for {:?}", invalid); + } + + // Entries that merely cannot match must not refuse to start: curl + // tolerates them, and httpjail would otherwise be unusable wherever such + // a value is set globally. + for tolerated in ["https://internal.corp", "foo/bar", "", ".", "*,example.com"] { + assert!( + parse(tolerated).is_ok(), + "unexpected error for {tolerated:?}" + ); + } + + // Nothing is parsed that cannot affect the outcome: no proxy at all, and + // a wildcard bypass, both short-circuit before the invalid values are + // reached. + assert!( + UpstreamProxies::from_specs(None, None, Some("10.0.0.0/8x")) + .unwrap() + .is_none() + ); + assert!( + UpstreamProxies::from_specs(Some("http://["), None, Some("*")) + .unwrap() + .is_none() + ); + + // A NO_PROXY value can hold a pasted proxy URL, so the error must not + // echo the entry. + // `{:#}` renders the whole chain, which is what reaches the user. + let err = format!("{:#}", parse("10.0.0.0/8@user:pass").unwrap_err()); + assert!( + !err.contains("user") && !err.contains("pass"), + "credentials leaked into error: {err}" + ); + assert!( + err.contains("10.0.0.0"), + "error lacks the entry position: {err}" + ); + } + + /// The uppercase spelling wins, and the value survives untouched. Shared by + /// every proxy variable, so this fixes the precedence for all of them. + #[test] + fn uppercase_env_spelling_wins() { + assert_eq!(first_set(Some("upper"), Some("lower")), Some("upper")); + assert_eq!(first_set(None, Some("lower")), Some("lower")); + assert_eq!(first_set(Some(""), Some("lower")), Some("lower")); + // Unlike curl, a whitespace-only value does not shadow the other spelling. + assert_eq!(first_set(Some(" "), Some("lower")), Some("lower")); + assert_eq!(first_set(None, None), None); + assert_eq!(first_set(Some(""), Some(" ")), None); + // Returned verbatim: trimming here would turn `" * "` into a wildcard. + assert_eq!(first_set(Some(" * "), None), Some(" * ")); + } + /// Drive the proxy side of an in-memory duplex: read request headers up to /// the blank line, then reply with `response`. Returns the request text. async fn fake_proxy(mut end: tokio::io::DuplexStream, response: &'static [u8]) -> String { From 0b3e7b6614bdf32fa198a855cfacbd415b2ba859 Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Fri, 7 Aug 2026 19:27:42 +0000 Subject: [PATCH 13/23] fix: stop passing the parent's proxy environment to jailed processes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Weak mode merged the parent's NO_PROXY into the value it set for the jailed process. Every host that value named was then reached directly by that process, skipping httpjail and any rule evaluation — the jail's purpose, silently defeated by an environment variable. The hole predates the upstream proxy work, but that work is what gives NO_PROXY a reason to be set, so it turns a latent problem into a likely one. The parent's NO_PROXY is no longer merged; the jailed process gets only localhost, 127.0.0.1 and ::1, which exist to keep local traffic from looping back through httpjail. ALL_PROXY was never stripped anywhere. It can carry the upstream proxy's credentials, and a process that prefers it over the scheme-specific variables would talk to that proxy instead of to httpjail. It is now removed alongside HTTP_PROXY and HTTPS_PROXY in weak mode, native Linux jails and Docker. All of these are handled in both spellings. Removing only the uppercase one leaves the hole open, and for NO_PROXY the lowercase spelling is the dangerous one: curl reads it first. The three call sites now share PARENT_PROXY_ENV_VARS and remove_parent_proxy_env() so the list cannot drift apart again. The weak mode test sets both spellings of each variable to different values on the parent, so a leak through either one fails and names itself; asserting only on the uppercase spelling would have passed while the lowercase one leaked. --- docs/advanced/upstream-proxy.md | 7 +++ docs/guide/configuration.md | 8 ++- src/jail/linux/docker.rs | 4 +- src/jail/linux/mod.rs | 4 +- src/jail/mod.rs | 52 +++++++++++++++++++ src/jail/weak.rs | 26 +++++----- tests/weak_integration.rs | 91 ++++++++++++++++++--------------- 7 files changed, 133 insertions(+), 59 deletions(-) diff --git a/docs/advanced/upstream-proxy.md b/docs/advanced/upstream-proxy.md index dd67bfbf..b334393c 100644 --- a/docs/advanced/upstream-proxy.md +++ b/docs/advanced/upstream-proxy.md @@ -110,3 +110,10 @@ point sandboxed processes at httpjail itself. The jailed process talks to httpjail; the proxy env vars only affect the hop from httpjail to the outside world. + +None of the parent's proxy variables are passed on to the jailed process, in any +mode. `HTTP_PROXY`, `HTTPS_PROXY` and `ALL_PROXY` are removed so the process +cannot reach the upstream proxy directly or read its credentials, and `NO_PROXY` +is replaced with the local addresses only. Inheriting `NO_PROXY` would let the +process connect straight to every destination it named, with no rule evaluation +at all. In weak mode httpjail then sets the proxy variables to its own address. diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 3d7abfde..e085a5fb 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -84,7 +84,13 @@ traffic is redirected transparently without setting proxy variables. | `HTTPS_PROXY` | HTTPS proxy address | `http://127.0.0.1:34567` | | `SSL_CERT_FILE` | CA certificate path | `/tmp/httpjail-ca.pem` | | `SSL_CERT_DIR` | CA certificate directory | `/tmp/httpjail-certs/` | -| `NO_PROXY` | Bypass proxy for these hosts | `localhost,127.0.0.1` | +| `NO_PROXY` | Bypass proxy for these hosts | `localhost,127.0.0.1,::1` | + +The parent's proxy variables are never inherited by the jailed process: +`HTTP_PROXY`, `HTTPS_PROXY` and `ALL_PROXY` are removed (in both spellings) so +the process cannot reach the upstream proxy directly or read its credentials, and +`NO_PROXY` is set to the local addresses only rather than merged with the +parent's value, which would let the process bypass httpjail. ### Consumed by httpjail diff --git a/src/jail/linux/docker.rs b/src/jail/linux/docker.rs index 68bb7efa..e652da54 100644 --- a/src/jail/linux/docker.rs +++ b/src/jail/linux/docker.rs @@ -314,9 +314,7 @@ impl DockerLinux { // The parent process may use proxy env vars for httpjail's own egress. // Do not leak those credentials or settings into the Docker CLI process; // Docker network isolation routes container traffic through httpjail. - for key in ["HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"] { - cmd.env_remove(key); - } + crate::jail::remove_parent_proxy_env(&mut cmd); // Use our isolated Docker network cmd.args(["--network", &network_name]); diff --git a/src/jail/linux/mod.rs b/src/jail/linux/mod.rs index 8d7810cd..dd8b19de 100644 --- a/src/jail/linux/mod.rs +++ b/src/jail/linux/mod.rs @@ -547,9 +547,7 @@ impl Jail for LinuxJail { // The parent process may use proxy env vars for httpjail's own egress. // Do not leak those credentials or settings into the jailed command; // native Linux isolation redirects traffic transparently. - for key in ["HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"] { - cmd.env_remove(key); - } + crate::jail::remove_parent_proxy_env(&mut cmd); // Preserve SUDO environment variables for consistency with macOS if let Ok(sudo_user) = std::env::var("SUDO_USER") { diff --git a/src/jail/mod.rs b/src/jail/mod.rs index c389c293..609ead1e 100644 --- a/src/jail/mod.rs +++ b/src/jail/mod.rs @@ -1,5 +1,6 @@ use anyhow::Result; use rand::Rng; +use std::process::Command; pub mod weak; @@ -9,6 +10,39 @@ pub mod linux; #[cfg(any(target_os = "macos", target_os = "linux"))] pub mod managed; +/// Proxy environment variables that configure httpjail's *own* egress and must +/// never reach a jailed process. +/// +/// `HTTP_PROXY` / `HTTPS_PROXY` / `ALL_PROXY` can carry the upstream proxy's +/// credentials, and a jailed process that honored them would talk to that proxy +/// instead of to httpjail. `NO_PROXY` is worse: it names destinations to reach +/// directly, so an inherited value lets the process skip httpjail entirely and +/// escape rule evaluation. +/// +/// Both spellings of each name are listed. Tools differ in which they read — +/// curl prefers the lowercase one — so removing only the uppercase spelling +/// would leave the hole open. +pub const PARENT_PROXY_ENV_VARS: [&str; 8] = [ + "HTTP_PROXY", + "http_proxy", + "HTTPS_PROXY", + "https_proxy", + "ALL_PROXY", + "all_proxy", + "NO_PROXY", + "no_proxy", +]; + +/// Drop every variable in [`PARENT_PROXY_ENV_VARS`] from a child's environment. +/// +/// Callers that need a proxy variable set for the child (weak mode points the +/// process at httpjail) assign it *after* calling this. +pub fn remove_parent_proxy_env(cmd: &mut Command) { + for key in PARENT_PROXY_ENV_VARS { + cmd.env_remove(key); + } +} + /// Trait for platform-specific jail implementations #[allow(dead_code)] pub trait Jail: Send + Sync { @@ -213,4 +247,22 @@ mod tests { // We generated 1000 unique IDs assert_eq!(ids.len(), 1000); } + + /// Every proxy variable must be stripped in both spellings. Missing the + /// lowercase one is the dangerous case: curl reads it first, so a leftover + /// `no_proxy` would let a jailed process skip httpjail. + #[test] + fn parent_proxy_env_covers_both_spellings() { + for name in ["HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "NO_PROXY"] { + assert!( + PARENT_PROXY_ENV_VARS.contains(&name), + "{name} is not stripped from jailed processes" + ); + let lower = name.to_ascii_lowercase(); + assert!( + PARENT_PROXY_ENV_VARS.contains(&lower.as_str()), + "{lower} is not stripped from jailed processes" + ); + } + } } diff --git a/src/jail/weak.rs b/src/jail/weak.rs index 9835cd55..bf4e0c51 100644 --- a/src/jail/weak.rs +++ b/src/jail/weak.rs @@ -46,6 +46,11 @@ impl Jail for WeakJail { cmd.arg(arg); } + // The parent's proxy variables configure httpjail's own egress. None of + // them may survive into the jailed process, so clear them all before + // setting the ones that point at httpjail. + super::remove_parent_proxy_env(&mut cmd); + // Set proxy environment variables let http_proxy = format!("http://127.0.0.1:{}", self.config.http_proxy_port); let https_proxy = format!("http://127.0.0.1:{}", self.config.https_proxy_port); @@ -55,18 +60,15 @@ impl Jail for WeakJail { cmd.env("http_proxy", &http_proxy); cmd.env("https_proxy", &https_proxy); - // Also set NO_PROXY for localhost to avoid proxying local connections - // Preserve any existing NO_PROXY settings by appending them - let mut no_proxy_hosts = "localhost,127.0.0.1,::1".to_string(); - - if let Ok(existing) = std::env::var("NO_PROXY").or_else(|_| std::env::var("no_proxy")) - && !existing.is_empty() - { - no_proxy_hosts = format!("{},{}", existing, no_proxy_hosts); - } - - cmd.env("NO_PROXY", &no_proxy_hosts); - cmd.env("no_proxy", &no_proxy_hosts); + // Keep local connections off the proxy, which would otherwise loop back + // through httpjail. The parent's NO_PROXY is deliberately not merged in: + // any entry it names would let the jailed process reach that destination + // directly, with no rule evaluation at all. + // + // Both spellings are set because tools disagree on which they read. + let no_proxy_hosts = "localhost,127.0.0.1,::1"; + cmd.env("NO_PROXY", no_proxy_hosts); + cmd.env("no_proxy", no_proxy_hosts); // Set any extra environment variables for (key, value) in extra_env { diff --git a/tests/weak_integration.rs b/tests/weak_integration.rs index 8de3ba82..174a6dda 100644 --- a/tests/weak_integration.rs +++ b/tests/weak_integration.rs @@ -121,54 +121,65 @@ fn test_weak_mode_allows_localhost() { } } +/// The parent's proxy variables configure httpjail's own egress and must not +/// reach the jailed process. An inherited NO_PROXY entry in particular would let +/// the process reach that destination directly, with no rule evaluation. +/// +/// Both spellings are set on the parent, with different values, so that a leak +/// through either one is detected and named. #[test] -fn test_weak_mode_appends_no_proxy() { - // Ensure existing NO_PROXY values are preserved and localhost entries appended +fn test_weak_mode_does_not_inherit_parent_proxy_env() { let result = HttpjailCommand::new() .weak() .js("true") - .env("NO_PROXY", "example.com") + .env("NO_PROXY", "upper.internal.corp") + .env("no_proxy", "lower.internal.corp") + .env("ALL_PROXY", "http://upper:3128") + .env("all_proxy", "http://lower:3128") + .env("HTTP_PROXY", "http://upper:8080") + .env("http_proxy", "http://lower:8080") .verbose(2) .command(vec!["env"]) .execute(); - match result { - Ok((exit_code, stdout, _stderr)) => { - assert_eq!(exit_code, 0, "env command should succeed"); - - let mut found_upper = false; - let mut found_lower = false; - - for line in stdout.lines() { - if let Some((key, value)) = line.split_once('=') { - if key == "NO_PROXY" { - found_upper = true; - assert!( - value.contains("example.com") - && value.contains("localhost") - && value.contains("127.0.0.1") - && value.contains("::1"), - "NO_PROXY missing expected entries: {}", - value - ); - } else if key == "no_proxy" { - found_lower = true; - assert!( - value.contains("example.com") - && value.contains("localhost") - && value.contains("127.0.0.1") - && value.contains("::1"), - "no_proxy missing expected entries: {}", - value - ); - } - } - } - - assert!(found_upper, "NO_PROXY variable not found"); - assert!(found_lower, "no_proxy variable not found"); - } - Err(e) => panic!("Failed to execute httpjail: {}", e), + let (exit_code, stdout, _stderr) = result.expect("Failed to execute httpjail"); + assert_eq!(exit_code, 0, "env command should succeed"); + + let child_env: std::collections::HashMap<&str, &str> = stdout + .lines() + .filter_map(|line| line.split_once('=')) + .collect(); + + // ALL_PROXY has no httpjail equivalent, so it must simply be gone. + for key in ["ALL_PROXY", "all_proxy"] { + assert!( + !child_env.contains_key(key), + "{key} leaked into the jailed process: {:?}", + child_env.get(key) + ); + } + + // Both spellings are set for the child, and neither may carry the parent's + // entries: those hosts would bypass httpjail entirely. + for key in ["NO_PROXY", "no_proxy"] { + let value = child_env + .get(key) + .unwrap_or_else(|| panic!("{key} should be set for the jailed process")); + assert_eq!( + *value, "localhost,127.0.0.1,::1", + "{key} should list only local addresses" + ); + } + + // The child must talk to httpjail, not to the parent's proxy. + for key in ["HTTP_PROXY", "http_proxy", "HTTPS_PROXY", "https_proxy"] { + let value = child_env + .get(key) + .unwrap_or_else(|| panic!("{key} should point at httpjail")); + assert!( + value.starts_with("http://127.0.0.1:"), + "{key} should point at httpjail, got {value}" + ); } } From 3cd13f3685af409e7ec4eb64a723d33ba0ce306e Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Fri, 7 Aug 2026 19:38:21 +0000 Subject: [PATCH 14/23] fix: match a bare NO_PROXY address against host name destinations too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit curl decides how to read a NO_PROXY entry from what the destination is, not from what the entry looks like. When the destination is a host name every entry — including a bare address — goes through the domain suffix match, so `NO_PROXY=127.0.0.1` also covers `foo.127.0.0.1` and `127.0.0.1.`; the latter is the plausible one, since a trailing dot stops the destination from parsing as an address. Classifying by the entry instead sent both of those through the proxy. The entry now keeps its original text alongside the parsed address and offers it as a domain candidate, which restores curl's behavior without reclassifying anything per request. Label boundaries still apply, so `x127.0.0.1` does not match. An IPv6 entry carries its text as well and simply never matches on that path, because a host name cannot contain a colon — the same outcome curl reaches by comparing the token as a string. --- docs/advanced/upstream-proxy.md | 2 +- src/upstream.rs | 36 ++++++++++++++++++++++++--------- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/docs/advanced/upstream-proxy.md b/docs/advanced/upstream-proxy.md index b334393c..304b07ba 100644 --- a/docs/advanced/upstream-proxy.md +++ b/docs/advanced/upstream-proxy.md @@ -50,7 +50,7 @@ the upstream proxy. The syntax follows curl 8.14.1. | Wildcard | `*` | Only when the whole list is exactly `*`: no proxy is used at all | | IPv4 CIDR | `192.168.0.0/16` | Only for destinations written as an IP literal | | IPv6 CIDR | `2001:db8::/32` | Only for destinations written as an IP literal | -| Address | `192.168.1.1` | Without a prefix length, an exact address match | +| Address | `192.168.1.1` | Without a prefix length, an exact address match. Also matches host names ending in it, since an entry is read as a domain whenever the destination is a host name | Entries are separated by commas, matched case-insensitively, and one trailing dot is ignored on both the entry and the destination. Rule evaluation is unaffected: diff --git a/src/upstream.rs b/src/upstream.rs index e1c1081a..483867fc 100644 --- a/src/upstream.rs +++ b/src/upstream.rs @@ -62,18 +62,22 @@ const NO_PROXY_WILDCARD: &str = "*"; /// One parsed `NO_PROXY` entry. /// -/// The destination decides which variants can apply: an IP literal destination is -/// only ever compared against [`NoProxyRule::Ip`] and [`NoProxyRule::Net`], and a -/// host name only against [`NoProxyRule::Domain`]. Domain rules and address rules -/// never cross-match, matching curl. +/// curl decides how to read an entry from what the *destination* is, not from +/// what the entry looks like: against an IP literal destination every entry is +/// read as an address or network, and against a host name destination every entry +/// is read as a domain. So a prefix length only ever applies to an IP +/// destination, a domain never matches an IP destination, and a bare address is +/// also a domain candidate — `NO_PROXY=127.0.0.1` bypasses `foo.127.0.0.1` and +/// `127.0.0.1.` as well as `127.0.0.1` itself. #[derive(Clone, Debug)] enum NoProxyRule { /// Label-boundary suffix match. Already ASCII-lowercased with one leading /// and one trailing dot removed. Domain(String), - /// An entry without a prefix length: exact address match. - Ip(IpAddr), - /// An entry with a prefix length. + /// An entry without a prefix length: an exact match against an IP literal + /// destination, and `text` as a domain against a host name destination. + Ip { addr: IpAddr, text: String }, + /// An entry with a prefix length. Only an IP literal destination can match. Net(IpNet), } @@ -115,7 +119,7 @@ impl NoProxy { fn matches(&self, host: &str) -> bool { if let Ok(ip) = host.parse::() { return self.rules.iter().any(|rule| match rule { - NoProxyRule::Ip(entry) => *entry == ip, + NoProxyRule::Ip { addr, .. } => *addr == ip, NoProxyRule::Net(entry) => entry.contains(&ip), NoProxyRule::Domain(_) => false, }); @@ -125,7 +129,11 @@ impl NoProxy { let host = host.strip_suffix('.').unwrap_or(host); self.rules.iter().any(|rule| match rule { NoProxyRule::Domain(entry) => domain_matches(entry, host), - NoProxyRule::Ip(_) | NoProxyRule::Net(_) => false, + // A bare address is a domain candidate too, so `127.0.0.1` covers + // `foo.127.0.0.1`. An IPv6 entry simply never matches here, since a + // host name cannot contain a colon. + NoProxyRule::Ip { text, .. } => domain_matches(text, host), + NoProxyRule::Net(_) => false, }) } } @@ -152,7 +160,10 @@ fn parse_no_proxy_rule(index: usize, token: &str) -> Result> } if let Ok(addr) = token.parse::() { - return Ok(Some(NoProxyRule::Ip(addr))); + return Ok(Some(NoProxyRule::Ip { + addr, + text: token.to_ascii_lowercase(), + })); } // One leading and one trailing dot are ignored, trailing first, as curl @@ -863,6 +874,11 @@ mod tests { ("192.168.0.0/16", "http://192.169.4.5/", false), ("192.168.1.1", "http://192.168.1.1/", true), ("192.168.1.1", "http://192.168.1.2/", false), + // A bare address is read as a domain when the destination is a host + // name, so it covers names ending in it — still at a label boundary. + ("127.0.0.1", "http://foo.127.0.0.1/", true), + ("127.0.0.1", "http://127.0.0.1./", true), + ("127.0.0.1", "http://x127.0.0.1/", false), ("2001:db8::/32", "http://[2001:db8::1]/", true), ("2001:db8::/32", "http://[2001:db9::1]/", false), ("2001:db8::/65", "http://[2001:db8::1]/", true), From d39cf7fdc572951bd546daa799b53b930f1d2ec1 Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Sat, 8 Aug 2026 03:49:11 +0000 Subject: [PATCH 15/23] perf: read the CONNECT response in chunks instead of byte by byte read_connect_status awaited one read per byte so that it could stop exactly at the end of the headers: reading further would have consumed tunnel data with nowhere to put it. A short "HTTP/1.1 200 Connection established" answer therefore cost around forty awaits and syscalls, against the project's minimal-latency goal. The response is now read in 1 KiB chunks and the bytes that overshoot the headers are carried on the connection instead of being dropped. ProxyStream replays them before it touches the socket, so the destination TLS handshake sees exactly the byte order it would have seen before. Details worth keeping: - The header terminator is searched from three bytes before the newly read data, so a \r\n\r\n split across two reads is still found. - The size cap is applied to the header end, not to everything buffered. A single read can legitimately return headers within the cap plus tunnel data that pushes the total past it, and that is a success. - Only the headers are decoded as UTF-8. What follows is a TLS ClientHello and arbitrary binary. - BytesMut reports nearly unbounded space, so each read is capped with BufMut::limit rather than letting the buffer size the read. - One timeout still covers the whole exchange rather than each read, or a proxy dribbling out a byte at a time would never trip a deadline. --- src/upstream.rs | 214 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 177 insertions(+), 37 deletions(-) diff --git a/src/upstream.rs b/src/upstream.rs index 483867fc..4debbc62 100644 --- a/src/upstream.rs +++ b/src/upstream.rs @@ -25,6 +25,7 @@ use anyhow::{Context as _, Result, anyhow, bail}; use base64::{Engine as _, engine::general_purpose::STANDARD}; +use bytes::{Buf, BufMut, Bytes, BytesMut}; use hyper::Uri; use hyper::header::HeaderValue; use hyper::rt::{Read, ReadBufCursor, Write}; @@ -56,6 +57,11 @@ const PROXY_SETUP_TIMEOUT: Duration = Duration::from_secs(30); /// A well-behaved proxy answers with a short status line and a few headers. const MAX_CONNECT_RESPONSE_BYTES: usize = 16 * 1024; +/// How much of the proxy's `CONNECT` response to ask for per read. Large enough +/// that a well-behaved proxy's whole response arrives in one read, small enough +/// that the bytes read past the headers stay a bounded prefix. +const CONNECT_READ_CHUNK_BYTES: usize = 1024; + /// The only `NO_PROXY` value that acts as a wildcard. Compared against the whole /// list verbatim, as curl does, so `" * "` is not a wildcard. const NO_PROXY_WILDCARD: &str = "*"; @@ -493,7 +499,7 @@ impl Service for ProxyConnector { Err(_) => return Err(timed_out("connecting directly to destination")), }; let _ = tcp.set_nodelay(true); - return Ok(ProxyStream::new(tcp, false)); + return Ok(ProxyStream::new(tcp, Bytes::new(), false)); }; // Dial the proxy (TCP). The destination scheme is irrelevant here; @@ -506,25 +512,26 @@ impl Service for ProxyConnector { }; let _ = stream.set_nodelay(true); - let proxied = if dst.scheme_str() == Some("https") { + let (prefetched, proxied) = if dst.scheme_str() == Some("https") { let host = uri_host(&dst).ok_or_else(|| { BoxError::from(format!("CONNECT target has no host: {}", dst)) })?; let port = dst.port_u16().unwrap_or(443); - establish_connect_tunnel(&mut stream, host, port, proxy.auth.as_ref()) - .await - .map_err(|e| -> BoxError { e.into() })?; + let prefetched = + establish_connect_tunnel(&mut stream, host, port, proxy.auth.as_ref()) + .await + .map_err(|e| -> BoxError { e.into() })?; // The tunnel is transparent end-to-end; destination TLS is // layered on top by the surrounding HttpsConnector and the // request is sent in origin-form, so do not mark it proxied. - false + (prefetched, false) } else { // Plain HTTP: the proxy forwards absolute-form requests. Mark the // connection proxied so hyper emits absolute-form request lines. - true + (Bytes::new(), true) }; - Ok(ProxyStream::new(stream, proxied)) + Ok(ProxyStream::new(stream, prefetched, proxied)) }) } } @@ -538,13 +545,18 @@ fn timed_out(phase: &str) -> BoxError { /// consults to decide between absolute-form and origin-form request lines. pub struct ProxyStream { io: TokioIo, + /// Tunnel bytes read ahead of time while consuming the `CONNECT` response. + /// Replayed before anything is taken from the socket so the byte order the + /// destination TLS handshake sees is unchanged. + prefetched: Bytes, proxied: bool, } impl ProxyStream { - fn new(io: TcpStream, proxied: bool) -> Self { + fn new(io: TcpStream, prefetched: Bytes, proxied: bool) -> Self { ProxyStream { io: TokioIo::new(io), + prefetched, proxied, } } @@ -560,9 +572,21 @@ impl Read for ProxyStream { fn poll_read( self: Pin<&mut Self>, cx: &mut Context<'_>, - buf: ReadBufCursor<'_>, + mut buf: ReadBufCursor<'_>, ) -> Poll> { - Pin::new(&mut self.get_mut().io).poll_read(cx, buf) + let this = self.get_mut(); + + // Drain the read-ahead first, and return without touching the socket + // while any of it remains. Mixing the two in one poll would reorder the + // stream. + if !this.prefetched.is_empty() { + let take = this.prefetched.len().min(buf.remaining()); + buf.put_slice(&this.prefetched[..take]); + this.prefetched.advance(take); + return Poll::Ready(Ok(())); + } + + Pin::new(&mut this.io).poll_read(cx, buf) } } @@ -596,14 +620,17 @@ impl Write for ProxyStream { } } -/// Send a `CONNECT` request to the upstream proxy and validate its response, -/// leaving `stream` positioned at the start of the tunnel payload on success. +/// Send a `CONNECT` request to the upstream proxy and validate its response. +/// +/// Returns any tunnel bytes that arrived in the same read as the end of the +/// response headers. Those bytes belong to the tunnel and must be replayed +/// before anything further is read from `stream`; see [`ProxyStream`]. async fn establish_connect_tunnel( stream: &mut S, host: &str, port: u16, auth: Option<&HeaderValue>, -) -> Result<()> +) -> Result where S: AsyncRead + AsyncWrite + Unpin, { @@ -630,25 +657,30 @@ where Err(_) => bail!("Timeout flushing CONNECT request to upstream proxy"), } - let status = match timeout(PROXY_SETUP_TIMEOUT, read_connect_status(stream)).await { + // One timeout for the whole exchange rather than one per read: a proxy that + // dribbles the response out a byte at a time would otherwise never trip a + // per-read deadline and could hold the setup open indefinitely. + let response = match timeout(PROXY_SETUP_TIMEOUT, read_connect_response(stream)).await { Ok(result) => result?, Err(_) => bail!("Timeout reading CONNECT response from upstream proxy"), }; - if !(200..300).contains(&status) { + if !(200..300).contains(&response.status) { bail!( "Upstream proxy refused CONNECT to {}:{} with status {}", host, port, - status + response.status ); } debug!( - "Established CONNECT tunnel to {}:{} via upstream proxy", - host, port + "Established CONNECT tunnel to {}:{} via upstream proxy ({} byte(s) of tunnel data already read)", + host, + port, + response.prefetched.len() ); - Ok(()) + Ok(response.prefetched) } /// The destination host as a bare host name or IP literal. @@ -676,38 +708,80 @@ fn host_port_authority(host: &str, port: u16) -> String { } } -/// Read the proxy's `CONNECT` response up to the end of its headers and return -/// the HTTP status code. Reads are bounded by [`MAX_CONNECT_RESPONSE_BYTES`] to -/// avoid consuming tunnel payload and to bound memory. -async fn read_connect_status(stream: &mut S) -> Result +/// The proxy's answer to `CONNECT`, plus whatever came after it. +struct ConnectResponse { + status: u16, + /// Tunnel bytes that arrived in the same read as the end of the headers. + /// Reading in chunks means the response and the first tunnel data can land + /// together; discarding the remainder would corrupt the TLS handshake that + /// follows. + prefetched: Bytes, +} + +/// Read the proxy's `CONNECT` response up to the end of its headers. +/// +/// Reads in chunks of [`CONNECT_READ_CHUNK_BYTES`] rather than a byte at a time, +/// and hands back the bytes that overshot the headers instead of dropping them. +/// The headers themselves are capped at [`MAX_CONNECT_RESPONSE_BYTES`], so memory +/// stays within that plus one chunk. +async fn read_connect_response(stream: &mut S) -> Result where S: AsyncRead + Unpin, { - let mut buf = Vec::with_capacity(128); - let mut byte = [0u8; 1]; - loop { - let n = stream.read(&mut byte).await?; - if n == 0 { + const TERMINATOR: &[u8] = b"\r\n\r\n"; + + let mut buf = BytesMut::with_capacity(CONNECT_READ_CHUNK_BYTES); + let header_end = loop { + let filled = buf.len(); + + // `BytesMut` reports nearly unbounded space, so cap each read explicitly + // rather than letting it size the read for us. + let mut chunk = (&mut buf).limit(CONNECT_READ_CHUNK_BYTES); + if stream.read_buf(&mut chunk).await? == 0 { bail!("Upstream proxy closed connection during CONNECT"); } - buf.push(byte[0]); - if buf.ends_with(b"\r\n\r\n") { - break; + + // A terminator can straddle two reads, so rescan the last three bytes of + // what was already there instead of only the newly added bytes. + let search_from = filled.saturating_sub(TERMINATOR.len() - 1); + if let Some(offset) = find_subslice(&buf[search_from..], TERMINATOR) { + break search_from + offset + TERMINATOR.len(); } - if buf.len() > MAX_CONNECT_RESPONSE_BYTES { + + // Only reached with no terminator in hand: everything read so far is + // header, so the cap applies to all of it. + if buf.len() >= MAX_CONNECT_RESPONSE_BYTES { bail!("Upstream proxy CONNECT response exceeded size limit"); } + }; + + // Checked after the terminator is located, not before: a single read may + // carry headers within the cap plus tunnel data that pushes the total over + // it, and that case is a success. + if header_end > MAX_CONNECT_RESPONSE_BYTES { + bail!("Upstream proxy CONNECT response exceeded size limit"); } - // Parse the status code from the first line, e.g. - // `HTTP/1.1 200 Connection established`. + let prefetched = buf.split_off(header_end).freeze(); + + // Only the headers are text. The tunnel bytes are arbitrary binary (a TLS + // ClientHello, typically) and must never be run through a UTF-8 check. let head = std::str::from_utf8(&buf).context("Non-UTF8 CONNECT response")?; let first_line = head.lines().next().unwrap_or(""); - first_line + let status = first_line .split_whitespace() .nth(1) .and_then(|code| code.parse::().ok()) - .ok_or_else(|| anyhow!("Malformed CONNECT status line: {:?}", first_line)) + .ok_or_else(|| anyhow!("Malformed CONNECT status line: {:?}", first_line))?; + + Ok(ConnectResponse { status, prefetched }) +} + +/// Index of the first occurrence of `needle` in `haystack`. +fn find_subslice(haystack: &[u8], needle: &[u8]) -> Option { + haystack + .windows(needle.len()) + .position(|window| window == needle) } #[cfg(test)] @@ -1079,6 +1153,72 @@ mod tests { assert!(request.contains("Host: [::1]:443\r\n")); } + /// Reading in chunks can pull tunnel data in with the response headers. Those + /// bytes belong to the TLS handshake that follows and must survive intact, + /// including bytes that are not valid UTF-8. + #[tokio::test] + async fn connect_tunnel_returns_bytes_read_past_the_headers() { + const TUNNEL: &[u8] = &[0x16, 0x03, 0x01, 0x00, 0xff, 0x00, 0x80]; + + let (mut client_end, mut proxy_end) = tokio::io::duplex(1024); + let proxy = tokio::spawn(async move { + let mut buf = Vec::new(); + let mut byte = [0u8; 1]; + while proxy_end.read(&mut byte).await.unwrap() != 0 { + buf.push(byte[0]); + if buf.ends_with(b"\r\n\r\n") { + break; + } + } + // Headers and tunnel data in a single write, so they arrive together. + let mut response = b"HTTP/1.1 200 Connection established\r\n\r\n".to_vec(); + response.extend_from_slice(TUNNEL); + proxy_end.write_all(&response).await.unwrap(); + proxy_end.flush().await.unwrap(); + }); + + let prefetched = establish_connect_tunnel(&mut client_end, "example.com", 443, None) + .await + .unwrap(); + + assert_eq!(prefetched.as_ref(), TUNNEL); + proxy.await.unwrap(); + } + + /// The read-ahead has to come out before anything from the socket, and has to + /// survive being read in pieces smaller than itself. + #[tokio::test] + async fn proxy_stream_replays_prefetched_bytes_before_socket_bytes() { + use tokio::io::AsyncReadExt as _; + + const PREFIX: &[u8] = b"prefetched-"; + const BODY: &[u8] = b"from-socket"; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let server = tokio::spawn(async move { + let (mut sock, _) = listener.accept().await.unwrap(); + sock.write_all(BODY).await.unwrap(); + sock.flush().await.unwrap(); + }); + + let client = tokio::net::TcpStream::connect(addr).await.unwrap(); + let stream = ProxyStream::new(client, Bytes::from_static(PREFIX), false); + let mut io = TokioIo::new(stream); + + // Deliberately smaller than the prefix so the replay spans several reads. + let mut got = Vec::new(); + let mut chunk = [0u8; 4]; + while got.len() < PREFIX.len() + BODY.len() { + let n = io.read(&mut chunk).await.unwrap(); + assert_ne!(n, 0, "stream ended early: {:?}", got); + got.extend_from_slice(&chunk[..n]); + } + + assert_eq!(got, [PREFIX, BODY].concat()); + server.await.unwrap(); + } + #[tokio::test] async fn connect_tunnel_rejects_non_2xx() { let (mut client_end, proxy_end) = tokio::io::duplex(1024); From e2d603dd1c44ce87144d00e81f9d855b18735fb0 Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Sat, 8 Aug 2026 03:53:14 +0000 Subject: [PATCH 16/23] test: cover an HTTPS destination through the upstream CONNECT tunnel The CONNECT exchange and the plain-HTTP forwarding path each had tests, but nothing exercised them composed: an HTTPS destination reached by opening a tunnel through the proxy, running the destination TLS handshake inside it, and getting a response back. That is the most intricate path in the feature and it was the one with no coverage. The test stands up a real TLS origin with a self-signed certificate and a proxy that answers CONNECT and then splices the two sockets, so the handshake has to succeed over the tunnel for the request to arrive at all. It asserts the CONNECT names the destination authority rather than the proxy's own address, and that the origin receives the expected path. One timeout covers the request, the body collection and the two task joins together. Bounding only the request would let a regression that stalls after the response headers, or one that leaves the tunnel copy running, hang the suite instead of failing it. Verified to have teeth: skipping the CONNECT fails in three seconds rather than hanging on the thirty second upstream setup timeout, and a stall after the headers trips the same bound. Marking the tunnel proxied, which the test cannot detect, is not a gap this test should try to close: hyper-util's absolute_form() falls back to origin-form for HTTPS URIs on its own, so the flag cannot change what the origin sees. The comment says so rather than claiming coverage it does not have. Everything binds to port 0 and no external network, DNS, sudo or Docker is involved, so it runs in parallel with the rest of the suite. --- src/proxy.rs | 145 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 143 insertions(+), 2 deletions(-) diff --git a/src/proxy.rs b/src/proxy.rs index f44b40ba..ef9dca58 100644 --- a/src/proxy.rs +++ b/src/proxy.rs @@ -855,6 +855,147 @@ mod tests { )); } + /// The whole HTTPS path in one go: the connector opens a CONNECT tunnel + /// through the proxy, the HttpsConnector layers the destination TLS on top of + /// it, and the request reaches the origin server. + /// + /// Each half is covered on its own elsewhere; nothing else checks that they + /// compose. Skipping the CONNECT, tunneling to the wrong authority, or + /// failing to run the destination handshake over the tunnel all fail here. + /// + /// Note that the origin-form assertion below records the wire shape rather + /// than guarding the `proxied` flag: hyper-util's `absolute_form()` falls + /// back to origin-form for HTTPS URIs on its own, so marking the tunnel + /// proxied would not change what the origin sees. + #[tokio::test] + async fn https_destination_through_connect_tunnel_reaches_origin() { + use http_body_util::Empty; + use hyper::server::conn::http1 as server_http1; + use hyper::service::service_fn; + use std::sync::Mutex; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::{TcpListener, TcpStream}; + + const ORIGIN_HOST: &str = "target.test"; + const BODY: &str = "through-connect-ok"; + + // Origin: a real TLS server with its own self-signed certificate, so the + // handshake has to succeed over the tunnel for the request to arrive. + let cert = rcgen::generate_simple_self_signed(vec![ORIGIN_HOST.to_string()]).unwrap(); + let tls_config = rustls::ServerConfig::builder() + .with_no_client_auth() + .with_single_cert( + vec![cert.cert.der().clone()], + rustls::pki_types::PrivateKeyDer::Pkcs8(cert.key_pair.serialize_der().into()), + ) + .unwrap(); + let acceptor = tokio_rustls::TlsAcceptor::from(Arc::new(tls_config)); + + let origin_listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let origin_addr = origin_listener.local_addr().unwrap(); + let seen_uri = Arc::new(Mutex::new(String::new())); + let origin_seen_uri = Arc::clone(&seen_uri); + let origin = tokio::spawn(async move { + let (sock, _) = origin_listener.accept().await.unwrap(); + let tls = acceptor.accept(sock).await.unwrap(); + let service = service_fn(move |req: Request| { + let seen = Arc::clone(&origin_seen_uri); + async move { + *seen.lock().unwrap() = req.uri().to_string(); + Ok::<_, HyperError>( + Response::builder() + // Close after one response so the tunnel copy ends. + .header(hyper::header::CONNECTION, "close") + .body(Full::new(Bytes::from(BODY))) + .unwrap(), + ) + } + }); + let _ = server_http1::Builder::new() + .serve_connection(TokioIo::new(tls), service) + .await; + }); + + // Proxy: answer CONNECT, then splice the connection to the origin. + let proxy_listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let proxy_addr = proxy_listener.local_addr().unwrap(); + let proxy = tokio::spawn(async move { + let (mut client, _) = proxy_listener.accept().await.unwrap(); + let mut head = Vec::new(); + let mut byte = [0u8; 1]; + while client.read(&mut byte).await.unwrap() != 0 { + head.push(byte[0]); + if head.ends_with(b"\r\n\r\n") { + break; + } + } + let mut origin = TcpStream::connect(origin_addr).await.unwrap(); + client + .write_all(b"HTTP/1.1 200 Connection established\r\n\r\n") + .await + .unwrap(); + client.flush().await.unwrap(); + let _ = tokio::io::copy_bidirectional(&mut client, &mut origin).await; + String::from_utf8_lossy(&head).into_owned() + }); + + let upstream = + crate::upstream::UpstreamProxy::parse(&format!("http://{proxy_addr}")).unwrap(); + let proxies = UpstreamProxies::all(upstream); + let connector = ProxyConnector::with_config(proxies.clone()); + // The HttpsConnector is the point of the test: it must be able to run the + // destination handshake on top of what the connector returns. + let https = + hyper_rustls::HttpsConnector::from((connector, create_dangerous_client_config())); + let client = UpstreamClient::Proxied { + client: build_pooled_client(https), + proxies, + }; + + let body = Empty::::new() + .map_err(|never| match never {}) + .boxed(); + let req = Request::builder() + .uri(format!( + "https://{ORIGIN_HOST}:{}/through-connect", + origin_addr.port() + )) + .body(body) + .unwrap(); + + // One bound over the whole exchange, not just the request: a regression + // that stalls after the response headers, or leaves the tunnel copy + // running, would otherwise hang here instead of failing. + let exchange = async move { + let resp = client.request(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + let got = resp.into_body().collect().await.unwrap().to_bytes(); + assert_eq!(got, Bytes::from(BODY)); + + // Let the pooled connection go so the tunnel copy sees EOF. + drop(client); + let connect_head = proxy.await.unwrap(); + origin.await.unwrap(); + connect_head + }; + let connect_head = tokio::time::timeout(Duration::from_secs(3), exchange) + .await + .expect("timed out waiting for the tunneled request to complete"); + + // The proxy is dialed by address but must be asked for the destination. + assert!( + connect_head.starts_with(&format!( + "CONNECT {ORIGIN_HOST}:{} HTTP/1.1\r\n", + origin_addr.port() + )), + "unexpected CONNECT request: {connect_head}" + ); + // Reaching the origin at all proves the tunnel and the destination + // handshake worked; the path confirms the request was not rewritten on + // the way through. + assert_eq!(seen_uri.lock().unwrap().as_str(), "/through-connect"); + } + /// A plain-HTTP request routed through an upstream proxy must be forwarded in /// absolute-form with the configured `Proxy-Authorization` header. #[tokio::test] @@ -885,8 +1026,8 @@ mod tests { let proxy = crate::upstream::UpstreamProxy::parse(&format!("http://user:pass@{}", addr)).unwrap(); - let proxies = UpstreamProxies::all(proxy.clone()); - let connector = ProxyConnector::new(proxy); + let proxies = UpstreamProxies::all(proxy); + let connector = ProxyConnector::with_config(proxies.clone()); let https = hyper_rustls::HttpsConnector::from((connector, create_dangerous_client_config())); let client = UpstreamClient::Proxied { From 1dd901c6acee2aee3376617cfd73122cc907b1e2 Mon Sep 17 00:00:00 2001 From: shintaro-t Date: Sat, 8 Aug 2026 03:54:18 +0000 Subject: [PATCH 17/23] refactor: close the upstream connector's test-only constructors ProxyConnector::new and UpstreamProxies::all were public but never reached from production code: new() only wrapped all(), and all() only existed to feed new(). Both were left over from before from_specs() became the way a configuration is built, and they advertised a second construction path that nothing supports. new() is gone, with_config() is now pub(crate) since its only caller is UpstreamClient::new in this crate, and all() is #[cfg(test)] so it does not appear in a normal build at all. The two upstream proxy tests build their connector the same way now, through with_config. No behavior or public documentation changes. --- src/upstream.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/upstream.rs b/src/upstream.rs index 4debbc62..c9f90711 100644 --- a/src/upstream.rs +++ b/src/upstream.rs @@ -315,6 +315,10 @@ impl UpstreamProxies { self.proxy_for_uri(uri).and_then(UpstreamProxy::http_auth) } + /// Route every scheme through one proxy, with no bypass list. Only the + /// tests build a configuration this way; `from_specs` is the real entry + /// point. + #[cfg(test)] pub(crate) fn all(proxy: UpstreamProxy) -> Self { Self { http: Some(proxy.clone()), @@ -462,11 +466,7 @@ pub struct ProxyConnector { } impl ProxyConnector { - pub fn new(proxy: UpstreamProxy) -> Self { - Self::with_config(UpstreamProxies::all(proxy)) - } - - pub fn with_config(proxies: UpstreamProxies) -> Self { + pub(crate) fn with_config(proxies: UpstreamProxies) -> Self { let mut http = HttpConnector::new(); // The proxy is addressed via an http(s) URL; allow non-http schemes so // the connector does not reject the dial target. From 5f15d3a836c08b8c17d8621fe0dd3cd23501c986 Mon Sep 17 00:00:00 2001 From: "shintaro-t@iij.ad.jp" Date: Sun, 9 Aug 2026 01:31:52 +0900 Subject: [PATCH 18/23] docs: clarify weak-mode proxy credential exposure --- README.md | 7 +++++++ docs/advanced/upstream-proxy.md | 21 +++++++++++++++------ docs/guide/configuration.md | 9 ++++++--- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index bf39b8df..dee7e95d 100644 --- a/README.md +++ b/README.md @@ -86,6 +86,13 @@ through the proxy. - In weak mode, httpjail overwrites proxy env vars inside the jailed process to point sandboxed processes at httpjail itself. +Removing the upstream proxy variables from the command's own environment does +not make them secret from an untrusted command. Neither weak nor strong mode +provides process isolation from httpjail itself, and process-inspection +permissions depend on the platform and how httpjail was started. Do not put +proxy credentials in the environment when running untrusted commands unless a +separate OS or external credential boundary prevents access to them. + ## Documentation Docs are stored in the `docs/` directory and served diff --git a/docs/advanced/upstream-proxy.md b/docs/advanced/upstream-proxy.md index 304b07ba..30eedd9a 100644 --- a/docs/advanced/upstream-proxy.md +++ b/docs/advanced/upstream-proxy.md @@ -111,9 +111,18 @@ point sandboxed processes at httpjail itself. The jailed process talks to httpjail; the proxy env vars only affect the hop from httpjail to the outside world. -None of the parent's proxy variables are passed on to the jailed process, in any -mode. `HTTP_PROXY`, `HTTPS_PROXY` and `ALL_PROXY` are removed so the process -cannot reach the upstream proxy directly or read its credentials, and `NO_PROXY` -is replaced with the local addresses only. Inheriting `NO_PROXY` would let the -process connect straight to every destination it named, with no rule evaluation -at all. In weak mode httpjail then sets the proxy variables to its own address. +None of the parent's proxy variables are included in the jailed process's own +environment. `HTTP_PROXY`, `HTTPS_PROXY` and `ALL_PROXY` are removed so +cooperating applications do not use the upstream proxy directly, and `NO_PROXY` +is replaced with the local addresses only. Inheriting `NO_PROXY` would let an +application connect straight to every destination it named, with no rule +evaluation at all. In weak mode httpjail then sets the proxy variables to its own +address. + +Removing variables from the command's own environment is not credential +isolation. Neither weak nor strong mode creates a PID namespace or otherwise +guarantees that the command cannot inspect the httpjail process. Strong mode +isolates network access, but visibility of the parent process and permission to +read its environment depend on the platform, UID setup and other OS controls. +When running an untrusted command, use a credential-free proxy or a separate OS +or external credential boundary that prevents access to httpjail's environment. diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index e085a5fb..243dbfab 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -88,9 +88,12 @@ traffic is redirected transparently without setting proxy variables. The parent's proxy variables are never inherited by the jailed process: `HTTP_PROXY`, `HTTPS_PROXY` and `ALL_PROXY` are removed (in both spellings) so -the process cannot reach the upstream proxy directly or read its credentials, and -`NO_PROXY` is set to the local addresses only rather than merged with the -parent's value, which would let the process bypass httpjail. +cooperating applications do not use the upstream proxy directly, and `NO_PROXY` +is set to the local addresses only rather than merged with the parent's value, +which would let the process bypass httpjail. This does not hide the parent +process's environment from an untrusted command. Neither weak nor strong mode +provides process credential isolation from httpjail itself; access depends on +the platform, UID setup and other OS controls. ### Consumed by httpjail From a4f8276dc795a96fd0aaac3017d2daee94cd1fa7 Mon Sep 17 00:00:00 2001 From: "shintaro-t@iij.ad.jp" Date: Sun, 9 Aug 2026 01:32:57 +0900 Subject: [PATCH 19/23] test: isolate max-tx proxy listeners --- tests/weak_integration_max_tx_bytes.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/weak_integration_max_tx_bytes.rs b/tests/weak_integration_max_tx_bytes.rs index 724a274c..989d6df4 100644 --- a/tests/weak_integration_max_tx_bytes.rs +++ b/tests/weak_integration_max_tx_bytes.rs @@ -100,6 +100,11 @@ async fn start_httpjail(js_config: &str, proxy_port: u16) -> std::process::Child .arg("--js") .arg(js_config) .env("HTTPJAIL_HTTP_BIND", proxy_port.to_string()) + // These tests use only HTTP. Avoid racing on the default HTTPS port. + .env("HTTPJAIL_HTTPS_BIND", "127.0.0.1:0") + // The backend is local and must not inherit the developer's proxy route. + .env("NO_PROXY", "127.0.0.1") + .env("no_proxy", "127.0.0.1") .env("HTTPJAIL_SKIP_KEYCHAIN_INSTALL", "1") .stdout(Stdio::piped()) .stderr(Stdio::piped()) From 604ba137fea54dd7f83e257cf8b34c24ffa6d98a Mon Sep 17 00:00:00 2001 From: "shintaro-t@iij.ad.jp" Date: Sun, 9 Aug 2026 01:37:06 +0900 Subject: [PATCH 20/23] fix: time out destination TLS setup --- docs/advanced/upstream-proxy.md | 6 +-- src/proxy.rs | 11 +++-- src/upstream.rs | 86 +++++++++++++++++++++++++++++++-- 3 files changed, 94 insertions(+), 9 deletions(-) diff --git a/docs/advanced/upstream-proxy.md b/docs/advanced/upstream-proxy.md index 30eedd9a..7decd5d8 100644 --- a/docs/advanced/upstream-proxy.md +++ b/docs/advanced/upstream-proxy.md @@ -94,9 +94,9 @@ such as the unsupported forms above, are ignored and logged at debug level. the `Proxy-Authorization` header attached when credentials are configured. The header is never sent to a destination that `NO_PROXY` bypasses, nor to an HTTPS destination, whose request travels inside the tunnel to the origin server. -- Only connection setup (the TCP connect and the `CONNECT` exchange) is bounded - by a timeout. The established tunnel carries no timeout, so long-running - connections such as WebSocket and gRPC keep working. +- Connection setup (the TCP connect, `CONNECT` exchange and destination TLS + handshake) is bounded by a timeout. The established tunnel carries no timeout, + so long-running connections such as WebSocket and gRPC keep working. ## Relationship to jailed process proxy variables diff --git a/src/proxy.rs b/src/proxy.rs index ef9dca58..afe8274c 100644 --- a/src/proxy.rs +++ b/src/proxy.rs @@ -3,7 +3,7 @@ use crate::dangerous_verifier::create_dangerous_client_config; use crate::rules::{Action, RuleEngine}; #[allow(unused_imports)] use crate::tls::CertificateManager; -use crate::upstream::{ProxyConnector, UpstreamProxies}; +use crate::upstream::{ConnectionSetupTimeout, ProxyConnector, UpstreamProxies}; use anyhow::Result; use bytes::Bytes; use http_body_util::{BodyExt, Full, combinators::BoxBody}; @@ -167,8 +167,10 @@ type DirectClient = Client, BoxBody< /// Upstream client that routes every re-originated request through an upstream /// (corporate) proxy via a [`ProxyConnector`]. -type ProxiedClient = - Client, BoxBody>; +type ProxiedClient = Client< + ConnectionSetupTimeout>, + BoxBody, +>; /// Upstream client: either contacts destinations directly or routes through a /// configured upstream proxy. Both variants are high-level pooled clients. @@ -211,6 +213,7 @@ impl UpstreamClient { }; let connector = ProxyConnector::with_config(proxies.clone()); let https = hyper_rustls::HttpsConnector::from((connector, config)); + let https = ConnectionSetupTimeout::new(https); debug!("Upstream client initialized to route through the upstream proxy"); UpstreamClient::Proxied { client: build_pooled_client(https), @@ -947,6 +950,7 @@ mod tests { // destination handshake on top of what the connector returns. let https = hyper_rustls::HttpsConnector::from((connector, create_dangerous_client_config())); + let https = ConnectionSetupTimeout::new(https); let client = UpstreamClient::Proxied { client: build_pooled_client(https), proxies, @@ -1030,6 +1034,7 @@ mod tests { let connector = ProxyConnector::with_config(proxies.clone()); let https = hyper_rustls::HttpsConnector::from((connector, create_dangerous_client_config())); + let https = ConnectionSetupTimeout::new(https); let client = UpstreamClient::Proxied { client: build_pooled_client(https), proxies, diff --git a/src/upstream.rs b/src/upstream.rs index c9f90711..8de2caf7 100644 --- a/src/upstream.rs +++ b/src/upstream.rs @@ -48,9 +48,9 @@ use url::{Host, Url}; type BoxError = Box; -/// Timeout for establishing the tunnel through the upstream proxy (TCP connect -/// and the `CONNECT` exchange). This bounds setup only; the resulting tunnel -/// carries no timeout so long-running connections keep working. +/// Timeout for establishing an upstream connection (TCP connect, the `CONNECT` +/// exchange, and the destination TLS handshake). This bounds setup only; the +/// resulting connection carries no timeout so long-running connections work. const PROXY_SETUP_TIMEOUT: Duration = Duration::from_secs(30); /// Upper bound on the size of the upstream proxy's `CONNECT` response headers. @@ -465,6 +465,58 @@ pub struct ProxyConnector { proxies: Arc, } +/// Bound the complete connection setup performed by an inner connector. +/// +/// Wrapping the final HTTPS connector, rather than [`UpstreamClient::request`], +/// includes the destination TLS handshake without placing a deadline on the +/// established connection or its request/response streams. +#[derive(Clone)] +pub struct ConnectionSetupTimeout { + inner: C, + duration: Duration, +} + +impl ConnectionSetupTimeout { + pub(crate) fn new(inner: C) -> Self { + Self { + inner, + duration: PROXY_SETUP_TIMEOUT, + } + } + + #[cfg(test)] + fn with_timeout(inner: C, duration: Duration) -> Self { + Self { inner, duration } + } +} + +impl Service for ConnectionSetupTimeout +where + C: Service, + C::Future: Send + 'static, + C::Response: Send + 'static, + C::Error: Into, +{ + type Response = C::Response; + type Error = BoxError; + type Future = Pin> + Send>>; + + fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { + self.inner.poll_ready(cx).map_err(Into::into) + } + + fn call(&mut self, dst: Uri) -> Self::Future { + let connect = self.inner.call(dst); + let duration = self.duration; + Box::pin(async move { + match timeout(duration, connect).await { + Ok(result) => result.map_err(Into::into), + Err(_) => Err(timed_out("establishing connection")), + } + }) + } +} + impl ProxyConnector { pub(crate) fn with_config(proxies: UpstreamProxies) -> Self { let mut http = HttpConnector::new(); @@ -788,6 +840,34 @@ fn find_subslice(haystack: &[u8], needle: &[u8]) -> Option { mod tests { use super::*; + #[derive(Clone)] + struct StalledConnector; + + impl Service for StalledConnector { + type Response = (); + type Error = io::Error; + type Future = Pin> + Send>>; + + fn poll_ready(&mut self, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn call(&mut self, _dst: Uri) -> Self::Future { + Box::pin(std::future::pending()) + } + } + + #[tokio::test] + async fn connection_setup_timeout_includes_inner_connector() { + let mut connector = + ConnectionSetupTimeout::with_timeout(StalledConnector, Duration::from_millis(10)); + let err = connector + .call("https://target.test".parse().unwrap()) + .await + .unwrap_err(); + assert!(err.to_string().contains("Timeout establishing connection")); + } + #[test] fn parse_plain_proxy() { let p = UpstreamProxy::parse("http://proxy.corp:3128").unwrap(); From d8589369c01cb19493d69973b6d592af4cd42b53 Mon Sep 17 00:00:00 2001 From: "shintaro-t@iij.ad.jp" Date: Sun, 9 Aug 2026 01:38:23 +0900 Subject: [PATCH 21/23] fix: accept binary CONNECT header values --- src/upstream.rs | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/upstream.rs b/src/upstream.rs index 8de2caf7..9021d5e0 100644 --- a/src/upstream.rs +++ b/src/upstream.rs @@ -816,15 +816,23 @@ where let prefetched = buf.split_off(header_end).freeze(); - // Only the headers are text. The tunnel bytes are arbitrary binary (a TLS + // HTTP field values may contain obs-text bytes, so only parse the status + // code token as text. The tunnel bytes are arbitrary binary as well (a TLS // ClientHello, typically) and must never be run through a UTF-8 check. - let head = std::str::from_utf8(&buf).context("Non-UTF8 CONNECT response")?; - let first_line = head.lines().next().unwrap_or(""); + let first_line_end = find_subslice(&buf, b"\r\n").unwrap_or(buf.len()); + let first_line = &buf[..first_line_end]; let status = first_line - .split_whitespace() + .split(|byte| byte.is_ascii_whitespace()) + .filter(|token| !token.is_empty()) .nth(1) + .and_then(|code| std::str::from_utf8(code).ok()) .and_then(|code| code.parse::().ok()) - .ok_or_else(|| anyhow!("Malformed CONNECT status line: {:?}", first_line))?; + .ok_or_else(|| { + anyhow!( + "Malformed CONNECT status line: {:?}", + String::from_utf8_lossy(first_line) + ) + })?; Ok(ConnectResponse { status, prefetched }) } @@ -1205,6 +1213,19 @@ mod tests { assert!(request.contains("Proxy-Authorization: Basic dXNlcjpwYXNz\r\n")); } + #[tokio::test] + async fn connect_tunnel_accepts_non_utf8_header_values() { + let (mut client_end, proxy_end) = tokio::io::duplex(1024); + tokio::spawn(fake_proxy( + proxy_end, + b"HTTP/1.1 200 Connection established\r\nX-Binary: \xff\r\n\r\n", + )); + + establish_connect_tunnel(&mut client_end, "example.com", 443, None) + .await + .unwrap(); + } + /// An IPv6 literal destination must reach the proxy as `[::1]:443`, taking /// the host from the destination `Uri` exactly as the connector does. /// `Uri::host()` returns the literal already bracketed, so feeding it From 9621c81d35011e76ac5fa37bfc321f2fb630441a Mon Sep 17 00:00:00 2001 From: "shintaro-t@iij.ad.jp" Date: Sun, 9 Aug 2026 01:39:20 +0900 Subject: [PATCH 22/23] fix: preserve proxy credential bytes --- src/upstream.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/upstream.rs b/src/upstream.rs index 9021d5e0..de9e594b 100644 --- a/src/upstream.rs +++ b/src/upstream.rs @@ -444,9 +444,13 @@ pub fn redact_proxy_spec(spec: &str) -> String { /// Build a `Proxy-Authorization: Basic ...` header value from `user:pass` /// userinfo, percent-decoding each component first. fn build_basic_auth(user: &str, pass: Option<&str>) -> Result { - let user = percent_decode_str(user).decode_utf8_lossy(); - let pass = percent_decode_str(pass.unwrap_or("")).decode_utf8_lossy(); - let token = STANDARD.encode(format!("{user}:{pass}")); + let user = percent_decode_str(user).collect::>(); + let pass = percent_decode_str(pass.unwrap_or("")).collect::>(); + let mut credentials = Vec::with_capacity(user.len() + 1 + pass.len()); + credentials.extend_from_slice(&user); + credentials.push(b':'); + credentials.extend_from_slice(&pass); + let token = STANDARD.encode(credentials); HeaderValue::from_str(&format!("Basic {}", token)) .context("Invalid characters in upstream proxy credentials") } @@ -940,6 +944,13 @@ mod tests { ); } + #[test] + fn parse_credentials_preserves_non_utf8_octets() { + let p = UpstreamProxy::parse("http://%FF:%80@proxy.corp:3128").unwrap(); + // base64([0xff, b':', 0x80]) + assert_eq!(p.auth.unwrap().to_str().unwrap(), "Basic /zqA"); + } + #[test] fn redact_proxy_spec_removes_userinfo() { assert_eq!( From f2359472043fc8df60afa9c2f69436a507e89305 Mon Sep 17 00:00:00 2001 From: "shintaro-t@iij.ad.jp" Date: Sun, 9 Aug 2026 01:40:07 +0900 Subject: [PATCH 23/23] fix: mark proxy authorization sensitive --- src/upstream.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/upstream.rs b/src/upstream.rs index de9e594b..2dfcf635 100644 --- a/src/upstream.rs +++ b/src/upstream.rs @@ -451,8 +451,10 @@ fn build_basic_auth(user: &str, pass: Option<&str>) -> Result { credentials.push(b':'); credentials.extend_from_slice(&pass); let token = STANDARD.encode(credentials); - HeaderValue::from_str(&format!("Basic {}", token)) - .context("Invalid characters in upstream proxy credentials") + let mut value = HeaderValue::from_str(&format!("Basic {}", token)) + .context("Invalid characters in upstream proxy credentials")?; + value.set_sensitive(true); + Ok(value) } /// A hyper connector that routes outbound connections through an @@ -951,6 +953,13 @@ mod tests { assert_eq!(p.auth.unwrap().to_str().unwrap(), "Basic /zqA"); } + #[test] + fn proxy_credentials_are_sensitive() { + let p = UpstreamProxy::parse("http://user:secret@proxy.corp:3128").unwrap(); + assert!(p.auth.as_ref().unwrap().is_sensitive()); + assert!(!format!("{p:?}").contains("dXNlcjpzZWNyZXQ=")); + } + #[test] fn redact_proxy_spec_removes_userinfo() { assert_eq!(