From 1c7341f3ac9bcd20a813726a55f8d08b7749eee6 Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Fri, 27 Mar 2026 15:04:14 -0700 Subject: [PATCH 01/11] updated in response to ossf scorecard --- CHANGELOG.md | 3 + crates/kingfisher-scanner/Cargo.toml | 3 +- .../src/validation/http_validation.rs | 218 +++++++++++++++++- .../kingfisher-scanner/src/validation/jwt.rs | 29 +-- .../kingfisher-scanner/src/validation/mod.rs | 3 +- docs/USAGE.md | 36 +++ src/cli/global.rs | 9 + src/direct_validate.rs | 2 +- src/scanner/runner.rs | 2 +- src/validation.rs | 75 ++++-- src/validation/utils.rs | 75 +++++- tests/int_allowlist.rs | 1 + tests/int_bitbucket.rs | 1 + tests/int_dedup.rs | 1 + tests/int_github.rs | 1 + tests/int_gitlab.rs | 2 + tests/int_redact.rs | 1 + tests/int_slack.rs | 1 + tests/int_teams.rs | 1 + tests/int_validation_cache.rs | 1 + tests/int_vulnerable_files.rs | 1 + 21 files changed, 425 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d33cf4..f1b9832 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,9 @@ All notable changes to this project will be documented in this file. ## [v1.91.0] +- Added SSRF protection for credential validation: outbound HTTP requests now block connections to loopback, private, link-local, and other non-public IP addresses. Redirect targets are also validated. Use `--allow-internal-ips` to opt out when scanning internal infrastructure. +- Consolidated JWT SSRF checks to use the shared `is_ssrf_safe_ip` function, covering additional reserved ranges (CGNAT, documentation, benchmarking, IPv6 unique-local). +- Removed `ipnet` dependency from `kingfisher-scanner` (no longer needed). - Remediated current RustSec vulnerability findings by upgrading core dependencies including `gix`, `mysql_async`, `axum`, `indicatif`, `quick-xml`, and `console`. - Added `make audit-deps` to run `cargo audit` locally and report vulnerable dependencies. - Refreshed pinned GitHub Actions for `swatinem/rust-cache`, `msys2/setup-msys2`, and `ncipollo/release-action`, and configured Dependabot to ignore selected GitHub Action major-version bumps. diff --git a/crates/kingfisher-scanner/Cargo.toml b/crates/kingfisher-scanner/Cargo.toml index 2966d6e..d7b2c3f 100644 --- a/crates/kingfisher-scanner/Cargo.toml +++ b/crates/kingfisher-scanner/Cargo.toml @@ -74,7 +74,6 @@ validation-gcp = [ validation-jwt = [ "validation-http", "dep:chrono", - "dep:ipnet", "dep:jsonwebtoken", "dep:tokio", ] @@ -166,7 +165,7 @@ sha2 = { workspace = true, optional = true } pem = { version = "3.0.6", optional = true } percent-encoding = { workspace = true, optional = true } ring = { version = "0.17", optional = true } -ipnet = { version = "2.11", optional = true } + jsonwebtoken = { version = "10.2.0", features = ["aws-lc-rs"], optional = true } p256 = { version = "0.13.2", optional = true } ed25519-dalek = { version = "2.2", features = ["pkcs8"], optional = true } diff --git a/crates/kingfisher-scanner/src/validation/http_validation.rs b/crates/kingfisher-scanner/src/validation/http_validation.rs index 52b3ab8..9071570 100644 --- a/crates/kingfisher-scanner/src/validation/http_validation.rs +++ b/crates/kingfisher-scanner/src/validation/http_validation.rs @@ -1,4 +1,4 @@ -use std::{collections::BTreeMap, future::Future, str::FromStr, time::Duration}; +use std::{collections::BTreeMap, future::Future, net::IpAddr, str::FromStr, time::Duration}; use anyhow::{anyhow, Error, Result}; use http::StatusCode; @@ -392,10 +392,220 @@ pub fn validate_response( word_ok && status_ok && header_ok && json_ok && xml_ok && html_ok } -/// Check if a URL can be resolved via DNS. -pub async fn check_url_resolvable(url: &Url) -> Result<(), Box> { +/// Returns `true` if the IP address is safe for outbound validation requests +/// (i.e., it is a publicly routable address, not internal/reserved). +pub fn is_ssrf_safe_ip(ip: &IpAddr) -> bool { + if ip.is_loopback() || ip.is_unspecified() || ip.is_multicast() { + return false; + } + match ip { + IpAddr::V4(v4) => { + let octets = v4.octets(); + // Private ranges (RFC 1918) + if octets[0] == 10 { + return false; + } + if octets[0] == 172 && (16..=31).contains(&octets[1]) { + return false; + } + if octets[0] == 192 && octets[1] == 168 { + return false; + } + // Link-local (169.254.0.0/16) — includes AWS metadata 169.254.169.254 + if octets[0] == 169 && octets[1] == 254 { + return false; + } + // CGNAT / Shared Address Space (100.64.0.0/10) + if octets[0] == 100 && (64..=127).contains(&octets[1]) { + return false; + } + // Documentation ranges (RFC 5737) + if octets[0] == 192 && octets[1] == 0 && octets[2] == 2 { + return false; + } + if octets[0] == 198 && octets[1] == 51 && octets[2] == 100 { + return false; + } + if octets[0] == 203 && octets[1] == 0 && octets[2] == 113 { + return false; + } + // Benchmarking (198.18.0.0/15) + if octets[0] == 198 && (18..=19).contains(&octets[1]) { + return false; + } + // Broadcast + if octets == [255, 255, 255, 255] { + return false; + } + true + } + IpAddr::V6(v6) => { + let segments = v6.segments(); + // Unique local (fc00::/7) + if segments[0] & 0xfe00 == 0xfc00 { + return false; + } + // Link-local (fe80::/10) + if segments[0] & 0xffc0 == 0xfe80 { + return false; + } + true + } + } +} + +/// Check if a URL can be resolved via DNS, with SSRF protection against +/// internal/private IP addresses. +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); - lookup_host(addr).await?.next().ok_or_else(|| "Failed to resolve URL".into()).map(|_| ()) + 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(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; + + #[test] + fn rejects_ipv4_loopback() { + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)))); + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(127, 255, 255, 255)))); + } + + #[test] + fn rejects_ipv4_unspecified() { + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::UNSPECIFIED))); + } + + #[test] + fn rejects_ipv4_private_rfc1918() { + // 10.0.0.0/8 + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1)))); + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(10, 255, 255, 255)))); + // 172.16.0.0/12 + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(172, 16, 0, 1)))); + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(172, 31, 255, 255)))); + // 192.168.0.0/16 + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(192, 168, 0, 1)))); + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(192, 168, 255, 255)))); + } + + #[test] + fn rejects_link_local_and_metadata() { + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(169, 254, 0, 1)))); + // AWS/GCP/Azure metadata endpoint + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(169, 254, 169, 254)))); + } + + #[test] + fn rejects_cgnat() { + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(100, 64, 0, 1)))); + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(100, 127, 255, 255)))); + } + + #[test] + fn rejects_documentation_ranges() { + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(192, 0, 2, 1)))); + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(198, 51, 100, 1)))); + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(203, 0, 113, 1)))); + } + + #[test] + fn rejects_benchmarking() { + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(198, 18, 0, 1)))); + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(198, 19, 255, 255)))); + } + + #[test] + fn rejects_broadcast() { + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::BROADCAST))); + } + + #[test] + fn rejects_multicast() { + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(224, 0, 0, 1)))); + } + + #[test] + fn rejects_ipv6_loopback() { + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::LOCALHOST))); + } + + #[test] + fn rejects_ipv6_unspecified() { + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::UNSPECIFIED))); + } + + #[test] + fn rejects_ipv6_unique_local() { + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0xfc00, 0, 0, 0, 0, 0, 0, 1)))); + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 1)))); + } + + #[test] + fn rejects_ipv6_link_local() { + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 1)))); + } + + #[test] + fn accepts_public_ipv4() { + assert!(is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8)))); + assert!(is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(1, 1, 1, 1)))); + assert!(is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(93, 184, 216, 34)))); + } + + #[test] + fn accepts_public_ipv6() { + assert!(is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0x2606, 0x4700, 0, 0, 0, 0, 0, 0x1111)))); + } + + #[test] + fn accepts_edge_cases_outside_private_ranges() { + // 172.15.x.x is NOT private (private is 172.16-31) + assert!(is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(172, 15, 255, 255)))); + // 172.32.x.x is NOT private + assert!(is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(172, 32, 0, 1)))); + // 100.63.x.x is NOT CGNAT (CGNAT is 100.64-127) + assert!(is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(100, 63, 255, 255)))); + // 100.128.x.x is NOT CGNAT + assert!(is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(100, 128, 0, 1)))); + } + + #[tokio::test] + async fn check_url_resolvable_rejects_localhost() { + let url = Url::parse("https://localhost/test").unwrap(); + let result = check_url_resolvable(&url, false).await; + assert!(result.is_err()); + let err = result.unwrap_err().to_string(); + assert!(err.contains("SSRF protection"), "expected SSRF error, got: {}", err); + } + + #[tokio::test] + async fn check_url_resolvable_allows_localhost_when_permitted() { + let url = Url::parse("https://localhost/test").unwrap(); + // With allow_internal_ips=true, localhost should resolve successfully + let result = check_url_resolvable(&url, true).await; + assert!(result.is_ok(), "expected Ok with allow_internal_ips=true, got: {:?}", result); + } } diff --git a/crates/kingfisher-scanner/src/validation/jwt.rs b/crates/kingfisher-scanner/src/validation/jwt.rs index 455ef1e..3e48d1a 100644 --- a/crates/kingfisher-scanner/src/validation/jwt.rs +++ b/crates/kingfisher-scanner/src/validation/jwt.rs @@ -1,7 +1,6 @@ use anyhow::{anyhow, Result}; use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _}; use chrono::Utc; -use ipnet::IpNet; use jsonwebtoken::{ decode, decode_header, jwk::JwkSet, Algorithm, DecodingKey, Validation as JwtValidation, }; @@ -10,7 +9,7 @@ use reqwest::{redirect::Policy, Client, Url}; use serde::Deserialize; use tokio::net::lookup_host; -use super::http_validation::check_url_resolvable; +use super::http_validation::{check_url_resolvable, is_ssrf_safe_ip}; /// Global redirect-free client with strict TLS validation. static STRICT_CLIENT: Lazy = Lazy::new(|| { @@ -38,9 +37,6 @@ fn get_client(lax_tls: bool) -> &'static Client { } } -/// RFC 1918 + loopback + link-local nets we refuse to contact. -const BLOCKED_NETS: &[&str] = - &["10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", "127.0.0.0/8", "169.254.0.0/16"]; #[derive(Debug, Deserialize)] #[serde(untagged)] @@ -66,11 +62,16 @@ pub struct ValidateOptions { } /// Backwards-compatible entry point with secure defaults. -pub async fn validate_jwt(token: &str, lax_tls: bool) -> Result<(bool, String)> { +pub async fn validate_jwt( + token: &str, + lax_tls: bool, + allow_internal_ips: bool, +) -> Result<(bool, String)> { validate_jwt_with( token, &ValidateOptions { allow_alg_none: false, fallback_decoding_key: None }, lax_tls, + allow_internal_ips, ) .await } @@ -80,6 +81,7 @@ pub async fn validate_jwt_with( token: &str, opts: &ValidateOptions, lax_tls: bool, + allow_internal_ips: bool, ) -> Result<(bool, String)> { let client = get_client(lax_tls); let claims: Claims = { @@ -197,13 +199,17 @@ pub async fn validate_jwt_with( )); } - for addr in lookup_host((jwks_host.as_str(), 443)).await? { - if is_blocked_ip(addr.ip()) { - return Ok((false, "jwks_uri resolves to private or link-local IP".to_string())); + 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())); + } } } - check_url_resolvable(&url).await.map_err(|e| anyhow!("jwks uri unresolvable: {e}"))?; + check_url_resolvable(&url, allow_internal_ips) + .await + .map_err(|e| anyhow!("jwks uri unresolvable: {e}"))?; let jwks_resp = client.get(url).send().await.map_err(|e| anyhow!("jwks fetch failed: {e}"))?; if !jwks_resp.status().is_success() { @@ -240,9 +246,6 @@ fn extract_aud_strings(claims: &Claims) -> Vec { } } -fn is_blocked_ip(ip: std::net::IpAddr) -> bool { - BLOCKED_NETS.iter().filter_map(|cidr| cidr.parse::().ok()).any(|net| net.contains(&ip)) -} fn normalize_issuer_url(issuer: &str) -> Result { let trimmed = issuer.trim(); diff --git a/crates/kingfisher-scanner/src/validation/mod.rs b/crates/kingfisher-scanner/src/validation/mod.rs index 2da8631..82addc0 100644 --- a/crates/kingfisher-scanner/src/validation/mod.rs +++ b/crates/kingfisher-scanner/src/validation/mod.rs @@ -60,7 +60,8 @@ pub use validation_body::{as_str, clone_as_string, from_string, ValidationRespon #[cfg(feature = "validation-http")] pub use http_validation::{ - build_request_builder, check_url_resolvable, generate_http_cache_key_parts, parse_http_method, + build_request_builder, check_url_resolvable, generate_http_cache_key_parts, is_ssrf_safe_ip, + parse_http_method, process_headers, retry_multipart_request, retry_request, validate_response, }; diff --git a/docs/USAGE.md b/docs/USAGE.md index 3514df4..cfe4293 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -986,6 +986,42 @@ The legacy `--ignore-certs` flag is still supported as an alias for `--tls-mode= --- +## SSRF Protection + +Kingfisher makes outbound HTTP requests during credential validation, with URLs sometimes constructed from user-controlled data found in scanned content (e.g., domain names extracted alongside API keys). To prevent Server-Side Request Forgery (SSRF), Kingfisher blocks validation requests that would connect to non-public IP addresses. + +### What is blocked + +By default, validation requests are rejected if the target hostname resolves to any of these address ranges: + +| Range | Description | +| ----- | ----------- | +| `127.0.0.0/8`, `::1` | Loopback (localhost) | +| `0.0.0.0`, `::` | Unspecified | +| `10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16` | Private networks (RFC 1918) | +| `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 | + +HTTP redirects to IP-literal addresses in these ranges are also blocked. + +### `--allow-internal-ips` + +If you are scanning infrastructure that uses internal endpoints for credential validation (e.g., self-hosted GitLab, Artifactory, or Vault behind a private network), use `--allow-internal-ips` to disable SSRF protections: + +```bash +# Scan with SSRF protection disabled (allows requests to internal IPs) +kingfisher scan --allow-internal-ips ./repo + +# Also works with validate and revoke commands +kingfisher validate --allow-internal-ips --rule kingfisher.artifactory.1 +``` + +> **Warning:** Only use `--allow-internal-ips` when you trust the content being scanned. Malicious content could cause Kingfisher to make requests to internal services. + +--- + ## Understanding the Scan Summary After each scan, Kingfisher displays a summary with validation statistics: diff --git a/src/cli/global.rs b/src/cli/global.rs index c68e172..36d9237 100644 --- a/src/cli/global.rs +++ b/src/cli/global.rs @@ -119,6 +119,14 @@ pub struct GlobalArgs { #[arg(global = true, long, value_enum, default_value = "strict")] pub tls_mode: TlsMode, + /// Allow validation requests to internal/private IP addresses. + /// + /// By default, Kingfisher blocks HTTP requests to loopback, private, + /// and link-local addresses during credential validation to prevent SSRF. + /// Use this flag when scanning infrastructure that uses internal endpoints. + #[arg(global = true, long = "allow-internal-ips", default_value_t = false)] + pub allow_internal_ips: bool, + /// Disable TLS certificate validation (deprecated: use --tls-mode=off) #[arg(global = true, long, hide = true)] pub ignore_certs: bool, @@ -149,6 +157,7 @@ impl Default for GlobalArgs { verbose: 0, quiet: false, tls_mode: TlsMode::Strict, + allow_internal_ips: false, ignore_certs: false, self_update: false, no_update_check: false, diff --git a/src/direct_validate.rs b/src/direct_validate.rs index d957447..87caf06 100644 --- a/src/direct_validate.rs +++ b/src/direct_validate.rs @@ -727,7 +727,7 @@ pub async fn run_direct_validation( Validation::JWT => { // JWT expects a JWT token as the secret - match validate_jwt(&secret, use_lax_tls).await { + match validate_jwt(&secret, use_lax_tls, global_args.allow_internal_ips).await { Ok((is_valid, message)) => DirectValidationResult { rule_id: String::new(), rule_name: String::new(), diff --git a/src/scanner/runner.rs b/src/scanner/runner.rs index ffd8c3c..f21eefc 100644 --- a/src/scanner/runner.rs +++ b/src/scanner/runner.rs @@ -164,7 +164,7 @@ pub async fn run_async_scan( info!("Starting secret validation phase..."); Some(Arc::new(( register_all(liquid::ParserBuilder::with_stdlib()).build()?, - crate::validation::ValidationClients::new(global_args.tls_mode)?, + crate::validation::ValidationClients::new(global_args.tls_mode, global_args.allow_internal_ips)?, Arc::new(SkipMap::new()), validation_rate_limiter.clone(), ))) diff --git a/src/validation.rs b/src/validation.rs index f2e805b..661c31b 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -118,19 +118,57 @@ pub struct ValidationClients { lax: Client, /// The global TLS mode from CLI arguments. pub global_mode: TlsMode, + /// When true, skip SSRF IP validation and allow requests to internal/private addresses. + pub allow_internal_ips: bool, +} + +/// Build a redirect policy that blocks redirects to non-public IP addresses. +fn ssrf_safe_redirect_policy() -> reqwest::redirect::Policy { + reqwest::redirect::Policy::custom(|attempt| { + if let Some(host) = attempt.url().host_str() { + // For IP-literal hosts, check directly without DNS. + if let Ok(ip) = host.parse::() { + if !kingfisher_scanner::validation::is_ssrf_safe_ip(&ip) { + return attempt.error(format!( + "SSRF protection: redirect to non-public IP {} blocked", + ip + )); + } + } + // 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() + }) } impl ValidationClients { /// Create validation clients based on the global TLS mode. - pub fn new(global_mode: TlsMode) -> anyhow::Result { + pub fn new(global_mode: TlsMode, allow_internal_ips: bool) -> anyhow::Result { let timeout = std::time::Duration::from_secs(30); - let strict = - Client::builder().danger_accept_invalid_certs(false).timeout(timeout).build()?; + let strict = Client::builder() + .danger_accept_invalid_certs(false) + .redirect(if allow_internal_ips { + reqwest::redirect::Policy::default() + } else { + ssrf_safe_redirect_policy() + }) + .timeout(timeout) + .build()?; - let lax = Client::builder().danger_accept_invalid_certs(true).timeout(timeout).build()?; + let lax = Client::builder() + .danger_accept_invalid_certs(true) + .redirect(if allow_internal_ips { + reqwest::redirect::Policy::default() + } else { + ssrf_safe_redirect_policy() + }) + .timeout(timeout) + .build()?; - Ok(Self { strict, lax, global_mode }) + Ok(Self { strict, lax, global_mode, allow_internal_ips }) } /// Get the appropriate client for a given rule's TLS mode. @@ -288,6 +326,7 @@ async fn render_and_parse_url( globals: &liquid::Object, rule_name: &str, template_url: &str, + allow_internal_ips: bool, ) -> Result { let rendered_url_str = render_template(parser, globals, rule_name, template_url).await.map_err(|e| { @@ -302,8 +341,8 @@ async fn render_and_parse_url( error_msg })?; - // Check if the URL is resolvable. - utils::check_url_resolvable(&url).await.map_err(|e| { + // Check if the URL is resolvable (with SSRF protection). + utils::check_url_resolvable(&url, allow_internal_ips).await.map_err(|e| { let error_msg = format!("URL <{}> resolution failed: {}", &url, e); error_msg })?; @@ -534,6 +573,7 @@ async fn timed_validate_single_match<'a>( &globals, &rule_syntax.name, &http_validation.request.url, + clients.allow_internal_ips, ) .await { @@ -792,6 +832,7 @@ async fn timed_validate_single_match<'a>( &globals, &rule_syntax.name, &grpc_validation_cfg.request.url, + clients.allow_internal_ips, ) .await { @@ -1168,7 +1209,7 @@ async fn timed_validate_single_match<'a>( return; } - match jwt::validate_jwt(&token, use_lax_tls).await { + match jwt::validate_jwt(&token, use_lax_tls, clients.allow_internal_ips).await { Ok((ok, msg)) => { m.validation_success = ok; m.validation_response_body = validation_body::from_string(msg); @@ -1494,19 +1535,19 @@ mod tests { #[test] fn validation_clients_new_creates_both_clients() { - let clients = ValidationClients::new(TlsMode::Strict).unwrap(); + let clients = ValidationClients::new(TlsMode::Strict, false).unwrap(); assert_eq!(clients.global_mode, TlsMode::Strict); - let clients_lax = ValidationClients::new(TlsMode::Lax).unwrap(); + let clients_lax = ValidationClients::new(TlsMode::Lax, false).unwrap(); assert_eq!(clients_lax.global_mode, TlsMode::Lax); - let clients_off = ValidationClients::new(TlsMode::Off).unwrap(); + let clients_off = ValidationClients::new(TlsMode::Off, false).unwrap(); assert_eq!(clients_off.global_mode, TlsMode::Off); } #[test] fn client_for_rule_strict_mode_always_returns_strict_client() { - let clients = ValidationClients::new(TlsMode::Strict).unwrap(); + let clients = ValidationClients::new(TlsMode::Strict, false).unwrap(); // With no rule TLS mode let client1 = clients.client_for_rule(None); @@ -1522,7 +1563,7 @@ mod tests { #[test] fn client_for_rule_off_mode_always_returns_lax_client() { - let clients = ValidationClients::new(TlsMode::Off).unwrap(); + let clients = ValidationClients::new(TlsMode::Off, false).unwrap(); // With no rule TLS mode let client1 = clients.client_for_rule(None); @@ -1538,7 +1579,7 @@ mod tests { #[test] fn client_for_rule_lax_mode_respects_rule_preference() { - let clients = ValidationClients::new(TlsMode::Lax).unwrap(); + let clients = ValidationClients::new(TlsMode::Lax, false).unwrap(); // Get references to understand which is which let strict_client = clients.client_for_rule(None); @@ -1565,7 +1606,7 @@ mod tests { #[test] fn should_use_lax_off_mode_always_returns_true() { - let clients = ValidationClients::new(TlsMode::Off).unwrap(); + let clients = ValidationClients::new(TlsMode::Off, false).unwrap(); assert!(clients.should_use_lax(None)); assert!(clients.should_use_lax(Some(kingfisher_rules::TlsMode::Strict))); @@ -1574,7 +1615,7 @@ mod tests { #[test] fn should_use_lax_strict_mode_always_returns_false() { - let clients = ValidationClients::new(TlsMode::Strict).unwrap(); + let clients = ValidationClients::new(TlsMode::Strict, false).unwrap(); assert!(!clients.should_use_lax(None)); assert!(!clients.should_use_lax(Some(kingfisher_rules::TlsMode::Strict))); @@ -1583,7 +1624,7 @@ mod tests { #[test] fn should_use_lax_lax_mode_respects_rule_preference() { - let clients = ValidationClients::new(TlsMode::Lax).unwrap(); + let clients = ValidationClients::new(TlsMode::Lax, false).unwrap(); // Only true when rule explicitly opts in assert!(!clients.should_use_lax(None)); diff --git a/src/validation/utils.rs b/src/validation/utils.rs index 1031436..2cf6157 100644 --- a/src/validation/utils.rs +++ b/src/validation/utils.rs @@ -3,6 +3,9 @@ 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; + /// Return (NAME, value, start, end) for the captures we care about. /// /// * Named captures keep their (upper-cased) name @@ -106,11 +109,30 @@ pub fn find_closest_variable( best_before.or(best_overlap).or(best_after).map(|(_, value)| value) } -pub async fn check_url_resolvable(url: &Url) -> Result<(), Box> { +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); - lookup_host(addr).await?.next().ok_or_else(|| "Failed to resolve URL".into()).map(|_| ()) + 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(()) } // ----------------------------------------------------------------------------- @@ -246,4 +268,53 @@ mod tests { assert_eq!(result, "after".to_string()); } + + // ---- SSRF IP validation tests ---- + + #[test] + fn ssrf_rejects_loopback() { + assert!(!is_ssrf_safe_ip(&"127.0.0.1".parse().unwrap())); + assert!(!is_ssrf_safe_ip(&"::1".parse().unwrap())); + } + + #[test] + fn ssrf_rejects_unspecified() { + assert!(!is_ssrf_safe_ip(&"0.0.0.0".parse().unwrap())); + assert!(!is_ssrf_safe_ip(&"::".parse().unwrap())); + } + + #[test] + fn ssrf_rejects_private_ranges() { + assert!(!is_ssrf_safe_ip(&"10.0.0.1".parse().unwrap())); + assert!(!is_ssrf_safe_ip(&"172.16.0.1".parse().unwrap())); + assert!(!is_ssrf_safe_ip(&"192.168.1.1".parse().unwrap())); + } + + #[test] + fn ssrf_rejects_link_local_and_metadata() { + assert!(!is_ssrf_safe_ip(&"169.254.169.254".parse().unwrap())); + assert!(!is_ssrf_safe_ip(&"169.254.1.1".parse().unwrap())); + } + + #[test] + fn ssrf_accepts_public_ips() { + assert!(is_ssrf_safe_ip(&"8.8.8.8".parse().unwrap())); + assert!(is_ssrf_safe_ip(&"1.1.1.1".parse().unwrap())); + assert!(is_ssrf_safe_ip(&"2606:4700::1111".parse().unwrap())); + } + + #[tokio::test] + async fn check_url_resolvable_blocks_localhost() { + let url = Url::parse("https://localhost/path").unwrap(); + let result = check_url_resolvable(&url, false).await; + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("SSRF protection")); + } + + #[tokio::test] + async fn check_url_resolvable_allows_localhost_when_opted_in() { + let url = Url::parse("https://localhost/path").unwrap(); + let result = check_url_resolvable(&url, true).await; + assert!(result.is_ok()); + } } diff --git a/tests/int_allowlist.rs b/tests/int_allowlist.rs index 96a8b6a..f40ce8e 100644 --- a/tests/int_allowlist.rs +++ b/tests/int_allowlist.rs @@ -197,6 +197,7 @@ fn run_skiplist(skip_regex: Vec, skip_skipword: Vec) -> Result Result<()> { ignore_certs: false, user_agent_suffix: None, tls_mode: TlsMode::Strict, + allow_internal_ips: false, }; let datastore = Arc::new(Mutex::new(FindingsStore::new(clone_dir))); diff --git a/tests/int_dedup.rs b/tests/int_dedup.rs index b47a31b..48ef032 100644 --- a/tests/int_dedup.rs +++ b/tests/int_dedup.rs @@ -202,6 +202,7 @@ rules: ignore_certs: false, user_agent_suffix: None, tls_mode: TlsMode::Strict, + allow_internal_ips: false, }; // ── load rules once ───────────────────────────────────────────── diff --git a/tests/int_github.rs b/tests/int_github.rs index e9ae3fa..90f7883 100644 --- a/tests/int_github.rs +++ b/tests/int_github.rs @@ -189,6 +189,7 @@ fn test_github_remote_scan() -> Result<()> { ignore_certs: false, user_agent_suffix: None, tls_mode: TlsMode::Strict, + allow_internal_ips: false, }; // Create in-memory datastore let datastore = Arc::new(Mutex::new(FindingsStore::new(clone_dir))); diff --git a/tests/int_gitlab.rs b/tests/int_gitlab.rs index c9cbf12..19e3f46 100644 --- a/tests/int_gitlab.rs +++ b/tests/int_gitlab.rs @@ -187,6 +187,7 @@ fn test_gitlab_remote_scan() -> Result<()> { ignore_certs: false, user_agent_suffix: None, tls_mode: TlsMode::Strict, + allow_internal_ips: false, }; let datastore = Arc::new(Mutex::new(FindingsStore::new(clone_dir))); @@ -362,6 +363,7 @@ fn test_gitlab_remote_scan_no_history() -> Result<()> { ignore_certs: false, user_agent_suffix: None, tls_mode: TlsMode::Strict, + allow_internal_ips: false, }; let datastore = Arc::new(Mutex::new(FindingsStore::new(clone_dir))); diff --git a/tests/int_redact.rs b/tests/int_redact.rs index a68ad2c..c69cb14 100644 --- a/tests/int_redact.rs +++ b/tests/int_redact.rs @@ -165,6 +165,7 @@ async fn test_redact_hashes_finding_values() -> Result<()> { ignore_certs: false, user_agent_suffix: None, tls_mode: TlsMode::Strict, + allow_internal_ips: false, }; let loaded = RuleLoader::from_rule_specifiers(&scan_args.rules).load(&scan_args)?; diff --git a/tests/int_slack.rs b/tests/int_slack.rs index e162269..45eedd6 100644 --- a/tests/int_slack.rs +++ b/tests/int_slack.rs @@ -331,6 +331,7 @@ async fn test_scan_slack_messages() -> Result<()> { ignore_certs: false, user_agent_suffix: None, tls_mode: TlsMode::Strict, + allow_internal_ips: false, }; let datastore = Arc::new(Mutex::new(FindingsStore::new(clone_dir))); diff --git a/tests/int_teams.rs b/tests/int_teams.rs index 3d78dfa..49a136c 100644 --- a/tests/int_teams.rs +++ b/tests/int_teams.rs @@ -205,6 +205,7 @@ async fn test_scan_teams_messages() -> Result<()> { ignore_certs: false, user_agent_suffix: None, tls_mode: TlsMode::Strict, + allow_internal_ips: false, }; let loaded = RuleLoader::from_rule_specifiers(&scan_args.rules).load(&scan_args)?; diff --git a/tests/int_validation_cache.rs b/tests/int_validation_cache.rs index c91a348..5122bbf 100644 --- a/tests/int_validation_cache.rs +++ b/tests/int_validation_cache.rs @@ -263,6 +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, }; let update_status = UpdateStatus::default(); diff --git a/tests/int_vulnerable_files.rs b/tests/int_vulnerable_files.rs index e81d8b6..72ca410 100644 --- a/tests/int_vulnerable_files.rs +++ b/tests/int_vulnerable_files.rs @@ -336,6 +336,7 @@ impl TestContext { ignore_certs: false, user_agent_suffix: None, tls_mode: TlsMode::Strict, + allow_internal_ips: false, }; let datastore = Arc::new(Mutex::new(FindingsStore::new(clone_dir))); From 411aeefa92740f59fe4bf78049a5ec24cbef0001 Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Fri, 27 Mar 2026 17:22:21 -0700 Subject: [PATCH 02/11] updated in response to ossf scorecard --- crates/kingfisher-scanner/Cargo.toml | 1 + .../src/validation/http_validation.rs | 30 ++++++++++++++++++ .../kingfisher-scanner/src/validation/jwt.rs | 2 +- docs/USAGE.md | 4 ++- src/validation.rs | 13 ++++++-- src/validation/utils.rs | 31 ++----------------- tests/int_validation_cache.rs | 2 +- 7 files changed, 48 insertions(+), 35 deletions(-) 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(); From 051e4ffdd274707ff2c60acd7a421e78e7c466bf Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Fri, 27 Mar 2026 21:08:39 -0700 Subject: [PATCH 03/11] updated in response to ossf scorecard --- CHANGELOG.md | 2 +- .../src/validation/http_validation.rs | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1b9832..2b5221a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ All notable changes to this project will be documented in this file. ## [v1.91.0] -- Added SSRF protection for credential validation: outbound HTTP requests now block connections to loopback, private, link-local, and other non-public IP addresses. Redirect targets are also validated. Use `--allow-internal-ips` to opt out when scanning internal infrastructure. +- Added SSRF protection for credential validation: outbound HTTP requests now block connections to loopback, private, link-local, and other non-public IP addresses. Redirects to non-public IP-literal addresses are also blocked. Use `--allow-internal-ips` to opt out when scanning internal infrastructure. - Consolidated JWT SSRF checks to use the shared `is_ssrf_safe_ip` function, covering additional reserved ranges (CGNAT, documentation, benchmarking, IPv6 unique-local). - Removed `ipnet` dependency from `kingfisher-scanner` (no longer needed). - Remediated current RustSec vulnerability findings by upgrading core dependencies including `gix`, `mysql_async`, `axum`, `indicatif`, `quick-xml`, and `console`. diff --git a/crates/kingfisher-scanner/src/validation/http_validation.rs b/crates/kingfisher-scanner/src/validation/http_validation.rs index a083fe3..40cbae0 100644 --- a/crates/kingfisher-scanner/src/validation/http_validation.rs +++ b/crates/kingfisher-scanner/src/validation/http_validation.rs @@ -401,6 +401,10 @@ pub fn is_ssrf_safe_ip(ip: &IpAddr) -> bool { match ip { IpAddr::V4(v4) => { let octets = v4.octets(); + // 0.0.0.0/8 — "This host on this network" (RFC 1122); not routable + if octets[0] == 0 { + return false; + } // Private ranges (RFC 1918) if octets[0] == 10 { return false; @@ -446,6 +450,12 @@ pub fn is_ssrf_safe_ip(ip: &IpAddr) -> bool { return is_ssrf_safe_ip(&IpAddr::V4(mapped)); } let segments = v6.segments(); + // IPv4-compatible IPv6 addresses (::/96, e.g., ::127.0.0.1) are + // deprecated (RFC 4291 §2.5.5.1) and can bypass IPv4-only checks. + // Reject the entire ::/96 range. + if segments[..6].iter().all(|&s| s == 0) { + return false; + } // Unique local (fc00::/7) if segments[0] & 0xfe00 == 0xfc00 { return false; @@ -507,6 +517,13 @@ mod tests { assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::UNSPECIFIED))); } + #[test] + fn rejects_ipv4_this_network() { + // 0.0.0.0/8 — "This host on this network" (RFC 1122) + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(0, 0, 0, 1)))); + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(0, 255, 255, 255)))); + } + #[test] fn rejects_ipv4_private_rfc1918() { // 10.0.0.0/8 @@ -598,6 +615,16 @@ mod tests { assert!(is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0xffff, 0x0808, 0x0808)))); } + #[test] + fn rejects_ipv4_compatible_ipv6() { + // ::127.0.0.1 — deprecated IPv4-compatible IPv6 (loopback) + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0x7f00, 0x0001)))); + // ::10.0.0.1 — deprecated IPv4-compatible IPv6 (private) + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0x0a00, 0x0001)))); + // ::8.8.8.8 — even public IPv4 in ::/96 is rejected (deprecated range) + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0x0808, 0x0808)))); + } + #[test] fn accepts_public_ipv4() { assert!(is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8)))); From 9c8c63db909066d630abfd5aa51aa965511075d6 Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Fri, 27 Mar 2026 21:08:52 -0700 Subject: [PATCH 04/11] updated in response to ossf scorecard --- .../src/validation/http_validation.rs | 20 ++++++++++++++----- .../kingfisher-scanner/src/validation/jwt.rs | 2 -- .../kingfisher-scanner/src/validation/mod.rs | 3 +-- src/scanner/runner.rs | 5 ++++- src/validation/utils.rs | 1 - 5 files changed, 20 insertions(+), 11 deletions(-) diff --git a/crates/kingfisher-scanner/src/validation/http_validation.rs b/crates/kingfisher-scanner/src/validation/http_validation.rs index 40cbae0..f5af606 100644 --- a/crates/kingfisher-scanner/src/validation/http_validation.rs +++ b/crates/kingfisher-scanner/src/validation/http_validation.rs @@ -598,19 +598,29 @@ mod tests { 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)))); + 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)))); + 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)))); + 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)))); + 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)))); + 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)))); } diff --git a/crates/kingfisher-scanner/src/validation/jwt.rs b/crates/kingfisher-scanner/src/validation/jwt.rs index 3322ce2..5d5d35b 100644 --- a/crates/kingfisher-scanner/src/validation/jwt.rs +++ b/crates/kingfisher-scanner/src/validation/jwt.rs @@ -37,7 +37,6 @@ fn get_client(lax_tls: bool) -> &'static Client { } } - #[derive(Debug, Deserialize)] #[serde(untagged)] enum Aud { @@ -246,7 +245,6 @@ fn extract_aud_strings(claims: &Claims) -> Vec { } } - fn normalize_issuer_url(issuer: &str) -> Result { let trimmed = issuer.trim(); if trimmed.is_empty() { diff --git a/crates/kingfisher-scanner/src/validation/mod.rs b/crates/kingfisher-scanner/src/validation/mod.rs index 82addc0..fd5dcc2 100644 --- a/crates/kingfisher-scanner/src/validation/mod.rs +++ b/crates/kingfisher-scanner/src/validation/mod.rs @@ -61,8 +61,7 @@ pub use validation_body::{as_str, clone_as_string, from_string, ValidationRespon #[cfg(feature = "validation-http")] pub use http_validation::{ build_request_builder, check_url_resolvable, generate_http_cache_key_parts, is_ssrf_safe_ip, - parse_http_method, - process_headers, retry_multipart_request, retry_request, validate_response, + parse_http_method, process_headers, retry_multipart_request, retry_request, validate_response, }; #[cfg(feature = "validation-aws")] diff --git a/src/scanner/runner.rs b/src/scanner/runner.rs index f21eefc..2a37445 100644 --- a/src/scanner/runner.rs +++ b/src/scanner/runner.rs @@ -164,7 +164,10 @@ pub async fn run_async_scan( info!("Starting secret validation phase..."); Some(Arc::new(( register_all(liquid::ParserBuilder::with_stdlib()).build()?, - crate::validation::ValidationClients::new(global_args.tls_mode, global_args.allow_internal_ips)?, + crate::validation::ValidationClients::new( + global_args.tls_mode, + global_args.allow_internal_ips, + )?, Arc::new(SkipMap::new()), validation_rate_limiter.clone(), ))) diff --git a/src/validation/utils.rs b/src/validation/utils.rs index ca09855..056214d 100644 --- a/src/validation/utils.rs +++ b/src/validation/utils.rs @@ -106,7 +106,6 @@ pub fn find_closest_variable( best_before.or(best_overlap).or(best_after).map(|(_, value)| value) } - // ----------------------------------------------------------------------------- // tests // ----------------------------------------------------------------------------- From 4e9a7364cd13a21a8ea0d1ed0c52babd3e1c3f88 Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Fri, 27 Mar 2026 21:25:56 -0700 Subject: [PATCH 05/11] updated in response to ossf scorecard --- .../src/validation/http_validation.rs | 25 +++++++++++++++++++ docs/USAGE.md | 2 +- src/direct_validate.rs | 14 ++++++++++- src/validation.rs | 2 +- 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/crates/kingfisher-scanner/src/validation/http_validation.rs b/crates/kingfisher-scanner/src/validation/http_validation.rs index f5af606..07080c9 100644 --- a/crates/kingfisher-scanner/src/validation/http_validation.rs +++ b/crates/kingfisher-scanner/src/validation/http_validation.rs @@ -480,6 +480,21 @@ pub async fn check_url_resolvable( allow_internal_ips: bool, ) -> Result<(), Box> { let host = url.host_str().ok_or("No host in URL")?; + + // If the host is already an IP literal, check it directly without DNS. + if let Ok(ip) = host.parse::() { + if !allow_internal_ips && !is_ssrf_safe_ip(&ip) { + return Err(format!( + "SSRF protection: resolved IP {} for host '{}' is not a public address. \ + Use --allow-internal-ips to permit internal addresses.", + ip, host + ) + .into()); + } + return Ok(()); + } + + // Hostname — resolve via DNS and check each resolved address. let port = url.port().unwrap_or(if url.scheme() == "https" { 443 } else { 80 }); let addr = format!("{}:{}", host, port); let mut resolved_any = false; @@ -675,4 +690,14 @@ mod tests { let result = check_url_resolvable(&url, true).await; assert!(result.is_ok(), "expected Ok with allow_internal_ips=true, got: {:?}", result); } + + #[tokio::test] + async fn check_url_resolvable_rejects_ipv6_loopback_literal() { + // IPv6 literal URL — brackets are handled by reqwest::Url, host_str() returns "::1" + let url = Url::parse("https://[::1]/test").unwrap(); + let result = check_url_resolvable(&url, false).await; + assert!(result.is_err()); + let err = result.unwrap_err().to_string(); + assert!(err.contains("SSRF protection"), "expected SSRF error, got: {}", err); + } } diff --git a/docs/USAGE.md b/docs/USAGE.md index 72bcc55..bf2f6fd 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -1016,7 +1016,7 @@ If you are scanning infrastructure that uses internal endpoints for credential v # Scan with SSRF protection disabled (allows requests to internal IPs) kingfisher scan --allow-internal-ips ./repo -# Also works with validate and revoke commands +# Also works with the validate command kingfisher validate --allow-internal-ips --rule kingfisher.artifactory.1 ``` diff --git a/src/direct_validate.rs b/src/direct_validate.rs index 87caf06..e35df49 100644 --- a/src/direct_validate.rs +++ b/src/direct_validate.rs @@ -296,10 +296,16 @@ async fn execute_http_validation( parser: &liquid::Parser, timeout: Duration, retries: u32, + allow_internal_ips: bool, ) -> Result { // Render the URL let url = render_and_parse_url(parser, globals, &http_validation.request.url).await?; + // SSRF check: verify the resolved IP is public before making the request + crate::validation::utils::check_url_resolvable(&url, allow_internal_ips) + .await + .map_err(|e| anyhow!("URL resolution failed: {}", e))?; + debug!("Validating against URL: {}", url); // Build the request @@ -438,11 +444,16 @@ pub async fn run_direct_validation( crate::cli::global::TlsMode::Off | crate::cli::global::TlsMode::Lax ); - // Build HTTP client + // Build HTTP client with SSRF-safe redirect policy when applicable let client = Client::builder() .danger_accept_invalid_certs(use_lax_tls) .timeout(Duration::from_secs(args.timeout)) .user_agent(GLOBAL_USER_AGENT.as_str()) + .redirect(if global_args.allow_internal_ips { + reqwest::redirect::Policy::default() + } else { + crate::validation::ssrf_safe_redirect_policy() + }) .gzip(true) .deflate(true) .brotli(true) @@ -569,6 +580,7 @@ pub async fn run_direct_validation( &parser, timeout, args.retries, + global_args.allow_internal_ips, ) .await? } diff --git a/src/validation.rs b/src/validation.rs index d9e2f72..58758d0 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -133,7 +133,7 @@ pub struct ValidationClients { /// 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 { +pub(crate) fn ssrf_safe_redirect_policy() -> reqwest::redirect::Policy { reqwest::redirect::Policy::custom(|attempt| { if let Some(host) = attempt.url().host_str() { // For IP-literal hosts, check directly without DNS. From b04865e174927a23a28361737b5474a272a461d2 Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Fri, 27 Mar 2026 21:38:58 -0700 Subject: [PATCH 06/11] updated in response to ossf scorecard --- CHANGELOG.md | 2 +- .../src/validation/http_validation.rs | 27 ++++++++++++-- docs/USAGE.md | 6 ++-- src/direct_validate.rs | 15 +++++++- src/validation.rs | 35 +++++++------------ 5 files changed, 56 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b5221a..27be9a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ All notable changes to this project will be documented in this file. ## [v1.91.0] -- Added SSRF protection for credential validation: outbound HTTP requests now block connections to loopback, private, link-local, and other non-public IP addresses. Redirects to non-public IP-literal addresses are also blocked. Use `--allow-internal-ips` to opt out when scanning internal infrastructure. +- Added SSRF protection for credential validation: outbound HTTP requests now block connections to loopback, private, link-local, and other non-public IP addresses. Automatic HTTP redirects are blocked during validation to prevent redirect-based SSRF. Use `--allow-internal-ips` to opt out when scanning internal infrastructure. - Consolidated JWT SSRF checks to use the shared `is_ssrf_safe_ip` function, covering additional reserved ranges (CGNAT, documentation, benchmarking, IPv6 unique-local). - Removed `ipnet` dependency from `kingfisher-scanner` (no longer needed). - Remediated current RustSec vulnerability findings by upgrading core dependencies including `gix`, `mysql_async`, `axum`, `indicatif`, `quick-xml`, and `console`. diff --git a/crates/kingfisher-scanner/src/validation/http_validation.rs b/crates/kingfisher-scanner/src/validation/http_validation.rs index 07080c9..2e24bf7 100644 --- a/crates/kingfisher-scanner/src/validation/http_validation.rs +++ b/crates/kingfisher-scanner/src/validation/http_validation.rs @@ -437,8 +437,8 @@ pub fn is_ssrf_safe_ip(ip: &IpAddr) -> bool { if octets[0] == 198 && (18..=19).contains(&octets[1]) { return false; } - // Broadcast - if octets == [255, 255, 255, 255] { + // Reserved for future use (240.0.0.0/4) — not routable + if octets[0] >= 240 { return false; } true @@ -464,6 +464,10 @@ pub fn is_ssrf_safe_ip(ip: &IpAddr) -> bool { if segments[0] & 0xffc0 == 0xfe80 { return false; } + // Site-local (fec0::/10) — deprecated (RFC 3879) but still non-routable + if segments[0] & 0xffc0 == 0xfec0 { + return false; + } // Documentation (2001:db8::/32) if segments[0] == 0x2001 && segments[1] == 0x0db8 { return false; @@ -475,6 +479,13 @@ pub fn is_ssrf_safe_ip(ip: &IpAddr) -> bool { /// Check if a URL can be resolved via DNS, with SSRF protection against /// internal/private IP addresses. +/// +/// **Note:** This is a preflight check — the HTTP client will perform its own +/// DNS resolution when connecting. A DNS-rebinding attack could theoretically +/// return a public IP for this check and a private IP for the actual connection. +/// Fully eliminating this TOCTOU gap would require a custom resolver/connector +/// that pins resolved IPs; automatic redirect blocking (see +/// `ssrf_safe_redirect_policy`) mitigates the most practical exploitation path. pub async fn check_url_resolvable( url: &Url, allow_internal_ips: bool, @@ -579,7 +590,10 @@ mod tests { } #[test] - fn rejects_broadcast() { + fn rejects_reserved_and_broadcast() { + // 240.0.0.0/4 — reserved for future use (includes broadcast) + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(240, 0, 0, 1)))); + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(250, 1, 2, 3)))); assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::BROADCAST))); } @@ -609,6 +623,13 @@ mod tests { assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0xfe80, 0, 0, 0, 0, 0, 0, 1)))); } + #[test] + fn rejects_ipv6_site_local() { + // fec0::/10 — deprecated site-local (RFC 3879) + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0xfec0, 0, 0, 0, 0, 0, 0, 1)))); + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0xfeff, 0, 0, 0, 0, 0, 0, 1)))); + } + #[test] fn rejects_ipv6_documentation() { // 2001:db8::/32 — documentation range (RFC 3849) diff --git a/docs/USAGE.md b/docs/USAGE.md index bf2f6fd..9d69ebc 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -1004,9 +1004,11 @@ By default, validation requests are rejected if the target hostname resolves to | `fc00::/7` | IPv6 unique-local | | `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 | +| `240.0.0.0/4` | Reserved for future use (includes broadcast) | +| `fec0::/10` | IPv6 site-local (deprecated, RFC 3879) | +| Multicast, benchmarking ranges | Other reserved ranges | -HTTP redirects to IP-literal addresses in these ranges are also blocked. +Automatic HTTP redirects are blocked during credential validation to prevent redirect-based SSRF bypasses. ### `--allow-internal-ips` diff --git a/src/direct_validate.rs b/src/direct_validate.rs index e35df49..5d68d22 100644 --- a/src/direct_validate.rs +++ b/src/direct_validate.rs @@ -357,10 +357,16 @@ async fn execute_grpc_validation( globals: &Object, parser: &liquid::Parser, timeout: Duration, + allow_internal_ips: bool, ) -> Result { // Render the URL let url = render_and_parse_url(parser, globals, &grpc_validation_cfg.request.url).await?; + // SSRF check: verify the resolved IP is public before making the request + crate::validation::utils::check_url_resolvable(&url, allow_internal_ips) + .await + .map_err(|e| anyhow!("URL resolution failed: {}", e))?; + debug!("Validating against gRPC URL: {}", url); let res = grpc_validation::grpc_unary_call_from_rule( @@ -585,7 +591,14 @@ pub async fn run_direct_validation( .await? } Validation::Grpc(grpc_validation_cfg) => { - execute_grpc_validation(grpc_validation_cfg, &globals, &parser, timeout).await? + execute_grpc_validation( + grpc_validation_cfg, + &globals, + &parser, + timeout, + global_args.allow_internal_ips, + ) + .await? } Validation::AWS => { diff --git a/src/validation.rs b/src/validation.rs index 58758d0..44779c7 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -122,31 +122,22 @@ pub struct ValidationClients { pub allow_internal_ips: bool, } -/// Build a redirect policy that blocks redirects to non-public IP addresses. +/// Build a redirect policy that prevents SSRF via HTTP redirects. /// -/// **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. +/// Automatic redirects are blocked entirely when SSRF protection is enabled. +/// An attacker could supply a URL that initially points to a public endpoint +/// but redirects to an internal/private address (including via a hostname that +/// resolves to a private IP). Because reqwest's redirect callback is synchronous +/// and cannot perform async DNS resolution, selectively filtering redirect +/// targets is insufficient. Blocking all redirects is the safe default; +/// credential validation endpoints rarely require redirect-following. pub(crate) fn ssrf_safe_redirect_policy() -> reqwest::redirect::Policy { reqwest::redirect::Policy::custom(|attempt| { - if let Some(host) = attempt.url().host_str() { - // For IP-literal hosts, check directly without DNS. - if let Ok(ip) = host.parse::() { - if !kingfisher_scanner::validation::is_ssrf_safe_ip(&ip) { - return attempt.error(format!( - "SSRF protection: redirect to non-public IP {} blocked", - ip - )); - } - } - } - attempt.follow() + let url = attempt.url().clone(); + attempt.error(format!( + "SSRF protection: redirect to {} blocked (automatic redirects disabled)", + url + )) }) } From e0a403607f1fc0368c3bbcaa2bbc1c3b0ba153f8 Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Fri, 27 Mar 2026 22:26:35 -0700 Subject: [PATCH 07/11] updated in response to ossf scorecard --- CHANGELOG.md | 2 +- .../src/validation/http_validation.rs | 5 +- docs/USAGE.md | 3 +- src/validation.rs | 56 +++++++++++++++---- 4 files changed, 50 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27be9a9..7b4462f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ All notable changes to this project will be documented in this file. ## [v1.91.0] -- Added SSRF protection for credential validation: outbound HTTP requests now block connections to loopback, private, link-local, and other non-public IP addresses. Automatic HTTP redirects are blocked during validation to prevent redirect-based SSRF. Use `--allow-internal-ips` to opt out when scanning internal infrastructure. +- Added SSRF protection for credential validation: outbound HTTP requests now block connections to loopback, private, link-local, and other non-public IP addresses. HTTP redirect targets are DNS-resolved and validated against the same SSRF rules. Use `--allow-internal-ips` to opt out when scanning internal infrastructure. - Consolidated JWT SSRF checks to use the shared `is_ssrf_safe_ip` function, covering additional reserved ranges (CGNAT, documentation, benchmarking, IPv6 unique-local). - Removed `ipnet` dependency from `kingfisher-scanner` (no longer needed). - Remediated current RustSec vulnerability findings by upgrading core dependencies including `gix`, `mysql_async`, `axum`, `indicatif`, `quick-xml`, and `console`. diff --git a/crates/kingfisher-scanner/src/validation/http_validation.rs b/crates/kingfisher-scanner/src/validation/http_validation.rs index 2e24bf7..5638907 100644 --- a/crates/kingfisher-scanner/src/validation/http_validation.rs +++ b/crates/kingfisher-scanner/src/validation/http_validation.rs @@ -484,8 +484,9 @@ pub fn is_ssrf_safe_ip(ip: &IpAddr) -> bool { /// DNS resolution when connecting. A DNS-rebinding attack could theoretically /// return a public IP for this check and a private IP for the actual connection. /// Fully eliminating this TOCTOU gap would require a custom resolver/connector -/// that pins resolved IPs; automatic redirect blocking (see -/// `ssrf_safe_redirect_policy`) mitigates the most practical exploitation path. +/// that pins resolved IPs. In practice, callers should also disable automatic +/// redirects or use a redirect-blocking policy on the HTTP client to mitigate +/// the most practical exploitation paths. pub async fn check_url_resolvable( url: &Url, allow_internal_ips: bool, diff --git a/docs/USAGE.md b/docs/USAGE.md index 9d69ebc..656c405 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -1004,11 +1004,12 @@ By default, validation requests are rejected if the target hostname resolves to | `fc00::/7` | IPv6 unique-local | | `2001:db8::/32` | IPv6 documentation (RFC 3849) | | `::ffff:0:0/96` | IPv4-mapped IPv6 (checked against IPv4 rules) | +| `::/96` | IPv4-compatible IPv6 (deprecated) | | `240.0.0.0/4` | Reserved for future use (includes broadcast) | | `fec0::/10` | IPv6 site-local (deprecated, RFC 3879) | | Multicast, benchmarking ranges | Other reserved ranges | -Automatic HTTP redirects are blocked during credential validation to prevent redirect-based SSRF bypasses. +HTTP redirects during credential validation are also validated: each redirect target is resolved via DNS and checked against the same SSRF rules above. Redirects to non-public IPs are blocked. When `--allow-internal-ips` is used, redirect validation is disabled along with all other SSRF protections. ### `--allow-internal-ips` diff --git a/src/validation.rs b/src/validation.rs index 44779c7..5ad795f 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -122,22 +122,54 @@ pub struct ValidationClients { pub allow_internal_ips: bool, } -/// Build a redirect policy that prevents SSRF via HTTP redirects. +/// Build a redirect policy that validates redirect targets against SSRF rules. /// -/// Automatic redirects are blocked entirely when SSRF protection is enabled. -/// An attacker could supply a URL that initially points to a public endpoint -/// but redirects to an internal/private address (including via a hostname that -/// resolves to a private IP). Because reqwest's redirect callback is synchronous -/// and cannot perform async DNS resolution, selectively filtering redirect -/// targets is insufficient. Blocking all redirects is the safe default; -/// credential validation endpoints rarely require redirect-following. +/// Each redirect hop is checked: IP-literal targets are validated directly via +/// `is_ssrf_safe_ip`, and hostname targets are resolved synchronously via +/// `std::net::ToSocketAddrs` so that all resolved IPs can be checked. This +/// closes the hostname-redirect SSRF gap (e.g., a public URL that 302s to an +/// attacker-controlled hostname resolving to `169.254.169.254`). +/// +/// **Note:** Using blocking DNS in the redirect callback is acceptable because +/// reqwest runs redirect callbacks on its connection thread, not the tokio +/// event loop, and the DNS latency is negligible relative to the HTTP request. pub(crate) fn ssrf_safe_redirect_policy() -> reqwest::redirect::Policy { reqwest::redirect::Policy::custom(|attempt| { + // Cap redirect depth (reqwest default is 10) + if attempt.previous().len() >= 10 { + return attempt.error("too many redirects"); + } + // Extract URL info before potentially moving `attempt`. let url = attempt.url().clone(); - attempt.error(format!( - "SSRF protection: redirect to {} blocked (automatic redirects disabled)", - url - )) + if let Some(host) = url.host_str() { + if let Ok(ip) = host.parse::() { + // IP-literal: check directly without DNS. + if !kingfisher_scanner::validation::is_ssrf_safe_ip(&ip) { + return attempt.error(format!( + "SSRF protection: redirect to non-public IP {} blocked", + ip + )); + } + } else { + // Hostname: resolve synchronously and check all resolved IPs. + let port = url.port().unwrap_or(if url.scheme() == "https" { 443 } else { 80 }); + if let Ok(addrs) = std::net::ToSocketAddrs::to_socket_addrs(&(host, port as u16)) { + for addr in addrs { + if !kingfisher_scanner::validation::is_ssrf_safe_ip(&addr.ip()) { + return attempt.error(format!( + "SSRF protection: redirect to '{}' resolves to non-public IP {} — blocked", + host, + addr.ip() + )); + } + } + } + // If DNS resolution fails, allow the redirect — reqwest will + // fail on connect anyway, and we don't want to silently block + // valid hostnames that are transiently unresolvable. + } + } + attempt.follow() }) } From 93cd6e940c4b4aa8513e60dbfa5faadaad12515c Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Fri, 27 Mar 2026 22:43:50 -0700 Subject: [PATCH 08/11] updated in response to ossf scorecard --- .../src/validation/http_validation.rs | 7 +++++ .../kingfisher-scanner/src/validation/mod.rs | 6 ++-- src/validation.rs | 29 ++++++++++++------- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/crates/kingfisher-scanner/src/validation/http_validation.rs b/crates/kingfisher-scanner/src/validation/http_validation.rs index 5638907..34e0089 100644 --- a/crates/kingfisher-scanner/src/validation/http_validation.rs +++ b/crates/kingfisher-scanner/src/validation/http_validation.rs @@ -528,6 +528,13 @@ pub async fn check_url_resolvable( Ok(()) } +/// Backwards-compatible wrapper: checks URL resolvability with SSRF protection +/// enabled (i.e., `allow_internal_ips = false`). +#[deprecated(since = "0.1.0", note = "use check_url_resolvable(url, allow_internal_ips) instead")] +pub async fn check_url_resolvable_safe(url: &Url) -> Result<(), Box> { + check_url_resolvable(url, false).await +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/kingfisher-scanner/src/validation/mod.rs b/crates/kingfisher-scanner/src/validation/mod.rs index fd5dcc2..e1914a7 100644 --- a/crates/kingfisher-scanner/src/validation/mod.rs +++ b/crates/kingfisher-scanner/src/validation/mod.rs @@ -59,9 +59,11 @@ pub use utils::{find_closest_variable, process_captures}; pub use validation_body::{as_str, clone_as_string, from_string, ValidationResponseBody}; #[cfg(feature = "validation-http")] +#[allow(deprecated)] pub use http_validation::{ - build_request_builder, check_url_resolvable, generate_http_cache_key_parts, is_ssrf_safe_ip, - parse_http_method, process_headers, retry_multipart_request, retry_request, validate_response, + build_request_builder, check_url_resolvable, check_url_resolvable_safe, + generate_http_cache_key_parts, is_ssrf_safe_ip, parse_http_method, process_headers, + retry_multipart_request, retry_request, validate_response, }; #[cfg(feature = "validation-aws")] diff --git a/src/validation.rs b/src/validation.rs index 5ad795f..b74e359 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -153,20 +153,27 @@ pub(crate) fn ssrf_safe_redirect_policy() -> reqwest::redirect::Policy { } else { // Hostname: resolve synchronously and check all resolved IPs. let port = url.port().unwrap_or(if url.scheme() == "https" { 443 } else { 80 }); - if let Ok(addrs) = std::net::ToSocketAddrs::to_socket_addrs(&(host, port as u16)) { - for addr in addrs { - if !kingfisher_scanner::validation::is_ssrf_safe_ip(&addr.ip()) { - return attempt.error(format!( - "SSRF protection: redirect to '{}' resolves to non-public IP {} — blocked", - host, - addr.ip() - )); + match std::net::ToSocketAddrs::to_socket_addrs(&(host, port as u16)) { + Ok(addrs) => { + for addr in addrs { + if !kingfisher_scanner::validation::is_ssrf_safe_ip(&addr.ip()) { + return attempt.error(format!( + "SSRF protection: redirect to '{}' resolves to non-public IP {} — blocked", + host, + addr.ip() + )); + } } } + Err(e) => { + // Fail closed: if we cannot resolve the hostname, we + // cannot guarantee the redirect target is SSRF-safe. + return attempt.error(format!( + "SSRF protection: cannot resolve redirect host '{}' ({}) — blocked", + host, e + )); + } } - // If DNS resolution fails, allow the redirect — reqwest will - // fail on connect anyway, and we don't want to silently block - // valid hostnames that are transiently unresolvable. } } attempt.follow() From 993a76ded1d22f01768d374aacee4be6ec40cc95 Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Fri, 27 Mar 2026 22:57:19 -0700 Subject: [PATCH 09/11] updated in response to ossf scorecard --- .../src/validation/http_validation.rs | 32 +++++++++++++++++++ src/validation.rs | 9 ++++-- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/crates/kingfisher-scanner/src/validation/http_validation.rs b/crates/kingfisher-scanner/src/validation/http_validation.rs index 34e0089..fb32b28 100644 --- a/crates/kingfisher-scanner/src/validation/http_validation.rs +++ b/crates/kingfisher-scanner/src/validation/http_validation.rs @@ -394,6 +394,8 @@ pub fn validate_response( /// Returns `true` if the IP address is safe for outbound validation requests /// (i.e., it is a publicly routable address, not internal/reserved). +/// +/// Covers all IANA special-purpose ranges from RFC 6890 and RFC 8190. pub fn is_ssrf_safe_ip(ip: &IpAddr) -> bool { if ip.is_loopback() || ip.is_unspecified() || ip.is_multicast() { return false; @@ -423,10 +425,18 @@ pub fn is_ssrf_safe_ip(ip: &IpAddr) -> bool { if octets[0] == 100 && (64..=127).contains(&octets[1]) { return false; } + // IANA Special Purpose (192.0.0.0/24, RFC 6890) + if octets[0] == 192 && octets[1] == 0 && octets[2] == 0 { + return false; + } // Documentation ranges (RFC 5737) if octets[0] == 192 && octets[1] == 0 && octets[2] == 2 { return false; } + // 6to4 relay anycast (192.88.99.0/24, RFC 7526 — deprecated) + if octets[0] == 192 && octets[1] == 88 && octets[2] == 99 { + return false; + } if octets[0] == 198 && octets[1] == 51 && octets[2] == 100 { return false; } @@ -468,6 +478,10 @@ pub fn is_ssrf_safe_ip(ip: &IpAddr) -> bool { if segments[0] & 0xffc0 == 0xfec0 { return false; } + // Benchmarking (2001:2::/48, RFC 5180) + if segments[0] == 0x2001 && segments[1] == 0x0002 && segments[2] == 0 { + return false; + } // Documentation (2001:db8::/32) if segments[0] == 0x2001 && segments[1] == 0x0db8 { return false; @@ -679,6 +693,24 @@ mod tests { assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0x0808, 0x0808)))); } + #[test] + fn rejects_iana_special_purpose() { + // 192.0.0.0/24 — IANA special-purpose (RFC 6890) + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(192, 0, 0, 1)))); + } + + #[test] + fn rejects_6to4_relay_anycast() { + // 192.88.99.0/24 — 6to4 relay anycast (RFC 7526, deprecated) + assert!(!is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(192, 88, 99, 1)))); + } + + #[test] + fn rejects_ipv6_benchmarking() { + // 2001:2::/48 — benchmarking (RFC 5180) + assert!(!is_ssrf_safe_ip(&IpAddr::V6(Ipv6Addr::new(0x2001, 0x0002, 0, 0, 0, 0, 0, 1)))); + } + #[test] fn accepts_public_ipv4() { assert!(is_ssrf_safe_ip(&IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8)))); diff --git a/src/validation.rs b/src/validation.rs index b74e359..bda079c 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -130,9 +130,12 @@ pub struct ValidationClients { /// closes the hostname-redirect SSRF gap (e.g., a public URL that 302s to an /// attacker-controlled hostname resolving to `169.254.169.254`). /// -/// **Note:** Using blocking DNS in the redirect callback is acceptable because -/// reqwest runs redirect callbacks on its connection thread, not the tokio -/// event loop, and the DNS latency is negligible relative to the HTTP request. +/// **Note:** reqwest runs redirect callbacks on Tokio worker threads, so the +/// blocking DNS lookup here can briefly stall other async tasks on that thread. +/// This is acceptable for a scanner workload because DNS is typically cached +/// by the system resolver (<5ms), redirect hops are infrequent, and the +/// alternative (disabling automatic redirects and following them manually with +/// async DNS) would add significant complexity for minimal practical benefit. pub(crate) fn ssrf_safe_redirect_policy() -> reqwest::redirect::Policy { reqwest::redirect::Policy::custom(|attempt| { // Cap redirect depth (reqwest default is 10) From afd0eb5713455c21f6ea8ba7490e659bdfcf4432 Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Fri, 27 Mar 2026 23:07:02 -0700 Subject: [PATCH 10/11] updated in response to ossf scorecard --- .../src/validation/http_validation.rs | 2 +- crates/kingfisher-scanner/src/validation/jwt.rs | 17 +++++++---------- src/validation.rs | 11 ++++++++--- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/crates/kingfisher-scanner/src/validation/http_validation.rs b/crates/kingfisher-scanner/src/validation/http_validation.rs index fb32b28..4658092 100644 --- a/crates/kingfisher-scanner/src/validation/http_validation.rs +++ b/crates/kingfisher-scanner/src/validation/http_validation.rs @@ -395,7 +395,7 @@ pub fn validate_response( /// Returns `true` if the IP address is safe for outbound validation requests /// (i.e., it is a publicly routable address, not internal/reserved). /// -/// Covers all IANA special-purpose ranges from RFC 6890 and RFC 8190. +/// Blocks common IANA special-purpose ranges from RFC 6890 and RFC 8190. pub fn is_ssrf_safe_ip(ip: &IpAddr) -> bool { if ip.is_loopback() || ip.is_unspecified() || ip.is_multicast() { return false; diff --git a/crates/kingfisher-scanner/src/validation/jwt.rs b/crates/kingfisher-scanner/src/validation/jwt.rs index 5d5d35b..33828f4 100644 --- a/crates/kingfisher-scanner/src/validation/jwt.rs +++ b/crates/kingfisher-scanner/src/validation/jwt.rs @@ -1,3 +1,4 @@ +use super::http_validation::check_url_resolvable; use anyhow::{anyhow, Result}; use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _}; use chrono::Utc; @@ -7,9 +8,6 @@ use jsonwebtoken::{ use once_cell::sync::Lazy; use reqwest::{redirect::Policy, Client, Url}; use serde::Deserialize; -use tokio::net::lookup_host; - -use super::http_validation::{check_url_resolvable, is_ssrf_safe_ip}; /// Global redirect-free client with strict TLS validation. static STRICT_CLIENT: Lazy = Lazy::new(|| { @@ -198,18 +196,17 @@ 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()) { + match check_url_resolvable(&url, allow_internal_ips).await { + Ok(()) => {} + Err(e) => { + let msg = e.to_string(); + if msg.contains("SSRF protection") { return Ok((false, "jwks_uri resolves to non-public or reserved IP".to_string())); } + return Err(anyhow!("jwks uri unresolvable: {e}")); } } - check_url_resolvable(&url, allow_internal_ips) - .await - .map_err(|e| anyhow!("jwks uri unresolvable: {e}"))?; - let jwks_resp = client.get(url).send().await.map_err(|e| anyhow!("jwks fetch failed: {e}"))?; if !jwks_resp.status().is_success() { return Ok((false, format!("jwks fetch failed: {}", jwks_resp.status()))); diff --git a/src/validation.rs b/src/validation.rs index bda079c..edf38a3 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -127,8 +127,13 @@ pub struct ValidationClients { /// Each redirect hop is checked: IP-literal targets are validated directly via /// `is_ssrf_safe_ip`, and hostname targets are resolved synchronously via /// `std::net::ToSocketAddrs` so that all resolved IPs can be checked. This -/// closes the hostname-redirect SSRF gap (e.g., a public URL that 302s to an -/// attacker-controlled hostname resolving to `169.254.169.254`). +/// significantly reduces the hostname-redirect SSRF risk (e.g., a public URL +/// that 302s to an attacker-controlled hostname resolving to `169.254.169.254`). +/// This is a best-effort check: reqwest performs its own DNS resolution when +/// connecting, so a malicious DNS server could return different IPs between +/// this check and the actual request (DNS rebinding / TOCTOU). A future +/// hardening step would be a pinned/custom resolver so that validated IPs are +/// exactly those used for the outbound connection. /// /// **Note:** reqwest runs redirect callbacks on Tokio worker threads, so the /// blocking DNS lookup here can briefly stall other async tasks on that thread. @@ -156,7 +161,7 @@ pub(crate) fn ssrf_safe_redirect_policy() -> reqwest::redirect::Policy { } else { // Hostname: resolve synchronously and check all resolved IPs. let port = url.port().unwrap_or(if url.scheme() == "https" { 443 } else { 80 }); - match std::net::ToSocketAddrs::to_socket_addrs(&(host, port as u16)) { + match std::net::ToSocketAddrs::to_socket_addrs(&(host, port)) { Ok(addrs) => { for addr in addrs { if !kingfisher_scanner::validation::is_ssrf_safe_ip(&addr.ip()) { From b14522351bcc6523871d0a85250f1b14e7c8274d Mon Sep 17 00:00:00 2001 From: Mick Grove Date: Fri, 27 Mar 2026 23:18:56 -0700 Subject: [PATCH 11/11] updated in response to ossf scorecard --- .../src/validation/http_validation.rs | 22 +++++++++++++++---- .../kingfisher-scanner/src/validation/jwt.rs | 14 +++++------- .../kingfisher-scanner/src/validation/mod.rs | 11 ++++++---- docs/USAGE.md | 2 +- src/validation.rs | 18 ++++++++------- 5 files changed, 41 insertions(+), 26 deletions(-) diff --git a/crates/kingfisher-scanner/src/validation/http_validation.rs b/crates/kingfisher-scanner/src/validation/http_validation.rs index 4658092..2dc70de 100644 --- a/crates/kingfisher-scanner/src/validation/http_validation.rs +++ b/crates/kingfisher-scanner/src/validation/http_validation.rs @@ -14,6 +14,20 @@ use sha1::{Digest, Sha1}; use tokio::{net::lookup_host, time::sleep}; use tracing::debug; +/// Error returned by [`check_url_resolvable`] when an IP address fails the +/// SSRF safety check. Callers can downcast `Box` to distinguish +/// SSRF blocks from other resolution failures. +#[derive(Debug)] +pub struct SsrfBlockedError(pub String); + +impl std::fmt::Display for SsrfBlockedError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.0) + } +} + +impl std::error::Error for SsrfBlockedError {} + use super::GLOBAL_USER_AGENT; use kingfisher_rules::ResponseMatcher; @@ -510,11 +524,11 @@ pub async fn check_url_resolvable( // If the host is already an IP literal, check it directly without DNS. if let Ok(ip) = host.parse::() { if !allow_internal_ips && !is_ssrf_safe_ip(&ip) { - return Err(format!( + return Err(SsrfBlockedError(format!( "SSRF protection: resolved IP {} for host '{}' is not a public address. \ Use --allow-internal-ips to permit internal addresses.", ip, host - ) + )) .into()); } return Ok(()); @@ -527,12 +541,12 @@ pub async fn check_url_resolvable( 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!( + return Err(SsrfBlockedError(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()); } } diff --git a/crates/kingfisher-scanner/src/validation/jwt.rs b/crates/kingfisher-scanner/src/validation/jwt.rs index 33828f4..b04a206 100644 --- a/crates/kingfisher-scanner/src/validation/jwt.rs +++ b/crates/kingfisher-scanner/src/validation/jwt.rs @@ -1,4 +1,4 @@ -use super::http_validation::check_url_resolvable; +use super::http_validation::{check_url_resolvable, SsrfBlockedError}; use anyhow::{anyhow, Result}; use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _}; use chrono::Utc; @@ -196,15 +196,11 @@ pub async fn validate_jwt_with( )); } - match check_url_resolvable(&url, allow_internal_ips).await { - Ok(()) => {} - Err(e) => { - let msg = e.to_string(); - if msg.contains("SSRF protection") { - return Ok((false, "jwks_uri resolves to non-public or reserved IP".to_string())); - } - return Err(anyhow!("jwks uri unresolvable: {e}")); + if let Err(e) = check_url_resolvable(&url, allow_internal_ips).await { + if e.downcast_ref::().is_some() { + return Ok((false, "jwks_uri resolves to non-public or reserved IP".to_string())); } + return Err(anyhow!("jwks uri unresolvable: {e}")); } let jwks_resp = client.get(url).send().await.map_err(|e| anyhow!("jwks fetch failed: {e}"))?; diff --git a/crates/kingfisher-scanner/src/validation/mod.rs b/crates/kingfisher-scanner/src/validation/mod.rs index e1914a7..443f821 100644 --- a/crates/kingfisher-scanner/src/validation/mod.rs +++ b/crates/kingfisher-scanner/src/validation/mod.rs @@ -59,13 +59,16 @@ pub use utils::{find_closest_variable, process_captures}; pub use validation_body::{as_str, clone_as_string, from_string, ValidationResponseBody}; #[cfg(feature = "validation-http")] -#[allow(deprecated)] pub use http_validation::{ - build_request_builder, check_url_resolvable, check_url_resolvable_safe, - generate_http_cache_key_parts, is_ssrf_safe_ip, parse_http_method, process_headers, - retry_multipart_request, retry_request, validate_response, + build_request_builder, check_url_resolvable, generate_http_cache_key_parts, is_ssrf_safe_ip, + parse_http_method, process_headers, retry_multipart_request, retry_request, validate_response, + SsrfBlockedError, }; +#[cfg(feature = "validation-http")] +#[allow(deprecated)] +pub use http_validation::check_url_resolvable_safe; + #[cfg(feature = "validation-aws")] pub use aws::{ aws_key_to_account_number, generate_aws_cache_key, revoke_aws_access_key, diff --git a/docs/USAGE.md b/docs/USAGE.md index 656c405..69c07fe 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -997,7 +997,7 @@ By default, validation requests are rejected if the target hostname resolves to | Range | Description | | ----- | ----------- | | `127.0.0.0/8`, `::1` | Loopback (localhost) | -| `0.0.0.0`, `::` | Unspecified | +| `0.0.0.0/8`, `::` | Unspecified / "this network" (RFC 1122) | | `10.0.0.0/8`, `172.16.0.0/12`, `192.168.0.0/16` | Private networks (RFC 1918) | | `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 | diff --git a/src/validation.rs b/src/validation.rs index edf38a3..fd33662 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -135,12 +135,9 @@ pub struct ValidationClients { /// hardening step would be a pinned/custom resolver so that validated IPs are /// exactly those used for the outbound connection. /// -/// **Note:** reqwest runs redirect callbacks on Tokio worker threads, so the -/// blocking DNS lookup here can briefly stall other async tasks on that thread. -/// This is acceptable for a scanner workload because DNS is typically cached -/// by the system resolver (<5ms), redirect hops are infrequent, and the -/// alternative (disabling automatic redirects and following them manually with -/// async DNS) would add significant complexity for minimal practical benefit. +/// **Note:** reqwest runs redirect callbacks on Tokio worker threads. The DNS +/// lookup uses `tokio::task::block_in_place` so the runtime can compensate +/// (e.g., spawn additional worker threads) rather than silently stalling. pub(crate) fn ssrf_safe_redirect_policy() -> reqwest::redirect::Policy { reqwest::redirect::Policy::custom(|attempt| { // Cap redirect depth (reqwest default is 10) @@ -159,9 +156,14 @@ pub(crate) fn ssrf_safe_redirect_policy() -> reqwest::redirect::Policy { )); } } else { - // Hostname: resolve synchronously and check all resolved IPs. + // Hostname: resolve and check all resolved IPs. We use + // block_in_place to signal Tokio that this thread is about to + // block on synchronous DNS, so the runtime can compensate. let port = url.port().unwrap_or(if url.scheme() == "https" { 443 } else { 80 }); - match std::net::ToSocketAddrs::to_socket_addrs(&(host, port)) { + let dns_result = tokio::task::block_in_place(|| { + std::net::ToSocketAddrs::to_socket_addrs(&(host, port)) + }); + match dns_result { Ok(addrs) => { for addr in addrs { if !kingfisher_scanner::validation::is_ssrf_safe_ip(&addr.ip()) {