updated in response to ossf scorecard

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;

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<Client> = 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())));

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()) {