diff --git a/CHANGELOG.md b/CHANGELOG.md index 08438ed..9cb4aae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 default), and return a clear `DevkitError::RateLimited` when the cap would be exceeded. +### Security + +- `devkit-rpc`: harden the HTTP transport against SSRF. Endpoint validation now + refuses private, link-local, and other non-public address literals, DNS + answers are filtered before connection, and only re-validated same-host + redirects are followed. The final URL is checked again before reading the + response. See `docs/NETWORK_SECURITY.md`. + ## [Unreleased] ### Added diff --git a/SECURITY.md b/SECURITY.md index 1297ce5..f74da33 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -44,7 +44,10 @@ Out of scope: ## Security model -- No network access is performed by default. No network features exist yet. +- No network access is performed by default. Network access is explicit, and + `HttpTransport` enforces a fail-closed SSRF policy: non-public addresses are + refused, DNS answers are filtered, and only same-host redirects are followed. + See [docs/NETWORK_SECURITY.md](docs/NETWORK_SECURITY.md). - Decoding is bounded; malformed input is handled with structured errors, not panics. - No secrets are read, logged, or committed. diff --git a/crates/devkit-rpc/src/lib.rs b/crates/devkit-rpc/src/lib.rs index 26c4c9b..0e8b0f7 100644 --- a/crates/devkit-rpc/src/lib.rs +++ b/crates/devkit-rpc/src/lib.rs @@ -10,19 +10,28 @@ //! * Endpoints are validated before use; plaintext `http` is only allowed for //! loopback hosts, credentials in URLs are rejected, and unknown schemes are //! refused. +//! * Remote endpoints may not use private, link-local, unspecified, multicast, +//! or loopback address literals. DNS answers are filtered through the same +//! policy before a connection is made, so a public name cannot resolve to an +//! internal address. +//! * Redirects are followed only to the original endpoint host, and every hop +//! is re-validated. All other redirects fail closed. //! * Responses are parsed with structured errors; malformed responses never //! panic. use std::cell::RefCell; use std::collections::VecDeque; use std::io::Read; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, ToSocketAddrs}; use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::Arc; use std::time::Duration; use devkit_core::DevkitError; +use reqwest::dns::{Addrs, Name, Resolve, Resolving}; use serde::{Deserialize, Serialize}; use serde_json::{json, Value}; -use url::Url; +use url::{Host, Url}; const MAX_RPC_RESPONSE_BYTES: usize = 4 * 1024 * 1024; const MAX_RPC_RESPONSE_DEPTH: usize = 64; @@ -48,36 +57,14 @@ impl Endpoint { /// `::1`), to avoid sending traffic in plaintext to a remote host. /// * Any other scheme is rejected. /// * Embedded credentials are rejected. + /// * A remote endpoint may not be an IP literal in a private, link-local, + /// unspecified, multicast, or loopback range. Loopback literals are only + /// allowed because they are the local-endpoint case. pub fn parse(input: &str) -> Result { let url = Url::parse(input.trim()) .map_err(|e| DevkitError::invalid(format!("invalid endpoint URL: {e}")))?; - match url.scheme() { - "https" => {} - "http" => { - if !is_loopback(&url) { - return Err(DevkitError::invalid( - "plaintext http is only allowed for loopback hosts; use https", - )); - } - } - other => { - return Err(DevkitError::invalid(format!( - "unsupported endpoint scheme: {other}" - ))) - } - } - - if !url.username().is_empty() || url.password().is_some() { - return Err(DevkitError::invalid( - "credentials embedded in the endpoint URL are not allowed", - )); - } - - if url.host_str().is_none() { - return Err(DevkitError::invalid("endpoint URL has no host")); - } - + validate_endpoint_url(&url)?; Ok(Endpoint { url }) } @@ -92,15 +79,196 @@ impl Endpoint { } } +fn validate_endpoint_url(url: &Url) -> Result<(), DevkitError> { + match url.scheme() { + "https" => {} + "http" => { + if !is_loopback(url) { + return Err(DevkitError::invalid( + "plaintext http is only allowed for loopback hosts; use https", + )); + } + } + other => { + return Err(DevkitError::invalid(format!( + "unsupported endpoint scheme: {other}" + ))) + } + } + + if !url.username().is_empty() || url.password().is_some() { + return Err(DevkitError::invalid( + "credentials embedded in the endpoint URL are not allowed", + )); + } + + let host = url + .host() + .ok_or_else(|| DevkitError::invalid("endpoint URL has no host"))?; + if host_is_loopback(&host) { + return Ok(()); + } + let blocked = match host { + Host::Ipv4(ip) => is_blocked_ip(IpAddr::V4(ip)), + Host::Ipv6(ip) => is_blocked_ip(IpAddr::V6(ip)), + Host::Domain(_) => false, + }; + if blocked { + return Err(DevkitError::invalid(format!( + "endpoint host `{}` is a private, link-local, or otherwise non-public address", + url.host_str().unwrap_or("") + ))); + } + Ok(()) +} + fn is_loopback(url: &Url) -> bool { - match url.host_str() { - Some("localhost") | Some("127.0.0.1") | Some("::1") | Some("[::1]") => true, - Some(host) => host - .parse::() - .map(|ip| ip.is_loopback()) - .unwrap_or(false), - None => false, + url.host().is_some_and(|host| host_is_loopback(&host)) +} + +fn host_is_loopback(host: &Host<&str>) -> bool { + match host { + Host::Domain(domain) => { + domain.eq_ignore_ascii_case("localhost") + || domain + .to_ascii_lowercase() + .strip_suffix(".localhost") + .is_some() + } + Host::Ipv4(ip) => ip.is_loopback(), + Host::Ipv6(ip) => ip_is_loopback(IpAddr::V6(*ip)), + } +} + +fn ip_is_loopback(ip: IpAddr) -> bool { + match ip { + IpAddr::V4(ip) => ip.is_loopback(), + IpAddr::V6(ip) => { + ip.is_loopback() || ip.to_ipv4_mapped().is_some_and(|ip| ip.is_loopback()) + } + } +} + +fn is_blocked_ip(ip: IpAddr) -> bool { + match ip { + IpAddr::V4(ip) => is_blocked_ipv4(ip), + IpAddr::V6(ip) => is_blocked_ipv6(ip), + } +} + +fn is_blocked_ipv4(ip: Ipv4Addr) -> bool { + let octets = ip.octets(); + ip.is_private() + || ip.is_link_local() + || ip.is_unspecified() + || ip.is_broadcast() + || ip.is_multicast() + || octets[0] == 0 + || (octets[0] == 100 && (64..=127).contains(&octets[1])) + || (octets[0] == 198 && matches!(octets[1], 18 | 19)) +} + +fn is_blocked_ipv6(ip: Ipv6Addr) -> bool { + if let Some(ip) = ip.to_ipv4_mapped() { + return is_blocked_ipv4(ip); + } + let segments = ip.segments(); + ip.is_unspecified() + || ip.is_multicast() + || (segments[0] & 0xfe00) == 0xfc00 + || (segments[0] & 0xffc0) == 0xfe80 +} + +fn resolved_ip_is_allowed(ip: IpAddr, allow_loopback: bool) -> bool { + if ip_is_loopback(ip) { + allow_loopback + } else { + !is_blocked_ip(ip) + } +} + +/// Resolves endpoint hostnames with the same address policy as URL validation. +/// +/// The guard runs before reqwest connects, so a public hostname that resolves +/// to an internal address is refused instead of being used for SSRF. +#[derive(Clone, Copy)] +struct SsrfGuardResolver { + allow_loopback: bool, +} + +impl Resolve for SsrfGuardResolver { + fn resolve(&self, name: Name) -> Resolving { + let host = name.as_str().to_owned(); + let result = resolve_guarded(&host, self.allow_loopback); + Box::pin(std::future::ready(result)) + } +} + +fn resolve_guarded( + host: &str, + allow_loopback: bool, +) -> Result> { + let addresses = (host, 0u16).to_socket_addrs().map_err(|error| { + std::io::Error::other(format!( + "failed to resolve RPC endpoint host `{host}`: {error}" + )) + })?; + let mut allowed = Vec::new(); + let mut refused = false; + for address in addresses { + if resolved_ip_is_allowed(address.ip(), allow_loopback) { + allowed.push(address); + } else { + refused = true; + } + } + if allowed.is_empty() { + let reason = if refused { + "only loopback, private, link-local, or otherwise non-public addresses" + } else { + "no addresses" + }; + return Err(Box::new(std::io::Error::other(format!( + "refusing RPC endpoint `{host}`: resolved to {reason}" + )))); + } + Ok(Box::new(allowed.into_iter()) as Addrs) +} + +fn redirect_policy(allowed_host: String) -> reqwest::redirect::Policy { + reqwest::redirect::Policy::custom(move |attempt| { + if attempt.previous().len() > 5 { + return attempt.error("too many redirects"); + } + let next = attempt.url().clone(); + if !next + .host_str() + .is_some_and(|host| host.eq_ignore_ascii_case(&allowed_host)) + { + return attempt.error(format!( + "refusing redirect to non-allowlisted host `{}`", + next.host_str().unwrap_or("") + )); + } + match validate_endpoint_url(&next) { + Ok(()) => attempt.follow(), + Err(error) => attempt.error(error.to_string()), + } + }) +} + +fn validate_final_url(url: &Url, allowed_host: &str) -> Result<(), DevkitError> { + if !url + .host_str() + .is_some_and(|host| host.eq_ignore_ascii_case(allowed_host)) + { + return Err(DevkitError::Io(format!( + "RPC transport refused a response from unexpected host `{}`", + url.host_str().unwrap_or("") + ))); } + validate_endpoint_url(url) + .map_err(|error| DevkitError::Io(format!("RPC transport refused final URL: {error}"))) } /// A JSON-RPC transport. Implementations may perform I/O; the rest of the crate @@ -165,10 +333,12 @@ impl Transport for MockTransport { /// /// Responses are capped before JSON parsing. Requests have a finite timeout, /// JSON-RPC identifiers are checked, and server error messages are returned as -/// structured errors. Redirects are disabled by the HTTP client to avoid -/// silently forwarding RPC requests to an unexpected host. +/// structured errors. Redirects are followed only to the endpoint host and are +/// re-validated at every hop; resolved addresses are filtered by the same +/// endpoint policy, so DNS cannot redirect a request to an internal network. pub struct HttpTransport { endpoint: String, + allowed_host: String, client: reqwest::blocking::Client, next_id: AtomicU64, } @@ -181,15 +351,24 @@ impl HttpTransport { "RPC timeout must be greater than zero", )); } + let allowed_host = endpoint + .url + .host_str() + .ok_or_else(|| DevkitError::invalid("endpoint URL has no host"))? + .to_owned(); let client = reqwest::blocking::Client::builder() .timeout(timeout) - .redirect(reqwest::redirect::Policy::none()) + .redirect(redirect_policy(allowed_host.clone())) + .dns_resolver(Arc::new(SsrfGuardResolver { + allow_loopback: endpoint.is_loopback(), + })) .build() .map_err(|error| { DevkitError::Io(format!("failed to build RPC HTTP client: {error}")) })?; Ok(Self { endpoint: endpoint.as_str().to_owned(), + allowed_host, client, next_id: AtomicU64::new(1), }) @@ -218,7 +397,16 @@ impl Transport for HttpTransport { .post(&self.endpoint) .json(&request) .send() - .map_err(|error| DevkitError::Io(format!("RPC HTTP request failed: {error}")))?; + .map_err(|error| { + let mut message = format!("RPC HTTP request failed: {error}"); + let mut source = std::error::Error::source(&error); + while let Some(cause) = source { + message.push_str(&format!(": {cause}")); + source = cause.source(); + } + DevkitError::Io(message) + })?; + validate_final_url(response.url(), &self.allowed_host)?; let status = response.status(); if status == reqwest::StatusCode::TOO_MANY_REQUESTS { let retry_after = response @@ -592,6 +780,52 @@ mod tests { assert!(Endpoint::parse("https://user:pass@example.com").is_err()); } + #[test] + fn rejects_private_and_link_local_ip_literals() { + for input in [ + "https://10.0.0.1", + "https://172.16.0.1", + "https://192.168.1.1", + "https://169.254.169.254", + "https://100.64.0.1", + "https://[fd00::1]", + "https://[fe80::1]", + "https://[::ffff:192.168.1.1]", + ] { + assert!( + Endpoint::parse(input).is_err(), + "{input} should be refused by the endpoint policy" + ); + } + } + + #[test] + fn allows_loopback_literals_for_local_nodes() { + assert!(Endpoint::parse("https://127.0.0.1").is_ok()); + assert!(Endpoint::parse("https://[::1]").is_ok()); + assert!(Endpoint::parse("http://localhost:8000").is_ok()); + assert!(Endpoint::parse("http://api.localhost:8000").is_ok()); + assert!(Endpoint::parse("https://[::ffff:127.0.0.1]").is_ok()); + } + + #[test] + fn resolved_addresses_follow_the_endpoint_policy() { + assert!(resolved_ip_is_allowed("127.0.0.1".parse().unwrap(), true)); + assert!(!resolved_ip_is_allowed("127.0.0.1".parse().unwrap(), false)); + assert!(!resolved_ip_is_allowed( + "192.168.0.1".parse().unwrap(), + true + )); + assert!(!resolved_ip_is_allowed( + "169.254.169.254".parse().unwrap(), + true + )); + assert!(!resolved_ip_is_allowed("fe80::1".parse().unwrap(), true)); + assert!(resolved_ip_is_allowed("1.1.1.1".parse().unwrap(), false)); + assert!(resolve_guarded("localhost", true).is_ok()); + assert!(resolve_guarded("localhost", false).is_err()); + } + #[test] fn parses_health_response() { let transport = MockTransport::with_responses(vec![json!({ @@ -744,6 +978,74 @@ mod tests { server.join().expect("test server exits"); } + #[test] + fn http_transport_follows_same_host_redirects_and_revalidates_them() { + use std::io::Write; + use std::net::TcpListener; + + let listener = TcpListener::bind("127.0.0.1:0").expect("bind test server"); + let address = listener.local_addr().expect("test server address"); + let server = std::thread::spawn(move || { + for _ in 0..2 { + let (mut stream, _) = listener.accept().expect("accept request"); + let mut request = [0u8; 1024]; + let _ = stream.read(&mut request); + if request.starts_with(b"POST /final") { + let body = r#"{"jsonrpc":"2.0","id":1,"result":{"status":"healthy"}}"#; + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\n\r\n{body}", + body.len() + ); + stream + .write_all(response.as_bytes()) + .expect("write response"); + } else { + stream + .write_all( + b"HTTP/1.1 307 Temporary Redirect\r\nLocation: /final\r\nConnection: close\r\nContent-Length: 0\r\n\r\n", + ) + .expect("write redirect"); + } + } + }); + + let endpoint = Endpoint::parse(&format!("http://{address}")).expect("loopback endpoint"); + let transport = HttpTransport::with_default_timeout(endpoint).expect("transport"); + let health = transport + .call("getHealth", json!({})) + .expect("same-host redirect should be followed"); + assert_eq!(health["status"], "healthy"); + server.join().expect("test server exits"); + } + + #[test] + fn http_transport_refuses_cross_host_redirects() { + use std::io::Write; + use std::net::TcpListener; + + let listener = TcpListener::bind("127.0.0.1:0").expect("bind test server"); + let address = listener.local_addr().expect("test server address"); + let server = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept request"); + let mut request = [0u8; 1024]; + let _ = stream.read(&mut request); + stream + .write_all( + b"HTTP/1.1 302 Found\r\nLocation: http://example.com/rpc\r\nConnection: close\r\nContent-Length: 0\r\n\r\n", + ) + .expect("write redirect"); + }); + + let endpoint = Endpoint::parse(&format!("http://{address}")).expect("loopback endpoint"); + let transport = HttpTransport::with_default_timeout(endpoint).expect("transport"); + let error = transport.call("getHealth", json!({})).unwrap_err(); + assert!( + error.to_string().contains("non-allowlisted"), + "unexpected error: {error}" + ); + server.join().expect("test server exits"); + } + #[test] fn retry_policy_retries_rate_limits_and_uses_bounded_default_backoff() { let policy = RetryPolicy::new(2); diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index a049a0d..7ab775d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -75,11 +75,12 @@ Design rules: `devkit-rpc` separates protocol from transport: -- `Endpoint` validates schemes, rejects embedded credentials, and permits - plaintext HTTP only for loopback hosts. +- `Endpoint` validates schemes, rejects embedded credentials, permits plaintext + HTTP only for loopback hosts, and refuses non-public address literals. - `Transport` is the injection point. `MockTransport` is deterministic and used by tests; `HttpTransport` performs bounded JSON-RPC over reqwest with a finite - timeout and redirects disabled. + timeout, guarded DNS resolution, same-host-only redirects, and final-URL + re-validation. See [NETWORK_SECURITY.md](NETWORK_SECURITY.md). - Typed models cover `getHealth`, `getNetwork`, and `getLatestLedger`. Other RPC methods return raw JSON until they are modeled. - `RetryPolicy` retries transport failures and rate-limit responses. It does not @@ -142,8 +143,9 @@ Commands: `version`, `doctor`, `strkey`, `scval`, `envelope`, `event`, - **No network access by default.** The only networked command is `rpc-health`, which requires an explicit endpoint (argument, environment, or config). -- The HTTP transport uses a finite timeout, disables redirects, caps response - bytes before parsing, and validates JSON-RPC responses. +- The HTTP transport uses a finite timeout, refuses private/link-local targets, + follows only re-validated same-host redirects, caps response bytes before + parsing, and validates JSON-RPC responses. - Scanned/decoded input is **untrusted**: decoding is bounded and fuzz targets exist for `ScVal`, `TransactionEnvelope`, and `ContractEvent`. - No secrets are read, logged, or committed. `.gitignore` excludes `.env*`, diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index c69ac96..70bb347 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -60,8 +60,9 @@ stellar-foundry rpc-health https://soroban-testnet.stellar.org receive an endpoint after precedence is resolved. - Diagnostics emit event names only; do not add URLs, headers, or payloads to diagnostic output. -- Endpoint policy rejects embedded credentials and permits plaintext HTTP only - for loopback hosts. +- Endpoint policy rejects embedded credentials, permits plaintext HTTP only for + loopback hosts, and refuses private/link-local targets and cross-host + redirects. See [NETWORK_SECURITY.md](NETWORK_SECURITY.md). ## Not implemented yet diff --git a/docs/DRIPS_READINESS.md b/docs/DRIPS_READINESS.md index 6fd7fdf..06351fe 100644 --- a/docs/DRIPS_READINESS.md +++ b/docs/DRIPS_READINESS.md @@ -36,7 +36,7 @@ Merged capabilities: fixtures. - RPC endpoint policy validation and a typed RPC client with deterministic mock transport. -- HTTP JSON-RPC transport with timeout, redirects disabled, a response-size cap, +- HTTP JSON-RPC transport with timeout, SSRF-hardened same-host redirects, a response-size cap, response validation, and structured errors. - Non-executing WASM validation, structural metadata, and bounded decoding of `contractspecv0` XDR entries into entry kind/name/definition records. diff --git a/docs/NETWORK_SECURITY.md b/docs/NETWORK_SECURITY.md new file mode 100644 index 0000000..2bc2323 --- /dev/null +++ b/docs/NETWORK_SECURITY.md @@ -0,0 +1,53 @@ +# Network security policy + +This document describes the fail-closed policy applied by +`devkit-rpc::HttpTransport`. Network access is always explicit: the only +networked CLI command is `rpc-health`, which requires a configured endpoint. + +## Endpoint URLs + +`Endpoint::parse` validates a URL before any transport is created: + +- `https` endpoints are allowed. +- `http` is allowed only for loopback hosts (`localhost`, `*.localhost`, + `127.0.0.0/8`, and `::1`). Plaintext traffic is never sent to a remote host. +- Any other scheme is rejected. +- Embedded credentials (`user:password@host`) are rejected. +- IP literals in private, link-local, loopback (unless the endpoint is itself + loopback), unspecified, multicast, broadcast, or other non-public ranges are + rejected. This includes IPv4-mapped IPv6 forms. + +## DNS resolution + +`HttpTransport` installs a guarded resolver. Every address returned for the +endpoint hostname is checked against the same policy before a connection is +made. Loopback answers are accepted only for an explicitly loopback endpoint. +If a hostname resolves exclusively to internal addresses, the request fails with +a transport error. Filtering happens on the addresses actually handed to the +HTTP client, so a public name cannot be used to reach an internal service. + +## Redirects + +Redirects are followed only when the next URL still targets the original endpoint +host. Each hop is re-validated with the endpoint URL policy, including the +scheme and address checks. Cross-host redirects, scheme downgrades to plaintext +remote hosts, and chains longer than five hops fail closed. The final response +URL is re-validated as defense in depth before the body is read. + +The allowlist therefore contains exactly one host: the configured endpoint host. +There is no API for expanding it, because an RPC client only needs to talk to the +endpoint it was configured with. + +## Other transport bounds + +- A finite request timeout is required; a zero timeout is rejected. +- Response bodies are capped at 4 MiB before JSON parsing. +- JSON-RPC identifiers must match the request. +- Server error messages are returned as structured errors and never panic. + +## Out of scope + +- Proxying or tunneling. +- Reaching Stellar RPC through an operator-managed forward proxy. +- Protecting against a compromised endpoint that legitimately resolves to a + public address.