From 29a5b16395660260325cdd046764a6a79cbb82e4 Mon Sep 17 00:00:00 2001 From: ginuerzh Date: Wed, 3 Jun 2026 21:14:42 +0800 Subject: [PATCH] fix(handler/tunnel): respect user-supplied host in handleBind, add ingress conflict detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- handler/tunnel/bind.go | 41 +++++++---- handler/tunnel/bind_test.go | 131 ++++++++++++++++++++++++++++++++- handler/tunnel/connect_test.go | 9 ++- 3 files changed, 165 insertions(+), 16 deletions(-) diff --git a/handler/tunnel/bind.go b/handler/tunnel/bind.go index 78340415..044bb61a 100644 --- a/handler/tunnel/bind.go +++ b/handler/tunnel/bind.go @@ -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. - host = endpoint + 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{} @@ -73,11 +86,11 @@ func (h *tunnelHandler) handleBind(ctx context.Context, conn net.Conn, network, }, ) if _, err = resp.WriteTo(conn); err != nil { - log.Error(err) - return - } + log.Error(err) + return + } - // Upgrade connection to multiplex session. + // Upgrade connection to multiplex session. session, err := mux.ClientSession(conn, h.md.muxCfg) if err != nil { return @@ -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(), diff --git a/handler/tunnel/bind_test.go b/handler/tunnel/bind_test.go index f6c8c2fa..da8b0d7f 100644 --- a/handler/tunnel/bind_test.go +++ b/handler/tunnel/bind_test.go @@ -2,6 +2,8 @@ package tunnel import ( "context" + "crypto/md5" + "encoding/hex" "net" "testing" "time" @@ -438,4 +440,131 @@ func TestHandleBind_WithHostEndpoint(t *testing.T) { if ing.rule == nil { t.Error("expected ingress rule to be set") } -} \ No newline at end of file +} +// 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") + } +} diff --git a/handler/tunnel/connect_test.go b/handler/tunnel/connect_test.go index 2f6f073a..796c9f1f 100644 --- a/handler/tunnel/connect_test.go +++ b/handler/tunnel/connect_test.go @@ -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 + 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 }