Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions docs/bwrap-support/bubblewrap-backend.md
Original file line number Diff line number Diff line change
Expand Up @@ -260,10 +260,10 @@ Bubblewrap because it requires **no root and no `CAP_NET_ADMIN`**.
supply their own proxy via `localhost: <port>` or `url: <url>`.
2. The sandbox is then started **without** `--unshare-net` so the sandbox
shares the host network namespace and can reach the loopback proxy.
3. The command builder sets `HTTP_PROXY`, `HTTPS_PROXY`, `http_proxy`, and
`https_proxy` inside the sandbox via `bwrap --setenv` (any
caller-supplied values for these keys, including `NO_PROXY` /
`no_proxy`, are stripped before injection). The runner deliberately
3. The command builder sets `HTTP_PROXY`, `HTTPS_PROXY`, `ALL_PROXY`,
`FTP_PROXY`, and their lowercase variants inside the sandbox via
`bwrap --setenv` (caller-supplied values for these keys, including
`NO_PROXY` / `no_proxy`, are stripped before injection). The runner deliberately
does **not** set `NO_PROXY`: since the sandbox shares the host netns,
a `NO_PROXY=localhost,127.0.0.1` entry would let cooperating clients
bypass the proxy for host-loopback destinations, defeating
Expand Down
24 changes: 23 additions & 1 deletion src/backends/bubblewrap/common/src/bwrap_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ pub fn build_args(request: &ExecutionRequest, proxy_address: Option<&ProxyAddres
/// - drops `--unshare-net` (the sandbox needs to reach the loopback proxy on
/// the host's network namespace),
/// - strips any caller-supplied `HTTP_PROXY` / `HTTPS_PROXY` / `ALL_PROXY` /
/// `NO_PROXY` entries from `request.env`,
/// `FTP_PROXY` / `NO_PROXY` entries from `request.env`,
/// - emits `--setenv` for the proxy keys (all but `NO_PROXY`) pointing at the
/// proxy URL.
///
Expand Down Expand Up @@ -731,6 +731,9 @@ mod tests {
"HTTP_PROXY=http://attacker.example:9999".into(),
"https_proxy=http://attacker.example:9999".into(),
"ALL_PROXY=http://attacker.example:9999".into(),
"FTP_PROXY=http://attacker.example:9999".into(),
"ftp_proxy=http://attacker.example:9999".into(),
"NO_PROXY=*".into(),
"PATH=/usr/bin".into(),
];
let addr = ProxyAddress::new("127.0.0.1".into(), 9000);
Expand All @@ -749,6 +752,25 @@ mod tests {
// The proxy URL is the one we set, not the attacker's.
let http_pos = args.iter().position(|a| a == "HTTP_PROXY").unwrap();
assert_eq!(args[http_pos + 1], "http://127.0.0.1:9000");

// FTP variables point at the configured proxy rather than a
// caller-controlled alternative.
for key in ["FTP_PROXY", "ftp_proxy"] {
let pos = args
.iter()
.position(|arg| arg == key)
.unwrap_or_else(|| panic!("missing --setenv {key} in {args:?}"));
assert_eq!(args[pos + 1], "http://127.0.0.1:9000");
}

// Bypass variables remain absent after clearing the caller's
// environment.
for key in ["NO_PROXY", "no_proxy"] {
assert!(
!args.iter().any(|arg| arg == key),
"proxy mode must not emit caller-controlled {key}: {args:?}"
);
}
}

#[test]
Expand Down
21 changes: 18 additions & 3 deletions src/core/wxc_common/src/proxy_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
//! workload cannot pre-disable the proxy via its own `HTTP_PROXY` (or a
//! `NO_PROXY` exemption). This only sanitizes the *initial* env; the model
//! is cooperative, so a workload can still mutate its own env at runtime.
//! 2. **Set** the HTTP/HTTPS/ALL proxy keys ([`PROXY_SET_KEYS`]) to the
//! 2. **Set** the HTTP/HTTPS/ALL/FTP proxy keys ([`PROXY_SET_KEYS`]) to the
//! configured URL — never `NO_PROXY` (a host exemption list, not a target).
//!
//! `NO_PROXY` is kept out of [`PROXY_SET_KEYS`] and handled per-backend:
Expand All @@ -35,25 +35,29 @@ pub const PROXY_ENV_KEYS: &[&str] = &[
"HTTP_PROXY",
"HTTPS_PROXY",
"ALL_PROXY",
"FTP_PROXY",
"http_proxy",
"https_proxy",
"all_proxy",
"ftp_proxy",
Comment thread
SohamDas2021 marked this conversation as resolved.
"NO_PROXY",
"no_proxy",
];

/// Proxy env var keys that are actively *set* to the configured proxy URL.
///
/// The HTTP/HTTPS/ALL keys (upper- and lower-case) are set. `NO_PROXY` is
/// The HTTP/HTTPS/ALL/FTP keys (upper- and lower-case) are set. `NO_PROXY` is
/// deliberately omitted (it is a host-exemption list, not a proxy target; see
/// module docs and [`PROXY_NEUTRALIZE_KEYS`]).
pub const PROXY_SET_KEYS: &[&str] = &[
"HTTP_PROXY",
"HTTPS_PROXY",
"ALL_PROXY",
"FTP_PROXY",
"http_proxy",
"https_proxy",
"all_proxy",
"ftp_proxy",
];

/// Proxy env var keys that [`apply_cooperative_proxy_env`] sets to the *empty
Expand Down Expand Up @@ -129,7 +133,7 @@ mod tests {
use super::*;

#[test]
fn sets_all_http_https_proxy_keys_to_url() {
fn sets_all_proxy_keys_to_url() {
let env = apply_cooperative_proxy_env(&[], "http://127.0.0.1:8080");
for key in PROXY_SET_KEYS {
assert!(
Expand All @@ -139,6 +143,15 @@ mod tests {
}
}

#[test]
fn sets_ftp_proxy_to_override_image_environment() {
// WSLc merges these entries over image ENV, so emitting both spellings
// is what overrides an image-baked FTP proxy.
let env = apply_cooperative_proxy_env(&[], "http://127.0.0.1:8080");
assert!(env.contains(&"FTP_PROXY=http://127.0.0.1:8080".to_string()));
assert!(env.contains(&"ftp_proxy=http://127.0.0.1:8080".to_string()));
}

#[test]
fn sets_no_proxy_empty() {
// NO_PROXY / no_proxy are forced to the empty string (never a value),
Expand Down Expand Up @@ -166,6 +179,8 @@ mod tests {
"FOO=bar".to_string(),
"HTTP_PROXY=http://attacker.example:9999".to_string(),
"https_proxy=http://attacker.example:9999".to_string(),
"FTP_PROXY=http://attacker.example:9999".to_string(),
"ftp_proxy=http://attacker.example:9999".to_string(),
"NO_PROXY=example.com".to_string(),
"PATH=/usr/bin".to_string(),
];
Expand Down
Loading