fix(redirect/tcp): forward protocols the sniffer does not handle

Since 3.3.0 a transparent red service with sniffing enabled closed every connection whose protocol Sniff does not classify as HTTP or TLS. SSH, Redis, and any raw TCP — plus server-speaks-first protocols such as SMTP, where Sniff returns "" after sniffing.timeout — took no matching branch of the switch on proto, so sniffErr stayed nil and the handler returned nil, closing the connection via the deferred conn.Close(). No dial was attempted, so the upstream node never saw the connection, and nothing was logged above info. This traces to c23a2f1, which introduced the error-returning switch in order to replay sniffed bytes for the empty-SNI case (#891). Before it, the switch had no error variable and simply fell out into raw forwarding, so every unclassified protocol was proxied as-is. sniffing.fallback is not a usable workaround: it also changes dial() so that a failure to reach the sniffed host re-dials the original IP, where domain-based hop/node bypass rules no longer match. That can route a domain through a node its bypass excludes. Mark the two handled protocols explicitly and let everything else fall through to raw forwarding unconditionally, restoring the 3.2.x behaviour. sniffing.fallback keeps its intended meaning: it governs only what happens after HTTP/TLS handling itself fails (#891). Closes #913
2026-10-19 16:16:32 +00:00 · 2026-10-11 10:15:11 +00:00
parent 1d0b6de411
commit 0954e57dcb
2 changed files with 356 additions and 4 deletions
@@ -245,8 +245,10 @@ func (h *redirectHandler) Handle(ctx context.Context, conn net.Conn, opts ...han
 		sniffConn := xnet.NewReadWriteConn(io.TeeReader(br, capture), origConn, origConn)

 		var sniffErr error
+		var handled bool
 		switch proto {
 		case sniffing.ProtoHTTP:
+			handled = true
 			sniffErr = sniffer.HandleHTTP(ctx, "tcp", sniffConn,
 				sniffing.WithService(h.options.Service),
 				sniffing.WithDial(dial),
@@ -260,6 +262,7 @@ func (h *redirectHandler) Handle(ctx context.Context, conn net.Conn, opts ...han
 				return nil
 			}
 		case sniffing.ProtoTLS:
+			handled = true
 			sniffErr = sniffer.HandleTLS(ctx, ro.Network, sniffConn,
 				sniffing.WithService(h.options.Service),
 				sniffing.WithDial(dial),
@@ -273,13 +276,22 @@ func (h *redirectHandler) Handle(ctx context.Context, conn net.Conn, opts ...han
 			}
 		}

-		if h.md.sniffingFallback {
+		// A protocol the sniffer does not classify (ssh, redis, or "" when
+		// nothing arrives within sniffing.timeout) has no error to report and
+		// nothing to fall back from, so it always reaches raw forwarding to the
+		// original dstAddr — the way every non-HTTP/TLS protocol was handled
+		// before sniffing gained a failure path. sniffing.fallback only governs
+		// what happens after HTTP/TLS handling itself fails (e.g. an empty SNI,
+		// gost#891).
+		if !handled {
+			log.Debugf("sniffing: protocol %q not handled, forwarding raw", proto)
+		} else if h.md.sniffingFallback {
 			log.Debugf("sniffing(%s) failed, falling back: %v", proto, sniffErr)
-			conn = xnet.NewReadWriteConn(io.MultiReader(capture, br), origConn, origConn)
-			// fall through to raw forwarding below
 		} else {
 			return sniffErr
 		}
+		conn = xnet.NewReadWriteConn(io.MultiReader(capture, br), origConn, origConn)
+		// fall through to raw forwarding below
 	}

 	// Re-check bypass on dstAddr before raw forwarding. In whitelist mode the
@@ -4,13 +4,15 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"io"
 	"net"
 	"slices"
 	"testing"
+	"time"

-	dissector "github.com/go-gost/tls-dissector"
 	"github.com/go-gost/core/chain"
 	"github.com/go-gost/core/handler"
+	dissector "github.com/go-gost/tls-dissector"
 	xbypass "github.com/go-gost/x/bypass"
 	xlogger "github.com/go-gost/x/logger"
 	mdata "github.com/go-gost/x/metadata"
@@ -69,6 +71,230 @@ func tlsClientHello(t *testing.T, name string) []byte {
 	return buf.Bytes()
 }

+// dialRouter ignores the requested address and dials target instead, so a test
+// can stand up a real backend and assert the bytes that reach it.
+type dialRouter struct {
+	target string
+}
+
+func (r *dialRouter) Options() *chain.RouterOptions { return nil }
+func (r *dialRouter) Dial(ctx context.Context, network, _ string, _ ...chain.DialOption) (net.Conn, error) {
+	return (&net.Dialer{}).DialContext(ctx, network, r.target)
+}
+func (r *dialRouter) Bind(context.Context, string, string, ...chain.BindOption) (net.Listener, error) {
+	return nil, errors.New("bind failed")
+}
+
+// gost#913: an unhandled protocol must not only reach the backend, it must
+// arrive intact — the sniffed-but-unread bytes have to be replayed through
+// io.MultiReader ahead of the rest of the stream.
+func TestHandler_UnhandledProtoPayloadReplayedIntact(t *testing.T) {
+	// Backend that reads a fixed number of bytes and echoes a reply, standing
+	// in for a server-speaks-first protocol such as SMTP.
+	const reply = "hello from destination\r\n"
+	backend, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer backend.Close()
+	go func() {
+		for {
+			c, err := backend.Accept()
+			if err != nil {
+				return
+			}
+			go func(c net.Conn) {
+				defer c.Close()
+				buf := make([]byte, 14)
+				if _, err := io.ReadFull(c, buf); err != nil {
+					return
+				}
+				c.Write([]byte(reply))
+			}(c)
+		}
+	}()
+
+	h := NewHandler(
+		handler.RouterOption(&dialRouter{target: backend.Addr().String()}),
+		handler.LoggerOption(xlogger.NewLogger()),
+	)
+	if err := h.Init(mdata.NewMetadata(map[string]any{
+		"tproxy":   true,
+		"sniffing": true,
+	})); err != nil {
+		t.Fatal(err)
+	}
+
+	l, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer l.Close()
+
+	server := make(chan net.Conn, 1)
+	go func() {
+		if c, err := l.Accept(); err == nil {
+			server <- c
+		}
+	}()
+
+	client, err := net.Dial("tcp", l.Addr().String())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer client.Close()
+
+	conn := <-server
+	defer conn.Close()
+	go h.Handle(context.Background(), conn)
+
+	// The client speaks first, with a greeting the sniffer classifies as ssh.
+	const greeting = "SSH-2.0-gost\r\n"
+	if _, err := client.Write([]byte(greeting)); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := client.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil {
+		t.Fatal(err)
+	}
+	buf := make([]byte, len(reply))
+	if _, err := io.ReadFull(client, buf); err != nil {
+		t.Fatalf("backend never replied, the greeting was not forwarded: %v", err)
+	}
+	if string(buf) != reply {
+		t.Fatalf("got %q, want %q", buf, reply)
+	}
+}
+
+// gost#891 / gost#913: sniffing.fallback governs only what happens after
+// HTTP/TLS handling fails. A ClientHello that Sniff classifies as TLS but
+// ParseClientHello rejects must be dropped when fallback is off, and replayed
+// verbatim to the backend when it is on — the bytes the sniffer consumed must
+// not be lost in either case.
+func TestHandler_TLSHandlingFailureRespectsFallback(t *testing.T) {
+	// TLS record header (handshake, TLS 1.0) followed by a payload that is not
+	// a parsable ClientHello.
+	badHello := append([]byte{0x16, 0x03, 0x01, 0x00, 0x05}, []byte("junkjunk")...)
+
+	t.Run("fallback off", func(t *testing.T) {
+		router := &recordingRouter{}
+		h := NewHandler(
+			handler.RouterOption(router),
+			handler.LoggerOption(xlogger.NewLogger()),
+		)
+		if err := h.Init(mdata.NewMetadata(map[string]any{
+			"tproxy":            true,
+			"sniffing":          true,
+			"sniffing.fallback": false,
+		})); err != nil {
+			t.Fatal(err)
+		}
+
+		l, err := net.Listen("tcp", "127.0.0.1:0")
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer l.Close()
+
+		server := make(chan net.Conn, 1)
+		go func() {
+			if c, err := l.Accept(); err == nil {
+				server <- c
+			}
+		}()
+
+		client, err := net.Dial("tcp", l.Addr().String())
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer client.Close()
+
+		conn := <-server
+		defer conn.Close()
+
+		if _, err := client.Write(badHello); err != nil {
+			t.Fatal(err)
+		}
+
+		if err := h.Handle(context.Background(), conn); err == nil {
+			t.Fatal("got nil error, want the sniffer's parse failure")
+		}
+		if len(router.hosts) != 0 {
+			t.Fatalf("failed TLS handling fell back to raw forwarding: %v", router.hosts)
+		}
+	})
+
+	t.Run("fallback on", func(t *testing.T) {
+		// Backend echoes back exactly what it received.
+		backend, err := net.Listen("tcp", "127.0.0.1:0")
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer backend.Close()
+		go func() {
+			for {
+				c, err := backend.Accept()
+				if err != nil {
+					return
+				}
+				go func(c net.Conn) {
+					defer c.Close()
+					io.Copy(c, c)
+				}(c)
+			}
+		}()
+
+		h := NewHandler(
+			handler.RouterOption(&dialRouter{target: backend.Addr().String()}),
+			handler.LoggerOption(xlogger.NewLogger()),
+		)
+		if err := h.Init(mdata.NewMetadata(map[string]any{
+			"tproxy":            true,
+			"sniffing":          true,
+			"sniffing.fallback": true,
+		})); err != nil {
+			t.Fatal(err)
+		}
+
+		l, err := net.Listen("tcp", "127.0.0.1:0")
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer l.Close()
+
+		server := make(chan net.Conn, 1)
+		go func() {
+			if c, err := l.Accept(); err == nil {
+				server <- c
+			}
+		}()
+
+		client, err := net.Dial("tcp", l.Addr().String())
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer client.Close()
+
+		conn := <-server
+		defer conn.Close()
+		go h.Handle(context.Background(), conn)
+
+		if _, err := client.Write(badHello); err != nil {
+			t.Fatal(err)
+		}
+		if err := client.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil {
+			t.Fatal(err)
+		}
+		buf := make([]byte, len(badHello))
+		if _, err := io.ReadFull(client, buf); err != nil {
+			t.Fatalf("fallback did not forward to the backend: %v", err)
+		}
+		if !bytes.Equal(buf, badHello) {
+			t.Fatalf("backend got %x, want %x", buf, badHello)
+		}
+	})
+}
+
 // gost#899: a whitelist bypass containing only domains must not reject the
 // destination IP before sniffing runs. The sniffer matches the SNI host, so a
 // whitelisted domain (example.com) is forwarded even though its bare IP is
@@ -131,6 +357,120 @@ func TestHandler_WhitelistDomainForwarded(t *testing.T) {
 	}
 }

+// gost#913: a protocol the sniffer does not classify must fall through to raw
+// forwarding to the original dstAddr, exactly as it did before 3.3.0, no matter
+// what sniffing.fallback is set to. sniffing.fallback only governs what happens
+// after HTTP/TLS handling fails.
+func TestHandler_UnhandledProtoFallsThroughToRawForwarding(t *testing.T) {
+	tests := []struct {
+		name    string
+		payload []byte
+	}{
+		{"ssh", []byte("SSH-2.0-gost\r\n")},
+		{"redis", []byte("*1\r\n$4\r\nPING\r\n")},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			router := &recordingRouter{}
+			h := NewHandler(
+				handler.RouterOption(router),
+				handler.LoggerOption(xlogger.NewLogger()),
+			)
+			// sniffing.fallback is deliberately left off (the default).
+			if err := h.Init(mdata.NewMetadata(map[string]any{
+				"tproxy":   true,
+				"sniffing": true,
+			})); err != nil {
+				t.Fatal(err)
+			}
+
+			l, err := net.Listen("tcp", "127.0.0.1:0")
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer l.Close()
+
+			server := make(chan net.Conn, 1)
+			go func() {
+				if c, err := l.Accept(); err == nil {
+					server <- c
+				}
+			}()
+
+			client, err := net.Dial("tcp", l.Addr().String())
+			if err != nil {
+				t.Fatal(err)
+			}
+			defer client.Close()
+
+			conn := <-server
+			defer conn.Close()
+
+			if _, err := client.Write(tt.payload); err != nil {
+				t.Fatal(err)
+			}
+
+			// The stub router fails the dial, but only after recording the
+			// address, so a non-empty hosts list proves the handler reached
+			// the raw forwarding stage instead of closing the connection.
+			h.Handle(context.Background(), conn)
+
+			if len(router.hosts) == 0 {
+				t.Fatalf("unhandled protocol %q was not forwarded to the original dstAddr", tt.name)
+			}
+		})
+	}
+}
+
+// gost#913: when the client sends nothing within sniffing.timeout, Sniff
+// returns an empty proto. That is also "not handled by the sniffer" and must
+// reach raw forwarding rather than closing the connection.
+func TestHandler_SniffTimeoutFallsThroughToRawForwarding(t *testing.T) {
+	router := &recordingRouter{}
+	h := NewHandler(
+		handler.RouterOption(router),
+		handler.LoggerOption(xlogger.NewLogger()),
+	)
+	if err := h.Init(mdata.NewMetadata(map[string]any{
+		"tproxy":            true,
+		"sniffing":          true,
+		"sniffing.timeout":  "100ms",
+		"sniffing.fallback": false,
+	})); err != nil {
+		t.Fatal(err)
+	}
+
+	l, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer l.Close()
+
+	server := make(chan net.Conn, 1)
+	go func() {
+		if c, err := l.Accept(); err == nil {
+			server <- c
+		}
+	}()
+
+	client, err := net.Dial("tcp", l.Addr().String())
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer client.Close()
+
+	conn := <-server
+	defer conn.Close()
+
+	// The client stays silent, so the sniffing read deadline expires.
+	h.Handle(context.Background(), conn)
+
+	if len(router.hosts) == 0 {
+		t.Fatal("connection with no sniffed protocol was not forwarded to the original dstAddr")
+	}
+}
+
 // gost#899 (security): a non-whitelisted host must still be rejected, so the
 // pre-sniffing skip does not let arbitrary traffic through in whitelist mode.
 func TestHandler_WhitelistRejectsNonWhitelistedHost(t *testing.T) {