diff --git a/core/archipelago/src/api/rpc/middleware.rs b/core/archipelago/src/api/rpc/middleware.rs index 0c9bd6ae..da078543 100644 --- a/core/archipelago/src/api/rpc/middleware.rs +++ b/core/archipelago/src/api/rpc/middleware.rs @@ -179,13 +179,91 @@ pub(super) fn extract_cookie(headers: &hyper::HeaderMap, name: &str) -> Option IpAddr { +/// The TCP peer address of the connection a request arrived on, injected +/// into request extensions by the server accept loop. +#[derive(Debug, Clone, Copy)] +pub struct PeerAddr(pub std::net::SocketAddr); + +/// Extract the client IP for rate limiting. +/// +/// `X-Real-IP`/`X-Forwarded-For` are only honored when the connection +/// itself comes from loopback โ€” i.e. from our local nginx, which sets +/// `X-Real-IP $remote_addr`. On a direct connection (the FIPS peer +/// listener, or anything that isn't the local proxy) the headers are +/// client-supplied, so trusting them let an attacker rotate per-request +/// "IPs" and defeat the login rate limiter; there we use the socket +/// address instead. +pub(super) fn extract_client_ip(parts: &hyper::http::request::Parts) -> IpAddr { + let socket_ip = parts.extensions.get::().map(|p| p.0.ip()); + match socket_ip { + Some(ip) if ip.is_loopback() => forwarded_client_ip(&parts.headers).unwrap_or(ip), + Some(ip) => ip, + // No socket info recorded (shouldn't happen in the server path); + // fall back to the pre-extension behavior. + None => forwarded_client_ip(&parts.headers) + .unwrap_or(IpAddr::V4(std::net::Ipv4Addr::LOCALHOST)), + } +} + +/// The proxy-reported client IP, if a forwarded header carries one. +fn forwarded_client_ip(headers: &hyper::HeaderMap) -> Option { headers .get("x-real-ip") .or_else(|| headers.get("x-forwarded-for")) .and_then(|v| v.to_str().ok()) .and_then(|s| s.split(',').next()) .and_then(|s| s.trim().parse::().ok()) - .unwrap_or(IpAddr::V4(std::net::Ipv4Addr::LOCALHOST)) +} + +#[cfg(test)] +mod client_ip_tests { + use super::*; + use std::net::SocketAddr; + + fn parts_with( + peer: Option<&str>, + real_ip: Option<&str>, + ) -> hyper::http::request::Parts { + let mut builder = hyper::Request::builder().uri("/rpc/v1"); + if let Some(ip) = real_ip { + builder = builder.header("x-real-ip", ip); + } + let (mut parts, _) = builder.body(()).unwrap().into_parts(); + if let Some(addr) = peer { + parts + .extensions + .insert(PeerAddr(addr.parse::().unwrap())); + } + parts + } + + #[test] + fn loopback_connection_trusts_forwarded_header() { + // nginx on loopback forwards the real client IP โ€” use it. + let parts = parts_with(Some("127.0.0.1:44412"), Some("192.168.1.50")); + assert_eq!( + extract_client_ip(&parts), + "192.168.1.50".parse::().unwrap() + ); + } + + #[test] + fn direct_connection_ignores_spoofed_header() { + // A direct (non-proxy) client rotating X-Real-IP per request must + // still bucket under its socket address. + let parts = parts_with(Some("203.0.113.9:9999"), Some("10.0.0.1")); + assert_eq!( + extract_client_ip(&parts), + "203.0.113.9".parse::().unwrap() + ); + } + + #[test] + fn loopback_connection_without_header_uses_socket_ip() { + let parts = parts_with(Some("127.0.0.1:5000"), None); + assert_eq!( + extract_client_ip(&parts), + "127.0.0.1".parse::().unwrap() + ); + } } diff --git a/core/archipelago/src/api/rpc/mod.rs b/core/archipelago/src/api/rpc/mod.rs index e474cbe6..2dc3dd39 100644 --- a/core/archipelago/src/api/rpc/mod.rs +++ b/core/archipelago/src/api/rpc/mod.rs @@ -58,6 +58,7 @@ use middleware::{ derive_csrf_token, extract_client_ip, extract_cookie, sanitize_error_message, CACHEABLE_METHODS, UNAUTHENTICATED_METHODS, }; +pub use middleware::PeerAddr; use response::{cookie_header, json_response, ResponseCache, RpcError, RpcRequest, RpcResponse}; /// Default dev password when no user is set up (matches mock-backend). @@ -369,7 +370,7 @@ impl RpcHandler { // Rate limit login attempts if rpc_req.method == "auth.login" { - let client_ip = extract_client_ip(&parts.headers); + let client_ip = extract_client_ip(&parts); if !self.login_rate_limiter.check(client_ip).await { return Ok(self.rate_limit_response()); } @@ -377,7 +378,7 @@ impl RpcHandler { // Rate limit sensitive endpoints { - let client_ip = extract_client_ip(&parts.headers); + let client_ip = extract_client_ip(&parts); if !self .endpoint_rate_limiter .check(&rpc_req.method, client_ip) @@ -451,7 +452,7 @@ impl RpcHandler { let mut response = json_response(StatusCode::OK, &resp_body); // Post-dispatch: set cookies for auth-related methods - let client_ip = extract_client_ip(&parts.headers); + let client_ip = extract_client_ip(&parts); self.apply_auth_cookies( &rpc_req.method, &mut rpc_resp, diff --git a/core/archipelago/src/server.rs b/core/archipelago/src/server.rs index 807a27f7..4a52851d 100644 --- a/core/archipelago/src/server.rs +++ b/core/archipelago/src/server.rs @@ -1034,9 +1034,13 @@ async fn accept_loop( let permit = active_connections.clone().acquire_owned().await; tokio::spawn(async move { let _permit = permit; - let service = service_fn(move |req: hyper::Request| { + let service = service_fn(move |mut req: hyper::Request| { let handler = handler.clone(); async move { + // Record the TCP peer so rate limiting only trusts + // forwarded headers on loopback (nginx) connections. + req.extensions_mut() + .insert(crate::api::rpc::PeerAddr(peer_addr)); if peer_only && !is_peer_allowed_path(req.uri().path()) { let resp = hyper::Response::builder() .status(hyper::StatusCode::NOT_FOUND) diff --git a/docs/1.8.0-RELEASE-HARDENING-PLAN.md b/docs/1.8.0-RELEASE-HARDENING-PLAN.md index 67a5c887..b223951c 100644 --- a/docs/1.8.0-RELEASE-HARDENING-PLAN.md +++ b/docs/1.8.0-RELEASE-HARDENING-PLAN.md @@ -131,14 +131,25 @@ modules; production request/boot paths are essentially panic-free. The real risk revalidate) instead of blocking on systemctl. `image_verifier.rs` cosign sites have no callers yet โ€” handled with the ยงA cosign item. Tests: container 155 / transport 29 / config 29 / package 46 all green. -- [ ] ๐ŸŸก **Restrict Bitcoin RPC exposure.** `bootstrap.rs:409` writes - `rpcallowip=0.0.0.0/0`. Scope to the container subnet / `127.0.0.1`. +- [ ] ๐ŸŸก **Restrict Bitcoin RPC exposure.** INVESTIGATED 2026-07-03 โ€” the fix is NOT + `rpcallowip`: under rootless-podman NAT every forwarded connection reaches bitcoind + with an in-subnet source IP, so scoping `rpcallowip` blocks nothing (and container + consumers use archy-net DNS anyway). The actual exposure is the host-side publish + `8332:8332` (binds 0.0.0.0 โ†’ LAN can hit RPC, auth-only barrier; 4 write sites: + `bootstrap.rs:424`, `package/config.rs:694`, `package/install.rs:1333/2450`, plus + the knots manifest). Real fix = `127.0.0.1:8332:8332` host bind (P2P 8333 stays + public; zmq 28332/28333 should get the same look โ€” unauthenticated). โš  May break + external wallets pointed straight at nodeIP:8332 โ€” needs a user call + on-node gate + re-run, so NOT changed drive-by from the dev box. - [ ] ๐ŸŸก **Move generated secrets from env to file mounts.** `manifest.rs:1208-1226` injects secrets as `-e KEY=value`, readable via `podman inspect` / `/proc//environ`. Prefer bind-mounting the existing `0600` secret file or `podman --secret`. -- [ ] ๐ŸŸก **Harden rate-limit IP extraction.** `middleware.rs:120-128` trusts - client-spoofable `X-Real-IP`/`X-Forwarded-For` โ†’ per-request bucket rotation defeats the - login limiter. Trust forwarded headers only from a configured proxy; have nginx set them. +- [x] ๐ŸŸก **Harden rate-limit IP extraction.** DONE 2026-07-03: the accept loop injects the + TCP `PeerAddr` into request extensions; `extract_client_ip` honors + `X-Real-IP`/`X-Forwarded-For` ONLY when the connection is from loopback (our nginx, + which sets `X-Real-IP $remote_addr`) โ€” direct connections (e.g. the FIPS peer + listener) bucket under their socket IP, so per-request header rotation no longer + defeats the login limiter. 3 unit tests. - [ ] ๐ŸŸข **Include `seq` in the mesh signed preimage.** `message_types.rs:245-288` signs `(t,v,ts)` but sets the anti-replay `seq` after signing โ†’ a radio MITM can alter ordering without breaking the signature.