Skip to content

Commit 9ed392f

Browse files
committed
fix(gl): refuse signed loopback http when a proxy carries it off-machine
1 parent 8def1ca commit 9ed392f

1 file changed

Lines changed: 141 additions & 2 deletions

File tree

‎crates/gl/src/http.rs‎

Lines changed: 141 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,75 @@ fn is_insecure_remote(url: &str) -> bool {
6363
true
6464
}
6565

66+
/// True when `no_proxy` covers `host`: a `*` entry, an exact match, or a
67+
/// domain suffix match (`example.com` covers `a.example.com`).
68+
fn no_proxy_covers(host: &str, no_proxy: &str) -> bool {
69+
no_proxy
70+
.split(',')
71+
.map(str::trim)
72+
.map(|e| e.trim_start_matches('.'))
73+
.filter(|e| !e.is_empty())
74+
.any(|e| e == "*" || host == e || host.ends_with(&format!(".{e}")))
75+
}
76+
77+
/// True when a plaintext request to a loopback `host` would still leave this
78+
/// machine. reqwest reads HTTP_PROXY/ALL_PROXY (and lowercase) at client
79+
/// build; a configured proxy without a NO_PROXY entry for the host carries the
80+
/// signed request off-machine in cleartext, which is the leak this module
81+
/// guards against.
82+
fn loopback_goes_off_machine(host: &str) -> bool {
83+
let proxied = ["HTTP_PROXY", "http_proxy", "ALL_PROXY", "all_proxy"]
84+
.iter()
85+
.any(|k| std::env::var_os(k).is_some_and(|v| !v.is_empty()));
86+
if !proxied {
87+
return false;
88+
}
89+
let no_proxy = std::env::var("NO_PROXY")
90+
.or_else(|_| std::env::var("no_proxy"))
91+
.unwrap_or_default();
92+
!no_proxy_covers(host, &no_proxy)
93+
}
94+
95+
/// The normalized host when `url` is an `http://` loopback address, else None.
96+
/// Same loopback rules as [`is_insecure_remote`].
97+
fn loopback_http_host(url: &str) -> Option<String> {
98+
let parsed = reqwest::Url::parse(url.trim()).ok()?;
99+
if parsed.scheme() != "http" {
100+
return None;
101+
}
102+
let host = parsed
103+
.host_str()?
104+
.trim_end_matches('.')
105+
.to_ascii_lowercase();
106+
if host == "localhost" {
107+
return Some(host);
108+
}
109+
let bare = host
110+
.trim_start_matches('[')
111+
.trim_end_matches(']')
112+
.to_string();
113+
let ip = bare.parse::<std::net::IpAddr>().ok()?;
114+
if ip.is_loopback() {
115+
return Some(bare);
116+
}
117+
if let std::net::IpAddr::V6(v6) = ip {
118+
if v6.to_ipv4_mapped().is_some_and(|v4| v4.is_loopback()) {
119+
return Some(bare);
120+
}
121+
}
122+
None
123+
}
124+
66125
/// Refuse to sign a request destined for a cleartext hop off this machine
67126
/// unless the operator has opted in. `GITLAWB_ALLOW_INSECURE_HTTP` exists for a
68127
/// private LAN where the operator has decided that is acceptable; its presence
69-
/// alone opts in, matching git-remote-gitlawb's guard for the same hop.
128+
/// alone opts in, matching git-remote-gitlawb's guard for the same hop. The
129+
/// loopback exemption does not apply when a proxy would carry the request
130+
/// off-machine anyway.
70131
fn ensure_signing_transport(node_base: &str, allow_insecure: bool) -> Result<()> {
71-
if allow_insecure || !is_insecure_remote(node_base) {
132+
let insecure = is_insecure_remote(node_base)
133+
|| loopback_http_host(node_base).is_some_and(|h| loopback_goes_off_machine(&h));
134+
if allow_insecure || !insecure {
72135
return Ok(());
73136
}
74137
anyhow::bail!(
@@ -1465,4 +1528,80 @@ mod tests {
14651528
assert!(!insecure_http_allowed());
14661529
}
14671530
}
1531+
1532+
#[test]
1533+
fn no_proxy_covers_matches_exact_suffix_and_star() {
1534+
assert!(no_proxy_covers("localhost", "localhost,127.0.0.1"));
1535+
assert!(no_proxy_covers("127.0.0.1", "localhost, 127.0.0.1"));
1536+
assert!(no_proxy_covers("a.example.com", ".example.com"));
1537+
assert!(no_proxy_covers("a.example.com", "example.com"));
1538+
assert!(no_proxy_covers("anything", "*"));
1539+
assert!(!no_proxy_covers("localhost", "127.0.0.1"));
1540+
assert!(!no_proxy_covers("127.0.0.1", ""));
1541+
assert!(!no_proxy_covers("notevil.com", "evil.com"));
1542+
}
1543+
1544+
/// A proxied loopback plaintext hop must be refused. The env plumbing is
1545+
/// checked in a re-execed child so the process-global proxy vars never
1546+
/// touch sibling tests' reqwest clients.
1547+
#[test]
1548+
fn proxied_loopback_http_is_refused() {
1549+
if std::env::var_os("GL_PROXY_CHILD").is_none() {
1550+
let out = std::process::Command::new(std::env::current_exe().unwrap())
1551+
.args([
1552+
"--exact",
1553+
"tests::http::tests::proxied_loopback_http_is_refused",
1554+
])
1555+
.env("GL_PROXY_CHILD", "1")
1556+
.env("HTTP_PROXY", "http://10.9.9.9:3128")
1557+
.env_remove("http_proxy")
1558+
.env_remove("ALL_PROXY")
1559+
.env_remove("all_proxy")
1560+
.env_remove("NO_PROXY")
1561+
.env_remove("no_proxy")
1562+
.env_remove("GITLAWB_ALLOW_INSECURE_HTTP")
1563+
.output()
1564+
.unwrap();
1565+
assert!(
1566+
out.status.success(),
1567+
"child failed: {}",
1568+
String::from_utf8_lossy(&out.stderr)
1569+
);
1570+
return;
1571+
}
1572+
let err =
1573+
ensure_signing_transport("http://127.0.0.1:1", insecure_http_allowed()).unwrap_err();
1574+
assert!(
1575+
err.to_string().contains("plaintext http"),
1576+
"a proxy without NO_PROXY carries the signed request off-machine: {err}"
1577+
);
1578+
}
1579+
1580+
/// And NO_PROXY covering the host restores the loopback exemption.
1581+
#[test]
1582+
fn proxied_loopback_with_no_proxy_is_allowed() {
1583+
if std::env::var_os("GL_PROXY_CHILD2").is_none() {
1584+
let out = std::process::Command::new(std::env::current_exe().unwrap())
1585+
.args([
1586+
"--exact",
1587+
"tests::http::tests::proxied_loopback_with_no_proxy_is_allowed",
1588+
])
1589+
.env("GL_PROXY_CHILD2", "1")
1590+
.env("HTTP_PROXY", "http://10.9.9.9:3128")
1591+
.env("NO_PROXY", "localhost,127.0.0.1")
1592+
.env_remove("GITLAWB_ALLOW_INSECURE_HTTP")
1593+
.output()
1594+
.unwrap();
1595+
assert!(
1596+
out.status.success(),
1597+
"child failed: {}",
1598+
String::from_utf8_lossy(&out.stderr)
1599+
);
1600+
return;
1601+
}
1602+
assert!(
1603+
ensure_signing_transport("http://127.0.0.1:1", insecure_http_allowed()).is_ok(),
1604+
"NO_PROXY covering the host keeps the request on this machine"
1605+
);
1606+
}
14681607
}

0 commit comments

Comments
 (0)