Fix SSRF allow-list bypass via URL parser differential

CHANGELOG.md

@@ -14,6 +14,10 @@ Pre-1.0 note: while `pg_durable` is in major version `0`, minor releases may inc
 
 - **Restricted HTTP transport (#342):** restricted allow-list builds now require HTTPS so credentials and request bodies cannot be sent over plaintext HTTP; development-only `http-allow-all` builds continue to permit HTTP.
 
+### Security
+
+- **HTTP allow-list URL parsing:** request URLs are now parsed once and the same canonical URL is used for validation and transport, preventing parser differences from approving a different host than the request targets.
+
 ## [0.2.6] - 2026-08-23
 
 ### Added

Cargo.toml

@@ -60,6 +60,11 @@ chrono = "0.4"
 # stays on native-tls with no rustls/ring/aws-lc in the tree.
 reqwest = { version = "0.13", default-features = false, features = ["json", "native-tls", "multipart"] }
 
+# Canonical URL parsing for SSRF validation. Semver-compatible with reqwest's own
+# url dependency, so Cargo unifies them and `url::Url` is `reqwest::Url` — the
+# validated URL and the sent URL are the same value.
+url = "2.5"
+
 # For df.http_multipart() — base64-decodes part payloads carried in the config JSON.
 base64 = "0.22"
 

docs/http-security.md

@@ -49,7 +49,7 @@ df.http() is disabled. Rebuild with the 'http-allow-azure-domains' Cargo feature
 
 Because `df.nodes` rows can be inserted by hand (bypassing the DSL), the same
 block is enforced again at execution time inside `execute_http.rs` via
-`validate_url_allowlist`.
+`validate_allowlist`.
 
 ---
 
@@ -225,6 +225,11 @@ The SSRF-safe DNS resolver (`SsrfSafeResolver`) wraps the system resolver and
 filters blocked IPs **inline** — the same IP that passes the check is the one
 used for the TCP connection.  There is no window for a rebinding attack.
 
+Restricted builds disable reqwest's system and environment proxy discovery.
+An HTTP proxy resolves the destination itself, outside `SsrfSafeResolver`, so
+inheriting `HTTP_PROXY`, `HTTPS_PROXY`, or platform proxy settings would bypass
+the destination-IP check.
+
 Only the single IP address that `reqwest` actually connects to is checked.  If
 DNS returns multiple A/AAAA records, the others are not checked because they
 are never used.  This is intentional, not a gap — checking unused addresses
@@ -234,7 +239,7 @@ would create false positives without any security benefit.
 
 Bare IP literals in URLs (e.g. `http://169.254.169.254/...`) bypass DNS
 entirely — `reqwest` connects directly without calling the resolver.
-`validate_url_allowlist` blocks all bare IPs unconditionally, so these never
+`validate_allowlist` blocks all bare IPs unconditionally, so these never
 reach the resolver.
 
 ---
@@ -285,11 +290,28 @@ Matched exactly — subdomains and lookalikes are rejected.
 
 ### 5.4 Bare IP rejection
 
-All bare IPv4 and IPv6 addresses are rejected by `validate_url_allowlist`
+All bare IPv4 and IPv6 addresses are rejected by `validate_allowlist`
 regardless of feature flag — even under `http-allow-azure-domains`.
 Because the allowlist blocks all bare IPs, there is no separate IP-literal
 check; the allowlist is the definitive gate for IP-literal URLs.
 
+### 5.4 One parse, one URL
+
+The host judged by the allow-list is read from the `url::Url` that is then
+handed to `reqwest`, never from the caller's string. Comparing a separately
+parsed host against the allow-list would let the two parsers disagree: WHATWG
+ends the authority of an `http(s)` URL at `\`, so in
+`https://evil.example\@acct.blob.core.windows.net/` the host is
+`evil.example` and the allow-listed name is merely path. A scan that read the
+name after the last `@` would approve a request aimed elsewhere — and with an
+IP literal in that position it would also clear the bare-IP rule, the only
+gate for targets that skip DNS.
+
+Judging the parsed host means the allow-list follows canonicalisation as
+`reqwest` performs it: percent-encoded host characters are decoded,
+non-dotted IPv4 notation (`http://2130706433/`) becomes an IP literal, and
+internationalised names are compared in their Punycode form.
+
 ---
 
 ## 6. Additional Hardening

scripts/test-e2e-docker.sh

@@ -34,6 +34,7 @@ SKIP_TESTS=(
     "45_connection_limit_timeout"
     "46_connection_limit_startup_validation"
     "66_new_transaction_launch_limit"
+    "67_host_guc"
     "47_http_dsl_disabled"
     "48_http_allow_all"
     # Needs the "reconcile" phase GUCs (reconcile_interval=2, retention_days=0)

src/activities/execute_http.rs

@@ -58,6 +58,10 @@ async fn check_http_privilege(pool: &PgPool, submitted_by: &str) -> Result<(), S
 /// could host a 302 redirecting to `http://169.254.169.254/...`, and reqwest
 /// would follow it without calling our DNS resolver (since the target is an IP
 /// literal).
+///
+/// Restricted builds also disable environment/system proxies. A proxy resolves
+/// the destination itself, which would bypass `SsrfSafeResolver`'s check of the
+/// address reqwest ultimately reaches.
 pub(crate) fn build_client(timeout: Duration) -> Result<reqwest::Client, String> {
     let builder = reqwest::Client::builder()
         .timeout(timeout)
@@ -70,7 +74,7 @@ pub(crate) fn build_client(timeout: Duration) -> Result<reqwest::Client, String>
         use crate::ssrf::{SsrfSafeResolver, SystemResolver};
         use std::sync::Arc;
         let resolver = SsrfSafeResolver::wrapping(Arc::new(SystemResolver));
-        builder.dns_resolver(Arc::new(resolver))
+        builder.no_proxy().dns_resolver(Arc::new(resolver))
     };
 
     builder
@@ -108,6 +112,9 @@ pub async fn execute(
     //   3. DNS resolver (SsrfSafeResolver): catches DNS rebinding — a hostname
     //                 that passes the allowlist but resolves to a private IP at
     //                 connect time.
+    //
+    // Steps 1 and 2 inspect the parsed URL that step 4 sends, so no parser
+    // differential can separate what we approve from what we request.
 
     // --- Privilege check (Layer 0): submitted_by must have EXECUTE on df.http() ---
     check_http_privilege(&pool, audit_user)
@@ -119,16 +126,23 @@ pub async fn execute(
             ));
         })?;
 
