From 8ea75cc987a541950f994c05bcdb9522a61ff644 Mon Sep 17 00:00:00 2001 From: Varun Agarwal Date: Wed, 9 Sep 2026 22:32:39 +0530 Subject: [PATCH 01/11] fix(ipn/proxies): restore getproxytimeout for proxyFor lock-read guard proxyFor()'s internal timeout for its RLock'd map-read goroutine (intended purely as a deadlock-recovery guard, per the ProxyFor doc-comment: "if it takes longer than getproxytimeout, it returns an error") was inadvertently changed from getproxytimeout (5s) to minWaitPeriodSec/2 (1s), with no accompanying doc-comment update and no explanation in the commit message. Impact: ProxyFor() only retries/waits for a missing proxy when isWellknown(id) is true (WG/Orbot/pip/internal/global-h1 ids). For any other, app-registered custom proxy id, proxyFor() is the only lookup attempt - there is no fallback wait. On a loaded or low-RAM device, the paired px.Lock() in AddProxy/RemoveProxy can legitimately hold the mutex for longer than 1s while a proxy is being registered/torn down (especially proxies whose constructor performs real I/O). Previously this had up to 5s of slack before proxyFor() gave up; now it has only 1s, turning a previously-recoverable, momentary lock stall into a permanent, unretried "proxy not found" for that connection - observed downstream (celzero/rethink-app-derived fork) as an intermittently failing custom local HTTP proxy route on Android TV / Fire TV Stick hardware, causing affected app connections to fail and the calling app to retry indefinitely. Fix: restore timeout := getproxytimeout, matching the function's existing doc-comment and preserving the deadlock-recovery intent this guard was designed for, without touching the minWaitPeriodSec change (3s->2s) which only affects the separate wellknown-id retry/backoff path and is not implicated in this regression. No behavior change for the intended deadlock-recovery case (an actual hang still errors out, just with the originally-documented 5s grace period instead of 1s). --- intra/ipn/proxies.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/intra/ipn/proxies.go b/intra/ipn/proxies.go index 797941b4..f752fd3b 100644 --- a/intra/ipn/proxies.go +++ b/intra/ipn/proxies.go @@ -929,7 +929,26 @@ func (px *proxifier) proxyFor(id string) (Proxy, error) { // Ingress (dummy): no fast path, fall through to general lookup } - timeout := time.Duration(minWaitPeriodSec/2) * time.Second + // Regression fix: this used to be getproxytimeout (5s) and was + // inadvertently shortened to minWaitPeriodSec/2 (1s) in 8677a52c + // ("core/volatile: cr by muse spark" era commit chain). proxyFor is + // called for every proxy id, including non-wellknown, app-registered + // ids (see isWellknown/ProxyFor above) for which there is NO retry/ + // wait fallback -- ProxyFor returns immediately with errProxyNotFound + // for those ids, so this is the *only* window a caller gets to find + // a just-registered proxy. The lookup itself is a cheap RLock'd map + // read (see below), but on loaded/low-RAM devices the paired Lock() + // in AddProxy/RemoveProxy can legitimately hold the mutex for longer + // than 1s during proxy setup/teardown, especially for proxies that do + // real I/O in their constructor. Shortening this guard to 1s turns a + // rare, recoverable stall into a hard, unretried lookup failure for + // any non-wellknown proxy id registered right around this window -- + // observed in production as a permanently-failing custom local proxy + // route until the next reconnect. Restoring getproxytimeout (5s) + // keeps this a deadlock-recovery guard (its original documented + // purpose, see the ProxyFor doc-comment above) rather than a + // register-race timeout. + timeout := getproxytimeout // go.dev/play/p/xCug1W3OcMH p, completed := core.Grx("pxr.ProxyFor: "+id, func(_ context.Context) (Proxy, error) { px.RLock() From 44f5c0316baceafa9af895bc4eb4914d3f4f59f7 Mon Sep 17 00:00:00 2001 From: Varun Agarwal Date: Sun, 13 Sep 2026 05:11:40 +0530 Subject: [PATCH 02/11] fix(intra): don't discard rwext read/write deadlines when TCP_USER_TIMEOUT is set forward() unwrapped the remote conn from its rwext deadline wrapper whenever rwext.SetTimeout() reported didSet=true, treating a successfully-applied low-level sockopt as equivalent to having a software read/write deadline in place. SetTimeout() only sets TCP_USER_TIMEOUT via core.SetTimeoutSockOpt(). TCP_USER_TIMEOUT bounds how long unacknowledged outbound data may go unacked before the kernel force-closes the connection - it has no effect on a blocking Read() that is simply waiting to receive more data from a peer that has gone idle without sending RST/FIN. It does not implement a receive/idle timeout. Once remote was unwrapped, the only mechanism that could bound such a Read() - rwext's extendr()/extendw(), which apply Go's real per-call SetReadDeadline/SetWriteDeadline via settings.DialerOpts - was discarded entirely. As a result, a relayed TCP connection to a peer that silently stops sending (common with some CDN/load-balancer behavior on idle keep-alive connections, or after a NAT/middlebox timeout that never surfaces an RST) blocks forward()'s Read() forever. The socket stays visibly ESTABLISHED with zero rx/tx queue activity indefinitely, and the app-level effect is a permanent hang (e.g., a media player stuck in a buffering state) with no path to recovery short of killing the connection/process. This was reproduced consistently on-device: a live TCP socket to a video CDN would enter this idle-ESTABLISHED state with zero queue bytes and never recover, while process CPU stayed idle (ruling out a busy loop) and DNS/WAN connectivity remained healthy throughout - pointing squarely at a stuck blocking Read() in the relay path. Fix: only unwrap remote from rwext when timeoutsecs <= 0, i.e. when no read/write deadline is configured at all and rwext.Read/Write would be a true no-op. When a positive timeout is configured, keep remote wrapped so extendr()/extendw() continue to enforce a genuine per-call deadline in addition to (not instead of) the TCP_USER_TIMEOUT sockopt optimization. --- intra/common.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/intra/common.go b/intra/common.go index 827e5cf3..8ca0a332 100644 --- a/intra/common.go +++ b/intra/common.go @@ -312,12 +312,20 @@ func (h *baseHandler) forward(local, remote net.Conn, smm *FlowSummary) { isrwext := false didSet := false timeoutsecs := 0 - // enable core.Pipe (sendfile/zero-copy) optimizations on TCP if - // read & write deadlines are not set (as in rwext is effectively - // a no-op) by unwrapping the underlying remote conn from rwext. + // enable core.Pipe (sendfile/zero-copy) optimizations on TCP only + // when no read/write deadline is actually configured (timeoutsecs + // <= 0), in which case rwext is a no-op wrapper and unwrapping is + // safe. Do NOT unwrap merely because didSet is true: SetTimeout + // (via core.SetTimeoutSockOpt) only sets TCP_USER_TIMEOUT, which + // bounds unacknowledged *writes*, not idle *reads*. If remote is + // unwrapped here while a positive timeoutsecs is configured, the + // only mechanism that can bound a stalled Read() (rwext's + // extendr/extendw, which set a real per-call deadline) is lost, + // and a peer that silently stops sending (no RST/FIN) causes + // Read() -- and thus this whole forward() -- to block forever. if r, ok := remote.(rwext); ok { isrwext = true - if timeoutsecs, didSet = r.SetTimeout(); didSet || timeoutsecs <= 0 { + if timeoutsecs, didSet = r.SetTimeout(); timeoutsecs <= 0 { remote = r.Unwrap() // c may be *net.TCPConn or *demuxconn or *dialers.retrier|splitter } } From c73f5da796031080ac91750413b31f311e0fabf0 Mon Sep 17 00:00:00 2001 From: Varun Agarwal Date: Sun, 13 Sep 2026 13:29:33 +0530 Subject: [PATCH 03/11] fix(dialers/retrier): re-apply read/write deadlines on every call retrier.Read()/Write() only pushed the caller-configured deadline down to the underlying conn once, right when the retry sequence completed; every subsequent call left the raw conn's deadline frozen at whatever was last applied, so callers (eg: rwext) refreshing an idle timeout on every Read/Write had no effect once retries were done. Additionally, retrier.WriteTo/ReadFrom's non-optimized fallback path streamed directly against the raw underlying conn (bypassing retrier.Read/Write entirely), so even a correct deadline on r never protected connections relayed via io.Copy-style WriteTo/ReadFrom (preferred by core.Pipe when available), leaving such connections with no idle-read/write timeout for their entire lifetime once the initial handshake completed. Fix: always (re)apply the current read/write deadline to the underlying conn before every raw Read/Write, and route the WriteTo/ReadFrom fallback through retrier's own Read/Write so it benefits from that refresh instead of bypassing it. --- intra/dialers/retrier.go | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/intra/dialers/retrier.go b/intra/dialers/retrier.go index e5e0f464..45bc285f 100644 --- a/intra/dialers/retrier.go +++ b/intra/dialers/retrier.go @@ -476,9 +476,17 @@ func (r *retrier) Read(buf []byte) (n int, err error) { r.mu.Lock() c := r.conn // r.conn may be provisional or final connection + rdeadline := r.readDeadline r.mu.Unlock() if c != nil { + // always (re)apply the caller's current read deadline (as set via + // SetReadDeadline, eg: rwext.extendr) to the underlying conn before + // reading from it; otherwise, once the retry sequence completes + // (see below), this deadline is applied to c just once and never + // again, so subsequent idle reads on c can block indefinitely even + // as callers keep extending r.readDeadline on every call. + _ = c.SetReadDeadline(rdeadline) for reads := range maxEmptyReads { n, err = c.Read(buf) if n == 0 && err == nil { // no data and no error @@ -670,6 +678,7 @@ func (r *retrier) Write(b []byte) (int, error) { r.mu.Lock() c := r.conn // retry has completed, so r.conn is final and may not need locking? + wdeadline := r.writeDeadline r.mu.Unlock() if c == nil { cerr := log.EE("retrier: write: %s: [] => %s (b: %d, tee: %d), not retrying, but no conn; after: %s", @@ -677,6 +686,8 @@ func (r *retrier) Write(b []byte) (int, error) { return 0, core.JoinErr(cerr, errNilConn) } + // always (re)apply the caller's current write deadline; see Read() for why. + _ = c.SetWriteDeadline(wdeadline) n, err := c.Write(b) if err != nil { err = log.EE("retrier: write: %s: [%s=>%s]; b: %d/%d (retried? %t); after: %s; err? %v", @@ -746,8 +757,11 @@ func (r *retrier) WriteTo(w io.Writer) (bytes int64, err error) { } if !optimizedWriteTo { - // write to w from c until EOF - b, err = core.Stream(w, c) + // write to w from r (not raw c) until EOF, so that r.Read's + // per-call deadline refresh (see Read()) stays in effect; reading + // from c directly bypasses that refresh and can hang indefinitely + // once the retry sequence above has completed. + b, err = core.Stream(w, r) bytes += b } @@ -842,8 +856,8 @@ func (r *retrier) ReadFrom(reader io.Reader) (bytes int64, err error) { } if !optimizedReadFrom { - // read from reader into c until EOF - b, err = core.Stream(c, reader) + // read from reader into r (not raw c) until EOF; see WriteTo for why. + b, err = core.Stream(r, reader) bytes += b } From c50f77e15d6b29165a0e97e24403ce570d14ac47 Mon Sep 17 00:00:00 2001 From: Varun Agarwal Date: Sun, 13 Sep 2026 13:42:24 +0530 Subject: [PATCH 04/11] fix(rwconn): scope rwext's deadline-disabling zero-copy path to *net.TCPConn only rwext.WriteTo/ReadFrom disabled all read/write deadlines (extendForever) whenever the wrapped conn satisfied the generic io.WriterTo/io.ReaderFrom interface, then fully delegated to it. This is safe for a genuine os-level conn (*net.TCPConn), where the underlying syscall (sendfile/ splice) is a single bounded operation. It is not safe for higher-level wrapper types -- eg *dialers.retrier, which also implements these interfaces but can internally fall back to a plain, chunked copy loop of its own (via core.Stream) when the peer isn't itself splice-capable. Because rwext handed off unconditionally and never called it again, such wrapper types never received a real, refreshed idle-read/write deadline: rwext's own deadlines() config was simply never applied. This left connections relayed this way (the common case for the 'Exit' proxy/dialer-retry path, which core.Pipe prefers via WriteTo/ReadFrom) with no idle timeout for their entire lifetime, regardless of settings.DialerOpts or the prior forward()/retrier fixes. Fix: only take the deadline-disabling fast path when the wrapped conn is specifically *net.TCPConn. For anything else (including *retrier), fall through to core.Stream(w, rw)/core.Stream(rw, r), which calls rwext's own Read()/Write() and so correctly refreshes a real idle deadline on every chunk, same as the plain (non zero-copy) path. Confirmed on-device: without this fix, a relayed TCP socket to a video CDN remained idle-ESTABLISHED (zero rx/tx queue bytes) for minutes, well past the configured 10s idle timeout, even with the forward() and retrier.go fixes already live in the running binary. --- intra/rwconn.go | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/intra/rwconn.go b/intra/rwconn.go index 812471de..4c7a3135 100644 --- a/intra/rwconn.go +++ b/intra/rwconn.go @@ -62,25 +62,28 @@ func (rw rwext) Write(b []byte) (n int, err error) { // ReadFrom implements core.RetrierConn. func (rw rwext) ReadFrom(r io.Reader) (n int64, err error) { switch c := rw.Unwrap().(type) { - case io.ReaderFrom: - // disable read and write deadlines for rw.Conn as - // io.ReaderFrom does not support io.Reader+io.Writer - // semantics which rwext relies on to extend deadlines. + case *net.TCPConn: + // disable read and write deadlines for rw.Conn as io.ReaderFrom + // (splice/sendfile) does not support io.Reader+io.Writer semantics + // which rwext relies on to extend deadlines; safe only for a true + // os-level conn where the syscall itself is a single operation, not + // an unbounded higher-level relay loop that can go idle forever. rw.extendForever() return c.ReadFrom(r) default: } - // nb: stream rw (which extends deadlines) not rw.Conn + // nb: stream rw (which extends deadlines) not rw.Conn; this also covers + // wrapper types (eg: *dialers.retrier) that implement io.ReaderFrom but + // may internally fall back to a plain, idle-able copy loop of their own + // -- such types must not have their deadlines disabled outright. return core.Stream(rw, r) } // WriteTo implements core.RetrierConn. func (rw rwext) WriteTo(w io.Writer) (n int64, err error) { switch c := rw.Unwrap().(type) { - case io.WriterTo: - // disable read and write deadlines for rw.Conn as - // io.WriterTo does not support io.Reader+io.Writer - // semantics which rwext relies on to extend deadlines. + case *net.TCPConn: + // see ReadFrom for why this is scoped to a genuine os-level conn. rw.extendForever() return c.WriteTo(w) default: From 76f51973dd023de6aa2573cb37fa646c514d2a94 Mon Sep 17 00:00:00 2001 From: Varun Agarwal Date: Sun, 13 Sep 2026 14:34:46 +0530 Subject: [PATCH 05/11] protect: clamp outbound TCP MSS to mitigate PMTUD-blackhole silent stalls Kernel-level diagnosis (via /proc/net/tcp rx_queue/tx_queue/retransmit counters polled across multiple hang reproductions) proved that after an Exit-proxied TCP connection's first response read (always <=~2861B, consistent with one or two full-MTU segments), the socket receives zero further bytes and sits with rx_queue==0 the entire time before our existing 10s idle-read deadline correctly closes it. This ruled out a data-delivery/read-dispatch bug in the retrier/relay code (the kernel itself has nothing buffered/unread) and pointed to a network- path issue: a classic PMTU-discovery blackhole, where the remote peer sends a segment that needs in-flight fragmentation, the ICMP fragmentation-needed reply required for real PMTUD is filtered/lost on some hop, and the peer's retransmits of the oversized segment never arrive. Reproduced identically across many unrelated destinations/CDNs simultaneously, ruling out any single origin's WAF/anti-bot behavior. Mitigation: clamp TCP_MAXSEG (pre-connect, via the existing Control/ ifbind socket-setup hook used by all outbound TCP dials) to a conservative 1400 bytes, forcing remote peers to never send us a segment large enough to need in-path fragmentation on typical access networks/tunnels. This is a standard, low-risk mitigation for this failure class -- it slightly reduces peak per-segment throughput on very large transfers but does not affect correctness. --- intra/protect/protect.go | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/intra/protect/protect.go b/intra/protect/protect.go index b6ecde06..015d2dde 100644 --- a/intra/protect/protect.go +++ b/intra/protect/protect.go @@ -36,6 +36,7 @@ import ( b "github.com/celzero/firestack/intra/backend" "github.com/celzero/firestack/intra/core" "github.com/celzero/firestack/intra/log" + "golang.org/x/sys/unix" ) // See: ipmap.LookupNetIP; Selfhost -> dnsx.Default; Systemhost -> dnsx.System @@ -55,6 +56,22 @@ const ( // if true, only protects the socket from routing loops & binds to active network. onlyProtectWildcardAddrs = false + + // clampedMSS is a conservative TCP MSS ceiling advertised (pre-connect, + // via TCP_MAXSEG) on all outbound TCP sockets. Mitigates a path-MTU + // discovery blackhole class of bug: a remote peer sends a full-MTU-sized + // segment that needs in-flight fragmentation on some hop of the path; + // the fragmentation-needed ICMP reply required for real PMTUD is + // filtered/lost (common middlebox misconfiguration), so the peer's + // retransmits of the oversized segment never make it through and the + // connection goes silent forever. Clamping our advertised MSS forces + // the remote peer to never send us a segment large enough to trigger + // this, at the cost of marginally smaller segments on high-throughput + // transfers. 1400 leaves headroom under the standard 1500-byte Ethernet + // MTU for the worst-case combined IPv4/IPv6 + TCP header overhead plus + // any additional encapsulation (eg: PPPoE, WireGuard, GRE, VPN) that may + // exist on the path. + clampedMSS = 1400 ) var MyUid = strconv.Itoa(os.Getuid()) @@ -86,6 +103,7 @@ func ifbind(who string, ctl Controller) func(string, string, syscall.RawConn) er log.VV("control: netbinder: %s: %s(%s); err? %v", who, network, addr, err) return c.Control(func(fd uintptr) { sock := int(fd) + clampMSS(who, network, sock) if onlyProtectWildcardAddrs && !maybeGlobalUnicast(addr, true) { ctl.Protect(who, sock) return @@ -104,6 +122,20 @@ func ifbind(who string, ctl Controller) func(string, string, syscall.RawConn) er } } +// clampMSS sets a conservative TCP_MAXSEG ceiling (best-effort; errors are +// logged, never fatal) on newly-created, not-yet-connected TCP sockets, to +// guard against a path-MTU-discovery blackhole (see clampedMSS doc above). +// No-op for non-TCP networks. +func clampMSS(who, network string, sock int) { + switch network { + case "tcp", "tcp4", "tcp6": + if err := unix.SetsockoptInt(sock, unix.IPPROTO_TCP, unix.TCP_MAXSEG, clampedMSS); err != nil { + log.D("protect: %s: mss-clamp(%d) on %s sock; err? %v", who, clampedMSS, network, err) + } + default: // udp, unix, etc: no-op + } +} + // unused: Binds a socket to a local ip. func ipbind(p Protector) func(string, string, syscall.RawConn) error { return func(network, addr string, c syscall.RawConn) (err error) { From bb9723a7a3f42ae5dc24b4266bcc04304e48b48c Mon Sep 17 00:00:00 2001 From: Varun Agarwal Date: Sun, 13 Sep 2026 14:40:29 +0530 Subject: [PATCH 06/11] debug: add temporary instrumentation for Zee5/PMTUD-blackhole investigation Not intended for permanent inclusion; will be reverted once the root cause is confirmed. Adds: - protect.clampMSS: getsockopt(TCP_MAXSEG) readback logged alongside the setsockopt call, to confirm the kernel actually honored our requested clamp (some kernels silently cap/ignore it). - dialers/retrier.dialStratLocked: logs the resolved split-strategy per dial attempt, to get ground truth on whether split/anti- censorship is really disabled at the dial layer (vs trusting the UI-level setting alone). - dialers/retrier.Read: pre- and post-read golang.org/x/sys/unix TCP_INFO snapshots (retransmits, rtt, last_data_recv, unacked) via getsockopt(TCP_INFO), bracketing every raw socket Read() call. Lets us see, at the exact moment a read stalls/times out, whether the kernel ever observed a retransmit attempt from the peer during the stall -- distinguishing a genuine network-path blackhole (no retransmits seen) from data arriving-but-undelivered (would show non-zero retransmits / rx_queue), which would instead point back at our own relay code. - netstack.GTCPConn.Read/Write: app-facing (netstack) side byte-count and error visibility (log.VV, requires raising the in-app Go log level to Verbose/Very-Verbose to observe), to check independently whether the app itself stops reading/writing during a stall. --- intra/dialers/retrier.go | 60 ++++++++++++++++++++++++++++++++++++++++ intra/netstack/tcp.go | 16 +++++++++-- intra/protect/protect.go | 11 ++++++-- 3 files changed, 82 insertions(+), 5 deletions(-) diff --git a/intra/dialers/retrier.go b/intra/dialers/retrier.go index 45bc285f..9d9df9a8 100644 --- a/intra/dialers/retrier.go +++ b/intra/dialers/retrier.go @@ -40,6 +40,7 @@ import ( "github.com/celzero/firestack/intra/log" "github.com/celzero/firestack/intra/protect" "github.com/celzero/firestack/intra/settings" + "golang.org/x/sys/unix" ) type zeroNetAddr struct{} @@ -67,6 +68,37 @@ const ( // TODO: with context.TODO, expmap's reaper goroutine will leak. var ippPins = core.NewSieve[netip.AddrPort, string](context.TODO(), "d.ippPins", desync_cache_ttl) +// dbgTCPInfo is a DEBUG-INSTRUMENTATION helper (temporary, remove once +// Zee5/PMTUD investigation is closed). It fetches kernel-level TCP_INFO +// stats (retransmits, rtt, last-data-received) for c, if c is backed by a +// real *net.TCPConn (best-effort; returns "" if unavailable). This lets us +// see, precisely at the moment a Read() stalls or times out, whether the +// kernel itself ever observed a retransmit from the peer during the stall +// -- distinguishing a genuine network-path blackhole (no retransmits seen, +// peer/path truly silent) from data arriving-but-undelivered (retransmits +// seen, rx_queue would be non-zero) which would point back at our code. +func dbgTCPInfo(c protect.Conn) string { + sc, ok := c.(syscall.Conn) + if !ok || sc == nil { + return "" + } + raw, err := sc.SyscallConn() + if err != nil || raw == nil { + return "" + } + var info *unix.TCPInfo + var operr error + cerr := raw.Control(func(fd uintptr) { + info, operr = unix.GetsockoptTCPInfo(int(fd), unix.IPPROTO_TCP, unix.TCP_INFO) + }) + if cerr != nil || operr != nil || info == nil { + return fmt.Sprintf("tcpinfo-err(ctl=%v,op=%v)", cerr, operr) + } + return fmt.Sprintf("tcpi[state=%d rtt=%dus rttvar=%dus retx=%d total_retx=%d last_data_recv=%dms last_data_sent=%dms unacked=%d]", + info.State, info.Rtt, info.Rttvar, info.Retransmits, info.Total_retrans, + info.Last_data_recv, info.Last_data_sent, info.Unacked) +} + // retrier implements the DuplexConn interface and must // be typecastable to *net.TCPConn (see: xdial.DialTCP) // inheritance: go.dev/play/p/mMiQgXsPM7Y @@ -308,6 +340,13 @@ func (r *retrier) dialStratLocked() (strat int32, err error) { strat = r.dialerOpts.Strat } + // DEBUG-INSTRUMENTATION (temporary, remove once Zee5/PMTUD investigation + // is closed): confirms at the dial layer exactly which split-strategy is + // actually being used per attempt, independent of what the UI/settings + // claim is configured. + log.I("retrier: dbg: %s: dialStrat: %s: strat=%d auto=%t retryStrat=%d split=%t retryCount=%d/%d", + r.dialerID(), r.raddr, strat, auto, retryStrat, split, r.retryCount, r.maxRetries) + return } @@ -487,6 +526,14 @@ func (r *retrier) Read(buf []byte) (n int, err error) { // again, so subsequent idle reads on c can block indefinitely even // as callers keep extending r.readDeadline on every call. _ = c.SetReadDeadline(rdeadline) + + // DEBUG-INSTRUMENTATION (temporary, remove once Zee5/PMTUD + // investigation is closed): kernel TCP_INFO snapshot right before + // issuing the read, so we can diff against the post-read snapshot + // below to see exactly what changed (or didn't) at the kernel level + // during this call, especially across a timeout/stall. + preTCPInfo := dbgTCPInfo(c) + for reads := range maxEmptyReads { n, err = c.Read(buf) if n == 0 && err == nil { // no data and no error @@ -496,6 +543,19 @@ func (r *retrier) Read(buf []byte) (n int, err error) { } // else: check if retry is needed (c == nil or err != nil) break } + + // DEBUG-INSTRUMENTATION (temporary, remove once Zee5/PMTUD + // investigation is closed): post-read snapshot. If Total_retrans + // increased between pre/post while err is a timeout, the kernel + // DID see retransmits from the peer during the stall (data was + // attempted but never fully arrived/ack'd) -- a genuine network + // issue. If Total_retrans is unchanged, the peer never even tried + // to resend, consistent with a silent path blackhole (eg PMTUD) + // upstream of this device entirely. + postTCPInfo := dbgTCPInfo(c) + log.I("retrier: dbg: %s: read-tcpinfo: [%s<=%s]; pre: %s; post: %s; n=%d err=%v", + r.dialerID(), laddr(c), r.raddr, preTCPInfo, postTCPInfo, n, err) + if n == 0 && err == nil { err = io.ErrNoProgress } diff --git a/intra/netstack/tcp.go b/intra/netstack/tcp.go index 640ddfe8..e5b58fc1 100644 --- a/intra/netstack/tcp.go +++ b/intra/netstack/tcp.go @@ -326,14 +326,26 @@ func (g *GTCPConn) RemoteAddr() net.Addr { func (g *GTCPConn) Write(data []byte) (int, error) { if c := g.conn(); c != nil { - return c.Write(data) + n, err := c.Write(data) + // DEBUG-INSTRUMENTATION (temporary, remove once Zee5/PMTUD + // investigation is closed): app-facing (netstack) side write + // visibility, to check whether the app itself ever stops writing + // (eg: waiting on a stalled read) vs our exit-side relay stalling + // independently. + log.VV("netstack: dbg: gconn(%s): write: b=%d/%d; err=%v", g.o, n, len(data), err) + return n, err } return 0, netError(g, "tcp", g.o+":write", io.ErrClosedPipe) } func (g *GTCPConn) Read(data []byte) (int, error) { if c := g.conn(); c != nil { - return c.Read(data) + n, err := c.Read(data) + // DEBUG-INSTRUMENTATION (temporary, remove once Zee5/PMTUD + // investigation is closed): app-facing (netstack) side read + // visibility, paired with the Write() log above. + log.VV("netstack: dbg: gconn(%s): read: b=%d/%d; err=%v", g.o, n, len(data), err) + return n, err } return 0, netError(g, "tcp", g.o+":read", io.ErrNoProgress) } diff --git a/intra/protect/protect.go b/intra/protect/protect.go index 015d2dde..54a6c840 100644 --- a/intra/protect/protect.go +++ b/intra/protect/protect.go @@ -126,12 +126,17 @@ func ifbind(who string, ctl Controller) func(string, string, syscall.RawConn) er // logged, never fatal) on newly-created, not-yet-connected TCP sockets, to // guard against a path-MTU-discovery blackhole (see clampedMSS doc above). // No-op for non-TCP networks. +// +// DEBUG-INSTRUMENTATION (temporary, remove once Zee5/PMTUD investigation is +// closed): logs a getsockopt readback of TCP_MAXSEG immediately after +// setting it, since some kernels silently cap/ignore the requested value. func clampMSS(who, network string, sock int) { switch network { case "tcp", "tcp4", "tcp6": - if err := unix.SetsockoptInt(sock, unix.IPPROTO_TCP, unix.TCP_MAXSEG, clampedMSS); err != nil { - log.D("protect: %s: mss-clamp(%d) on %s sock; err? %v", who, clampedMSS, network, err) - } + serr := unix.SetsockoptInt(sock, unix.IPPROTO_TCP, unix.TCP_MAXSEG, clampedMSS) + got, gerr := unix.GetsockoptInt(sock, unix.IPPROTO_TCP, unix.TCP_MAXSEG) + log.I("protect: dbg: %s: mss-clamp: set(%d)? err=%v; readback=%d; err=%v; %s sock", + who, clampedMSS, serr, got, gerr, network) default: // udp, unix, etc: no-op } } From 90986dbc0b5c6c942e7fce5bb649d787579d7712 Mon Sep 17 00:00:00 2001 From: Varun Agarwal Date: Sun, 13 Sep 2026 15:00:56 +0530 Subject: [PATCH 07/11] com: propagate abrupt TCP RST instead of a graceful FIN when a relayed conn errors out abnormally Closes a long-standing TODO ('Propagate TCP RST using local.Abort(), on appropriate errors'). download() previously always gracefully closed the app-facing conn's write-half (FIN) once its remote->local copy loop (core.Pipe) returned -- even when that return was due to a genuine abnormal error (eg: our own idle-read timeout firing because a CDN peer silently stopped sending more data on an otherwise fully healthy, ESTABLISHED connection -- confirmed via kernel TCP_INFO: retransmits=0, unacked=0 at the moment of timeout, ruling out packet loss/blackhole). A graceful FIN signals 'no more data, but everything received so far is a complete, valid response' to the peer's TCP/HTTP stack. Silently downgrading a real error into that signal is at best ambiguous and at worst actively misleading for HTTP clients relying on this connection (eg: a still-buffering in-flight response, or a keep-alive pool that may not immediately notice or correctly recover from the close), particularly for long-lived streaming responses without a client-known final length. Fix: resetOrClose() in common.go now inspects the actual copy-loop error. A plain io.EOF (peer's own graceful close) still results in a normal CloseOp/FIN as before. Any other, abnormal error now sends an RST via the app-facing *netstack.GTCPConn's existing Abort() method (added generically via a minimal local 'aborter' interface, so it's a no-op fallback to the prior graceful-close behavior for any conn type that doesn't support Abort()), giving the app an unambiguous, immediate ECONNRESET signal instead. --- intra/common.go | 38 ++++++++++++++++++++++++++++++++++++-- intra/dialers/retrier.go | 5 +++-- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/intra/common.go b/intra/common.go index 8ca0a332..1254e89b 100644 --- a/intra/common.go +++ b/intra/common.go @@ -9,6 +9,7 @@ package intra import ( "context" "fmt" + "io" "math/rand" "net" "net/netip" @@ -576,8 +577,41 @@ func (h *baseHandler) Reset() { log.I("com: %s: handler reset", h.proto) } +// aborter is implemented by conns (currently just *netstack.GTCPConn) that +// can forcibly terminate with a TCP RST, as opposed to a graceful FIN. +type aborter interface { + Abort() +} + +// resetOrClose closes op on c, unless err is a genuine abnormal error (not a +// plain io.EOF, which indicates the peer closed gracefully) and c supports +// aborting -- in which case a TCP RST is sent instead of a graceful FIN. +// +// This addresses a previously-known gap (see the old "TODO: Propagate TCP +// RST using local.Abort(), on appropriate errors" note this replaces): +// download() closing the app-facing conn's write-half gracefully (FIN) even +// when the actual cause was an abnormal upstream failure (eg: our own +// idle-read timeout firing because the remote peer silently stopped +// sending) meant the app could observe what looks like a clean, complete +// close -- rather than an unambiguous connection-reset -- for a connection +// that, in fact, terminated abnormally. Some HTTP clients treat a graceful +// close of an in-flight response as ambiguous/retryable at best, or a +// silently-truncated "complete" response at worst; a RST is unambiguous. +func resetOrClose(c io.Closer, op core.CloserOp, err error) { + if err != nil && err != io.EOF { + if a, ok := c.(aborter); ok && a != nil && core.IsNotNil(a) { + // DEBUG-INSTRUMENTATION (temporary, remove once Zee5/PMTUD + // investigation is closed): confirms this RST-on-error path + // actually engages (vs falling through to a graceful close). + log.I("com: dbg: resetOrClose: aborting (RST) %T on err: %v", c, err) + a.Abort() + return + } + } + core.CloseOp(c, op) +} + // upload copies data from remote to local, and returns the number of bytes copied and error, if any. -// TODO: Propagate TCP RST using local.Abort(), on appropriate errors. func upload(id string, local, remote net.Conn, ioch chan<- ioinfo) { defer core.Recover(core.Exit11, "c.upload."+id) defer core.CloseOp(local, core.CopR) @@ -595,7 +629,7 @@ func upload(id string, local, remote net.Conn, ioch chan<- ioinfo) { // download copies data from local to remote, and returns the number of bytes copied and error, if any. func download(id string, local, remote net.Conn) (n int64, err error) { - defer core.CloseOp(local, core.CopW) + defer func() { resetOrClose(local, core.CopW, err) }() defer core.CloseOp(remote, core.CopR) n, err = core.Pipe(local, remote) diff --git a/intra/dialers/retrier.go b/intra/dialers/retrier.go index 9d9df9a8..bb293538 100644 --- a/intra/dialers/retrier.go +++ b/intra/dialers/retrier.go @@ -94,9 +94,10 @@ func dbgTCPInfo(c protect.Conn) string { if cerr != nil || operr != nil || info == nil { return fmt.Sprintf("tcpinfo-err(ctl=%v,op=%v)", cerr, operr) } - return fmt.Sprintf("tcpi[state=%d rtt=%dus rttvar=%dus retx=%d total_retx=%d last_data_recv=%dms last_data_sent=%dms unacked=%d]", + return fmt.Sprintf("tcpi[state=%d rtt=%dus rttvar=%dus retx=%d total_retx=%d last_data_recv=%dms last_data_sent=%dms unacked=%d snd_mss=%d rcv_mss=%d pmtu=%d]", info.State, info.Rtt, info.Rttvar, info.Retransmits, info.Total_retrans, - info.Last_data_recv, info.Last_data_sent, info.Unacked) + info.Last_data_recv, info.Last_data_sent, info.Unacked, + info.Snd_mss, info.Rcv_mss, info.Pmtu) } // retrier implements the DuplexConn interface and must From 48e24e6c515ead8cf1b2e28ff819f856af76aada Mon Sep 17 00:00:00 2001 From: Varun Agarwal Date: Sun, 13 Sep 2026 15:41:16 +0530 Subject: [PATCH 08/11] dns: recognize upstream loopback-sinkhole answers (127.0.0.1/::1) as blocks, not just 0.0.0.0/:: Some public ad/tracker-blocking DNS resolvers signal a blocked domain by answering with a loopback address (127.0.0.1 / ::1) instead of the more common unspecified address (0.0.0.0 / ::) that xdns.AQuadAUnspecified() already detects. Every block-detection check in this codebase only tested for the unspecified-address form, so a loopback-sinkholed answer was silently treated as a normal, real answer: - UpstreamBlocks was never set on the DNS summary, so on-device query logs report such domains as NOT blocked even though they effectively are. - Any TCP connect a client subsequently makes to that loopback IP never enters the VPN tunnel at all (Android does not route loopback-destined traffic through a VPN's tun interface), so it is invisible to FirewallManager/ConnectionTracker: the app just gets an instant local ECONNREFUSED from the OS with zero visibility or control from this engine. - DNS64 synthesis and the multi-resolver "Plus" fallback logic would both treat such a loopback answer as a legitimate, usable answer rather than recognizing it as an upstream block signal. Fix: add xdns.AQuadALoopback(), analogous to the existing xdns.AQuadAUnspecified(), and OR it into every upstream-answer block-detection check: alg.go's querySecondary block-detection and DNS64-summary block-detection, transport.go's ansblocked computation, plus.go's multi-resolver ipblock detection, and x64/dns64.go's two DNS64 synthesis guards. No functional change for upstream answers that were already correctly recognized as blocked (0.0.0.0/::); this only extends recognition to the loopback-sinkhole style some resolvers use. --- intra/dnsx/alg.go | 8 +++++--- intra/dnsx/plus.go | 2 +- intra/dnsx/transport.go | 6 ++++-- intra/x64/dns64.go | 4 ++-- intra/xdns/dnsutil.go | 30 ++++++++++++++++++++++++++++++ 5 files changed, 42 insertions(+), 8 deletions(-) diff --git a/intra/dnsx/alg.go b/intra/dnsx/alg.go index 1e66c021..cbe6b49f 100644 --- a/intra/dnsx/alg.go +++ b/intra/dnsx/alg.go @@ -1366,8 +1366,10 @@ func (t *dnsgateway) querySecondary(t2 Transport, uid, fid, network string, msg result.smm.Blocklists = blocklistnames result.smm.BlockedTarget = blockedtarget } - if xdns.AQuadAUnspecified(r) { - // A/AAAA must be 0.0.0.0/::, set UpstreamBlocks to true + if xdns.AQuadAUnspecified(r) || xdns.AQuadALoopback(r) { + // A/AAAA must be 0.0.0.0/:: (or, for some upstream resolvers, + // sinkholed to loopback 127.0.0.1/::1); either way, set + // UpstreamBlocks to true result.smm.UpstreamBlocks = true // discard all other answers result.ips = append(result.ips, anyaddr4, anyaddr6) @@ -1518,7 +1520,7 @@ func (t *dnsgateway) q(t1, t2 Transport, preset []netip.Addr, origin, network, s // for t1, ansin's already evaluated for ans0000 in querySecondary // (secans.pri is set to true). ansin may be from t2 (if t2 != nil), // ans64, which is a modified ansin, depending on settings.PtMode - ans0000 := xdns.AQuadAUnspecified(ansin) // ansin is not nil; ans64 may be nil + ans0000 := xdns.AQuadAUnspecified(ansin) || xdns.AQuadALoopback(ansin) // ansin is not nil; ans64 may be nil if ans0000 { smm.BlockedTarget = qname diff --git a/intra/dnsx/plus.go b/intra/dnsx/plus.go index 5890f0d9..11d5de09 100644 --- a/intra/dnsx/plus.go +++ b/intra/dnsx/plus.go @@ -384,7 +384,7 @@ func (t *plus) forward(network string, q *dns.Msg, outSmm *x.DNSSummary, all ... failed := xdns.IsServFailOrInvalid(ans) noans := !failed && !xdns.HasAnyAnswer(ans) - ipblock := xdns.HasAQuadAQuestion(q) && xdns.AQuadAUnspecified(ans) + ipblock := xdns.HasAQuadAQuestion(q) && (xdns.AQuadAUnspecified(ans) || xdns.AQuadALoopback(ans)) // HTTPS/SVCB blocks have 0 answer records when blocked svcbblock := (xdns.HasHTTPQuestion(q) || xdns.HasSVCBQuestion(q)) && noans diff --git a/intra/dnsx/transport.go b/intra/dnsx/transport.go index 35fcad22..f9b920e7 100644 --- a/intra/dnsx/transport.go +++ b/intra/dnsx/transport.go @@ -836,8 +836,10 @@ runagain: } // else: discard ans2 (which is always exclusively a blocked ans from rdns blocklists) realips := Netip2Csv(xdns.IPs(nonalg)) - // ans1 is upstream answer... does upstream block? - ansblocked := xdns.AQuadAUnspecified(ans1) + // ans1 is upstream answer... does upstream block? some public + // ad/tracker-blocking resolvers sinkhole to loopback (127.0.0.1/::1) + // instead of the unspecified address; treat both as upstream blocks. + ansblocked := xdns.AQuadAUnspecified(ans1) || xdns.AQuadALoopback(ans1) if log.Verbose { log.V("dns: fwd: 7 for %s[%s] (fid: %s); query %s:%d, r%d, onQueryTime: %s / onAnswerTime: %s, ips: %s; smm[data: %s, status: %d]; new-ans? %t, blocklists? %t, blocked? %t", diff --git a/intra/x64/dns64.go b/intra/x64/dns64.go index 67d23f55..2fedf097 100644 --- a/intra/x64/dns64.go +++ b/intra/x64/dns64.go @@ -262,7 +262,7 @@ func (d *dns64) eval(network string, force64 bool, ansin *dns.Msg, r, uid string // hasaaaq(true) hasans(true) rgood(true) ans0000(false) hasq6 := xdns.HasAAAAQuestion(ansin) hasans6 := xdns.HasAAAAAnswer(ansin) - ans00006 := xdns.AQuadAUnspecified(ansin) + ans00006 := xdns.AQuadAUnspecified(ansin) || xdns.AQuadALoopback(ansin) hasauth := xdns.IsDNSSECAnswerAuthenticated(ansin) // treat as if v6 answer missing if enforcing 6to4 if !hasq6 || ((hasauth || hasans6) && !force64) || ans00006 { @@ -289,7 +289,7 @@ func (d *dns64) eval(network string, force64 bool, ansin *dns.Msg, r, uid string ans4, err := d.query64(network, ansin, r, uid) rgood := xdns.HasRcodeSuccess(ans4) hasans := xdns.HasAnyAnswer(ans4) - ans0000 := xdns.AQuadAUnspecified(ans4) + ans0000 := xdns.AQuadAUnspecified(ans4) || xdns.AQuadALoopback(ans4) if err != nil || ans4 == nil || !hasans || ans0000 { log.W("dns64: skip: for %s, query(n:%s / a? %t) on resolver(%s[%s]/%s), code(good? %t / blocked? %t), err(%v)", uid, qname, hasans, r, id, network, rgood, ans0000, err) diff --git a/intra/xdns/dnsutil.go b/intra/xdns/dnsutil.go index e669ba96..7c0d9cb6 100644 --- a/intra/xdns/dnsutil.go +++ b/intra/xdns/dnsutil.go @@ -1433,6 +1433,36 @@ func AQuadAUnspecified(msg *dns.Msg) bool { return false } +// AQuadALoopback returns true if any A/AAAA record in msg resolves to a +// loopback address (127.0.0.0/8 or ::1). Some upstream resolvers (public +// ad/tracker-blocking DNS providers) signal a block by sinkholing the +// answer to loopback instead of the more common 0.0.0.0/:: unspecified +// address that AQuadAUnspecified detects. Without this check, such +// upstream-blocked answers are silently treated as valid, real answers: +// the resulting connection attempt never reaches the VPN's tun interface +// (loopback-destined traffic is routed by the OS outside any VPN tunnel), +// so it is invisible to on-device firewall/connection logs, and is never +// flagged as blocked even though nothing will ever answer on it. +func AQuadALoopback(msg *dns.Msg) bool { + if msg == nil { + return false + } + ans := msg.Answer + for _, rr := range ans { + switch v := rr.(type) { + case *dns.AAAA: + if v.AAAA.IsLoopback() { + return true + } + case *dns.A: + if v.A.IsLoopback() { + return true + } + } + } + return false +} + func Len(msg *dns.Msg) int { if msg == nil { return 0 From 6d246906098773fb4633f320bd73887fd1145266 Mon Sep 17 00:00:00 2001 From: Varun Agarwal Date: Sun, 13 Sep 2026 21:35:15 +0530 Subject: [PATCH 09/11] fix(rwconn): grant warmed TCP conns a longer idle-read grace period rwext.Read() reset its idle-read deadline to a flat settings.DialerOpts.ReadTimeoutSec (default 10s, see PersistentState.kt:dialTimeoutSec) on every call, regardless of whether the conn had ever exchanged data. That 10s default was introduced to kill PMTUD/censorship-blackholed conns that never respond at all (v2.0.1 Zee5 freeze fix). Root-caused via 3-state (vpn-off / vpn-on+warm-app-cache / vpn-on+cold-app-cache) logcat capture+diff (2026-09-13): on a cold Zee5 app-cache launch, the video CDN's persistent HTTPS conn receives the DASH/HLS manifest response, then legitimately sits fully idle for slightly over 10s while the client does DRM/token provisioning before issuing its next (segment-fetch) request on the same keep-alive conn. Firestack's 10s idle-read timeout fires first, RSTs the conn out from under the app, and Zee5 never retries -- a silent, permanent, no-error player hang. Track a per-conn warm flag (rwext.warm, *atomic.Bool) set the first time Read() returns >0 bytes. Once warm, deadlines() grants at least warmReadGraceSec (45s) on subsequent reads instead of the base timeout, since a conn that has already exchanged data is proven alive and not a blackhole candidate -- it's merely idling between two logical HTTP requests on a reused connection. Cold (never-yet-successful) conns are unaffected and keep the original short, aggressive deadline. --- intra/rwconn.go | 37 ++++++++++++++++++++++++++++++++----- intra/tcp.go | 5 +++-- intra/udp.go | 5 +++-- 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/intra/rwconn.go b/intra/rwconn.go index 4c7a3135..7f2d1490 100644 --- a/intra/rwconn.go +++ b/intra/rwconn.go @@ -9,6 +9,7 @@ package intra import ( "io" "net" + "sync/atomic" "syscall" "time" @@ -16,11 +17,28 @@ import ( "github.com/celzero/firestack/intra/settings" ) +// warmReadGraceSec is the minimum idle-read grace period (secs) granted to a +// TCP conn once it has successfully read at least one byte from the remote +// (ie: proven to not be a PMTUD/censorship blackhole -- the failure mode +// settings.DialerOpts.ReadTimeoutSec's default (10s) was introduced to catch, +// see: PersistentState.kt:dialTimeoutSec). Persistent HTTP/1.1|2 keep-alive +// conns routinely sit fully idle between a completed response and the app's +// next logical request; eg: Zee5's cold-cache DRM/token provisioning after +// receiving its DASH/HLS manifest response can legitimately exceed the base +// 10s idle deadline, causing Firestack to RST an already-healthy, still-in- +// use connection out from under the app before it's reused -- manifesting as +// a silent, permanent player hang (no retry, no app-visible error). Root +// caused via 3-state (vpn-off / vpn-on+warm-app-cache / vpn-on+cold-app- +// cache) logcat capture+diff, 2026-09-13. Never-yet-successful (cold) conns +// are unaffected and keep the short, aggressive base deadline. +const warmReadGraceSec = 45 + // rwext wraps MinConn and extends deadline to minimum(min, settings.DialerOpts) // on every read and write. type rwext struct { - net.Conn // underlying conn - minidle uint32 // min idle timeout in secs + net.Conn // underlying conn + minidle uint32 // min idle timeout in secs + warm *atomic.Bool // set once a read succeeds on this conn; never nil } // TODO? var _ core.DuplexCloser = (*rwext)(nil) @@ -51,7 +69,11 @@ func (rw rwext) Unwrap() net.Conn { func (rw rwext) Read(b []byte) (n int, err error) { rw.extendr() - return rw.Conn.Read(b) + n, err = rw.Conn.Read(b) + if n > 0 && rw.warm != nil { + rw.warm.Store(true) // conn proven alive; grant a longer idle grace hereon + } + return } func (rw rwext) Write(b []byte) (n int, err error) { @@ -103,8 +125,13 @@ func (rw rwext) SyscallConn() (syscall.RawConn, error) { func (rw rwext) deadlines() (r, w uint32) { dopt := settings.GetDialerOpts() // -ve ints go higher than 2^31 w/ uint: go.dev/play/p/Rrqk_V8a7W0 - return max(rw.minidle, uint32(dopt.ReadTimeoutSec)), - max(rw.minidle, uint32(dopt.WriteTimeoutSec)) + r = max(rw.minidle, uint32(dopt.ReadTimeoutSec)) + if rw.warm != nil && rw.warm.Load() { + // already exchanged >=1 byte on this conn: not a blackhole candidate; + // extend (never shrink) the read-idle grace for keep-alive reuse gaps. + r = max(r, warmReadGraceSec) + } + return r, max(rw.minidle, uint32(dopt.WriteTimeoutSec)) } func (rw rwext) extendForever() { diff --git a/intra/tcp.go b/intra/tcp.go index 4fcb86d6..8d8e2f99 100644 --- a/intra/tcp.go +++ b/intra/tcp.go @@ -31,6 +31,7 @@ import ( "net" "net/netip" "sync" + "sync/atomic" "time" "github.com/celzero/firestack/intra/core" @@ -193,7 +194,7 @@ func (h *tcpHandler) ReverseProxy(gconn *netstack.GTCPConn, in net.Conn, to, fro } core.Go("tcp.reverse:"+cid, func() { - h.forward(gconn, rwext{in, tcptimeout}, smm) + h.forward(gconn, rwext{Conn: in, minidle: tcptimeout, warm: new(atomic.Bool)}, smm) }) return true } @@ -483,7 +484,7 @@ func (h *tcpHandler) handle(px ipn.Proxy, gconn *netstack.GTCPConn, src, target core.Go("tcp.forward."+smm.ID, func() { defer h.loopUnassoc(smm) h.flowing(smm) - h.forward(gconn, rwext{dst, tcptimeout}, smm) // src always *gonet.TCPConn + h.forward(gconn, rwext{Conn: dst, minidle: tcptimeout, warm: new(atomic.Bool)}, smm) // src always *gonet.TCPConn // TODO: assoc if forward was successful if eim { h.natAssoc(smm.PID, src, dstlocal) diff --git a/intra/udp.go b/intra/udp.go index 59c4391a..081b566c 100644 --- a/intra/udp.go +++ b/intra/udp.go @@ -30,6 +30,7 @@ import ( "errors" "net" "net/netip" + "sync/atomic" "time" "github.com/celzero/firestack/intra/dnsx" @@ -124,7 +125,7 @@ func (h *udpHandler) ReverseProxy(gconn *netstack.GUDPConn, in net.Conn, to, fro } core.Go("udp.reverse:"+cid, func() { - h.forward(gconn, rwext{in, udptimeout}, smm) + h.forward(gconn, rwext{Conn: in, minidle: udptimeout, warm: new(atomic.Bool)}, smm) }) return true } @@ -201,7 +202,7 @@ func (h *udpHandler) proxy(gconn *netstack.GUDPConn, src, dst netip.AddrPort, dm core.Go("udp.forward."+cid, func() { defer h.loopUnassoc(smm) h.flowing(smm) - h.forward(gconn, rwext{remote, udptimeout}, smm) + h.forward(gconn, rwext{Conn: remote, minidle: udptimeout, warm: new(atomic.Bool)}, smm) }) return true // ok } From 5e0d505e617b220e43df3fba584b84c4117abf8e Mon Sep 17 00:00:00 2001 From: Varun Agarwal Date: Sun, 13 Sep 2026 21:59:09 +0530 Subject: [PATCH 10/11] fix(intra): remove idle-read deadline entirely for warm TCP conns The prior fix (commit 6d246906) granted a fixed 45s idle-read grace to 'warm' conns (proven alive via >=1 successful read) instead of the base 10s deadline. On-device re-test (2026-09-13, vpn-on+cold-cache) showed this was still insufficient: Zee5's cold-cache DRM/token provisioning think-time exceeded 45s on this run, so the connection was RST at the 45s mark instead of 10s -- delaying but not fixing the hang. Since no finite grace period can be guaranteed sufficient (app think-time is unbounded and variable), remove the idle-read deadline entirely for warm conns instead of picking a larger fixed number. A warm conn has already proven 2-way liveness, so it is provably not a PMTUD/censorship blackhole candidate; a genuine mid-stream death is still caught via a real RST/FIN from the peer or the OS TCP stack's own keepalive, without needing an app-level timer to force it. Cold (never-yet-successful) conns are unaffected and keep the original aggressive 10s deadline (settings.DialerOpts.ReadTimeoutSec), which is what catches genuine blackholed connections and is required to prevent the original Zee5 hang mode that motivated dialTimeoutSec's 0->10 change (see PersistentState.kt). UDP conns are unaffected: rw.minidle's 120s floor for udp is preserved even when warm, since UDP flows have no real end-of-stream signal equivalent to TCP's RST/FIN and still need bounded idle cleanup. --- intra/rwconn.go | 62 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 43 insertions(+), 19 deletions(-) diff --git a/intra/rwconn.go b/intra/rwconn.go index 7f2d1490..e40bd2f7 100644 --- a/intra/rwconn.go +++ b/intra/rwconn.go @@ -17,21 +17,37 @@ import ( "github.com/celzero/firestack/intra/settings" ) -// warmReadGraceSec is the minimum idle-read grace period (secs) granted to a -// TCP conn once it has successfully read at least one byte from the remote -// (ie: proven to not be a PMTUD/censorship blackhole -- the failure mode -// settings.DialerOpts.ReadTimeoutSec's default (10s) was introduced to catch, -// see: PersistentState.kt:dialTimeoutSec). Persistent HTTP/1.1|2 keep-alive -// conns routinely sit fully idle between a completed response and the app's -// next logical request; eg: Zee5's cold-cache DRM/token provisioning after -// receiving its DASH/HLS manifest response can legitimately exceed the base -// 10s idle deadline, causing Firestack to RST an already-healthy, still-in- -// use connection out from under the app before it's reused -- manifesting as -// a silent, permanent player hang (no retry, no app-visible error). Root -// caused via 3-state (vpn-off / vpn-on+warm-app-cache / vpn-on+cold-app- -// cache) logcat capture+diff, 2026-09-13. Never-yet-successful (cold) conns -// are unaffected and keep the short, aggressive base deadline. -const warmReadGraceSec = 45 +// A TCP conn's idle-read deadline is governed by +// settings.DialerOpts.ReadTimeoutSec (default 10s, see: +// PersistentState.kt:dialTimeoutSec). That deadline exists to catch conns +// that are PMTUD/censorship blackholed -- silently dropped with no RST/FIN -- +// which would otherwise hang forever with no timeout at all (the original +// failure mode dialTimeoutSec's 0->10 change was introduced to fix). +// +// But persistent HTTP/1.1|2 keep-alive conns routinely sit fully idle between +// a completed response and the app's next logical request, and that idle gap +// is not bounded by any protocol invariant -- it's purely a function of how +// long the app takes to decide it wants more data. Eg: Zee5's cold-cache +// DRM/token provisioning after receiving its DASH/HLS manifest response can +// legitimately, and variably, exceed even a generous fixed grace period (45s +// was tried and empirically still insufficient on some runs), causing +// Firestack to RST an already-healthy, still-in-use conn out from under the +// app before it's reused -- manifesting as a silent, permanent player hang +// (no retry, no app-visible error). Root caused via 3-state (vpn-off / +// vpn-on+warm-app-cache / vpn-on+cold-app-cache) logcat capture+diff, +// 2026-09-13; 45s-grace attempt empirically insufficient, re-tested same day. +// +// Since no finite grace period can be guaranteed sufficient, once a conn has +// successfully read >=1 byte from the remote it is proven NOT to be a +// blackhole candidate (a genuinely blackholed conn could never have done +// so) -- at that point the dopt.ReadTimeoutSec-derived deadline is dropped +// entirely, falling back to just the conn-type's own floor (rwext.minidle; +// 0 for tcp => extendr/extend treat that as "no deadline", see common.go). +// A real mid-stream death of a warm conn is still caught: either the peer +// eventually sends a real RST/FIN (no artificial timer needed), or the OS +// TCP stack's own keepalive eventually detects the dead link. Never-yet- +// successful (cold) conns are unaffected and keep the short, aggressive base +// deadline (settings.DialerOpts.ReadTimeoutSec). // rwext wraps MinConn and extends deadline to minimum(min, settings.DialerOpts) // on every read and write. @@ -124,12 +140,20 @@ func (rw rwext) SyscallConn() (syscall.RawConn, error) { func (rw rwext) deadlines() (r, w uint32) { dopt := settings.GetDialerOpts() - // -ve ints go higher than 2^31 w/ uint: go.dev/play/p/Rrqk_V8a7W0 - r = max(rw.minidle, uint32(dopt.ReadTimeoutSec)) if rw.warm != nil && rw.warm.Load() { // already exchanged >=1 byte on this conn: not a blackhole candidate; - // extend (never shrink) the read-idle grace for keep-alive reuse gaps. - r = max(r, warmReadGraceSec) + // no finite idle-read grace is guaranteed sufficient (see rationale + // above), so drop the dopt.ReadTimeoutSec-derived deadline entirely + // and fall back to just rw.minidle (the conn-type's own floor, if + // any: 0 for tcp => extendr/extend see <=0 and call + // SetDeadline(zero-value), ie: no deadline at all, see common.go; + // 120s for udp => unchanged from before, still bounded so idle udp + // "conns" -- which have no real end-of-stream signal like tcp's + // RST/FIN -- don't leak NAT/socket state forever). + r = rw.minidle + } else { + // -ve ints go higher than 2^31 w/ uint: go.dev/play/p/Rrqk_V8a7W0 + r = max(rw.minidle, uint32(dopt.ReadTimeoutSec)) } return r, max(rw.minidle, uint32(dopt.WriteTimeoutSec)) } From f748fa31f8296cca0971daa89951b2064aec1645 Mon Sep 17 00:00:00 2001 From: Varun Agarwal Date: Mon, 14 Sep 2026 00:51:40 +0530 Subject: [PATCH 11/11] fix(ipn): short-interval TCP keepalive for Exit-proxied conns Zee5's video CDN (Akamai-fronted) closes idle-but-alive connections via a clean FIN at ~120s; the app's HTTP client does not transparently retry on the dead pooled connection, causing a permanent hang. The existing keepalive tuning (sockopt.go defaultIdle=600s) is far too slow to matter against this window. When the user enables the existing tcpKeepAlive/LowerKeepAlive setting, Exit-proxied connections now use a 20s idle / 5s interval / 4-probe keepalive config instead of the slow default -- best-effort experiment; only helps if the peer's idle-close tracks raw TCP socket activity rather than HTTP-level activity. --- intra/ipn/auto.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/intra/ipn/auto.go b/intra/ipn/auto.go index 9696c655..d81fb0fa 100644 --- a/intra/ipn/auto.go +++ b/intra/ipn/auto.go @@ -23,6 +23,21 @@ const ( ttl30s = 30 * time.Second shortdelay = 100 * time.Millisecond delayForUnhealthyProxies = 2 * time.Second + + // STREAMSHIELD HOOK: short-interval TCP keepalive tuning for Exit-proxied + // connections. Some video-CDN/load-balancer stacks (observed: Zee5's + // Akamai-fronted CDN) close idle-but-alive connections at ~120s via a + // clean FIN, and the app's HTTP client fails to transparently retry on + // the dead pooled connection (permanent hang). sockopt.go's global + // defaults (600s idle) are far too slow to matter here. These constants + // are used only when the user enables the existing "TCP keep alive" + // toggle (settings.DialerOpts.LowerKeepAlive / PersistentState.tcpKeepAlive) + // -- best-effort experiment: only helps if the peer's idle-timeout tracks + // raw TCP socket activity (keepalive probes are pure ACKs) rather than + // HTTP-level request/response activity. + exitKeepAliveIdleSec = 20 // secs of inactivity before first probe + exitKeepAliveIntervalSec = 5 // secs between probes + exitKeepAliveCount = 4 // unacked probes before conn is declared dead ) // auto is a proxy that dials multiple, preset outbounds. @@ -624,8 +639,9 @@ func maybeKeepAlive2(c net.Conn) (keepingalive, ok bool) { } if opts := settings.GetDialerOpts(); opts.LowerKeepAlive { - // adjust socket's keepalive config - lowered := core.SetKeepAliveConfigSockOpt(c) + // STREAMSHIELD HOOK: use the short-interval tuning above instead of + // sockopt.go's slow global defaults (600s idle) -- see const block. + lowered := core.SetKeepAliveConfigSockOpt(c, exitKeepAliveIdleSec, exitKeepAliveIntervalSec, exitKeepAliveCount) keepingalive = lowered ok = lowered return