From 8d332797b6eea3cd610d24406ffa059489813861 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andri=20=C3=93skarsson?= Date: Sat, 28 Feb 2026 11:29:38 +0100 Subject: [PATCH] Fix transparent proxy: nil-pointer panic on portal API, missing WebSocket support, test cleanup --- DECISIONS.md | 2 + transparent.go | 70 +++++++++++++++++++++++- transparent_test.go | 128 ++++++++++++-------------------------------- 3 files changed, 106 insertions(+), 94 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index 1b01312..150e22a 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -75,3 +75,5 @@ - 2026-02-28 m+git@andri.dk — Transparent passthrough uses SNI extraction + raw TCP relay. When an `@@` excepted host is detected via SNI, the proxy replays the peeked ClientHello bytes to the upstream server and does bidirectional copy. The client's TLS handshake completes directly with upstream — functionally identical to explicit-mode passthrough. - 2026-02-28 m+git@andri.dk — Blocked hosts in transparent mode close the TCP connection immediately (before TLS handshake). Unlike explicit mode where a 403 response is returned over the CONNECT tunnel, transparent mode cannot send an HTTP error before TLS negotiation. The client sees a connection reset. - 2026-02-28 m+git@andri.dk — `replayConn` wraps a `net.Conn` with a `bufio.Reader` to replay peeked ClientHello bytes during the TLS handshake. The `extractSNI` function peeks at the TLS record without consuming bytes; `replayConn.Read` reads from the buffered reader first, then from the underlying connection. This avoids copying or buffering the entire ClientHello separately. +- 2026-02-28 m+git@andri.dk — Transparent HTTPS portal handler now passes `proxy.api` to `portalHandler`. Without it, any `/api/*` request on the transparent-mode portal would nil-pointer panic because the `api` field was zero-valued. +- 2026-02-28 m+git@andri.dk — Transparent HTTP forwarding now handles WebSocket upgrades. `forwardHTTPUpgrade` mirrors `proxyHandler.handleHTTPUpgrade` — re-adds hop-by-hop upgrade headers, hijacks both sides on 101, and does bidirectional copy. Without this, WebSocket connections through the transparent HTTP proxy would fail with a `RoundTrip` error. diff --git a/transparent.go b/transparent.go index 74af22e..7e44bc1 100644 --- a/transparent.go +++ b/transparent.go @@ -300,6 +300,12 @@ func (h *transparentHTTPHandler) forwardHTTP(w http.ResponseWriter, r *http.Requ return } + // WebSocket and other protocol upgrades need special handling + if isWebSocketUpgrade(r.Header) { + h.forwardHTTPUpgrade(w, r, targetURL) + return + } + start := time.Now() outReq, err := http.NewRequestWithContext(r.Context(), r.Method, targetURL, r.Body) @@ -341,6 +347,68 @@ func (h *transparentHTTPHandler) forwardHTTP(w http.ResponseWriter, r *http.Requ logRequest(r.Method, targetURL, resp.StatusCode, time.Since(start)) } +// forwardHTTPUpgrade handles WebSocket and other protocol upgrade requests +// in transparent HTTP mode. It mirrors proxyHandler.handleHTTPUpgrade but +// builds the target URL from the Host header. +func (h *transparentHTTPHandler) forwardHTTPUpgrade(w http.ResponseWriter, r *http.Request, targetURL string) { + outReq, err := http.NewRequestWithContext(r.Context(), r.Method, targetURL, r.Body) + if err != nil { + logError("transparent-http/upgrade/new-request", err) + http.Error(w, "bad request", http.StatusBadRequest) + return + } + + copyHeaders(outReq.Header, r.Header) + removeHopByHopHeaders(outReq.Header) + + // Re-add the upgrade headers that were stripped as hop-by-hop + outReq.Header.Set("Connection", "Upgrade") + outReq.Header.Set("Upgrade", r.Header.Get("Upgrade")) + + resp, err := h.proxy.transport.RoundTrip(outReq) + if err != nil { + logError("transparent-http/upgrade/roundtrip", err) + http.Error(w, "upstream error", http.StatusBadGateway) + return + } + + if resp.StatusCode != http.StatusSwitchingProtocols { + defer resp.Body.Close() + copyHeaders(w.Header(), resp.Header) + removeHopByHopHeaders(w.Header()) + w.WriteHeader(resp.StatusCode) + io.Copy(w, resp.Body) + return + } + + upstreamConn, ok := resp.Body.(io.ReadWriteCloser) + if !ok { + resp.Body.Close() + http.Error(w, "upstream does not support hijacking", http.StatusInternalServerError) + return + } + defer upstreamConn.Close() + + hijacker, ok := w.(http.Hijacker) + if !ok { + http.Error(w, "hijacking not supported", http.StatusInternalServerError) + return + } + + clientConn, clientBuf, err := hijacker.Hijack() + if err != nil { + logError("transparent-http/upgrade/hijack", err) + return + } + defer clientConn.Close() + + resp.Body = nil + resp.Write(clientConn) + clientBuf.Flush() + + bidirectionalCopy(clientConn, upstreamConn) +} + // serveTransparentTLS accepts raw TCP connections on the listener and // handles them as transparent HTTPS proxy connections. It peeks at the // TLS ClientHello to extract the SNI hostname, then either: @@ -442,7 +510,7 @@ func handleTransparentPortalTLS(conn net.Conn, br *bufio.Reader, proxy *proxyHan conn.SetDeadline(time.Time{}) // Serve portal pages over the TLS connection - portalH := &portalHandler{proxy: proxy} + portalH := &portalHandler{proxy: proxy, api: proxy.api} server := http.Server{Handler: portalH} serverConn := &singleConnListener{conn: tlsConn} server.Serve(serverConn) diff --git a/transparent_test.go b/transparent_test.go index 4665c36..43c44ca 100644 --- a/transparent_test.go +++ b/transparent_test.go @@ -387,12 +387,14 @@ func TestTransparentHTTPSTrustTracking(t *testing.T) { // --- Transparent HTTP proxy tests --- -func TestTransparentHTTPProxy(t *testing.T) { - upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - w.Write([]byte("hello http transparent")) - })) - defer upstream.Close() +type transparentHTTPTestEnv struct { + server *httptest.Server + proxy *proxyHandler + trustTracker *caTrustTracker +} + +func startTransparentHTTPTestEnv(t *testing.T) *transparentHTTPTestEnv { + t.Helper() caCert, caKey, err := ca.Generate() if err != nil { @@ -413,10 +415,26 @@ func TestTransparentHTTPProxy(t *testing.T) { } server := httptest.NewServer(transparentH) - defer server.Close() + t.Cleanup(server.Close) + + return &transparentHTTPTestEnv{ + server: server, + proxy: handler, + trustTracker: trustTracker, + } +} + +func TestTransparentHTTPProxy(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte("hello http transparent")) + })) + defer upstream.Close() + + env := startTransparentHTTPTestEnv(t) upstreamHost := strings.TrimPrefix(upstream.URL, "http://") - req, _ := http.NewRequest("GET", server.URL+"/test", nil) + req, _ := http.NewRequest("GET", env.server.URL+"/test", nil) req.Host = upstreamHost resp, err := http.DefaultClient.Do(req) @@ -437,28 +455,9 @@ func TestTransparentHTTPProxy(t *testing.T) { // --- Captive portal tests --- func TestCaptivePortalAppleDetection(t *testing.T) { - caCert, caKey, err := ca.Generate() - if err != nil { - t.Fatalf("ca.Generate: %v", err) - } - certs := ca.NewCache(caCert, caKey) - caCertPEM := ca.EncodeCertPEM(caCert) - handler := newProxyHandler(certs, caCertPEM) - handler.activityLog = NewActivityLog(100) - handler.portalOrigin = "https://proxy.local:8443" - handler.httpOrigin = "http://proxy.local:8080" - - trustTracker := newCATrustTracker() - transparentH := &transparentHTTPHandler{ - proxy: handler, - trustTracker: trustTracker, - portalHost: "proxy.local", - } - - server := httptest.NewServer(transparentH) - defer server.Close() + env := startTransparentHTTPTestEnv(t) - req, _ := http.NewRequest("GET", server.URL+"/hotspot-detect.html", nil) + req, _ := http.NewRequest("GET", env.server.URL+"/hotspot-detect.html", nil) req.Host = "captive.apple.com" client := &http.Client{ @@ -488,32 +487,13 @@ func TestCaptivePortalTrustedClientBypass(t *testing.T) { })) defer upstream.Close() - caCert, caKey, err := ca.Generate() - if err != nil { - t.Fatalf("ca.Generate: %v", err) - } - certs := ca.NewCache(caCert, caKey) - caCertPEM := ca.EncodeCertPEM(caCert) - handler := newProxyHandler(certs, caCertPEM) - handler.activityLog = NewActivityLog(100) - handler.portalOrigin = "https://proxy.local:8443" - handler.httpOrigin = "http://proxy.local:8080" - - trustTracker := newCATrustTracker() - transparentH := &transparentHTTPHandler{ - proxy: handler, - trustTracker: trustTracker, - portalHost: "proxy.local", - } - - server := httptest.NewServer(transparentH) - defer server.Close() + env := startTransparentHTTPTestEnv(t) // Mark client IP as trusted - trustTracker.markTrusted("127.0.0.1") + env.trustTracker.markTrusted("127.0.0.1") upstreamHost := strings.TrimPrefix(upstream.URL, "http://") - req, _ := http.NewRequest("GET", server.URL+"/hotspot-detect.html", nil) + req, _ := http.NewRequest("GET", env.server.URL+"/hotspot-detect.html", nil) req.Host = upstreamHost resp, err := http.DefaultClient.Do(req) @@ -532,28 +512,9 @@ func TestCaptivePortalTrustedClientBypass(t *testing.T) { } func TestCaptivePortalAndroidDetection(t *testing.T) { - caCert, caKey, err := ca.Generate() - if err != nil { - t.Fatalf("ca.Generate: %v", err) - } - certs := ca.NewCache(caCert, caKey) - caCertPEM := ca.EncodeCertPEM(caCert) - handler := newProxyHandler(certs, caCertPEM) - handler.activityLog = NewActivityLog(100) - handler.portalOrigin = "https://proxy.local:8443" - handler.httpOrigin = "http://proxy.local:8080" + env := startTransparentHTTPTestEnv(t) - trustTracker := newCATrustTracker() - transparentH := &transparentHTTPHandler{ - proxy: handler, - trustTracker: trustTracker, - portalHost: "proxy.local", - } - - server := httptest.NewServer(transparentH) - defer server.Close() - - req, _ := http.NewRequest("GET", server.URL+"/generate_204", nil) + req, _ := http.NewRequest("GET", env.server.URL+"/generate_204", nil) req.Host = "connectivitycheck.gstatic.com" client := &http.Client{ @@ -573,28 +534,9 @@ func TestCaptivePortalAndroidDetection(t *testing.T) { } func TestTransparentHTTPPortalAccess(t *testing.T) { - caCert, caKey, err := ca.Generate() - if err != nil { - t.Fatalf("ca.Generate: %v", err) - } - certs := ca.NewCache(caCert, caKey) - caCertPEM := ca.EncodeCertPEM(caCert) - handler := newProxyHandler(certs, caCertPEM) - handler.activityLog = NewActivityLog(100) - handler.portalOrigin = "https://proxy.local:8443" - handler.httpOrigin = "http://proxy.local:8080" - - trustTracker := newCATrustTracker() - transparentH := &transparentHTTPHandler{ - proxy: handler, - trustTracker: trustTracker, - portalHost: "proxy.local", - } - - server := httptest.NewServer(transparentH) - defer server.Close() + env := startTransparentHTTPTestEnv(t) - req, _ := http.NewRequest("GET", server.URL+"/", nil) + req, _ := http.NewRequest("GET", env.server.URL+"/", nil) req.Host = "proxy.local" resp, err := http.DefaultClient.Do(req) -- 2.51.2