From 38f2d9e6591bb2245db06edf15281d3c62e78850 Mon Sep 17 00:00:00 2001 From: ospab Date: Sat, 27 Jun 2026 16:30:42 +0300 Subject: [PATCH] =?UTF-8?q?=C2=A7B:=20port=20stability=20fixes=20from=200.?= =?UTF-8?q?3.x=20onto=20the=20clean=20base?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ported only the fixes that actually apply to the pre-refactor base (the handshake fixes 6eb7b36/d65af35 fix bugs the 0.3.1 multi-server refactor introduced into the new outbounds/ostp.rs; the base bridge.rs already waits for the handshake response with retransmit + NAT64 fallback, so they are intentionally skipped). - EMFILE (922cf0b): rlimit::increase_nofile_limit at CLI startup. - Logs (1151726): UoT connect/disconnect → debug; rate-limit the unauthorized-probe log to one line / ~30s so a junk/probe flood can't spam the log (and a client running junk-over-UDP can't self-ban). - Helper lifecycle + bypass routes (b6e78c1): * ostp-tun-helper forces std::process::exit(0) after run_server so the WinTun adapter and its metric-0 default route are reclaimed instead of lingering as a zombie that breaks the next connect. * windows_route: delete_routes_for_dest() purges stale /32s, dedupe bypass IPs, and log add failures at warn!. * windows: retry tun::create through the transient ERROR_INVALID_ PARAMETER window and widen the adapter-index lookup to ~15s. Co-Authored-By: Claude Opus 4.8 --- Cargo.lock | 10 ++++++++ ostp-server/src/lib.rs | 22 +++++++++++++++- ostp-server/src/transport/uot.rs | 5 ++-- ostp-tun-helper/src/main.rs | 14 ++++++++++- ostp-tun/src/windows.rs | 41 ++++++++++++++++++++++++------ ostp-tun/src/windows_route.rs | 43 +++++++++++++++++++++++++++++--- ostp/Cargo.toml | 1 + ostp/src/main.rs | 4 +++ 8 files changed, 125 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 33d5f25..ac5a771 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1395,6 +1395,7 @@ dependencies = [ "ostp-core", "ostp-server", "rand 0.8.5", + "rlimit", "serde", "serde_json", "tokio", @@ -1853,6 +1854,15 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "rlimit" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f35ee2729c56bb610f6dba436bf78135f728b7373bdffae2ec815b2d3eb98cc3" +dependencies = [ + "libc", +] + [[package]] name = "rust-embed" version = "8.11.0" diff --git a/ostp-server/src/lib.rs b/ostp-server/src/lib.rs index 072cd9b..48e326d 100644 --- a/ostp-server/src/lib.rs +++ b/ostp-server/src/lib.rs @@ -285,6 +285,10 @@ pub async fn run_server( // Headless event logger tokio::spawn(async move { + // Rate-limit unauthorized-probe logging so a junk/probe flood can't spam the log + // (and so a client running junk-over-UDP can't trigger a self-ban via log noise). + let mut probe_window_start: Option = None; + let mut probe_suppressed: u64 = 0; while let Some(ev) = ui_event_rx.recv().await { match ev { UiEvent::Log(msg) => { @@ -303,7 +307,23 @@ pub async fn run_server( } UiEvent::UnauthorizedProbe { peer, bytes } => { if debug { - tracing::debug!("Unauthorized probe from {peer} ({bytes} bytes)"); + let now = Instant::now(); + let elapsed = probe_window_start + .map(|s| now.duration_since(s)) + .unwrap_or(Duration::MAX); + if elapsed >= Duration::from_secs(30) { + if probe_suppressed > 0 { + tracing::debug!( + "(+{} more unauthorized probes suppressed in the previous ~30s)", + probe_suppressed + ); + } + probe_window_start = Some(now); + probe_suppressed = 0; + tracing::debug!("Unauthorized probe from {peer} ({bytes} bytes)"); + } else { + probe_suppressed += 1; + } } } UiEvent::PeerSeen { .. } => {} diff --git a/ostp-server/src/transport/uot.rs b/ostp-server/src/transport/uot.rs index 13d0a56..86749e8 100644 --- a/ostp-server/src/transport/uot.rs +++ b/ostp-server/src/transport/uot.rs @@ -5,7 +5,6 @@ use std::net::SocketAddr; use std::sync::Arc; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::sync::{mpsc, RwLock}; -use tracing::info; pub async fn handle_tcp_connection( stream: S, @@ -16,7 +15,7 @@ pub async fn handle_tcp_connection( where S: tokio::io::AsyncRead + tokio::io::AsyncWrite + Unpin + Send + 'static, { - info!("UoT client connected from {}", peer_addr); + tracing::debug!("UoT client connected from {}", peer_addr); // Register this connection in the map let (tx, mut rx) = mpsc::channel::(16384); @@ -54,6 +53,6 @@ where }); let _ = tokio::join!(writer_task, reader_task); - info!("UoT client disconnected: {}", peer_addr); + tracing::debug!("UoT client disconnected: {}", peer_addr); Ok(()) } diff --git a/ostp-tun-helper/src/main.rs b/ostp-tun-helper/src/main.rs index 0090948..3691655 100644 --- a/ostp-tun-helper/src/main.rs +++ b/ostp-tun-helper/src/main.rs @@ -88,7 +88,19 @@ async fn main() -> Result<()> { log_to_file(&format!("Fatal error: {}", e)); } log_to_file("Helper exiting"); - Ok(()) + + // The WinTun blocking `receive` runs on a thread that `task.abort()` cannot + // cancel, so it keeps the adapter handle — and the default route bound to it — + // alive and prevents the tokio runtime from shutting down. Without this the + // process lingers as a zombie: `ostp_tun` stays Up, its metric-0 default route + // competes with the physical one, and the NEXT connect fails to install the + // server bypass route, so traffic loops back into a dead tunnel (no internet). + // The GUI launches a fresh helper for every connect, so this process has no + // more work once run_server returns. Give the synchronous route/firewall + // teardown a moment to finish, then force the process to exit so the kernel + // reclaims the adapter and every route bound to it. (Ported from 0.3.x b6e78c1.) + tokio::time::sleep(Duration::from_millis(800)).await; + std::process::exit(0); } async fn run_server(expected_token: String, port: u16) -> Result<()> { diff --git a/ostp-tun/src/windows.rs b/ostp-tun/src/windows.rs index 78a0917..00a6fbd 100644 --- a/ostp-tun/src/windows.rs +++ b/ostp-tun/src/windows.rs @@ -72,32 +72,59 @@ pub async fn create(opts: OstpTunOptions) -> Result { .mtu(opts.mtu) .up(); - let dev = tun::create(&tun_cfg).map_err(|e| anyhow!("Failed to create TUN device: {}", e))?; + // The IpHelper calls the `tun` crate performs right after Adapter::create + // (set address / mtu) can transiently fail with ERROR_INVALID_PARAMETER + // (os error 87) when the freshly created interface is not yet registered + // in the IP stack. Retry a few times; on retry the crate reuses the + // existing adapter via Adapter::open. (Ported from 0.3.x b6e78c1.) + let dev = { + let mut attempt = 0; + loop { + attempt += 1; + match tun::create(&tun_cfg) { + Ok(d) => break d, + Err(e) if attempt < 5 => { + tracing::warn!( + "TUN device creation attempt {}/5 failed: {} — retrying in 300ms", + attempt, e + ); + tokio::time::sleep(std::time::Duration::from_millis(300)).await; + } + Err(e) => return Err(anyhow!("Failed to create TUN device after {} attempts: {}", attempt, e)), + } + } + }; let dev = tun::AsyncDevice::new(dev).map_err(|e| anyhow!("TUN device async failed: {}", e))?; tracing::info!("TUN device 'ostp_tun' created."); let current_exe = std::env::current_exe()?.to_string_lossy().into_owned(); + // A freshly created WinTun adapter can take several seconds to appear in + // GetAdaptersAddresses (it only shows up once it has an operational IPv4 + // binding). The default route via the TUN is what actually captures + // traffic, so this lookup is critical — give it a generous window (~15s). let mut tun_index = None; - for _ in 0..20 { + for _ in 0..75 { if let Some(idx) = windows_route::sys::get_interface_index("ostp_tun") { tun_index = Some(idx); break; } - tokio::time::sleep(std::time::Duration::from_millis(100)).await; + tokio::time::sleep(std::time::Duration::from_millis(200)).await; } if let Some(idx) = tun_index { - let _ = windows_route::sys::add_ipv4_route( + match windows_route::sys::add_ipv4_route( std::net::Ipv4Addr::new(0, 0, 0, 0), std::net::Ipv4Addr::new(0, 0, 0, 0), std::net::Ipv4Addr::new(10, 1, 0, 1), idx, 5, - ); - tracing::info!("Default route via TUN (if_index={idx}, metric=5) added."); + ) { + Ok(()) => tracing::info!("Default route via TUN (if_index={idx}, metric=5) added."), + Err(e) => tracing::error!("Failed to add default route via TUN (if_index={idx}): {e} — traffic will NOT be captured."), + } } else { - tracing::warn!("Could not find ostp_tun index in routing table — traffic may not be captured."); + tracing::error!("Could not find ostp_tun index in routing table after 15s — traffic will NOT be captured."); } let exe1 = current_exe.clone(); diff --git a/ostp-tun/src/windows_route.rs b/ostp-tun/src/windows_route.rs index 13ca88f..d090b89 100644 --- a/ostp-tun/src/windows_route.rs +++ b/ostp-tun/src/windows_route.rs @@ -168,6 +168,34 @@ pub mod sys { } } + /// Delete every routing-table entry whose destination is `dest`/`mask`, + /// regardless of its gateway or interface. Used to purge stale bypass routes + /// left by a previous session (possibly pointing at an old gateway after a + /// network change) so a fresh, correct one can be installed. (Ported from 0.3.x b6e78c1.) + pub fn delete_routes_for_dest(dest: Ipv4Addr, mask: Ipv4Addr) { + unsafe { + let mut size: ULONG = 0; + if GetIpForwardTable(ptr::null_mut(), &mut size, 0) != ERROR_INSUFFICIENT_BUFFER { + return; + } + let mut buf: Vec = vec![0; size as usize]; + let table = buf.as_mut_ptr() as *mut MIB_IPFORWARDTABLE; + if GetIpForwardTable(table, &mut size, 0) != NO_ERROR { + return; + } + let want_dest = ipv4_to_dword(dest); + let want_mask = ipv4_to_dword(mask); + let entries = + std::slice::from_raw_parts_mut((*table).table.as_mut_ptr(), (*table).dwNumEntries as usize); + for row in entries { + if row.dwForwardDest == want_dest && row.dwForwardMask == want_mask { + // Delete the exact existing row (its own nexthop/ifindex). + let _ = DeleteIpForwardEntry(row); + } + } + } + } + /// Add bypass routes for a list of resolved IP addresses (typically from exclusion config). /// Each IP gets a /32 host route via the physical gateway so it bypasses the TUN. /// Returns list of (ip, gw, if_index) that were successfully added, for later cleanup. @@ -178,15 +206,24 @@ pub mod sys { metric: u32, ) -> Vec<(Ipv4Addr, Ipv4Addr, u32)> { let mut added = Vec::new(); + let mut seen = std::collections::HashSet::new(); + let mask = Ipv4Addr::new(255, 255, 255, 255); for &ip in ips { - let mask = Ipv4Addr::new(255, 255, 255, 255); + // The server IP is passed both as server_ip and inside bypass_ips, so + // dedupe to avoid a guaranteed "already exists" failure on the second add. + if !seen.insert(ip) { + continue; + } + // Purge any pre-existing /32 for this dest (e.g. a stale route via an + // old gateway from a previous session) so add_ipv4_route below installs + // the correct one instead of failing with ERROR_OBJECT_ALREADY_EXISTS. + delete_routes_for_dest(ip, mask); match add_ipv4_route(ip, mask, gw, if_index, metric) { Ok(()) => { added.push((ip, gw, if_index)); } Err(e) => { - // 87 = ERROR_INVALID_PARAMETER (route may already exist) - tracing::debug!("bypass route add {ip}/32 via {gw}: {e}"); + tracing::warn!("bypass route add {ip}/32 via {gw} (if {if_index}) failed: {e}"); } } } diff --git a/ostp/Cargo.toml b/ostp/Cargo.toml index 139d29e..0139621 100644 --- a/ostp/Cargo.toml +++ b/ostp/Cargo.toml @@ -20,3 +20,4 @@ tracing.workspace = true tracing-subscriber = { version = "0.3", features = ["env-filter"] } ostp-core = { path = "../ostp-core" } colored = "2.1" +rlimit = "0.11.0" diff --git a/ostp/src/main.rs b/ostp/src/main.rs index 91a3511..ffed45d 100644 --- a/ostp/src/main.rs +++ b/ostp/src/main.rs @@ -363,6 +363,10 @@ struct MuxConfig { #[tokio::main] async fn main() -> Result<()> { + // Raise the open-file-descriptor limit to avoid EMFILE under many concurrent + // connections (ported from 0.3.x fix 922cf0b). No-op / best-effort on platforms + // where it does not apply. + let _ = rlimit::increase_nofile_limit(1048576); ostp_client::logging::setup_panic_hook(); let _log_guard = ostp_client::logging::init_tracing("info", "ostp-cli", env!("CARGO_PKG_VERSION"));