+    let request_url = crate::ssrf::parse_request_url(&config.url).inspect_err(|_| {
+        ctx.trace_info(format!(
+            "HTTP BLOCKED (malformed) url={} submitted_by={audit_user}",
+            config.url
+        ));
+    })?;
+
     // --- Scheme validation (always enforced, regardless of feature flag) ---
-    crate::ssrf::validate_url_scheme(&config.url).inspect_err(|_| {
+    crate::ssrf::validate_scheme(&request_url).inspect_err(|_| {
         ctx.trace_info(format!(
             "HTTP BLOCKED (scheme) url={} submitted_by={audit_user}",
             config.url
         ));
     })?;
 
     // --- Azure endpoint allow-list (blocks all bare IPs + non-Azure domains) ---
-    crate::ssrf::validate_url_allowlist(&config.url).inspect_err(|_| {
+    crate::ssrf::validate_allowlist(&request_url).inspect_err(|_| {
         ctx.trace_info(format!(
             "HTTP BLOCKED (allowlist) url={} submitted_by={audit_user}",
             config.url
@@ -146,11 +160,11 @@ pub async fn execute(
 
     // Build request based on method
     let mut request = match config.method.as_str() {
-        "GET" => client.get(&config.url),
-        "POST" => client.post(&config.url),
-        "PUT" => client.put(&config.url),
-        "DELETE" => client.delete(&config.url),
-        "PATCH" => client.patch(&config.url),
+        "GET" => client.get(request_url),
+        "POST" => client.post(request_url),
+        "PUT" => client.put(request_url),
+        "DELETE" => client.delete(request_url),
+        "PATCH" => client.patch(request_url),
         _ => return Err(format!("Unsupported HTTP method: {}", config.method)),
     };
 
@@ -249,3 +263,89 @@ pub async fn execute(
     // 4xx are client errors - user should handle in workflow logic
     Ok(result.to_string())
 }
+
+#[cfg(all(test, not(feature = "http-allow-all")))]
+mod tests {
+    use super::*;
+    use std::ffi::OsString;
+    use std::io::{Read, Write};
+    use std::net::TcpListener;
+    use std::sync::mpsc;
+
+    struct EnvGuard {
+        name: &'static str,
+        original: Option<OsString>,
+    }
+
+    impl EnvGuard {
+        fn set(name: &'static str, value: &str) -> Self {
+            let original = std::env::var_os(name);
+            std::env::set_var(name, value);
+            Self { name, original }
+        }
+
+        fn remove(name: &'static str) -> Self {
+            let original = std::env::var_os(name);
+            std::env::remove_var(name);
+            Self { name, original }
+        }
+    }
+
+    impl Drop for EnvGuard {
+        fn drop(&mut self) {
+            match &self.original {
+                Some(value) => std::env::set_var(self.name, value),
+                None => std::env::remove_var(self.name),
+            }
+        }
+    }
+
+    #[tokio::test]
+    async fn restricted_builds_ignore_environment_proxy() {
+        let listener = TcpListener::bind("127.0.0.1:0").unwrap();
+        listener.set_nonblocking(true).unwrap();
+        let proxy_url = format!("http://{}", listener.local_addr().unwrap());
+
+        let _http_proxy_upper = EnvGuard::set("HTTP_PROXY", &proxy_url);
+        let _http_proxy_lower = EnvGuard::set("http_proxy", &proxy_url);
+        let _no_proxy_upper = EnvGuard::remove("NO_PROXY");
+        let _no_proxy_lower = EnvGuard::remove("no_proxy");
+
+        let (stop_tx, stop_rx) = mpsc::channel();
+        let proxy_thread = std::thread::spawn(move || loop {
+            match listener.accept() {
+                Ok((mut stream, _)) => {
+                    stream
+                        .set_read_timeout(Some(Duration::from_secs(1)))
+                        .unwrap();
+                    let mut request = [0; 1024];
+                    let _ = stream.read(&mut request);
+                    stream
+                        .write_all(b"HTTP/1.1 502 Bad Gateway\r\nContent-Length: 0\r\n\r\n")
+                        .unwrap();
+                    return true;
+                }
+                Err(error) if error.kind() == std::io::ErrorKind::WouldBlock => {
+                    if stop_rx.try_recv().is_ok() {
+                        return false;
+                    }
+                    std::thread::yield_now();
+                }
+                Err(error) => panic!("proxy listener failed: {error}"),
+            }
+        });
+
+        let client = build_client(Duration::from_secs(1)).unwrap();
+        let _ = client
+            .get("http://pg-durable-proxy-test.invalid/")
+            .send()
+            .await;
+        stop_tx.send(()).unwrap();
+        let proxy_was_used = proxy_thread.join().unwrap();
+
+        assert!(
+            !proxy_was_used,
+            "restricted HTTP modes must bypass system proxies"
+        );
+    }
+}

src/activities/execute_multipart.rs

@@ -108,16 +108,23 @@ pub async fn execute(
             ));
         })?;
 
+    let request_url = crate::ssrf::parse_request_url(&config.url).inspect_err(|_| {
+        ctx.trace_info(format!(
+            "HTTP_MULTIPART BLOCKED (malformed) url={} submitted_by={audit_user}",
+            config.url
+        ));
+    })?;
+
     // --- Scheme validation (always enforced) ---
-    crate::ssrf::validate_url_scheme(&config.url).inspect_err(|_| {
+    crate::ssrf::validate_scheme(&request_url).inspect_err(|_| {
         ctx.trace_info(format!(
             "HTTP_MULTIPART BLOCKED (scheme) url={} submitted_by={audit_user}",
             config.url
         ));
     })?;
 
     // --- Azure endpoint allow-list ---
-    crate::ssrf::validate_url_allowlist(&config.url).inspect_err(|_| {
+    crate::ssrf::validate_allowlist(&request_url).inspect_err(|_| {
         ctx.trace_info(format!(
             "HTTP_MULTIPART BLOCKED (allowlist) url={} submitted_by={audit_user}",
             config.url
@@ -139,9 +146,9 @@ pub async fn execute(
     // body-carrying methods; the DSL guard restricts to POST/PUT/PATCH and we
     // defend in depth here.
     let mut request = match config.method.as_str() {
-        "POST" => client.post(&config.url),
-        "PUT" => client.put(&config.url),
-        "PATCH" => client.patch(&config.url),
+        "POST" => client.post(request_url),
+        "PUT" => client.put(request_url),
+        "PATCH" => client.patch(request_url),
         _ => {
             return Err(format!(
                 "Unsupported HTTP method for multipart: {}",

src/dsl.rs

@@ -497,7 +497,7 @@ pub fn http(
     // Skip the check when the URL contains variable placeholders ({...}) —
     // substitution happens at execution time so the scheme is not yet known.
     if !url.contains('{') {
-        if let Err(e) = crate::ssrf::validate_url_scheme(url) {
+        if let Err(e) = crate::ssrf::precheck_url_scheme(url) {
             pgrx::error!("{}", e);
         }
     }
@@ -578,7 +578,7 @@ pub fn http_multipart(
     // Validate URL scheme at DSL time (skip when URL contains variable
     // placeholders — substitution happens at execution time). Mirrors df.http.
     if !url.contains('{') {
-        if let Err(e) = crate::ssrf::validate_url_scheme(url) {
+        if let Err(e) = crate::ssrf::precheck_url_scheme(url) {
             pgrx::error!("{}", e);
         }
     }