fix(handler/tunnel): respect user-supplied host in handleBind, add ingress conflict detection

The response address sent back to the tunnel client was unconditionally set to the md5 hash of the tunnel ID. This meant the user's custom host (e.g. "dash" from "dash:8081") was ignored — the client listened on the hash instead. Now the user-supplied host is used directly when: 1. A host is present in the bind address AND ingress is configured. 2. No other tunnel has already claimed that host in ingress (conflict check via GetRule). Fall back to the md5 hash when no host is supplied, ingress is nil, or the host conflicts with an existing ingress rule. Also fixes indentation in the WriteTo error handling block (extra tab removed). Tests: - TestHandleBind_CustomHost: response AddrFeature uses "dash", not hash - TestHandleBind_CustomHostConflict: conflicting host falls back to hash - fakeIngress enhanced with ruleByHost map for conflict simulation
2026-06-03 21:14:42 +08:00
parent 91920aa805
commit 29a5b16395
3 changed files with 165 additions and 16 deletions
@@ -17,16 +17,27 @@ import (

 // handleBind handles a CmdBind request from an internal client.
 //
+// The bind address host is resolved as follows:
+//  1. If the user supplied a host (e.g. "dash" from "dash:8081") AND
+//     ingress is configured, use that host directly — the tunnel is
+//     reachable via the user's custom name.
+//  2. Before using the custom host, check for ingress conflicts: if
+//     another tunnel already owns this hostname, fall back to the
+//     md5 hash to avoid route hijacking.
+//  3. If no host was supplied (e.g. ":8081") or ingress is nil, use
+//     the deterministic md5 hash of tunnelID as a stable ingress key.
+//
 // Flow:
 //  1. Generate a random connector ID (copies weight from tunnelID).
-//  2. Compute an 8-hex-char endpoint from md5(tunnelID) for ingress routing.
-//  3. Send relay response with address + tunnel features back to client.
+//  2. Resolve the response address host per the rules above.
+//  3. Send relay response with the bind address + tunnel features back.
 //  4. Upgrade the TCP connection to a mux.ClientSession (smux).
 //  5. Create a Connector wrapping the mux session.
 //  6. Register:
 //     a. Add Connector to ConnectorPool (under tunnelID).
-//     b. If ingress is configured, set rules: endpoint → tunnelID, and
-//        the bind address host → tunnelID.
+//     b. Set ingress rules: the resolved host → tunnelID. When the user
+//        supplied a custom host that differs from the hash, also set a
+//        fallback rule: hash → tunnelID.
 //     c. If SD is configured, register the service (tunnelID, node, network).
 //
 // The mux session ownership is transferred to the Connector — conn is NOT
@@ -55,11 +66,13 @@ func (h *tunnelHandler) handleBind(ctx context.Context, conn net.Conn, network,
 	endpoint := hex.EncodeToString(v[:8])

 	host, port, _ := net.SplitHostPort(address)
-	// Always use the endpoint hash as the host — this provides a stable,
-	// deterministic ingress key regardless of what the internal client sends.
-	// The original host is ignored; the endpoint hash routes consistently
-	// across reconnects and multi-node deployments.
+	if host == "" || h.md.ingress == nil {
 		host = endpoint
+	} else if host != endpoint {
+		if rule := h.md.ingress.GetRule(ctx, host, ingress.WithService(h.options.Service)); rule != nil && rule.Endpoint != tunnelID.String() {
+			host = endpoint
+		}
+	}
 	addr := net.JoinHostPort(host, port)

 	af := &relay.AddrFeature{}
@@ -101,7 +114,7 @@ func (h *tunnelHandler) handleBind(ctx context.Context, conn net.Conn, network,
 			Hostname: endpoint,
 			Endpoint: tunnelID.String(),
 		}, ingress.WithService(h.options.Service))
-		if host != "" {
+		if host != "" && host != endpoint {
 			h.md.ingress.SetRule(ctx, &ingress.Rule{
 				Hostname: host,
 				Endpoint: tunnelID.String(),
@@ -2,6 +2,8 @@ package tunnel

 import (
 	"context"
+	"crypto/md5"
+	"encoding/hex"
 	"net"
 	"testing"
 	"time"
@@ -439,3 +441,130 @@ func TestHandleBind_WithHostEndpoint(t *testing.T) {
 		t.Error("expected ingress rule to be set")
 	}
 }
+// TestHandleBind_CustomHost tests that a user-supplied host is used as
+// the response address, not the md5 hash.
+func TestHandleBind_CustomHost(t *testing.T) {
+	tid := newTestTunnelID(t)
+
+	ing := &fakeIngress{}
+	h := &tunnelHandler{
+		options: handler.Options{
+			Logger: testLogger(),
+		},
+		md: metadata{
+			muxCfg:  &mux.Config{Version: 2},
+			ingress: ing,
+		},
+		id:   "node1",
+		pool: NewConnectorPool("node1"),
+		log:  testLogger(),
+	}
+	defer h.pool.Close()
+
+	client, server := net.Pipe()
+	defer client.Close()
+	defer server.Close()
+
+	errCh := make(chan error, 1)
+	go func() {
+		errCh <- h.handleBind(context.Background(), server, "tcp", "dash:8081", tid, testLogger())
+	}()
+
+	resp := &relay.Response{}
+	_, err := resp.ReadFrom(client)
+	if err != nil {
+		t.Fatalf("read response: %v", err)
+	}
+
+	// The response AddrFeature should use "dash", not the md5 hash.
+	var host string
+	for _, f := range resp.Features {
+		if f.Type() == relay.FeatureAddr {
+			if af, ok := f.(*relay.AddrFeature); ok {
+				host = af.Host
+			}
+		}
+	}
+	if host != "dash" {
+		t.Errorf("expected response host 'dash', got %q", host)
+	}
+
+	client.Close()
+	select {
+	case err := <-errCh:
+		t.Logf("handleBind returned: %v", err)
+	case <-time.After(2 * time.Second):
+		t.Fatal("handleBind did not complete")
+	}
+}
+
+// TestHandleBind_CustomHostConflict tests that when the user-supplied host
+// is already claimed by a different tunnel in ingress, handleBind falls
+// back to the md5 hash to avoid route hijacking.
+func TestHandleBind_CustomHostConflict(t *testing.T) {
+	tid := newTestTunnelID(t)
+	otherEndpoint := "other-tunnel-id"
+
+	ing := &fakeIngress{
+		ruleByHost: map[string]*ingress.Rule{
+			"dash": {
+				Hostname: "dash",
+				Endpoint: otherEndpoint,
+			},
+		},
+	}
+	h := &tunnelHandler{
+		options: handler.Options{
+			Logger: testLogger(),
+		},
+		md: metadata{
+			muxCfg:  &mux.Config{Version: 2},
+			ingress: ing,
+		},
+		id:   "node1",
+		pool: NewConnectorPool("node1"),
+		log:  testLogger(),
+	}
+	defer h.pool.Close()
+
+	client, server := net.Pipe()
+	defer client.Close()
+	defer server.Close()
+
+	errCh := make(chan error, 1)
+	go func() {
+		errCh <- h.handleBind(context.Background(), server, "tcp", "dash:8081", tid, testLogger())
+	}()
+
+	resp := &relay.Response{}
+	_, err := resp.ReadFrom(client)
+	if err != nil {
+		t.Fatalf("read response: %v", err)
+	}
+
+	// The response should fall back to the md5 hash because "dash" is
+	// already claimed by another tunnel.
+	var host string
+	for _, f := range resp.Features {
+		if f.Type() == relay.FeatureAddr {
+			if af, ok := f.(*relay.AddrFeature); ok {
+				host = af.Host
+			}
+		}
+	}
+
+	// Compute the expected fallback hash for this tunnelID.
+	v := md5.Sum([]byte(tid.String()))
+	expectedHash := hex.EncodeToString(v[:8])
+	if host != expectedHash {
+		t.Errorf("expected fallback hash %q for conflicting host, got %q", expectedHash, host)
+	}
+
+	client.Close()
+	select {
+	case err := <-errCh:
+		t.Logf("handleBind returned: %v", err)
+	case <-time.After(2 * time.Second):
+		t.Fatal("handleBind did not complete")
+	}
+}
@@ -23,11 +23,18 @@ func (b *fakeBypass) Contains(ctx context.Context, network, addr string, opts ..
 }

 // fakeIngress implements ingress.Ingress for testing.
+// It stores one rule per hostname for conflict simulation.
 type fakeIngress struct {
 	rule     *ingress.Rule
+	ruleByHost map[string]*ingress.Rule
 }

 func (i *fakeIngress) GetRule(ctx context.Context, host string, opts ...ingress.Option) *ingress.Rule {
+	if i.ruleByHost != nil {
+		if r, ok := i.ruleByHost[host]; ok {
+			return r
+		}
+	}
 	return i.rule
 }