Fix tls-spoof

common/tls/client.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"net"
 	"os"
+	"strings"
 
 	"github.com/sagernet/sing-box/common/badtls"
 	"github.com/sagernet/sing-box/common/tlsspoof"
@@ -33,6 +34,9 @@ func parseTLSSpoofOptions(serverName string, options option.OutboundTLSOptions)
 	if options.DisableSNI || serverName == "" || M.ParseAddr(serverName).IsValid() {
 		return "", 0, E.New("`spoof` requires TLS ClientHello with SNI")
 	}
+	if strings.EqualFold(options.Spoof, serverName) {
+		return "", 0, E.New("`spoof` must differ from `server_name`")
+	}
 	method, err := tlsspoof.ParseMethod(options.SpoofMethod)
 	if err != nil {
 		return "", 0, err
@@ -44,11 +48,7 @@ func applyTLSSpoof(conn net.Conn, spoof string, method tlsspoof.Method) (net.Con
 	if spoof == "" {
 		return conn, nil
 	}
-	spoofer, err := tlsspoof.NewSpoofer(conn, method)
-	if err != nil {
-		return nil, err
-	}
-	return tlsspoof.NewConn(conn, spoofer, spoof), nil
+	return tlsspoof.NewConn(conn, method, spoof)
 }
 
 func NewDialerFromOptions(ctx context.Context, logger logger.ContextLogger, dialer N.Dialer, serverAddress string, options option.OutboundTLSOptions) (N.Dialer, error) {

common/tls/client_test.go

@@ -0,0 +1,154 @@
+package tls
+
+import (
+	"context"
+	"crypto/tls"
+	"net"
+	"testing"
+
+	tf "github.com/sagernet/sing-box/common/tlsfragment"
+	"github.com/sagernet/sing-box/common/tlsspoof"
+	"github.com/sagernet/sing-box/option"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseTLSSpoofOptions_Disabled(t *testing.T) {
+	t.Parallel()
+	spoof, method, err := parseTLSSpoofOptions("example.com", option.OutboundTLSOptions{})
+	require.NoError(t, err)
+	require.Empty(t, spoof)
+	require.Equal(t, tlsspoof.MethodWrongSequence, method)
+}
+
+func TestParseTLSSpoofOptions_MethodWithoutSpoof(t *testing.T) {
+	t.Parallel()
+	_, _, err := parseTLSSpoofOptions("example.com", option.OutboundTLSOptions{
+		SpoofMethod: tlsspoof.MethodNameWrongChecksum,
+	})
+	require.Error(t, err)
+}
+
+func TestParseTLSSpoofOptions_IPLiteralRejected(t *testing.T) {
+	t.Parallel()
+	_, _, err := parseTLSSpoofOptions("1.2.3.4", option.OutboundTLSOptions{
+		Spoof: "example.com",
+	})
+	require.Error(t, err)
+}
+
+func TestParseTLSSpoofOptions_EmptyServerNameRejected(t *testing.T) {
+	t.Parallel()
+	_, _, err := parseTLSSpoofOptions("", option.OutboundTLSOptions{
+		Spoof: "example.com",
+	})
+	require.Error(t, err)
+}
+
+func TestParseTLSSpoofOptions_DisableSNIRejected(t *testing.T) {
+	t.Parallel()
+	_, _, err := parseTLSSpoofOptions("example.com", option.OutboundTLSOptions{
+		Spoof:      "decoy.com",
+		DisableSNI: true,
+	})
+	require.Error(t, err)
+}
+
+// TestParseTLSSpoofOptions_RejectsSameSNI is the primary regression test for
+// the "spoofed packet contains the original SNI" bug report: when a user
+// configures spoof equal to server_name, the rewriter produces a byte-identical
+// record, so the fake and real ClientHellos on the wire look the same. Reject
+// at parse time.
+func TestParseTLSSpoofOptions_RejectsSameSNI(t *testing.T) {
+	t.Parallel()
+	_, _, err := parseTLSSpoofOptions("example.com", option.OutboundTLSOptions{
+		Spoof: "example.com",
+	})
+	require.Error(t, err)
+
+	_, _, err = parseTLSSpoofOptions("example.com", option.OutboundTLSOptions{
+		Spoof: "EXAMPLE.com",
+	})
+	require.Error(t, err, "comparison must be case-insensitive")
+}
+
+func TestParseTLSSpoofOptions_UnknownMethodRejected(t *testing.T) {
+	t.Parallel()
+	_, _, err := parseTLSSpoofOptions("example.com", option.OutboundTLSOptions{
+		Spoof:       "decoy.com",
+		SpoofMethod: "nonsense",
+	})
+	require.Error(t, err)
+}
+
+func TestParseTLSSpoofOptions_DistinctSNIAccepted(t *testing.T) {
+	t.Parallel()
+	if !tlsspoof.PlatformSupported {
+		t.Skip("tlsspoof not supported on this platform")
+	}
+	spoof, method, err := parseTLSSpoofOptions("example.com", option.OutboundTLSOptions{
+		Spoof:       "decoy.com",
+		SpoofMethod: tlsspoof.MethodNameWrongSequence,
+	})
+	require.NoError(t, err)
+	require.Equal(t, "decoy.com", spoof)
+	require.Equal(t, tlsspoof.MethodWrongSequence, method)
+}
+
+// The following tests guard the wrap gate in STDClientConfig.Client():
+// tf.Conn must wrap the underlying connection whenever either `fragment` or
+// `record_fragment` is set, so that TLS fragmentation coexists with features
+// like tls_spoof that layer on top of tf.Conn.
+
+func newSTDClientConfigForGateTest(fragment, recordFragment bool) *STDClientConfig {
+	return &STDClientConfig{
+		ctx:            context.Background(),
+		config:         &tls.Config{ServerName: "example.com", InsecureSkipVerify: true},
+		fragment:       fragment,
+		recordFragment: recordFragment,
+	}
+}
+
+func TestSTDClient_Client_NoFragment_DoesNotWrap(t *testing.T) {
+	t.Parallel()
+	client, server := net.Pipe()
+	defer client.Close()
+	defer server.Close()
+	wrapped, err := newSTDClientConfigForGateTest(false, false).Client(client)
+	require.NoError(t, err)
+	_, isTF := wrapped.NetConn().(*tf.Conn)
+	require.False(t, isTF, "no fragment flags: must not wrap with tf.Conn")
+}
+
+func TestSTDClient_Client_FragmentOnly_Wraps(t *testing.T) {
+	t.Parallel()
+	client, server := net.Pipe()
+	defer client.Close()
+	defer server.Close()
+	wrapped, err := newSTDClientConfigForGateTest(true, false).Client(client)
+	require.NoError(t, err)
+	_, isTF := wrapped.NetConn().(*tf.Conn)
+	require.True(t, isTF, "fragment=true: must wrap with tf.Conn")
+}
+
+func TestSTDClient_Client_RecordFragmentOnly_Wraps(t *testing.T) {
+	t.Parallel()
+	client, server := net.Pipe()
+	defer client.Close()
+	defer server.Close()
+	wrapped, err := newSTDClientConfigForGateTest(false, true).Client(client)
+	require.NoError(t, err)
+	_, isTF := wrapped.NetConn().(*tf.Conn)
+	require.True(t, isTF, "record_fragment=true: must wrap with tf.Conn")
+}
+
+func TestSTDClient_Client_BothFragment_Wraps(t *testing.T) {
+	t.Parallel()
+	client, server := net.Pipe()
+	defer client.Close()
+	defer server.Close()
+	wrapped, err := newSTDClientConfigForGateTest(true, true).Client(client)
+	require.NoError(t, err)
+	_, isTF := wrapped.NetConn().(*tf.Conn)
+	require.True(t, isTF, "both fragment flags: must wrap with tf.Conn")
+}

common/tls/std_client.go

@@ -75,7 +75,7 @@ func (c *STDClientConfig) STDConfig() (*STDConfig, error) {
 }
 
 func (c *STDClientConfig) Client(conn net.Conn) (Conn, error) {
-	if c.recordFragment {
+	if c.fragment || c.recordFragment {
 		conn = tf.NewConn(conn, c.ctx, c.fragment, c.recordFragment, c.fragmentFallbackDelay)
 	}
 	conn, err := applyTLSSpoof(conn, c.spoof, c.spoofMethod)

common/tls/utls_client.go

@@ -83,7 +83,7 @@ func (c *UTLSClientConfig) STDConfig() (*STDConfig, error) {
 }
 
 func (c *UTLSClientConfig) Client(conn net.Conn) (Conn, error) {
-	if c.recordFragment {
+	if c.fragment || c.recordFragment {
 		conn = tf.NewConn(conn, c.ctx, c.fragment, c.recordFragment, c.fragmentFallbackDelay)
 	}
 	conn, err := applyTLSSpoof(conn, c.spoof, c.spoofMethod)

common/tls/utls_client_test.go

@@ -0,0 +1,73 @@
+//go:build with_utls
+
+package tls
+
+import (
+	"context"
+	"net"
+	"testing"
+
+	tf "github.com/sagernet/sing-box/common/tlsfragment"
+
+	utls "github.com/metacubex/utls"
+	"github.com/stretchr/testify/require"
+)
+
+// Guards the wrap gate in UTLSClientConfig.Client(): tf.Conn must wrap the
+// underlying connection whenever either `fragment` or `record_fragment` is
+// set. Mirrors the STDClientConfig gate tests to keep both code paths in
+// lockstep.
+
+func newUTLSClientConfigForGateTest(fragment, recordFragment bool) *UTLSClientConfig {
+	return &UTLSClientConfig{
+		ctx:            context.Background(),
+		config:         &utls.Config{ServerName: "example.com", InsecureSkipVerify: true},
+		id:             utls.HelloChrome_Auto,
+		fragment:       fragment,
+		recordFragment: recordFragment,
+	}
+}
+
+func TestUTLSClient_Client_NoFragment_DoesNotWrap(t *testing.T) {
+	t.Parallel()
+	client, server := net.Pipe()
+	defer client.Close()
+	defer server.Close()
+	wrapped, err := newUTLSClientConfigForGateTest(false, false).Client(client)
+	require.NoError(t, err)
+	_, isTF := wrapped.NetConn().(*tf.Conn)
+	require.False(t, isTF, "no fragment flags: must not wrap with tf.Conn")
+}
+
+func TestUTLSClient_Client_FragmentOnly_Wraps(t *testing.T) {
+	t.Parallel()
+	client, server := net.Pipe()
+	defer client.Close()
+	defer server.Close()
+	wrapped, err := newUTLSClientConfigForGateTest(true, false).Client(client)
+	require.NoError(t, err)
+	_, isTF := wrapped.NetConn().(*tf.Conn)
+	require.True(t, isTF, "fragment=true: must wrap with tf.Conn")
+}
+
+func TestUTLSClient_Client_RecordFragmentOnly_Wraps(t *testing.T) {
+	t.Parallel()
+	client, server := net.Pipe()
+	defer client.Close()
+	defer server.Close()
+	wrapped, err := newUTLSClientConfigForGateTest(false, true).Client(client)
+	require.NoError(t, err)
+	_, isTF := wrapped.NetConn().(*tf.Conn)
+	require.True(t, isTF, "record_fragment=true: must wrap with tf.Conn")
+}
+
+func TestUTLSClient_Client_BothFragment_Wraps(t *testing.T) {
+	t.Parallel()
+	client, server := net.Pipe()
+	defer client.Close()
+	defer server.Close()
+	wrapped, err := newUTLSClientConfigForGateTest(true, true).Client(client)
+	require.NoError(t, err)
+	_, isTF := wrapped.NetConn().(*tf.Conn)
+	require.True(t, isTF, "both fragment flags: must wrap with tf.Conn")
+}