From 1b1b21782649f7399686f79b0d89f7ab8e481b20 Mon Sep 17 00:00:00 2001 From: GatewayJ <835269233@qq.com> Date: Sat, 8 Aug 2026 19:28:47 +0800 Subject: [PATCH] fix(iam): preserve OIDC outbound policy errors (#5762) --- crates/iam/src/oidc.rs | 399 +++++++++++++++--- crates/utils/src/egress.rs | 188 ++++++++- docs/operations/keycloak-oidc-integration.md | 4 + docs/operations/outbound-connection-policy.md | 25 +- 4 files changed, 539 insertions(+), 77 deletions(-) diff --git a/crates/iam/src/oidc.rs b/crates/iam/src/oidc.rs index 5a1f2e900..727fd981c 100644 --- a/crates/iam/src/oidc.rs +++ b/crates/iam/src/oidc.rs @@ -21,16 +21,16 @@ use crate::oidc_state::{OidcAuthSession, OidcLogoutSession, OidcStateStore}; use openidconnect::core::{CoreAuthenticationFlow, CoreClient, CoreIdToken, CoreJsonWebKeySet}; use openidconnect::{ - AsyncHttpClient, Audience, AuthType, AuthorizationCode, ClientId, ClientSecret, CsrfToken, IssuerUrl, JsonWebKeySetUrl, - LogoutRequest, Nonce, PkceCodeChallenge, PkceCodeVerifier, PostLogoutRedirectUrl, ProviderMetadataWithLogout, RedirectUrl, - RequestTokenError, Scope, + AsyncHttpClient, Audience, AuthType, AuthorizationCode, ClientId, ClientSecret, CsrfToken, DiscoveryError, IssuerUrl, + JsonWebKeySetUrl, LogoutRequest, Nonce, PkceCodeChallenge, PkceCodeVerifier, PostLogoutRedirectUrl, + ProviderMetadataWithLogout, RedirectUrl, RequestTokenError, Scope, }; use reqwest::Client; use rustfs_config::oidc::*; use rustfs_config::server_config::{Config as ServerConfig, KVS}; use rustfs_config::{DEFAULT_DELIMITER, ENABLE_KEY, EnableState, MAX_OIDC_RESPONSE_SIZE}; use rustfs_policy::policy::{ClaimLookup, get_claim_case_insensitive}; -use rustfs_utils::egress::OutboundPolicy; +use rustfs_utils::egress::{ENV_OUTBOUND_ALLOW_ORIGINS, OutboundPolicy, find_outbound_dns_policy_rejection}; use serde::{Deserialize, Serialize}; use std::borrow::Cow; use std::collections::{HashMap, VecDeque}; @@ -38,6 +38,8 @@ use std::fmt; use std::future::Future; use std::net::IpAddr; use std::pin::Pin; +#[cfg(test)] +use std::sync::Arc; use std::sync::{LazyLock, Mutex, MutexGuard, RwLock}; use std::time::{Duration as StdDuration, Instant}; use tokio::time::sleep; @@ -51,6 +53,7 @@ const EVENT_OIDC_HTTP: &str = "oidc_http"; const OIDC_JWKS_REFRESH_INTERVAL: StdDuration = StdDuration::from_secs(24 * 60 * 60); const OIDC_DISCOVERY_TRANSPORT_RETRIES: usize = 3; const OIDC_DISCOVERY_TRANSPORT_RETRY_DELAY: StdDuration = StdDuration::from_millis(50); +const OIDC_DISCOVERY_BLOCKED_BY_OUTBOUND_POLICY: &str = "OIDC provider discovery blocked by outbound policy"; const OIDC_HTTP_REQUEST_TIMEOUT: StdDuration = StdDuration::from_secs(10); const OIDC_HTTP_CONNECT_TIMEOUT: StdDuration = StdDuration::from_secs(3); const OIDC_PLUGIN_AUTHN_WINDOW: StdDuration = StdDuration::from_secs(60); @@ -308,6 +311,8 @@ pub(crate) struct ReqwestHttpClient { /// `None` in production: the process-cached outbound policy from the environment is used. /// `Some(..)` only in tests, to explicitly allow a loopback mock endpoint. policy_override: Option, + #[cfg(test)] + dns_resolver_override: Option>, } /// Build a reqwest client pinned to the shared outbound egress policy for a single request. @@ -318,7 +323,11 @@ pub(crate) struct ReqwestHttpClient { /// connection so DNS rebinding fails closed. Redirects are not followed: a redirect target /// would otherwise skip URL-shape re-validation. The timeouts bound how long a slow or /// stalled provider can pin the calling task. -fn build_oidc_http_client(uri: &str, policy_override: Option<&OutboundPolicy>) -> Result { +fn build_oidc_http_client( + uri: &str, + policy_override: Option<&OutboundPolicy>, + #[cfg(test)] dns_resolver_override: Option>, +) -> Result<(Client, Url), OidcHttpError> { let url = Url::parse(uri).map_err(|_| OidcHttpError::ForbiddenOutbound("invalid outbound OIDC URL".to_string()))?; let resolver = match policy_override { Some(policy) => policy.resolver_for(&url), @@ -326,17 +335,48 @@ fn build_oidc_http_client(uri: &str, policy_override: Option<&OutboundPolicy>) - .map_err(|err| OidcHttpError::ForbiddenOutbound(err.to_string()))? .resolver_for(&url), } - .map_err(|err| OidcHttpError::ForbiddenOutbound(err.to_string()))?; + .map_err(|err| { + let base = err.to_string(); + let origin = url.origin().ascii_serialization(); + let can_allow_origin = + OutboundPolicy::from_allowed_origins(&origin).is_ok_and(|allowlisted| allowlisted.validate_url(&url).is_ok()); + oidc_forbidden_outbound_error(&url, base, can_allow_origin) + })?; + let bypass_proxy = should_bypass_proxy_for_oidc_uri(uri); + #[cfg(test)] + let bypass_proxy = bypass_proxy || dns_resolver_override.is_some(); + #[cfg(test)] + let resolver: Arc = dns_resolver_override.unwrap_or_else(|| Arc::new(resolver)); let mut builder = reqwest::Client::builder() .dns_resolver(resolver) .redirect(reqwest::redirect::Policy::none()) .timeout(OIDC_HTTP_REQUEST_TIMEOUT) .connect_timeout(OIDC_HTTP_CONNECT_TIMEOUT); - if should_bypass_proxy_for_oidc_uri(uri) { + if bypass_proxy { builder = builder.no_proxy(); } - builder.build().map_err(OidcHttpError::Reqwest) + builder.build().map(|client| (client, url)).map_err(OidcHttpError::Reqwest) +} + +fn oidc_forbidden_outbound_error(url: &Url, base: String, can_allow_origin: bool) -> OidcHttpError { + let reason = if can_allow_origin { + let origin = url.origin().ascii_serialization(); + format!( + "{base}; add {origin} to {ENV_OUTBOUND_ALLOW_ORIGINS} (comma-separated) and restart RustFS to allow this operator-owned OIDC provider (origin only, no path)" + ) + } else { + base + }; + OidcHttpError::ForbiddenOutbound(reason) +} + +fn oidc_http_error_from_reqwest(url: &Url, error: reqwest::Error) -> OidcHttpError { + if let Some(rejection) = find_outbound_dns_policy_rejection(&error) { + let base = rejection.to_string(); + return oidc_forbidden_outbound_error(url, base, rejection.allow_origin_can_recover()); + } + OidcHttpError::Reqwest(error) } /// Buffer a provider response body, failing closed once `limit` bytes have been seen. @@ -370,7 +410,11 @@ fn should_bypass_proxy_for_oidc_uri(uri: &str) -> bool { impl ReqwestHttpClient { fn new() -> Result { - Ok(Self { policy_override: None }) + Ok(Self { + policy_override: None, + #[cfg(test)] + dns_resolver_override: None, + }) } /// Test-only constructor that pins outbound requests to an explicit policy, so a @@ -379,6 +423,15 @@ impl ReqwestHttpClient { fn with_policy(policy: OutboundPolicy) -> Self { Self { policy_override: Some(policy), + dns_resolver_override: None, + } + } + + #[cfg(test)] + fn with_policy_and_dns_resolver(policy: OutboundPolicy, resolver: Arc) -> Self { + Self { + policy_override: Some(policy), + dns_resolver_override: Some(resolver), } } } @@ -408,7 +461,12 @@ impl<'c> AsyncHttpClient<'c> for ReqwestHttpClient { ); } - let client = build_oidc_http_client(&uri, self.policy_override.as_ref())?; + let (client, url) = build_oidc_http_client( + &uri, + self.policy_override.as_ref(), + #[cfg(test)] + self.dns_resolver_override.clone(), + )?; let response = client .request(parts.method, uri.clone()) .headers(parts.headers) @@ -421,6 +479,7 @@ impl<'c> AsyncHttpClient<'c> for ReqwestHttpClient { OIDC_PLUGIN_AUTHN_METRICS.record(elapsed_ms, succeeded); let response = response.map_err(|err| { + let error = oidc_http_error_from_reqwest(&url, err); error!( event = EVENT_OIDC_HTTP, component = LOG_COMPONENT_IAM, @@ -429,10 +488,10 @@ impl<'c> AsyncHttpClient<'c> for ReqwestHttpClient { method = %method, uri = %uri, elapsed_ms, - error = %err, + error = %error, "oidc outbound http" ); - OidcHttpError::Reqwest(err) + error })?; let status = response.status(); @@ -866,11 +925,11 @@ impl OidcSys { redirect_uri = %redirect_uri, request_error_kind = %request_error_kind, request_error_status = %request_error_status, - error = %e, + error = %err, "oidc token exchange failed" ); format!( - "token exchange failed: {e}: stage=token_request_failed, provider_id={}, config_url={}, issuer={}, token_endpoint={}, redirect_uri={}, client_id={}, request_error_kind={}, request_error_status={}", + "token exchange failed: stage=token_request_failed, provider_id={}, config_url={}, issuer={}, token_endpoint={}, redirect_uri={}, client_id={}, request_error_kind={}, request_error_status={}, request_error={}", session.provider_id, config.config_url, issuer, @@ -878,7 +937,8 @@ impl OidcSys { redirect_uri, config.client_id, request_error_kind, - request_error_status + request_error_status, + err ) } RequestTokenError::Parse(parse_err, body) => { @@ -1656,17 +1716,18 @@ impl OidcSys { let issuer_url = IssuerUrl::new(candidate_issuer.clone()).map_err(|e| format!("invalid issuer URL: {e}"))?; for attempt in 0..OIDC_DISCOVERY_TRANSPORT_RETRIES { - match ProviderMetadataWithLogout::discover_async(issuer_url.clone(), http_client) - .await - .map_err(|e| format!("discovery failed: {e}")) - { + match ProviderMetadataWithLogout::discover_async(issuer_url.clone(), http_client).await { Ok(metadata) => { return Ok(ProviderState { metadata, discovered_at: Instant::now(), }); } + Err(DiscoveryError::Request(OidcHttpError::ForbiddenOutbound(reason))) => { + return Err(format!("{OIDC_DISCOVERY_BLOCKED_BY_OUTBOUND_POLICY}: {reason}")); + } Err(error) => { + let error = format!("discovery failed: {error}"); let is_transient_transport = error.contains("Request failed"); let should_retry = is_transient_transport && attempt + 1 < OIDC_DISCOVERY_TRANSPORT_RETRIES; if should_retry { @@ -1728,10 +1789,13 @@ impl OidcSys { .body(Vec::new()) .map_err(|err| format!("failed to prepare discovery request: {err}"))?; - let response = http_client - .call(request) - .await - .map_err(|err| format!("discovery request failed: {err}"))?; + let response = match http_client.call(request).await { + Ok(response) => response, + Err(OidcHttpError::ForbiddenOutbound(reason)) => { + return Err(format!("{OIDC_DISCOVERY_BLOCKED_BY_OUTBOUND_POLICY}: {reason}")); + } + Err(err) => return Err(format!("discovery request failed: {err}")), + }; if response.status() != http::StatusCode::OK { return Err(format!("discovery failed: HTTP status code {} at {}", response.status(), discovery_url)); } @@ -1747,9 +1811,13 @@ impl OidcSys { } let jwks_url = jwks_url_from_config_url(&config.config_url, &issuer_url, provider_metadata.jwks_uri())?; - let jwks = CoreJsonWebKeySet::fetch_async(&jwks_url, http_client) - .await - .map_err(|err| format!("failed to fetch JWKS: {err}"))?; + let jwks = match CoreJsonWebKeySet::fetch_async(&jwks_url, http_client).await { + Ok(jwks) => jwks, + Err(DiscoveryError::Request(OidcHttpError::ForbiddenOutbound(reason))) => { + return Err(format!("JWKS request blocked by outbound policy: {reason}")); + } + Err(err) => return Err(format!("failed to fetch JWKS: {err}")), + }; Ok(ProviderState { metadata: provider_metadata.set_jwks(jwks), @@ -2046,6 +2114,25 @@ pub(crate) fn test_config(id: &str) -> OidcProviderConfig { #[cfg(test)] mod tests { use super::*; + use rustfs_utils::egress::OutboundDnsPolicyRejection; + use std::sync::atomic::{AtomicUsize, Ordering}; + + #[derive(Clone)] + struct RejectingDnsResolver { + allow_origin_can_recover: bool, + calls: Option>, + } + + impl reqwest::dns::Resolve for RejectingDnsResolver { + fn resolve(&self, name: reqwest::dns::Name) -> reqwest::dns::Resolving { + if let Some(calls) = &self.calls { + calls.fetch_add(1, Ordering::Relaxed); + } + let host = name.as_str().to_string(); + let rejection = OutboundDnsPolicyRejection::new(host, self.allow_origin_can_recover); + Box::pin(async move { Err(std::io::Error::new(std::io::ErrorKind::PermissionDenied, rejection).into()) }) + } + } #[test] fn test_extract_string_claim() { @@ -2853,6 +2940,32 @@ mod tests { assert!(sys.list_providers().is_empty()); } + #[test] + fn build_oidc_http_client_rejects_forbidden_targets_without_allowlist() { + // Cloud metadata endpoint is never allowed. + assert!( + matches!( + build_oidc_http_client("http://169.254.169.254/latest/meta-data/", None, None), + Err(OidcHttpError::ForbiddenOutbound(_)) + ), + "metadata endpoint must be rejected" + ); + // Loopback is rejected by default (no allow-origins configured). + assert!( + matches!( + build_oidc_http_client("http://127.0.0.1:8080/.well-known/openid-configuration", None, None), + Err(OidcHttpError::ForbiddenOutbound(_)) + ), + "loopback must be rejected by default" + ); + // A public hostname passes the up-front shape/host check; the resolved IP is still + // re-classified at connection time by the pinned resolver. + assert!( + build_oidc_http_client("https://accounts.example.com/.well-known/openid-configuration", None, None).is_ok(), + "public https endpoint should build" + ); + } + #[test] fn test_should_bypass_proxy_for_oidc_uri_loopback_only() { assert!(should_bypass_proxy_for_oidc_uri("http://127.0.0.1:9000/.well-known/openid-configuration")); @@ -2864,49 +2977,227 @@ mod tests { assert!(!should_bypass_proxy_for_oidc_uri("not-a-url")); } - #[test] - fn build_oidc_http_client_rejects_forbidden_targets_without_allowlist() { - // Cloud metadata endpoint is never allowed. - assert!( - matches!( - build_oidc_http_client("http://169.254.169.254/latest/meta-data/", None), - Err(OidcHttpError::ForbiddenOutbound(_)) - ), - "metadata endpoint must be rejected" - ); - // Loopback is rejected by default (no allow-origins configured). - assert!( - matches!( - build_oidc_http_client("http://127.0.0.1:8080/.well-known/openid-configuration", None), - Err(OidcHttpError::ForbiddenOutbound(_)) - ), - "loopback must be rejected by default" - ); - // A public hostname passes the up-front shape/host check; the resolved IP is still - // re-classified at connection time by the pinned resolver. - assert!( - build_oidc_http_client("https://accounts.example.com/.well-known/openid-configuration", None).is_ok(), - "public https endpoint should build" - ); - } - #[test] fn build_oidc_http_client_honors_explicit_allowlist_for_loopback() { let policy = OutboundPolicy::from_allowed_origins("http://127.0.0.1:8080").expect("origin should parse"); assert!( - build_oidc_http_client("http://127.0.0.1:8080/.well-known/openid-configuration", Some(&policy)).is_ok(), + build_oidc_http_client("http://127.0.0.1:8080/.well-known/openid-configuration", Some(&policy), None).is_ok(), "explicitly allow-listed loopback origin should build" ); // A metadata endpoint stays forbidden even when a loopback origin is allow-listed. assert!( matches!( - build_oidc_http_client("http://169.254.169.254/", Some(&policy)), + build_oidc_http_client("http://169.254.169.254/", Some(&policy), None), Err(OidcHttpError::ForbiddenOutbound(_)) ), "metadata endpoint stays forbidden despite an unrelated allow-list entry" ); } + #[tokio::test] + async fn oidc_discovery_reports_forbidden_outbound_without_retrying() { + let config_url = "http://192.168.65.254:8080/realms/rustfs/.well-known/openid-configuration"; + let config = build_mocked_oidc_provider_config("default", config_url); + let http_client = ReqwestHttpClient::with_policy(OutboundPolicy::default()); + + let error = match OidcSys::discover_provider(&config, &http_client).await { + Ok(_) => panic!("private OIDC provider should require an explicit allowlist origin"), + Err(error) => error, + }; + + assert!(error.contains("OIDC provider discovery blocked by outbound policy")); + assert!(error.contains(&format!("add http://192.168.65.254:8080 to {ENV_OUTBOUND_ALLOW_ORIGINS}"))); + assert!(!error.contains("discovery failed for all issuer variants")); + } + + #[tokio::test] + async fn oidc_explicit_issuer_reports_forbidden_discovery_endpoint() { + let config_url = "http://192.168.65.254:8080/realms/rustfs/.well-known/openid-configuration"; + let mut config = build_mocked_oidc_provider_config("default", config_url); + config.issuer = Some("https://idp.example.com/realms/rustfs".to_string()); + let http_client = ReqwestHttpClient::with_policy(OutboundPolicy::default()); + + let error = match OidcSys::discover_provider(&config, &http_client).await { + Ok(_) => panic!("private OIDC provider should require an explicit allowlist origin"), + Err(error) => error, + }; + + assert!(error.contains("OIDC provider discovery blocked by outbound policy")); + assert!(error.contains(&format!("add http://192.168.65.254:8080 to {ENV_OUTBOUND_ALLOW_ORIGINS}"))); + } + + #[tokio::test] + async fn oidc_explicit_issuer_reports_forbidden_jwks_endpoint() { + let Some((base, handle)) = start_mock_oidc_discovery_server( + |base| { + ( + format!("{base}/realms/rustfs"), + "http://192.168.65.254:8080/realms/rustfs/protocol/openid-connect/certs".to_string(), + "/unused".to_string(), + ) + }, + 1, + ) else { + return; + }; + let mut config = + build_mocked_oidc_provider_config("default", &format!("{base}/realms/rustfs/.well-known/openid-configuration")); + config.issuer = Some(format!("{base}/realms/rustfs")); + let policy = OutboundPolicy::from_allowed_origins(&base).expect("loopback discovery origin should be allowed"); + let http_client = ReqwestHttpClient::with_policy(policy); + + let error = match OidcSys::discover_provider(&config, &http_client).await { + Ok(_) => panic!("private JWKS endpoint should require an explicit allowlist origin"), + Err(error) => error, + }; + + assert!(error.contains("JWKS request blocked by outbound policy")); + assert!(error.contains(&format!("add http://192.168.65.254:8080 to {ENV_OUTBOUND_ALLOW_ORIGINS}"))); + assert!(handle.join().is_ok()); + } + + #[tokio::test] + async fn oidc_token_exchange_reports_forbidden_token_endpoint() { + let provider_id = "default"; + let token_endpoint = "http://192.168.65.254:8080/realms/rustfs/protocol/openid-connect/token"; + let metadata = serde_json::from_value::(serde_json::json!({ + "issuer": "https://idp.example.com/realms/rustfs", + "authorization_endpoint": "https://idp.example.com/realms/rustfs/protocol/openid-connect/auth", + "token_endpoint": token_endpoint, + "jwks_uri": "https://idp.example.com/realms/rustfs/protocol/openid-connect/certs", + "response_types_supported": ["code"], + "subject_types_supported": ["public"], + "id_token_signing_alg_values_supported": ["RS256"] + })) + .expect("provider metadata should parse"); + let config = build_mocked_oidc_provider_config( + provider_id, + "https://idp.example.com/realms/rustfs/.well-known/openid-configuration", + ); + let state_store = OidcStateStore::new(); + state_store + .insert( + "test-state".to_string(), + OidcAuthSession { + provider_id: provider_id.to_string(), + pkce_verifier: "test-pkce-verifier".to_string(), + nonce: "test-nonce".to_string(), + redirect_after: None, + }, + ) + .await; + let sys = OidcSys { + configs: HashMap::from([(provider_id.to_string(), config)]), + provider_states: RwLock::new(HashMap::from([( + provider_id.to_string(), + ProviderState { + metadata, + discovered_at: Instant::now(), + }, + )])), + state_store, + http_client: ReqwestHttpClient::with_policy(OutboundPolicy::default()), + }; + + let error = match sys + .exchange_code("test-state", "test-code", "https://console.example.com/oauth_callback") + .await + { + Ok(_) => panic!("private token endpoint should require an explicit allowlist origin"), + Err(error) => error, + }; + + assert!(error.contains("request_error_kind=forbidden_outbound")); + assert!(error.contains(&format!("add http://192.168.65.254:8080 to {ENV_OUTBOUND_ALLOW_ORIGINS}"))); + } + + #[tokio::test] + async fn oidc_reqwest_dns_policy_rejection_stays_typed() { + let calls = Arc::new(AtomicUsize::new(0)); + let config = build_mocked_oidc_provider_config( + "default", + "http://keycloak.internal:8080/realms/rustfs/.well-known/openid-configuration", + ); + let http_client = ReqwestHttpClient::with_policy_and_dns_resolver( + OutboundPolicy::default(), + Arc::new(RejectingDnsResolver { + allow_origin_can_recover: true, + calls: Some(calls.clone()), + }), + ); + + let error = match OidcSys::discover_provider(&config, &http_client).await { + Ok(_) => panic!("private DNS answer should fail discovery"), + Err(error) => error, + }; + + assert!(error.contains("OIDC provider discovery blocked by outbound policy")); + assert!(error.contains(&format!("add http://keycloak.internal:8080 to {ENV_OUTBOUND_ALLOW_ORIGINS}"))); + assert_eq!(calls.load(Ordering::Relaxed), 1, "policy rejection must not be retried"); + } + + #[tokio::test] + async fn oidc_explicit_issuer_preserves_dns_policy_rejection() { + let calls = Arc::new(AtomicUsize::new(0)); + let mut config = build_mocked_oidc_provider_config( + "default", + "http://keycloak.internal:8080/realms/rustfs/.well-known/openid-configuration", + ); + config.issuer = Some("https://idp.example.com/realms/rustfs".to_string()); + let http_client = ReqwestHttpClient::with_policy_and_dns_resolver( + OutboundPolicy::default(), + Arc::new(RejectingDnsResolver { + allow_origin_can_recover: true, + calls: Some(calls.clone()), + }), + ); + + let error = match OidcSys::discover_provider(&config, &http_client).await { + Ok(_) => panic!("private DNS answer should fail discovery"), + Err(error) => error, + }; + + assert!(error.contains("OIDC provider discovery blocked by outbound policy")); + assert!(error.contains(&format!("add http://keycloak.internal:8080 to {ENV_OUTBOUND_ALLOW_ORIGINS}"))); + assert_eq!(calls.load(Ordering::Relaxed), 1, "policy rejection must not be retried"); + } + + #[tokio::test] + async fn oidc_nonrecoverable_dns_policy_rejection_omits_allowlist_hint() { + let uri = "http://metadata.internal/latest/meta-data"; + let http_client = ReqwestHttpClient::with_policy_and_dns_resolver( + OutboundPolicy::default(), + Arc::new(RejectingDnsResolver { + allow_origin_can_recover: false, + calls: None, + }), + ); + let request = http::Request::builder() + .uri(uri) + .body(Vec::new()) + .expect("request should build"); + + let error = http_client + .call(request) + .await + .expect_err("metadata DNS answer should be rejected"); + let message = error.to_string(); + + assert!(matches!(error, OidcHttpError::ForbiddenOutbound(_))); + assert!(message.contains("metadata.internal")); + assert!(!message.contains(&format!("add http://metadata.internal to {ENV_OUTBOUND_ALLOW_ORIGINS}"))); + } + + #[test] + fn oidc_metadata_endpoint_rejection_does_not_offer_allowlist_bypass() { + let error = build_oidc_http_client("http://169.254.169.254/latest/meta-data/", Some(&OutboundPolicy::default()), None) + .expect_err("metadata endpoint must remain forbidden"); + let message = error.to_string(); + + assert!(message.contains("metadata endpoint")); + assert!(!message.contains(&format!("add http://169.254.169.254 to {ENV_OUTBOUND_ALLOW_ORIGINS}"))); + } + /// Serve exactly `body_len` bytes with no `Content-Length`, so the body ends only at EOF /// and the size guard cannot rely on an advertised length. fn start_unbounded_body_server(body_len: usize) -> Option<(String, std::thread::JoinHandle<()>)> { diff --git a/crates/utils/src/egress.rs b/crates/utils/src/egress.rs index 810eb57c3..8261d4f63 100644 --- a/crates/utils/src/egress.rs +++ b/crates/utils/src/egress.rs @@ -82,6 +82,59 @@ impl fmt::Display for OutboundPolicyError { impl std::error::Error for OutboundPolicyError {} +/// A DNS answer was rejected at the connection boundary because every resolved +/// address was forbidden by the outbound policy. +#[derive(Debug)] +pub struct OutboundDnsPolicyRejection { + host: String, + allow_origin_can_recover: bool, +} + +impl OutboundDnsPolicyRejection { + /// Records the rejected host and whether an exact operator allowlist origin + /// could permit at least one of its resolved addresses. + pub fn new(host: String, allow_origin_can_recover: bool) -> Self { + Self { + host, + allow_origin_can_recover, + } + } + + /// Whether an exact allowlist origin could permit at least one rejected address. + pub fn allow_origin_can_recover(&self) -> bool { + self.allow_origin_can_recover + } +} + +impl fmt::Display for OutboundDnsPolicyRejection { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "outbound DNS resolution for '{}' returned no allowed addresses", self.host) + } +} + +impl std::error::Error for OutboundDnsPolicyRejection {} + +/// Finds a typed DNS policy rejection through transport error wrappers. +pub fn find_outbound_dns_policy_rejection<'a>( + error: &'a (dyn std::error::Error + 'static), +) -> Option<&'a OutboundDnsPolicyRejection> { + let mut current = Some(error); + while let Some(error) = current { + if let Some(rejection) = error.downcast_ref::() { + return Some(rejection); + } + if let Some(rejection) = error + .downcast_ref::() + .and_then(std::io::Error::get_ref) + .and_then(|inner| inner.downcast_ref::()) + { + return Some(rejection); + } + current = error.source(); + } + None +} + #[derive(Debug, Default, PartialEq, Eq)] pub struct OutboundPolicy { allowed_restricted_origins: HashSet, @@ -226,14 +279,29 @@ impl reqwest::dns::Resolve for OutboundDnsResolver { .map_err(|err| std::io::Error::new(std::io::ErrorKind::NotFound, err))? .collect() }; + if addresses.is_empty() { + return Err(std::io::Error::new( + std::io::ErrorKind::NotFound, + format!("DNS resolution for '{host}' returned no addresses"), + ) + .into()); + } + let mut allow_origin_can_recover = false; let addrs = addresses .into_iter() - .filter(|address| resolved_ip_allowed(address.ip(), allow_restricted)) + .filter(|address| match validate_policy_ip(address.ip()) { + Ok(()) => true, + Err(reason) => { + let recoverable = restricted_reason_can_be_overridden(reason); + allow_origin_can_recover |= recoverable; + allow_restricted && recoverable + } + }) .collect::>(); if addrs.is_empty() { return Err(std::io::Error::new( std::io::ErrorKind::PermissionDenied, - format!("outbound DNS resolution for '{host}' returned no allowed addresses"), + OutboundDnsPolicyRejection::new(host, allow_origin_can_recover), ) .into()); } @@ -302,13 +370,6 @@ fn restricted_reason_can_be_overridden(reason: &str) -> bool { ) } -fn resolved_ip_allowed(ip: IpAddr, allow_restricted: bool) -> bool { - match validate_policy_ip(ip) { - Ok(()) => true, - Err(reason) => allow_restricted && restricted_reason_can_be_overridden(reason), - } -} - fn validate_policy_ip(ip: IpAddr) -> Result<(), &'static str> { if is_metadata_endpoint(ip) { return Err("metadata endpoint"); @@ -488,7 +549,7 @@ fn embedded_ipv4(v6: Ipv6Addr) -> Option { #[cfg(test)] mod tests { - use super::{OutboundPolicy, OutboundUrlError, validate_outbound_url}; + use super::{OutboundPolicy, OutboundUrlError, find_outbound_dns_policy_rejection, validate_outbound_url}; use std::collections::HashMap; use std::net::SocketAddr; use url::Url; @@ -835,12 +896,38 @@ mod tests { .map(|addr| addr.ip()) .collect::>(); assert_eq!(addrs, vec!["8.8.8.8".parse::().expect("public IP")]); - assert!( - reqwest::dns::Resolve::resolve(&resolver, "rebound.test".parse().expect("resolver hostname")) - .await - .is_err(), - "a rebound answer containing only restricted addresses must fail closed" - ); + let error = match reqwest::dns::Resolve::resolve(&resolver, "rebound.test".parse().expect("resolver hostname")).await { + Ok(_) => panic!("a rebound answer containing only restricted addresses must fail closed"), + Err(error) => error, + }; + let io_error = error + .downcast_ref::() + .expect("resolver must preserve its PermissionDenied root error"); + assert_eq!(io_error.kind(), std::io::ErrorKind::PermissionDenied); + let rejection = + find_outbound_dns_policy_rejection(&*error).expect("typed policy rejection must remain in the error chain"); + assert_eq!(rejection.host, "rebound.test"); + assert!(rejection.allow_origin_can_recover()); + } + + #[tokio::test] + async fn outbound_dns_resolver_does_not_classify_empty_answers_as_policy_rejections() { + let endpoint = Url::parse("https://empty.test/hook").expect("endpoint should parse"); + let resolver = OutboundPolicy::default() + .resolver_for(&endpoint) + .expect("public hostname should be accepted") + .with_overrides(HashMap::from([("empty.test".to_string(), Vec::new())])); + + let error = match reqwest::dns::Resolve::resolve(&resolver, "empty.test".parse().expect("resolver hostname")).await { + Ok(_) => panic!("an empty DNS answer must fail"), + Err(error) => error, + }; + let io_error = error + .downcast_ref::() + .expect("resolver errors must remain I/O errors"); + + assert_eq!(io_error.kind(), std::io::ErrorKind::NotFound); + assert!(find_outbound_dns_policy_rejection(&*error).is_none()); } #[tokio::test] @@ -976,4 +1063,73 @@ mod tests { "request must fail in the DNS policy layer: {error_chain:?}" ); } + + #[tokio::test] + async fn reqwest_preserves_recoverable_private_dns_policy_rejection() { + let endpoint = Url::parse("http://keycloak.internal:8080/realms/rustfs").expect("endpoint should parse"); + let resolver = OutboundPolicy::default() + .resolver_for(&endpoint) + .expect("hostname should pass the URL-shape check") + .with_overrides(HashMap::from([( + "keycloak.internal".to_string(), + vec![ + "10.96.0.20".parse().expect("private IP"), + "169.254.169.254".parse().expect("metadata IP"), + ], + )])); + let client = reqwest::Client::builder() + .no_proxy() + .dns_resolver(resolver) + .timeout(std::time::Duration::from_secs(2)) + .build() + .expect("test client should build"); + + let error = client + .get(endpoint) + .send() + .await + .expect_err("private DNS answer should be rejected before connecting"); + let rejection = find_outbound_dns_policy_rejection(&error).expect("typed DNS policy rejection should be preserved"); + let mut current: Option<&(dyn std::error::Error + 'static)> = Some(&error); + let mut io_kind = None; + while let Some(source) = current { + if let Some(io_error) = source.downcast_ref::() { + io_kind = Some(io_error.kind()); + break; + } + current = source.source(); + } + + assert_eq!(rejection.host, "keycloak.internal"); + assert!(rejection.allow_origin_can_recover()); + assert_eq!(io_kind, Some(std::io::ErrorKind::PermissionDenied)); + } + + #[tokio::test] + async fn reqwest_preserves_nonrecoverable_metadata_dns_policy_rejection() { + let endpoint = Url::parse("http://metadata.internal/latest").expect("endpoint should parse"); + let resolver = OutboundPolicy::default() + .resolver_for(&endpoint) + .expect("hostname should pass the URL-shape check") + .with_overrides(HashMap::from([( + "metadata.internal".to_string(), + vec!["169.254.169.254".parse().expect("metadata IP")], + )])); + let client = reqwest::Client::builder() + .no_proxy() + .dns_resolver(resolver) + .timeout(std::time::Duration::from_secs(2)) + .build() + .expect("test client should build"); + + let error = client + .get(endpoint) + .send() + .await + .expect_err("metadata DNS answer should be rejected before connecting"); + let rejection = find_outbound_dns_policy_rejection(&error).expect("typed DNS policy rejection should be preserved"); + + assert_eq!(rejection.host, "metadata.internal"); + assert!(!rejection.allow_origin_can_recover()); + } } diff --git a/docs/operations/keycloak-oidc-integration.md b/docs/operations/keycloak-oidc-integration.md index f26edfe95..829ec3f57 100644 --- a/docs/operations/keycloak-oidc-integration.md +++ b/docs/operations/keycloak-oidc-integration.md @@ -154,9 +154,11 @@ If RustFS reaches Keycloak through an internal URL while tokens use a public iss ```bash export RUSTFS_IDENTITY_OPENID_CONFIG_URL="http://keycloak.keycloak.svc.cluster.local:8080/realms/rustfs/.well-known/openid-configuration" export RUSTFS_IDENTITY_OPENID_ISSUER="https://keycloak.example.com/realms/rustfs" +export RUSTFS_OUTBOUND_ALLOW_ORIGINS="http://keycloak.keycloak.svc.cluster.local:8080" ``` Discovery and issuer-relative JWKS requests use the internal `CONFIG_URL` base. ID token issuer validation still uses `ISSUER`. +The outbound allowlist entry is the exact internal origin only; do not include the realm or discovery path. RustFS reads this process setting at startup, so restart every RustFS node after changing it. Use HTTPS with a trusted CA for the internal URL whenever possible. Discovery and JWKS define the token-signing trust root; use HTTP only on a network where DNS and traffic cannot be tampered with, because a compromised response can authorize forged tokens. For short-lived connectivity testing only, you may temporarily add: @@ -287,6 +289,7 @@ Expected flow: | Groups appear as `/consoleAdmin` | Keycloak `Full group path` is enabled | Disable `Full group path`. | | Console redirects to an internal host | Missing `RUSTFS_BROWSER_REDIRECT_URL` or incorrect proxy headers | Set `RUSTFS_BROWSER_REDIRECT_URL` to the public browser origin. | | Invalid or expired OIDC state | Callback reached a different RustFS node | Configure load-balancer session affinity for authorize and callback requests. | +| OIDC provider or login button is missing after upgrading to beta.12+ | The internal Keycloak origin is blocked by the outbound policy | Add the exact `scheme://host:port` origin to `RUSTFS_OUTBOUND_ALLOW_ORIGINS` and restart every RustFS node. | ## 7. Production Checklist @@ -299,3 +302,4 @@ Expected flow: - [ ] `role_policy=consoleAdmin` is not used as a permanent production shortcut. - [ ] The load balancer preserves query strings. - [ ] OIDC authorize and callback requests have session affinity to the same RustFS node. +- [ ] Internal Keycloak origins are listed exactly in `RUSTFS_OUTBOUND_ALLOW_ORIGINS` on every RustFS node. diff --git a/docs/operations/outbound-connection-policy.md b/docs/operations/outbound-connection-policy.md index d64770149..4494ba1ee 100644 --- a/docs/operations/outbound-connection-policy.md +++ b/docs/operations/outbound-connection-policy.md @@ -4,10 +4,11 @@ This document describes the outbound connection policy that RustFS applies to server-initiated HTTP(S) requests, and the `RUSTFS_OUTBOUND_ALLOW_ORIGINS` allowlist operators can use to reach endpoints on private or container networks. -It is written for operators who upgraded to `1.0.0-beta.11` (or later) and found -that event-notification webhooks, audit webhooks, or other outbound integrations -stopped reaching endpoints that worked before — typically Docker Compose service -names, `host.docker.internal`, or RFC 1918 addresses. +It is written for operators whose outbound integrations stopped reaching +endpoints after an upgrade — typically Docker Compose service names, +`host.docker.internal`, or RFC 1918 addresses. Webhook and audit clients adopted +this policy in `1.0.0-beta.11`; OIDC provider requests adopted it in +`1.0.0-beta.12`. ## Background: what the policy protects @@ -21,14 +22,16 @@ The policy governs the outbound clients used by: - event-notification webhooks (`RUSTFS_NOTIFY_WEBHOOK_*`); - audit webhooks (`RUSTFS_AUDIT_WEBHOOK_*`); -- OIDC identity-provider requests; +- OIDC identity-provider discovery, JWKS, and token requests (since `1.0.0-beta.12`); - S3 tiering (warm-backend) endpoints; - Keystone auth URLs. The webhook and audit outbound clients also **disable proxies and do not follow redirects**, so the destination must be reachable directly at the configured URL. -## What changed in beta.11 +## What changed in beta.11 (and for OIDC in beta.12) + +For webhook and audit clients: | | beta.10 | beta.11+ | |---|---|---| @@ -43,6 +46,11 @@ exact origin is on the allowlist. This is why a Compose setup that delivered events on beta.10 can go silent after the upgrade even though the configuration is unchanged. +OIDC joined the same policy in beta.12. An internal identity provider that +worked in beta.11 can therefore fail discovery after upgrading to beta.12 unless +its exact origin is allowlisted. The policy remains active for discovery, JWKS, +and token requests. + ## Symptoms - Bucket event rules and webhook configuration look correct. @@ -52,6 +60,9 @@ is unchanged. a loopback, private, shared, or reserved address. - Startup or target validation reports `webhook endpoint is not allowed: ...` with a reason such as `private address` or `loopback host`. +- An OIDC provider or login button is missing, and startup reports + `OIDC provider discovery blocked by outbound policy` with the exact origin to + allowlist. ## `RUSTFS_OUTBOUND_ALLOW_ORIGINS` @@ -127,7 +138,7 @@ services: The endpoint keeps its full path (`/events`); the allowlist entry is the origin (`http://logstash:8080`) only. -## Upgrade checklist (beta.10 → beta.11+) +## Upgrade checklist (beta.10 → beta.11+, or OIDC beta.11 → beta.12+) 1. List every outbound endpoint whose hostname resolves to a loopback, private, shared, or reserved address: notification webhooks, audit webhooks, OIDC