diff --git a/crates/kingfisher-scanner/Cargo.toml b/crates/kingfisher-scanner/Cargo.toml index d7b2c3f..2a760b7 100644 --- a/crates/kingfisher-scanner/Cargo.toml +++ b/crates/kingfisher-scanner/Cargo.toml @@ -195,3 +195,4 @@ rand = { version = "0.10", optional = true } [dev-dependencies] pretty_assertions = "1.4" tempfile = "3.23" +tokio = { version = "1.48", features = ["macros", "rt"] } diff --git a/crates/kingfisher-scanner/src/validation/http_validation.rs b/crates/kingfisher-scanner/src/validation/http_validation.rs index 9071570..a083fe3 100644 --- a/crates/kingfisher-scanner/src/validation/http_validation.rs +++ b/crates/kingfisher-scanner/src/validation/http_validation.rs @@ -440,6 +440,11 @@ pub fn is_ssrf_safe_ip(ip: &IpAddr) -> bool { true } IpAddr::V6(v6) => { + // IPv4-mapped IPv6 addresses (::ffff:x.x.x.x) — apply IPv4 checks + // to prevent bypassing via e.g. ::ffff:127.0.0.1 or ::ffff:10.0.0.1 + if let Some(mapped) = v6.to_ipv4_mapped() { + return is_ssrf_safe_ip(&IpAddr::V4(mapped)); + } let segments = v6.segments(); // Unique local (fc00::/7) if segments[0] & 0xfe00 == 0xfc00 { @@ -449,6 +454,10 @@ pub fn is_ssrf_safe_ip(ip: &IpAddr) -> bool { if segments[0] & 0xffc0 == 0xfe80 { return false; } + // Documentation (2001:db8::/32) + if segments[0] == 0x2001 && segments[1] == 0x0db8 { + return false; + } true } } @@ -568,6 +577,27 @@ mod tests { assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 1)))); } + #[test] + fn rejects_ipv6_documentation() { + // 2001:db8::/32 — documentation range (RFC 3849) + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0x2001, 0x0db8, 0, 0, 0, 0, 0, 1)))); + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0x2001, 0x0db8, 0xffff, 0, 0, 0, 0, 1)))); + } + + #[test] + fn rejects_ipv4_mapped_ipv6() { + // ::ffff:127.0.0.1 — IPv4-mapped loopback + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0xffff, 0x7f00, 0x0001)))); + // ::ffff:10.0.0.1 — IPv4-mapped private + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0xffff, 0x0a00, 0x0001)))); + // ::ffff:169.254.169.254 — IPv4-mapped metadata endpoint + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0xffff, 0xa9fe, 0xa9fe)))); + // ::ffff:192.168.1.1 — IPv4-mapped private + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0xffff, 0xc0a8, 0x0101)))); + // ::ffff:8.8.8.8 — IPv4-mapped public (should be allowed) + assert!(is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0xffff, 0x0808, 0x0808)))); + } + #[test] fn accepts_public_ipv4() { assert!(is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8)))); diff --git a/crates/kingfisher-scanner/src/validation/jwt.rs b/crates/kingfisher-scanner/src/validation/jwt.rs index 3e48d1a..3322ce2 100644 --- a/crates/kingfisher-scanner/src/validation/jwt.rs +++ b/crates/kingfisher-scanner/src/validation/jwt.rs @@ -202,7 +202,7 @@ pub async fn validate_jwt_with( if !allow_internal_ips { for addr in lookup_host((jwks_host.as_str(), 443)).await? { if !is_ssrf_safe_ip(&addr.ip()) { - return Ok((false, "jwks_uri resolves to private or link-local IP".to_string())); + return Ok((false, "jwks_uri resolves to non-public or reserved IP".to_string())); } } } diff --git a/docs/USAGE.md b/docs/USAGE.md index cfe4293..72bcc55 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -1002,7 +1002,9 @@ By default, validation requests are rejected if the target hostname resolves to | `169.254.0.0/16`, `fe80::/10` | Link-local (includes cloud metadata at `169.254.169.254`) | | `100.64.0.0/10` | CGNAT / Shared Address Space | | `fc00::/7` | IPv6 unique-local | -| Multicast, broadcast, documentation, benchmarking ranges | Other reserved ranges | +| `2001:db8::/32` | IPv6 documentation (RFC 3849) | +| `::ffff:0:0/96` | IPv4-mapped IPv6 (checked against IPv4 rules) | +| Multicast, broadcast, benchmarking ranges | Other reserved ranges | HTTP redirects to IP-literal addresses in these ranges are also blocked. diff --git a/src/validation.rs b/src/validation.rs index 661c31b..d9e2f72 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -123,6 +123,16 @@ pub struct ValidationClients { } /// Build a redirect policy that blocks redirects to non-public IP addresses. +/// +/// **Known limitation:** This only validates IP-literal redirect targets +/// (e.g., `http://10.0.0.1/...`). Redirects to *hostnames* that resolve to +/// internal IPs cannot be checked here because reqwest's redirect callback is +/// synchronous and cannot perform async DNS resolution. The initial request URL +/// is always validated via `check_url_resolvable` before the request is made, +/// but a redirect chain to an attacker-controlled hostname that resolves to an +/// internal IP would bypass this check. Fully closing this gap requires +/// disabling automatic redirects and manually following them with async DNS +/// validation at each hop — a future enhancement. fn ssrf_safe_redirect_policy() -> reqwest::redirect::Policy { reqwest::redirect::Policy::custom(|attempt| { if let Some(host) = attempt.url().host_str() { @@ -135,9 +145,6 @@ fn ssrf_safe_redirect_policy() -> reqwest::redirect::Policy { )); } } - // For hostnames, we cannot do async DNS in the sync redirect - // callback. The pre-request check_url_resolvable call validates - // DNS-resolved IPs before the initial request is made. } attempt.follow() }) diff --git a/src/validation/utils.rs b/src/validation/utils.rs index 2cf6157..ca09855 100644 --- a/src/validation/utils.rs +++ b/src/validation/utils.rs @@ -1,10 +1,7 @@ -use reqwest::Url; -use tokio::net::lookup_host; - use crate::validation::SerializableCaptures; // Re-export from the scanner crate so the rest of this module can use it. -pub use kingfisher_scanner::validation::is_ssrf_safe_ip; +pub use kingfisher_scanner::validation::{check_url_resolvable, is_ssrf_safe_ip}; /// Return (NAME, value, start, end) for the captures we care about. /// @@ -109,31 +106,6 @@ pub fn find_closest_variable( best_before.or(best_overlap).or(best_after).map(|(_, value)| value) } -pub async fn check_url_resolvable( - url: &Url, - allow_internal_ips: bool, -) -> Result<(), Box> { - let host = url.host_str().ok_or("No host in URL")?; - let port = url.port().unwrap_or(if url.scheme() == "https" { 443 } else { 80 }); - let addr = format!("{}:{}", host, port); - let mut resolved_any = false; - for socket_addr in lookup_host(&addr).await? { - resolved_any = true; - if !allow_internal_ips && !is_ssrf_safe_ip(&socket_addr.ip()) { - return Err(format!( - "SSRF protection: resolved IP {} for host '{}' is not a public address. \ - Use --allow-internal-ips to permit internal addresses.", - socket_addr.ip(), - host - ) - .into()); - } - } - if !resolved_any { - return Err("Failed to resolve URL".into()); - } - Ok(()) -} // ----------------------------------------------------------------------------- // tests @@ -144,6 +116,7 @@ mod tests { use super::*; use crate::matcher::{SerializableCapture, SerializableCaptures}; use pretty_assertions::assert_eq; + use reqwest::Url; use smallvec::smallvec; #[test] diff --git a/tests/int_validation_cache.rs b/tests/int_validation_cache.rs index 5122bbf..5edad66 100644 --- a/tests/int_validation_cache.rs +++ b/tests/int_validation_cache.rs @@ -263,7 +263,7 @@ async fn test_validation_cache_and_depvars() -> Result<()> { ignore_certs: false, user_agent_suffix: None, tls_mode: TlsMode::Strict, - allow_internal_ips: false, + allow_internal_ips: true, }; let update_status = UpdateStatus::default();