From 2f6115e0581423b031ed84f45e459cff91065d71 Mon Sep 17 00:00:00 2001 From: cxymds Date: Tue, 28 Jul 2026 17:04:58 +0800 Subject: [PATCH] fix(swift): enforce TempURL IP restrictions (#5377) --- Cargo.lock | 2 + crates/protocols/Cargo.toml | 4 + crates/protocols/src/swift/handler.rs | 55 +++++++- crates/protocols/src/swift/tempurl.rs | 174 +++++++++++++++++++++++--- 4 files changed, 215 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6086ee54c..920133174 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9714,6 +9714,7 @@ dependencies = [ "http-body-util", "hyper", "hyper-util", + "ipnetwork", "libunftp", "md5", "percent-encoding", @@ -9731,6 +9732,7 @@ dependencies = [ "rustfs-rio", "rustfs-storage-api", "rustfs-tls-runtime", + "rustfs-trusted-proxies", "rustfs-utils", "rustls", "s3s", diff --git a/crates/protocols/Cargo.toml b/crates/protocols/Cargo.toml index 2323a1d0e..d9ea48b41 100644 --- a/crates/protocols/Cargo.toml +++ b/crates/protocols/Cargo.toml @@ -49,6 +49,8 @@ swift = [ "dep:hmac", "dep:sha1", "dep:hex", + "dep:ipnetwork", + "dep:rustfs-trusted-proxies", "dep:astral-tokio-tar", "dep:base64", "dep:async-compression", @@ -111,6 +113,8 @@ quick-xml = { workspace = true, optional = true, features = ["serialize"] } hmac = { workspace = true, optional = true } sha1 = { workspace = true, optional = true } hex = { workspace = true, optional = true } +ipnetwork = { workspace = true, optional = true } +rustfs-trusted-proxies = { workspace = true, optional = true } astral-tokio-tar = { workspace = true, optional = true } base64 = { workspace = true, optional = true } async-compression = { workspace = true, optional = true, features = ["tokio", "gzip", "bzip2"] } diff --git a/crates/protocols/src/swift/handler.rs b/crates/protocols/src/swift/handler.rs index 4ad9294de..7e9f02983 100644 --- a/crates/protocols/src/swift/handler.rs +++ b/crates/protocols/src/swift/handler.rs @@ -28,7 +28,9 @@ use axum::http::{Method, Request, Response, StatusCode}; use futures::Future; use rustfs_credentials::Credentials; use rustfs_keystone::KEYSTONE_CREDENTIALS; +use rustfs_trusted_proxies::ClientInfo; use s3s::Body; +use std::net::{IpAddr, SocketAddr}; use std::pin::Pin; use std::task::{Context, Poll}; use tokio_util::io::StreamReader; @@ -40,6 +42,14 @@ const LOG_SUBSYSTEM_SWIFT_HANDLER: &str = "swift_handler"; const EVENT_SWIFT_ROUTE_STATE: &str = "swift_route_state"; const EVENT_SWIFT_TEMPURL_STATE: &str = "swift_tempurl_state"; +fn trusted_client_ip(req: &Request) -> Option { + req.extensions() + .get::() + .map(|info| info.real_ip) + .or_else(|| req.extensions().get::().map(SocketAddr::ip)) + .filter(|ip| !ip.is_unspecified()) +} + /// Swift-aware service that routes to Swift handlers or S3 service #[derive(Clone)] pub struct SwiftService { @@ -128,6 +138,7 @@ async fn handle_swift_request( route: SwiftRoute, credentials: Option, ) -> Result, SwiftError> { + let client_ip = trusted_client_ip(&req); // Extract parts let (parts, body) = req.into_parts(); let method = parts.method.clone(); @@ -164,7 +175,7 @@ async fn handle_swift_request( let tempurl = tempurl::TempURL::new(key); let path = uri.path(); - tempurl.validate_request(method.as_str(), path, &tempurl_params)?; + tempurl.validate_request(method.as_str(), path, &tempurl_params, client_ip)?; // TempURL is valid - proceed with request (no credentials needed) debug!( @@ -1497,7 +1508,10 @@ fn swift_error_to_response(error: SwiftError) -> Response { #[cfg(test)] mod tests { - use super::parse_range_header; + use super::{parse_range_header, trusted_client_ip}; + use axum::http::Request; + use rustfs_trusted_proxies::ClientInfo; + use std::net::SocketAddr; #[test] fn test_parse_range_header_start_end() { // bytes=100-199 @@ -1588,4 +1602,41 @@ mod tests { let result = parse_range_header("bytes=0-999", 1000); assert_eq!(result, Some((0, 999))); } + + #[test] + fn trusted_client_ip_ignores_untrusted_forwarded_headers() { + let mut request = Request::builder() + .header("x-forwarded-for", "198.51.100.9") + .header("x-real-ip", "198.51.100.10") + .body(()) + .expect("request"); + request + .extensions_mut() + .insert("192.0.2.7:9000".parse::().expect("socket address")); + + assert_eq!(trusted_client_ip(&request), Some("192.0.2.7".parse().expect("peer IP address"))); + } + + #[test] + fn trusted_client_ip_prefers_validated_proxy_identity() { + let mut request = Request::new(()); + request + .extensions_mut() + .insert("192.0.2.7:9000".parse::().expect("socket address")); + request.extensions_mut().insert(ClientInfo::direct( + "203.0.113.8:443".parse::().expect("validated client address"), + )); + + assert_eq!(trusted_client_ip(&request), Some("203.0.113.8".parse().expect("validated client IP"))); + } + + #[test] + fn trusted_client_ip_rejects_unspecified_fallback() { + let mut request = Request::new(()); + request + .extensions_mut() + .insert(ClientInfo::direct("0.0.0.0:0".parse::().expect("unspecified fallback"))); + + assert_eq!(trusted_client_ip(&request), None); + } } diff --git a/crates/protocols/src/swift/tempurl.rs b/crates/protocols/src/swift/tempurl.rs index 8c96b17c2..2c0e7fbad 100644 --- a/crates/protocols/src/swift/tempurl.rs +++ b/crates/protocols/src/swift/tempurl.rs @@ -21,7 +21,11 @@ use crate::swift::errors::SwiftError; use hmac::{Hmac, KeyInit, Mac}; +use ipnetwork::IpNetwork; +use percent_encoding::percent_decode_str; use sha1::Sha1; +use std::net::IpAddr; +use std::str::FromStr; use std::time::{SystemTime, UNIX_EPOCH}; type HmacSha1 = Hmac; @@ -50,12 +54,12 @@ impl TempURLParams { let mut ip_range = None; for param in query.split('&') { - let parts: Vec<&str> = param.split('=').collect(); - if parts.len() == 2 { - match parts[0] { - "temp_url_sig" => sig = Some(parts[1].to_string()), - "temp_url_expires" => expires = parts[1].parse().ok(), - "temp_url_ip_range" => ip_range = Some(parts[1].to_string()), + if let Some((name, value)) = param.split_once('=') { + let value = percent_decode_str(value).decode_utf8().ok()?; + match name { + "temp_url_sig" => sig = Some(value.into_owned()), + "temp_url_expires" => expires = value.parse().ok(), + "temp_url_ip_range" => ip_range = Some(value.into_owned()), _ => {} } } @@ -97,9 +101,22 @@ impl TempURL { /// # Returns /// Hex-encoded HMAC-SHA1 signature pub fn generate_signature(&self, method: &str, expires: u64, path: &str) -> Result { + self.generate_signature_with_ip_range(method, expires, path, None) + } + + /// Generate a TempURL signature, optionally binding it to an IP range. + pub fn generate_signature_with_ip_range( + &self, + method: &str, + expires: u64, + path: &str, + ip_range: Option<&str>, + ) -> Result { // Construct message for HMAC - // Format: "{METHOD}\n{expires}\n{path}" - let message = format!("{}\n{}\n{}", method.to_uppercase(), expires, path); + let message = match ip_range { + Some(ip_range) => format!("ip={}\n{}\n{}\n{}", ip_range, method.to_uppercase(), expires, path), + None => format!("{}\n{}\n{}", method.to_uppercase(), expires, path), + }; // Calculate HMAC-SHA1 let mut mac = HmacSha1::new_from_slice(self.key.as_bytes()) @@ -127,7 +144,13 @@ impl TempURL { /// # Returns /// - `Ok(())` if signature is valid and not expired /// - `Err(SwiftError::Unauthorized)` if invalid or expired - pub fn validate_request(&self, method: &str, path: &str, params: &TempURLParams) -> Result<(), SwiftError> { + pub fn validate_request( + &self, + method: &str, + path: &str, + params: &TempURLParams, + client_ip: Option, + ) -> Result<(), SwiftError> { // 1. Check expiration first (fast path for expired URLs) let now = SystemTime::now() .duration_since(UNIX_EPOCH) @@ -139,17 +162,25 @@ impl TempURL { } // 2. Generate expected signature - let expected_sig = self.generate_signature(method, params.temp_url_expires, path)?; + let expected_sig = + self.generate_signature_with_ip_range(method, params.temp_url_expires, path, params.temp_url_ip_range.as_deref())?; // 3. Constant-time comparison to prevent timing attacks if !constant_time_compare(params.temp_url_sig.as_bytes(), expected_sig.as_bytes()) { return Err(SwiftError::Unauthorized("Invalid TempURL signature".to_string())); } - // 4. TODO: Validate IP range if specified (future enhancement) - // if let Some(ip_range) = ¶ms.temp_url_ip_range { - // validate_ip_range(client_ip, ip_range)?; - // } + if let Some(ip_range) = ¶ms.temp_url_ip_range { + let client_ip = + client_ip.ok_or_else(|| SwiftError::Unauthorized("Trusted client address unavailable".to_string()))?; + let allowed = IpAddr::from_str(ip_range) + .map(|ip| ip == client_ip) + .or_else(|_| IpNetwork::from_str(ip_range).map(|network| network.contains(client_ip))) + .map_err(|_| SwiftError::Unauthorized("Invalid TempURL IP range".to_string()))?; + if !allowed { + return Err(SwiftError::Unauthorized("Client address is outside the TempURL IP range".to_string())); + } + } Ok(()) } @@ -308,7 +339,7 @@ mod tests { // Should validate successfully assert!( tempurl - .validate_request("GET", "/v1/AUTH_test/container/object", ¶ms) + .validate_request("GET", "/v1/AUTH_test/container/object", ¶ms, None) .is_ok() ); } @@ -331,7 +362,7 @@ mod tests { }; // Should reject expired URL - let result = tempurl.validate_request("GET", "/v1/AUTH_test/container/object", ¶ms); + let result = tempurl.validate_request("GET", "/v1/AUTH_test/container/object", ¶ms, None); assert!(result.is_err()); assert!(matches!(result.unwrap_err(), SwiftError::Unauthorized(_))); } @@ -349,7 +380,7 @@ mod tests { }; // Should reject invalid signature - let result = tempurl.validate_request("GET", "/v1/AUTH_test/container/object", ¶ms); + let result = tempurl.validate_request("GET", "/v1/AUTH_test/container/object", ¶ms, None); assert!(result.is_err()); assert!(matches!(result.unwrap_err(), SwiftError::Unauthorized(_))); } @@ -372,7 +403,7 @@ mod tests { }; // Try to validate with PUT method - let result = tempurl.validate_request("PUT", "/v1/AUTH_test/container/object", ¶ms); + let result = tempurl.validate_request("PUT", "/v1/AUTH_test/container/object", ¶ms, None); assert!(result.is_err()); assert!(matches!(result.unwrap_err(), SwiftError::Unauthorized(_))); } @@ -423,6 +454,14 @@ mod tests { assert_eq!(params.temp_url_ip_range.as_deref(), Some("192.168.1.0/24")); } + #[test] + fn test_parse_percent_encoded_ipv6_range() { + let query = "temp_url_sig=abc123&temp_url_expires=1609459200&temp_url_ip_range=2001%3Adb8%3A%3A%2F32"; + let params = TempURLParams::from_query(query).expect("encoded IPv6 TempURL parameters"); + + assert_eq!(params.temp_url_ip_range.as_deref(), Some("2001:db8::/32")); + } + #[test] fn test_parse_tempurl_params_missing_sig() { let query = "temp_url_expires=1609459200"; @@ -475,4 +514,103 @@ mod tests { .unwrap(); assert_eq!(sig, sig2); } + + #[test] + fn ip_bound_signature_matches_openstack_message_format() { + let tempurl = TempURL::new("mykey".to_string()); + let signature = tempurl + .generate_signature_with_ip_range("GET", 1648082711, "/v1/AUTH_account/container/object", Some("1.2.3.0/24")) + .expect("IP-bound signature generation"); + + assert_eq!(signature, "8698990d811e64cff1c8a2151340874fd5bda0c5"); + } + + fn ip_bound_params(tempurl: &TempURL, ip_range: &str) -> TempURLParams { + let expires = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system clock after Unix epoch") + .as_secs() + + 3600; + let signature = tempurl + .generate_signature_with_ip_range("GET", expires, "/v1/AUTH_test/container/object", Some(ip_range)) + .expect("IP-bound signature generation"); + TempURLParams { + temp_url_sig: signature, + temp_url_expires: expires, + temp_url_ip_range: Some(ip_range.to_string()), + } + } + + #[test] + fn ip_bound_tempurl_accepts_ipv4_inside_range_and_rejects_outside() { + let tempurl = TempURL::new("mykey".to_string()); + let params = ip_bound_params(&tempurl, "192.0.2.0/24"); + + assert!( + tempurl + .validate_request( + "GET", + "/v1/AUTH_test/container/object", + ¶ms, + Some("192.0.2.42".parse().expect("IPv4 address")), + ) + .is_ok() + ); + assert!(matches!( + tempurl.validate_request( + "GET", + "/v1/AUTH_test/container/object", + ¶ms, + Some("198.51.100.42".parse().expect("IPv4 address")), + ), + Err(SwiftError::Unauthorized(_)) + )); + } + + #[test] + fn ip_bound_tempurl_accepts_ipv6_inside_range_and_rejects_outside() { + let tempurl = TempURL::new("mykey".to_string()); + let params = ip_bound_params(&tempurl, "2001:db8::/32"); + + assert!( + tempurl + .validate_request( + "GET", + "/v1/AUTH_test/container/object", + ¶ms, + Some("2001:db8::42".parse().expect("IPv6 address")), + ) + .is_ok() + ); + assert!(matches!( + tempurl.validate_request( + "GET", + "/v1/AUTH_test/container/object", + ¶ms, + Some("2001:db9::42".parse().expect("IPv6 address")), + ), + Err(SwiftError::Unauthorized(_)) + )); + } + + #[test] + fn ip_bound_tempurl_rejects_missing_client_ip_and_invalid_range() { + let tempurl = TempURL::new("mykey".to_string()); + let params = ip_bound_params(&tempurl, "192.0.2.0/24"); + assert!(matches!( + tempurl.validate_request("GET", "/v1/AUTH_test/container/object", ¶ms, None), + Err(SwiftError::Unauthorized(_)) + )); + + let invalid_params = ip_bound_params(&tempurl, "not-a-network"); + assert!(matches!( + tempurl.validate_request( + "GET", + "/v1/AUTH_test/container/object", + &invalid_params, + Some("192.0.2.42".parse().expect("IPv4 address")), + ), + Err(SwiftError::Unauthorized(_)) + )); + } }