From 75d1ce87fc35cc2e9f5160e5f2130268a9e4b7c3 Mon Sep 17 00:00:00 2001 From: Blomios Date: Thu, 16 Jul 2026 23:10:40 +0200 Subject: [PATCH 1/2] =?UTF-8?q?feat(web-server):=20donner=20un=20effet=20r?= =?UTF-8?q?=C3=A9el=20=C3=A0=20la=20confiance=20reverse=20proxy=20+=20fix?= =?UTF-8?q?=20port=20CLI=20(#72)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `--trust-reverse-proxy` était un drapeau creux : il n'était lu que dans `ServerConfig::validate()` pour exiger sa propre présence. Le serveur n'a jamais vérifié qui se connectait ni ce que le proxy annonçait. Ce lot lui donne un effet. - `trusted_proxies: Vec` dans `ServerConfig` + `--trusted-proxy` (répétable, IP ou CIDR v4/v6). `validate()` refuse désormais au démarrage un bind non-loopback en mode remote sans au moins un proxy autorisé. - Guard runtime sur les trois surfaces (statique, `/api/*`, `/api/ws`) : le `peer_addr` réel est vérifié AVANT toute confiance accordée aux headers. Sur bind loopback, seul un pair loopback est accepté ; sinon le pair doit tomber dans un `--trusted-proxy`. - `X-Forwarded-Proto: https` obligatoire en mode proxy, avec un message de refus actionnable (la commande nginx exacte à ajouter). - `X-Forwarded-For` n'autorise jamais rien : il n'est que journalisé. Un header ne donne aucun droit, seul le pair TCP en donne. - Diagnostics : `UntrustedProxyPeer`, `ForwardedProtoRejected`, `ForwardedHostMismatch`. B0 — même bug de port que celui corrigé dans #68 B1, sur le chemin CLI cette fois : `run_server` construisait son `ServerState` AVANT le bind, donc `idea-serve --listen 127.0.0.1:0` comparait l'origine à `http://127.0.0.1:0` et rejetait tout en 403. La réconciliation du port effectif est factorisée dans `config_with_effective_listen`, partagée avec le chemin embarqué. `X-Forwarded-Host` ne provoque PAS de rejet : Architect l'a jugé redondant avec la vérification stricte d'`Origin`, et il cassait nginx en configuration par défaut. Il reste un warning de diagnostic. La vérification d'accès stricte demeure l'égalité d'`Origin` contre `--public-origin`. CHANGEMENT DE COMPORTEMENT ASSUMÉ — ce lot casse volontairement les configurations existantes : un bind non-loopback distant sans `--trusted-proxy` est désormais refusé au démarrage. C'est le prix d'un drapeau qui ne mentait plus. Le message d'erreur dit quoi ajouter. Origine du ticket : #72 est né d'une trouvaille de l'agent Git à la revue de la doc de #65 — c'est en vérifiant une phrase de sécurité qu'il a établi que `--trust-reverse-proxy` n'avait aucun effet runtime. QA ré-exécutée par Git HORS SANDBOX avant merge (DevBackend et QA sont tous deux bloqués par EPERM sur `TcpListener::bind` ; un vert sandboxé ne vaut rien ici, on s'est déjà fait avoir sur #68 B1) : `cargo test -p web-server` 66 passed, 0 échec · `-p backend` · `-p app-tauri` verts. Revue de sécurité par Git : guard câblé sur les 3 routes, chemin de production propageant toujours le vrai `peer_addr` (le repli `listen.ip()` est `#[cfg(test)]`, inatteignable en production), CIDR correct y compris `prefix 0`. Co-Authored-By: Claude Opus 4.8 --- crates/web-server/src/lib.rs | 609 +++++++++++++++++++++++++++++++++-- 1 file changed, 584 insertions(+), 25 deletions(-) diff --git a/crates/web-server/src/lib.rs b/crates/web-server/src/lib.rs index c93db18..08ee401 100644 --- a/crates/web-server/src/lib.rs +++ b/crates/web-server/src/lib.rs @@ -5,7 +5,7 @@ use std::collections::HashSet; use std::env; -use std::net::SocketAddr; +use std::net::{IpAddr, SocketAddr}; use std::path::{Path, PathBuf}; use std::process::ExitCode; use std::sync::atomic::{AtomicU64, Ordering}; @@ -67,19 +67,11 @@ pub fn run_from_args(args: Vec) -> ExitCode { } }; - let state = Arc::new(ServerState::new(config.clone())); - eprintln!("IdeA pairing code: {}", state.pairing_code()); - eprintln!( - "idea --serve: app data dir = {}", - config.app_data_dir.display() - ); - eprintln!("IdeA server listening on {}", config.listen); - match tokio::runtime::Builder::new_multi_thread() .enable_all() .build() { - Ok(runtime) => match runtime.block_on(run_server(config, state)) { + Ok(runtime) => match runtime.block_on(run_server(config)) { Ok(()) => ExitCode::SUCCESS, Err(err) => { eprintln!("idea --serve: {err}"); @@ -104,12 +96,78 @@ pub struct ServerConfig { pub allow_remote: bool, /// Whether a trusted HTTPS reverse proxy terminates remote access. pub trust_reverse_proxy: bool, + /// Authorized reverse proxy peer IPs or CIDR ranges. + pub trusted_proxies: Vec, /// IdeA application data directory. pub app_data_dir: PathBuf, /// Built frontend assets root. pub web_root: PathBuf, } +/// Authorized reverse proxy peer IP or CIDR range. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct TrustedProxy { + addr: IpAddr, + prefix: u8, +} + +impl TrustedProxy { + /// Parses an IP address or CIDR range accepted by `--trusted-proxy`. + pub fn parse(value: &str) -> Result { + let (addr, prefix) = match value.split_once('/') { + Some((addr, prefix)) => { + let addr = addr + .parse::() + .map_err(|_| format!("invalid --trusted-proxy address: {value}"))?; + let prefix = prefix + .parse::() + .map_err(|_| format!("invalid --trusted-proxy CIDR prefix: {value}"))?; + (addr, prefix) + } + None => { + let addr = value + .parse::() + .map_err(|_| format!("invalid --trusted-proxy address: {value}"))?; + let prefix = match addr { + IpAddr::V4(_) => 32, + IpAddr::V6(_) => 128, + }; + (addr, prefix) + } + }; + let max = match addr { + IpAddr::V4(_) => 32, + IpAddr::V6(_) => 128, + }; + if prefix > max { + return Err(format!("invalid --trusted-proxy CIDR prefix: {value}")); + } + Ok(Self { addr, prefix }) + } + + fn contains(&self, ip: IpAddr) -> bool { + match (self.addr, ip) { + (IpAddr::V4(net), IpAddr::V4(ip)) => { + let mask = if self.prefix == 0 { + 0 + } else { + u32::MAX << (32 - self.prefix) + }; + u32::from(net) & mask == u32::from(ip) & mask + } + (IpAddr::V6(net), IpAddr::V6(ip)) => { + let mask = if self.prefix == 0 { + 0 + } else { + u128::MAX << (128 - self.prefix) + }; + u128::from(net) & mask == u128::from(ip) & mask + } + _ => false, + } + } +} + impl ServerConfig { /// Parses the web-server CLI flags. pub fn from_args(args: Vec) -> Result { @@ -119,6 +177,7 @@ impl ServerConfig { let mut public_origin = None; let mut allow_remote = false; let mut trust_reverse_proxy = false; + let mut trusted_proxies = Vec::new(); let mut app_data_dir = default_app_data_dir(); let mut web_root = None; @@ -141,6 +200,12 @@ impl ServerConfig { } "--allow-remote" => allow_remote = true, "--trust-reverse-proxy" => trust_reverse_proxy = true, + "--trusted-proxy" => { + let value = it + .next() + .ok_or_else(|| "--trusted-proxy requires an IP or CIDR".to_owned())?; + trusted_proxies.push(TrustedProxy::parse(&value)?); + } "--app-data-dir" => { let value = it .next() @@ -164,6 +229,7 @@ impl ServerConfig { public_origin, allow_remote, trust_reverse_proxy, + trusted_proxies, app_data_dir, web_root, }) @@ -192,6 +258,9 @@ impl ServerConfig { if !self.trust_reverse_proxy { return Err("--allow-remote requires --trust-reverse-proxy".to_owned()); } + if !self.listen.ip().is_loopback() && self.trusted_proxies.is_empty() { + return Err("non-loopback remote bind requires --trusted-proxy ; add the IP address or CIDR of the reverse proxy that connects to this server".to_owned()); + } } Ok(()) @@ -208,7 +277,7 @@ impl ServerConfig { /// Human-readable CLI usage. #[must_use] pub fn usage() -> String { - "usage: idea-serve [--listen IP:PORT] [--app-data-dir PATH] [--web-root PATH] [--allow-remote --public-origin https://host --trust-reverse-proxy]".to_owned() + "usage: idea-serve [--listen IP:PORT] [--app-data-dir PATH] [--web-root PATH] [--allow-remote --public-origin https://host --trust-reverse-proxy [--trusted-proxy IP_OR_CIDR]...]".to_owned() } } @@ -288,8 +357,7 @@ pub async fn run_embedded_with_core( let local_addr = listener .local_addr() .map_err(|err| format!("failed to read listener address: {err}"))?; - let mut effective_config = config.clone(); - effective_config.listen = local_addr; + let effective_config = config_with_effective_listen(config.clone(), local_addr); let state = Arc::new(ServerState::with_core(effective_config, core)); let pairing_code = state.pairing_code().to_owned(); let (shutdown_tx, shutdown_rx) = oneshot::channel(); @@ -303,6 +371,11 @@ pub async fn run_embedded_with_core( }) } +fn config_with_effective_listen(mut config: ServerConfig, local_addr: SocketAddr) -> ServerConfig { + config.listen = local_addr; + config +} + fn resolve_web_root(explicit: Option) -> Result { if let Some(path) = explicit { return validate_web_root(path, "--web-root"); @@ -433,6 +506,19 @@ enum SecurityLogEvent { origin: Option, route: String, }, + UntrustedProxyPeer { + peer: IpAddr, + forwarded_for: Option, + }, + ForwardedProtoRejected { + peer: IpAddr, + proto: Option, + }, + ForwardedHostMismatch { + peer: IpAddr, + host: Option, + expected: String, + }, WsUpgradeRejected { origin: Option, reason: &'static str, @@ -480,6 +566,31 @@ fn security_log_line(event: &SecurityLogEvent) -> String { origin_label(origin) ) } + SecurityLogEvent::UntrustedProxyPeer { + peer, + forwarded_for, + } => { + format!( + "untrustedProxyPeer peer={peer} forwardedFor={}", + origin_label(forwarded_for) + ) + } + SecurityLogEvent::ForwardedProtoRejected { peer, proto } => { + format!( + "forwardedProtoRejected peer={peer} proto={}", + origin_label(proto) + ) + } + SecurityLogEvent::ForwardedHostMismatch { + peer, + host, + expected, + } => { + format!( + "forwardedHostMismatch peer={peer} host={} expected={expected}", + origin_label(host) + ) + } SecurityLogEvent::WsUpgradeRejected { origin, reason } => { format!( "websocket upgrade rejected reason={reason} origin={}", @@ -496,10 +607,21 @@ fn origin_label(origin: &Option) -> &str { origin.as_deref().unwrap_or("") } -async fn run_server(config: ServerConfig, state: Arc) -> Result<(), String> { +async fn run_server(config: ServerConfig) -> Result<(), String> { let listener = TcpListener::bind(config.listen) .await .map_err(|err| format!("failed to bind {}: {err}", config.listen))?; + let local_addr = listener + .local_addr() + .map_err(|err| format!("failed to read listener address: {err}"))?; + let effective_config = config_with_effective_listen(config.clone(), local_addr); + let state = Arc::new(ServerState::new(effective_config)); + eprintln!("IdeA pairing code: {}", state.pairing_code()); + eprintln!( + "idea --serve: app data dir = {}", + config.app_data_dir.display() + ); + eprintln!("IdeA server listening on {local_addr}"); let (_shutdown_tx, shutdown_rx) = oneshot::channel(); run_listener(listener, state, shutdown_rx).await } @@ -514,10 +636,11 @@ async fn run_listener( result = listener.accept() => result, _ = &mut shutdown => return Ok(()), }; - let (stream, _) = accepted.map_err(|err| format!("failed to accept connection: {err}"))?; + let (stream, peer_addr) = + accepted.map_err(|err| format!("failed to accept connection: {err}"))?; let state = Arc::clone(&state); tokio::spawn(async move { - if let Err(err) = handle_tcp_connection(stream, state).await { + if let Err(err) = handle_tcp_connection(stream, peer_addr, state).await { eprintln!("idea --serve: connection error: {err}"); } }); @@ -526,6 +649,7 @@ async fn run_listener( async fn handle_tcp_connection( mut stream: tokio::net::TcpStream, + peer_addr: SocketAddr, state: Arc, ) -> Result<(), String> { let mut buffer = Vec::with_capacity(8192); @@ -549,7 +673,7 @@ async fn handle_tcp_connection( let (method, uri, headers, content_length) = parse_http_request_head(&buffer[..header_end])?; if method == Method::GET && uri.path() == WS_PATH { - return handle_ws_upgrade(stream, headers, state).await; + return handle_ws_upgrade(stream, peer_addr.ip(), headers, state).await; } let body_start = header_end + 4; @@ -575,6 +699,7 @@ async fn handle_tcp_connection( uri, headers, Bytes::copy_from_slice(&buffer[body_start..body_start + content_length]), + Some(peer_addr.ip()), state, ) .await; @@ -585,6 +710,15 @@ async fn handle_tcp_connection( async fn handle_request( req: Request, state: Arc, +) -> Response { + handle_request_from_peer(req, Arc::clone(&state), Some(state.config.listen.ip())).await +} + +#[cfg(test)] +async fn handle_request_from_peer( + req: Request, + state: Arc, + peer_ip: Option, ) -> Response { let (parts, body) = req.into_parts(); let body = match body.collect().await { @@ -604,6 +738,7 @@ async fn handle_request( parts.uri, parts.headers, body, + peer_ip, Arc::clone(&state), ) .await @@ -688,10 +823,11 @@ async fn write_http_response( async fn handle_ws_upgrade( mut stream: tokio::net::TcpStream, + peer_ip: IpAddr, headers: HeaderMap, state: Arc, ) -> Result<(), String> { - let accept = match validate_ws_upgrade(&headers, &state) { + let accept = match validate_ws_upgrade(&headers, Some(peer_ip), &state) { Ok(accept) => accept, Err(response) => return write_http_response(&mut stream, *response).await, }; @@ -711,8 +847,13 @@ async fn handle_ws_upgrade( fn validate_ws_upgrade( headers: &HeaderMap, + peer_ip: Option, state: &ServerState, ) -> Result>> { + if let Err(response) = validate_reverse_proxy(headers, peer_ip, state, WS_PATH) { + return Err(response); + } + let origin = match validate_request_origin(headers, &state.config) { Ok(origin) => origin, Err(response) => { @@ -1232,6 +1373,7 @@ async fn dispatch_http( uri: Uri, headers: HeaderMap, body: Bytes, + peer_ip: Option, state: Arc, ) -> Response { if has_forbidden_query_secret(&uri) { @@ -1244,9 +1386,16 @@ async fn dispatch_http( } if !is_api_path(uri.path()) { + if let Err(response) = validate_reverse_proxy(&headers, peer_ip, &state, uri.path()) { + return *response; + } return serve_static(&state.config.web_root, method, uri.path()).await; } + if let Err(response) = validate_reverse_proxy(&headers, peer_ip, &state, uri.path()) { + return *response; + } + let origin = match validate_request_origin(&headers, &state.config) { Ok(origin) => origin, Err(response) => { @@ -1794,6 +1943,126 @@ fn validate_request_origin( } } +fn validate_reverse_proxy( + headers: &HeaderMap, + peer_ip: Option, + state: &ServerState, + _route: &str, +) -> Result<(), Box>> { + let config = &state.config; + if !config.allow_remote { + return Ok(()); + } + let Some(public_origin) = &config.public_origin else { + return Ok(()); + }; + + let peer = peer_ip.unwrap_or(config.listen.ip()); + if !trusted_proxy_peer(peer, config) { + let forwarded_for = forwarded_header_first(headers, "x-forwarded-for"); + state.log_security(SecurityLogEvent::UntrustedProxyPeer { + peer, + forwarded_for, + }); + return Err(Box::new(error_response( + StatusCode::FORBIDDEN, + "FORBIDDEN", + untrusted_proxy_message(peer, config.trusted_proxies.is_empty()), + None, + ))); + } + + let proto = forwarded_header_first(headers, "x-forwarded-proto"); + if !proto + .as_deref() + .is_some_and(|proto| proto.eq_ignore_ascii_case("https")) + { + state.log_security(SecurityLogEvent::ForwardedProtoRejected { peer, proto }); + return Err(Box::new(error_response( + StatusCode::FORBIDDEN, + "FORBIDDEN", + "IdeA rejected the request because the trusted proxy did not report X-Forwarded-Proto: https. Configure the reverse proxy to send it; for nginx add: proxy_set_header X-Forwarded-Proto $scheme;", + None, + ))); + } + + let expected = public_origin_authority(public_origin).unwrap_or_else(|| public_origin.clone()); + let host = forwarded_host_diagnostic(headers); + if !host + .as_deref() + .is_some_and(|host| host.trim().eq_ignore_ascii_case(&expected)) + { + state.log_security(SecurityLogEvent::ForwardedHostMismatch { + peer, + host, + expected: expected.clone(), + }); + } + + Ok(()) +} + +fn trusted_proxy_peer(peer: IpAddr, config: &ServerConfig) -> bool { + if config.listen.ip().is_loopback() { + return peer.is_loopback(); + } + config + .trusted_proxies + .iter() + .any(|trusted| trusted.contains(peer)) +} + +fn untrusted_proxy_message(peer: IpAddr, no_authorized_proxy: bool) -> String { + let reason = if no_authorized_proxy { + "but no authorized proxy is configured" + } else { + "but it is not an authorized proxy" + }; + format!( + "IdeA rejected the request because it came from {peer}, {reason}. Add --trusted-proxy {peer} or a restricted proxy CIDR such as --trusted-proxy {peer}/32." + ) +} + +fn forwarded_header_first(headers: &HeaderMap, name: &str) -> Option { + headers + .get(name) + .and_then(|value| value.to_str().ok()) + .and_then(|value| value.split(',').next()) + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(str::to_owned) +} + +fn forwarded_host_diagnostic(headers: &HeaderMap) -> Option { + forwarded_header_first(headers, "x-forwarded-host") + .or_else(|| forwarded_header_host_param(headers)) + .or_else(|| forwarded_header_first(headers, "host")) +} + +fn forwarded_header_host_param(headers: &HeaderMap) -> Option { + headers + .get("forwarded") + .and_then(|value| value.to_str().ok()) + .and_then(|value| { + value.split(',').find_map(|entry| { + entry.split(';').find_map(|part| { + let (name, value) = part.trim().split_once('=')?; + name.trim() + .eq_ignore_ascii_case("host") + .then(|| value.trim().trim_matches('"').to_owned()) + }) + }) + }) + .filter(|value| !value.is_empty()) +} + +fn public_origin_authority(origin: &str) -> Option { + origin.parse::().ok().and_then(|uri| { + uri.authority() + .map(|authority| authority.as_str().to_owned()) + }) +} + fn request_origin(headers: &HeaderMap) -> Option { headers .get(ORIGIN) @@ -2501,6 +2770,7 @@ mod tests { public_origin: None, allow_remote: false, trust_reverse_proxy: false, + trusted_proxies: Vec::new(), app_data_dir: std::env::temp_dir().join(format!("idea-server-test-{}", Uuid::new_v4())), web_root: create_web_root(), } @@ -2534,6 +2804,51 @@ mod tests { Arc::new(ServerState::new_for_test(test_config(), "PAIR1234")) } + fn trusted_proxy(value: &str) -> TrustedProxy { + TrustedProxy::parse(value).unwrap() + } + + fn remote_config() -> ServerConfig { + ServerConfig { + listen: "192.168.1.75:17373".parse().unwrap(), + public_origin: Some("https://idea.example.com".to_owned()), + allow_remote: true, + trust_reverse_proxy: true, + trusted_proxies: vec![trusted_proxy("192.168.1.22")], + ..test_config() + } + } + + async fn remote_pair_request( + state: Arc, + peer: &str, + origin: &str, + proto: &str, + host: Option<&str>, + extra_headers: &[(&str, &str)], + ) -> Response { + let mut builder = Request::builder() + .method(Method::POST) + .uri("/api/pair") + .header(ORIGIN, origin) + .header("x-forwarded-proto", proto) + .header(CONTENT_TYPE, "application/json"); + if let Some(host) = host { + builder = builder.header("x-forwarded-host", host); + } + for (name, value) in extra_headers { + builder = builder.header(HeaderName::from_bytes(name.as_bytes()).unwrap(), *value); + } + handle_request_from_peer( + builder + .body(Full::new(Bytes::from_static(br#"{"code":"PAIR1234"}"#))) + .unwrap(), + state, + Some(peer.parse().unwrap()), + ) + .await + } + #[tokio::test] async fn run_embedded_stop_shuts_down_accept_loop() { let config = ServerConfig { @@ -2993,7 +3308,8 @@ mod tests { let state = state(); let headers = ws_headers("http://127.0.0.1:17373", None); - let response = validate_ws_upgrade(&headers, &state).unwrap_err(); + let response = + validate_ws_upgrade(&headers, Some("127.0.0.1".parse().unwrap()), &state).unwrap_err(); assert_eq!(response.status(), StatusCode::UNAUTHORIZED); } @@ -3006,7 +3322,8 @@ mod tests { Some("idea_session=not-a-valid-session"), ); - let response = validate_ws_upgrade(&headers, &state).unwrap_err(); + let response = + validate_ws_upgrade(&headers, Some("127.0.0.1".parse().unwrap()), &state).unwrap_err(); assert_eq!(response.status(), StatusCode::UNAUTHORIZED); } @@ -3020,7 +3337,8 @@ mod tests { Some(&format!("idea_session={token}")), ); - let response = validate_ws_upgrade(&headers, &state).unwrap_err(); + let response = + validate_ws_upgrade(&headers, Some("127.0.0.1".parse().unwrap()), &state).unwrap_err(); assert_eq!(response.status(), StatusCode::FORBIDDEN); } @@ -3034,7 +3352,8 @@ mod tests { Some(&format!("idea_session={token}")), ); - let accept = validate_ws_upgrade(&headers, &state).unwrap(); + let accept = + validate_ws_upgrade(&headers, Some("127.0.0.1".parse().unwrap()), &state).unwrap(); assert_eq!(accept, "s3pPLMBiTxaQ9kYGzzhZRbK+xOo="); } @@ -3682,9 +4001,43 @@ mod tests { trust_reverse_proxy: true, ..config }; + let err = config.validate().unwrap_err(); + assert!(err.contains("--trusted-proxy ")); + assert!(!err.contains("192.168.1.22")); + + let config = ServerConfig { + trusted_proxies: vec![trusted_proxy("192.168.1.22")], + ..config + }; assert!(config.validate().is_ok()); } + #[test] + fn loopback_remote_reverse_proxy_does_not_require_trusted_proxy() { + let config = ServerConfig { + listen: "127.0.0.1:17373".parse().unwrap(), + allow_remote: true, + public_origin: Some("https://idea.example.com".to_owned()), + trust_reverse_proxy: true, + trusted_proxies: Vec::new(), + ..test_config() + }; + + assert!(config.validate().is_ok()); + } + + #[test] + fn effective_listen_replaces_port_zero_for_origin_checks() { + let config = ServerConfig { + listen: "127.0.0.1:0".parse().unwrap(), + ..test_config() + }; + let effective = config_with_effective_listen(config, "127.0.0.1:43689".parse().unwrap()); + + assert!(origin_allowed("http://127.0.0.1:43689", &effective)); + assert!(!origin_allowed("http://127.0.0.1:0", &effective)); + } + #[test] fn allow_remote_requires_https_origin_and_reverse_proxy() { let base = ServerConfig { @@ -3710,6 +4063,207 @@ mod tests { assert!(without_proxy.validate().is_err()); } + #[tokio::test] + async fn remote_bind_rejects_peer_when_no_authorized_proxy_is_configured() { + let logger = Arc::new(RecordingSecurityLogger::default()); + let config = ServerConfig { + trusted_proxies: Vec::new(), + ..remote_config() + }; + let state = Arc::new(ServerState::new_for_test_with_logger( + config, + "PAIR1234", + logger.clone(), + )); + + let response = remote_pair_request( + state, + "192.168.1.22", + "https://idea.example.com", + "https", + Some("idea.example.com"), + &[], + ) + .await; + let (status, body, _) = response_json(response).await; + + assert_eq!(status, StatusCode::FORBIDDEN); + assert!(body["message"] + .as_str() + .is_some_and(|message| message.contains("came from 192.168.1.22") + && message.contains("no authorized proxy is configured") + && message.contains("--trusted-proxy 192.168.1.22"))); + assert!(logger + .events() + .iter() + .any(|event| matches!(event, SecurityLogEvent::UntrustedProxyPeer { peer, .. } if *peer == "192.168.1.22".parse::().unwrap()))); + } + + #[tokio::test] + async fn remote_bind_rejects_untrusted_proxy_peer_without_using_x_forwarded_for() { + let logger = Arc::new(RecordingSecurityLogger::default()); + let state = Arc::new(ServerState::new_for_test_with_logger( + remote_config(), + "PAIR1234", + logger.clone(), + )); + + let response = remote_pair_request( + state, + "192.168.1.23", + "https://idea.example.com", + "https", + Some("idea.example.com"), + &[("x-forwarded-for", "192.168.1.22")], + ) + .await; + let (status, _, _) = response_json(response).await; + + assert_eq!(status, StatusCode::FORBIDDEN); + assert!(logger.events().iter().any(|event| matches!( + event, + SecurityLogEvent::UntrustedProxyPeer { + peer, + forwarded_for: Some(forwarded_for) + } if *peer == "192.168.1.23".parse::().unwrap() && forwarded_for == "192.168.1.22" + ))); + } + + #[tokio::test] + async fn trusted_proxy_requires_forwarded_https_proto() { + let logger = Arc::new(RecordingSecurityLogger::default()); + let state = Arc::new(ServerState::new_for_test_with_logger( + remote_config(), + "PAIR1234", + logger.clone(), + )); + + let response = remote_pair_request( + state, + "192.168.1.22", + "https://idea.example.com", + "http", + Some("idea.example.com"), + &[], + ) + .await; + let (status, _, _) = response_json(response).await; + + assert_eq!(status, StatusCode::FORBIDDEN); + assert!(logger.events().iter().any(|event| matches!( + event, + SecurityLogEvent::ForwardedProtoRejected { + peer, + proto: Some(proto) + } if *peer == "192.168.1.22".parse::().unwrap() && proto == "http" + ))); + } + + #[tokio::test] + async fn trusted_proxy_allows_forwarded_host_mismatch_with_warning() { + let logger = Arc::new(RecordingSecurityLogger::default()); + let state = Arc::new(ServerState::new_for_test_with_logger( + remote_config(), + "PAIR1234", + logger.clone(), + )); + + let response = remote_pair_request( + state, + "192.168.1.22", + "https://idea.example.com", + "https", + Some("other.example.com"), + &[], + ) + .await; + let (status, body, _) = response_json(response).await; + + assert_eq!(status, StatusCode::OK); + assert_eq!(body["paired"], true); + assert!(logger.events().iter().any(|event| matches!( + event, + SecurityLogEvent::ForwardedHostMismatch { + peer, + host: Some(host), + expected + } if *peer == "192.168.1.22".parse::().unwrap() + && host == "other.example.com" + && expected == "idea.example.com" + ))); + } + + #[tokio::test] + async fn trusted_proxy_allows_missing_forwarded_host_with_warning() { + let logger = Arc::new(RecordingSecurityLogger::default()); + let state = Arc::new(ServerState::new_for_test_with_logger( + remote_config(), + "PAIR1234", + logger.clone(), + )); + + let response = remote_pair_request( + state, + "192.168.1.22", + "https://idea.example.com", + "https", + None, + &[], + ) + .await; + let (status, body, _) = response_json(response).await; + + assert_eq!(status, StatusCode::OK); + assert_eq!(body["paired"], true); + assert!(logger.events().iter().any(|event| matches!( + event, + SecurityLogEvent::ForwardedHostMismatch { + peer, + host: None, + expected + } if *peer == "192.168.1.22".parse::().unwrap() + && expected == "idea.example.com" + ))); + } + + #[tokio::test] + async fn trusted_proxy_with_https_and_matching_host_allows_remote_api() { + let state = Arc::new(ServerState::new_for_test(remote_config(), "PAIR1234")); + + let response = remote_pair_request( + state, + "192.168.1.22", + "https://idea.example.com", + "https", + Some("idea.example.com"), + &[("x-forwarded-for", "203.0.113.44")], + ) + .await; + let (status, body, _) = response_json(response).await; + + assert_eq!(status, StatusCode::OK); + assert_eq!(body["paired"], true); + } + + #[tokio::test] + async fn trusted_proxy_still_rejects_non_matching_api_origin() { + let state = Arc::new(ServerState::new_for_test(remote_config(), "PAIR1234")); + + let response = remote_pair_request( + state, + "192.168.1.22", + "https://evil.example", + "https", + Some("idea.example.com"), + &[], + ) + .await; + let (status, body, _) = response_json(response).await; + + assert_eq!(status, StatusCode::FORBIDDEN); + assert_eq!(body["message"], "origin not allowed"); + } + #[tokio::test] async fn invoke_requires_valid_cookie() { let state = state(); @@ -3783,7 +4337,8 @@ mod tests { assert_eq!(body["code"], "UNAUTHORIZED"); let headers = ws_headers("http://127.0.0.1:17373", Some(&cookie)); - let response = validate_ws_upgrade(&headers, &state).unwrap_err(); + let response = + validate_ws_upgrade(&headers, Some("127.0.0.1".parse().unwrap()), &state).unwrap_err(); assert_eq!(response.status(), StatusCode::UNAUTHORIZED); } @@ -3833,7 +4388,7 @@ mod tests { "http://127.0.0.1:17373", Some("idea_session=invalid-token-value"), ); - let _ = validate_ws_upgrade(&headers, &state); + let _ = validate_ws_upgrade(&headers, Some("127.0.0.1".parse().unwrap()), &state); let events = logger.events(); assert!(events.iter().any(|event| matches!( @@ -3925,6 +4480,7 @@ mod tests { public_origin: Some("https://idea.example.com".to_owned()), allow_remote: true, trust_reverse_proxy: true, + trusted_proxies: vec![trusted_proxy("192.168.1.22")], ..test_config() }; let state = Arc::new(ServerState::new_for_test(config, "PAIR1234")); @@ -3933,13 +4489,16 @@ mod tests { .method(Method::POST) .uri("/api/pair") .header(ORIGIN, "https://idea.example.com") + .header("x-forwarded-proto", "https") + .header("x-forwarded-host", "idea.example.com") .header(CONTENT_TYPE, "application/json") .body(Full::new(Bytes::from_static(br#"{"code":"PAIR1234"}"#))) .unwrap(); *req.headers_mut().get_mut(CONTENT_TYPE).unwrap() = HeaderValue::from_static("application/json"); - let response = handle_request(req, state).await; + let response = + handle_request_from_peer(req, state, Some("192.168.1.22".parse().unwrap())).await; let (status, _, headers) = response_json(response).await; assert_eq!(status, StatusCode::OK); From 42804fc55d1c7f9f76d864303e0459bbb30e31a1 Mon Sep 17 00:00:00 2001 From: Blomios Date: Thu, 16 Jul 2026 23:10:47 +0200 Subject: [PATCH 2/2] docs(server): documenter --trusted-proxy et l'exigence X-Forwarded-Proto (#72) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Aligne la doc du mode distant sur le durcissement de #72 : exemple complet du cas proxy sur une autre machine avec `--trusted-proxy`, mention que tout bind non-loopback exige au moins un proxy autorisé, et obligation faite au proxy d'envoyer `X-Forwarded-Proto: https`. Précise que transmettre le host public est recommandé pour le diagnostic mais n'est pas la vérification d'accès : celle-ci reste l'égalité stricte d'`Origin` contre `--public-origin`. Bascule les IP d'exemple vers la plage de documentation RFC 5737 (`192.0.2.0/24`) au lieu d'un réseau privé plausible. Co-Authored-By: Claude Opus 4.8 --- docs/server-client-mode-remote.md | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/docs/server-client-mode-remote.md b/docs/server-client-mode-remote.md index f40bf6c..6b1b13d 100644 --- a/docs/server-client-mode-remote.md +++ b/docs/server-client-mode-remote.md @@ -61,18 +61,33 @@ idea --serve \ This example is for a reverse proxy running on the same machine as the server. If the proxy runs on another machine, bind an address reachable by that proxy, -for example `--listen 192.168.1.75:17373`, and point the proxy upstream to that -address and port. The link from proxy to server is still plain HTTP on the LAN; -only the public access is HTTPS. +for example `--listen 192.0.2.75:17373`, and point the proxy upstream to that +address and port. Also authorize only that proxy peer: + +```bash +idea --serve \ + --listen 192.0.2.75:17373 \ + --allow-remote \ + --public-origin https://idea.example.com \ + --trust-reverse-proxy \ + --trusted-proxy 192.0.2.22 +``` + +The link from proxy to server is still plain HTTP on the LAN; only the public +access is HTTPS. For any non-loopback bind, keep the full remote HTTPS configuration: -`--allow-remote`, `--public-origin https://...`, and `--trust-reverse-proxy`. -`ServerConfig::validate()` rejects a non-loopback bind without it. +`--allow-remote`, `--public-origin https://...`, `--trust-reverse-proxy`, and at +least one `--trusted-proxy IP_OR_CIDR`. `ServerConfig::validate()` rejects a +non-loopback bind without it. That check covers the configuration, not reality: it cannot verify that a proxy is actually terminating HTTPS in front of the server. Do not expose the backend on a public non-loopback address without TLS proxying. -The proxy must forward the same origin for the SPA, `/api/*`, and `/api/ws`. +The proxy must forward the same origin for the SPA, `/api/*`, and `/api/ws`, and +must send `X-Forwarded-Proto: https`. Forwarding the public host is recommended +for diagnostics, but the strict access check remains the `Origin` match against +`--public-origin`. `--public-origin` is matched by strict equality against the request `Origin`: open the UI through the configured domain, not through the server IP, otherwise API calls are rejected with 403.