diff --git a/cmd/apps/skysocks-client/commands/skysocks-client.go b/cmd/apps/skysocks-client/commands/skysocks-client.go index 5fa2ccf6ee..c6d9dd66f7 100644 --- a/cmd/apps/skysocks-client/commands/skysocks-client.go +++ b/cmd/apps/skysocks-client/commands/skysocks-client.go @@ -178,7 +178,30 @@ func RunSkysocksClient(ctx context.Context, args []string) error { // only on a clean ctx-done stop; any other return is a // (re-)connect trigger when --reconnect is set. runCycle := func() error { + // Bind :1080 up front and serve status.skysocks (and the branded + // interstitial for real traffic) in-process WHILE we dial the exit, so the + // reserved diagnostic page is reachable during the "still connecting" window + // instead of connection-refused. The live Client can only be built from the + // dialed session conn, so this sessionless listener owns :1080 until the dial + // succeeds, then releases it to client.ListenAndServe below. Best-effort: if + // the bind fails we just skip it and dial as before. + dctx, dcancel := context.WithCancel(cycleCtx) + ddone := make(chan struct{}) + if lis, lerr := net.Listen("tcp", addr); lerr == nil { + go func() { + defer close(ddone) + skysocks.ServeDisconnected(dctx, lis, appCl) + }() + } else { + log.WithError(lerr).Debug("disconnected status listener not bound") + close(ddone) + } + conn, err := dialServer(cycleCtx, appCl, pk, serverPort) + // Stop the disconnected listener and wait for it to release :1080 before the + // live Client rebinds the same addr. + dcancel() + <-ddone if err != nil { return fmt.Errorf("dial server: %w", err) } diff --git a/pkg/skysocks/client.go b/pkg/skysocks/client.go index 9efec85643..4d0db56fe0 100644 --- a/pkg/skysocks/client.go +++ b/pkg/skysocks/client.go @@ -889,14 +889,23 @@ func (c *Client) statusSnapshot() proxystatus.Snapshot { // local base. Kept separate so the RPC-unavailable path is a clean fallback that // never breaks the status page. func (c *Client) visorStatusSnapshot() proxystatus.Snapshot { + return baseStatusSnapshot(c.appCl) +} + +// baseStatusSnapshot builds the status.skysocks base snapshot from the app RPC: +// the visor-built rich snapshot when the RPC is reachable and carried real data +// (any Legs/Logs/Events), otherwise the minimal local base. It carries NO live +// session facts (Running/Note/Streams are overlaid by the caller) so it can be +// shared by the live Client and the sessionless disconnected listener alike. +func baseStatusSnapshot(appCl *app.Client) proxystatus.Snapshot { base := proxystatus.Snapshot{ Surface: proxystatus.SurfaceSkysocks, App: skyenv.SkysocksClientName, } - if c.appCl == nil { + if appCl == nil { return base } - rich, err := c.appCl.ProxyStatus() + rich, err := appCl.ProxyStatus() if err != nil { return base } diff --git a/pkg/skysocks/client_disconnected_test.go b/pkg/skysocks/client_disconnected_test.go new file mode 100644 index 0000000000..0605340220 --- /dev/null +++ b/pkg/skysocks/client_disconnected_test.go @@ -0,0 +1,139 @@ +// Package skysocks disconnected-state (no exit session) listener tests. +package skysocks + +import ( + "bufio" + "context" + "io" + "net" + "net/http" + "strings" + "testing" + "time" +) + +// serveDisconnected binds a free loopback addr, runs ServeDisconnected on it with a +// nil app client (no session, no visor RPC — the minimal-local-base path), and +// returns the addr plus a stop func the test defers. +func serveDisconnected(t *testing.T) (addr string, stop func()) { + t.Helper() + addr = freeAddr(t) + lis, err := net.Listen("tcp", addr) + if err != nil { + t.Fatalf("bind disconnected listener: %v", err) + } + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + defer close(done) + ServeDisconnected(ctx, lis, nil) + }() + waitDial(t, addr) + return addr, func() { + cancel() + <-done + } +} + +// TestDisconnectedServesStatusSkysocks is the regression test for the remaining +// gap: with NO session to the exit at all (the client is still connecting / the +// route group never came up), a CONNECT to status.skysocks — on a NON-80 port — +// must still return the in-process status page, never connection-refused and never +// the interstitial. +func TestDisconnectedServesStatusSkysocks(t *testing.T) { + addr, stop := serveDisconnected(t) + defer stop() + + conn := socks5Connect(t, addr, "status.skysocks", 8080) + defer conn.Close() //nolint:errcheck + if _, err := conn.Write([]byte("GET / HTTP/1.1\r\nHost: status.skysocks\r\nConnection: close\r\n\r\n")); err != nil { + t.Fatal(err) + } + _ = conn.SetReadDeadline(time.Now().Add(3 * time.Second)) //nolint:errcheck + resp, err := http.ReadResponse(bufio.NewReader(conn), nil) + if err != nil { + t.Fatalf("status page not served while disconnected: %v", err) + } + defer resp.Body.Close() //nolint:errcheck + body, _ := io.ReadAll(resp.Body) //nolint:errcheck + if !strings.Contains(string(body), "per-leg mux") { + t.Fatalf("status page not served in-process; body=%q", string(body)) + } + if strings.Contains(string(body), "Building a route") { + t.Fatalf("status.skysocks was shadowed by the interstitial; body=%q", string(body)) + } + // The "no active session to the exit" note is the disconnected snapshot's tell. + if !strings.Contains(string(body), "no active session to the exit") { + t.Fatalf("disconnected status page missing the no-session note; body=%q", string(body)) + } +} + +// TestDisconnectedRealHostGetsInterstitial verifies the flip side: a REAL host in +// the same sessionless state gets the branded "building a route" interstitial (a +// plaintext-HTTP target), NOT the status page and NOT a hang. Real traffic is never +// tunneled while disconnected — there is no exit to tunnel to. +func TestDisconnectedRealHostGetsInterstitial(t *testing.T) { + addr, stop := serveDisconnected(t) + defer stop() + + conn := socks5Connect(t, addr, "example.com", 80) + defer conn.Close() //nolint:errcheck + if _, err := conn.Write([]byte("GET / HTTP/1.1\r\nHost: example.com\r\nConnection: close\r\n\r\n")); err != nil { + t.Fatal(err) + } + _ = conn.SetReadDeadline(time.Now().Add(3 * time.Second)) //nolint:errcheck + resp, err := http.ReadResponse(bufio.NewReader(conn), nil) + if err != nil { + t.Fatalf("interstitial not served for real host while disconnected: %v", err) + } + defer resp.Body.Close() //nolint:errcheck + body, _ := io.ReadAll(resp.Body) //nolint:errcheck + if !strings.Contains(string(body), "Building a route") { + t.Fatalf("real host did not get the interstitial; body=%q", string(body)) + } + if strings.Contains(string(body), "per-leg mux") { + t.Fatalf("real host wrongly got the status page; body=%q", string(body)) + } +} + +// TestDisconnectedNonHTTPPortDeclined verifies a non-HTTP real target (e.g. a raw +// TLS 443 CONNECT) is declined rather than served a corrupting HTML body: the +// interstitial only carries plaintext HTTP. The connection is closed with no bytes +// beyond the SOCKS reply the caller already consumed. +func TestDisconnectedNonHTTPPortDeclined(t *testing.T) { + addr, stop := serveDisconnected(t) + defer stop() + + conn, err := net.Dial("tcp", addr) + if err != nil { + t.Fatal(err) + } + defer conn.Close() //nolint:errcheck + // Greeting → local no-auth method reply. + if _, err := conn.Write([]byte{0x05, 0x01, 0x00}); err != nil { + t.Fatal(err) + } + method := make([]byte, 2) + if _, err := io.ReadFull(conn, method); err != nil { + t.Fatalf("read method reply: %v", err) + } + if method[0] != 0x05 || method[1] != 0x00 { + t.Fatalf("unexpected method reply %v", method) + } + // CONNECT example.com:443 — a non-reserved host on a non-HTTP port. ServeSOCKS5 + // declines it (no CONNECT success reply, no HTML body) rather than corrupting a + // raw-TLS stream with an interstitial, and closes the conn. + host := "example.com" + req := []byte{0x05, 0x01, 0x00, 0x03, byte(len(host))} //nolint:gosec + req = append(req, []byte(host)...) + req = append(req, byte(443>>8), byte(443&0xff)) + if _, err := conn.Write(req); err != nil { + t.Fatal(err) + } + _ = conn.SetReadDeadline(time.Now().Add(3 * time.Second)) //nolint:errcheck + buf := make([]byte, 512) + n, _ := conn.Read(buf) //nolint:errcheck // expect EOF (0 bytes): declined + closed + if n != 0 { + t.Fatalf("expected the non-HTTP CONNECT to be declined with no body, got %d bytes: %q", n, string(buf[:n])) + } +} diff --git a/pkg/skysocks/disconnected.go b/pkg/skysocks/disconnected.go new file mode 100644 index 0000000000..ca5d002ea3 --- /dev/null +++ b/pkg/skysocks/disconnected.go @@ -0,0 +1,77 @@ +// Package skysocks pkg/skysocks/disconnected.go c4-app-proxy +// +// Disconnected-state loopback listener. skysocks-client dials the exit BEFORE it +// binds :1080 (see cmd/apps/skysocks-client): the live Client is constructed from +// the yamux session conn, so until the dial succeeds there is no Client and nothing +// listens on :1080. During that "still connecting / exit route group never came up" +// window a browser pointed at :1080 got connection-refused — including for the +// reserved status.skysocks diagnostic host, which is exactly when a user wants to +// see why the proxy isn't connected. +// +// ServeDisconnected fills that window: it owns :1080 while the dial is in flight and +// answers each browser SOCKS5 connection LOCALLY. The reserved diagnostic hosts +// (proxystatus.Match, e.g. status.skysocks) are rendered in-process with zero exit +// involvement; every real target gets the branded "building a route" interstitial +// (plaintext HTTP) or is declined (other ports). No real traffic is ever tunneled +// here — there is no exit to tunnel to — so a listening-but-not-connected :1080 +// never silently serves or blackholes real requests; it only surfaces the local +// diagnostic/interstitial pages, which never needed the exit anyway. +package skysocks + +import ( + "context" + "net" + + "github.com/skycoin/skywire/pkg/app" + "github.com/skycoin/skywire/pkg/proxyinterstitial" + "github.com/skycoin/skywire/pkg/proxystatus" +) + +// ServeDisconnected serves lis while the client has NO session to the exit yet +// (still dialing, or the exit route group never came up). Each accepted connection +// is answered locally by disconnectedConn. It returns when ctx is canceled (the +// dial succeeded and the caller is handing :1080 to the live Client, or the proc is +// shutting down) or lis fails; lis is closed on return so the port is released for +// the live listener. The caller owns lis. +func ServeDisconnected(ctx context.Context, lis net.Listener, appCl *app.Client) { + go func() { + <-ctx.Done() + _ = lis.Close() //nolint:errcheck,gosec + }() + for { + conn, err := lis.Accept() + if err != nil { + return + } + go disconnectedConn(conn, appCl) + } +} + +// disconnectedConn answers one browser SOCKS5 connection while disconnected from +// the exit. The reserved-host override renders the status page in-process for a +// proxystatus.Match host (regardless of port, resolved before ServeSOCKS5's +// HTTP-only gate), so status.skysocks stays reachable with no session at all; every +// other target falls through to the branded interstitial (plaintext HTTP) or is +// declined. exitReachable is nil — there is no session, so a real HTTP target gets +// the waiting interstitial rather than a fall-through reload. +func disconnectedConn(conn net.Conn, appCl *app.Client) { + defer conn.Close() //nolint:errcheck,gosec + override := func(host string) []byte { + if surface, ok := proxystatus.Match(host); ok && surface == proxystatus.SurfaceSkysocks { + return statusHTTPResponse(proxystatus.Render(disconnectedSnapshot(appCl))) + } + return nil + } + _ = proxyinterstitial.ServeSOCKS5(conn, "not connected to the exit yet", "skysocks", override, nil) //nolint:errcheck +} + +// disconnectedSnapshot is the status.skysocks snapshot for the sessionless state: +// the app-RPC base (rich per-leg view when the visor has one, else the minimal +// local base) overlaid with the "no active session to the exit" note — mirroring +// the else branch of (*Client).statusSnapshot for a torn-down session. +func disconnectedSnapshot(appCl *app.Client) proxystatus.Snapshot { + snap := baseStatusSnapshot(appCl) + snap.Running = false + snap.Note = "no active session to the exit" + return snap +}