derp/derphttp: add tests for proxied CONNECT port selection

Adds two tests covering the fix in 0e4c8fc92:

TestDialNodeUsingProxyPort exercises dialNodeUsingProxy directly via a
stub CONNECT proxy, asserting the recorded target across four cases:
HTTPS/HTTP default fallback and explicit DERPPort override for each.

TestConnectThroughProxyHonorsDERPPort drives the full path end-to-end:
a real derpserver on an ephemeral TLS port, a real CONNECT proxy that
tunnels bytes bidirectionally, and a region client routed through it
via feature.HookProxyFromEnvironment. Without the fix, Connect fails
because the proxy is asked to dial :443.

Signed-off-by: Martin Zihlmann <martizih@outlook.com>

derp/derphttp/derphttp_test.go

@@ -4,6 +4,7 @@
 package derphttp_test
 
 import (
+	"bufio"
 	"bytes"
 	"context"
 	"crypto/tls"
@@ -11,11 +12,14 @@ import (
 	"errors"
 	"flag"
 	"fmt"
+	"io"
 	"maps"
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"slices"
+	"strconv"
 	"strings"
 	"sync"
 	"testing"
@@ -25,6 +29,7 @@ import (
 	"tailscale.com/derp"
 	"tailscale.com/derp/derphttp"
 	"tailscale.com/derp/derpserver"
+	"tailscale.com/feature"
 	"tailscale.com/net/memnet"
 	"tailscale.com/net/netmon"
 	"tailscale.com/net/netx"
@@ -625,3 +630,162 @@ func TestURLDial(t *testing.T) {
 		t.Fatalf("rc.Connect: %v", err)
 	}
 }
+
+// startFakeCONNECTProxy starts a loopback HTTP proxy that records each
+// CONNECT target and best-effort tunnels bytes upstream.
+func startFakeCONNECTProxy(t *testing.T) (proxyURL *url.URL, targets <-chan string) {
+	t.Helper()
+	ln, err := net.Listen("tcp4", "localhost:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { ln.Close() })
+
+	ch := make(chan string, 1)
+	go func() {
+		for {
+			conn, err := ln.Accept()
+			if err != nil {
+				return
+			}
+			go func(conn net.Conn) {
+				defer conn.Close()
+				br := bufio.NewReader(conn)
+				req, err := http.ReadRequest(br)
+				if err != nil {
+					return
+				}
+				if req.Method != "CONNECT" {
+					fmt.Fprint(conn, "HTTP/1.1 400 Bad Request\r\n\r\n")
+					return
+				}
+				ch <- req.Host
+				fmt.Fprint(conn, "HTTP/1.1 200 OK\r\n\r\n")
+				up, err := net.Dial("tcp", req.Host)
+				if err != nil {
+					return
+				}
+				defer up.Close()
+				done := make(chan struct{}, 2)
+				go func() { io.Copy(up, br); done <- struct{}{} }()
+				go func() { io.Copy(conn, up); done <- struct{}{} }()
+				<-done
+			}(conn)
+		}
+	}()
+	return &url.URL{Scheme: "http", Host: ln.Addr().String()}, ch
+}
+
+// TestDialNodeUsingProxyPort verifies that the CONNECT target sent to an
+// HTTPS_PROXY honors DERPNode.DERPPort, and otherwise falls back to the
+// same defaults as the direct dial path (443 for HTTPS, 3340 for HTTP).
+// Regression test for #19748.
+func TestDialNodeUsingProxyPort(t *testing.T) {
+	tests := []struct {
+		name       string
+		clientURL  string // controls c.useHTTPS()
+		nodePort   int
+		wantTarget string
+	}{
+		{"https_default", "https://unused.example/", 0, "derp.example:443"},
+		{"http_default", "http://unused.example/", 0, "derp.example:3340"},
+		{"https_custom_port", "https://unused.example/", 8765, "derp.example:8765"},
+		{"http_custom_port", "http://unused.example/", 8765, "derp.example:8765"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			proxyURL, targets := startFakeCONNECTProxy(t)
+
+			c, err := derphttp.NewClient(key.NewNode(), tt.clientURL, t.Logf, netmon.NewStatic())
+			if err != nil {
+				t.Fatalf("NewClient: %v", err)
+			}
+			defer c.Close()
+
+			node := &tailcfg.DERPNode{HostName: "derp.example", DERPPort: tt.nodePort}
+			ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+			defer cancel()
+			conn, err := c.DialNodeUsingProxy(ctx, node, proxyURL)
+			if err != nil {
+				t.Fatalf("DialNodeUsingProxy: %v", err)
+			}
+			conn.Close()
+
+			if got := <-targets; got != tt.wantTarget {
+				t.Errorf("CONNECT target = %q, want %q", got, tt.wantTarget)
+			}
+		})
+	}
+}
+
+// TestConnectThroughProxyHonorsDERPPort drives the full proxy path
+// end-to-end: a real DERP server on a non-default port, behind a real
+// CONNECT proxy, with the client routed through that proxy via
+// feature.HookProxyFromEnvironment. Without the #19748 fix, the proxy
+// would be asked to tunnel to :443 and the dial would fail.
+func TestConnectThroughProxyHonorsDERPPort(t *testing.T) {
+	// Real DERP server on TLS, ephemeral loopback port.
+	serverKey := key.NewNode()
+	s := derpserver.New(serverKey, t.Logf)
+	defer s.Close()
+
+	derpSrv := httptest.NewUnstartedServer(derpserver.Handler(s))
+	derpSrv.StartTLS()
+	defer derpSrv.Close()
+
+	derpURL, err := url.Parse(derpSrv.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	derpPort, err := strconv.Atoi(derpURL.Port())
+	if err != nil {
+		t.Fatalf("parsing derp port %q: %v", derpURL.Port(), err)
+	}
+	if derpPort == 443 {
+		t.Fatalf("test server bound to :443; can't distinguish bug from fix")
+	}
+
+	proxyURL, targets := startFakeCONNECTProxy(t)
+	restore := feature.HookProxyFromEnvironment.SetForTest(func(*http.Request) (*url.URL, error) {
+		return proxyURL, nil
+	})
+	defer restore()
+
+	region := &tailcfg.DERPRegion{
+		RegionID:   1,
+		RegionCode: "test",
+		Nodes: []*tailcfg.DERPNode{{
+			Name:             "1a",
+			RegionID:         1,
+			HostName:         "127.0.0.1",
+			IPv4:             "127.0.0.1",
+			DERPPort:         derpPort,
+			InsecureForTests: true,
+		}},
+	}
+	c := derphttp.NewRegionClient(key.NewNode(), t.Logf, netmon.NewStatic(),
+		func() *tailcfg.DERPRegion { return region })
+	defer c.Close()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+	if err := c.Connect(ctx); err != nil {
+		t.Fatalf("Connect: %v", err)
+	}
+
+	// Pump Recv so the PONG gets dispatched.
+	go func() {
+		for {
+			if _, err := c.Recv(); err != nil {
+				return
+			}
+		}
+	}()
+	if err := c.Ping(ctx); err != nil {
+		t.Fatalf("Ping through proxy tunnel: %v", err)
+	}
+
+	if got, want := <-targets, fmt.Sprintf("127.0.0.1:%d", derpPort); got != want {
+		t.Errorf("proxy CONNECT target = %q, want %q", got, want)
+	}
+}

derp/derphttp/export_test.go

@@ -3,10 +3,22 @@
 
 package derphttp
 
+import (
+	"context"
+	"net"
+	"net/url"
+
+	"tailscale.com/tailcfg"
+)
+
 func SetTestHookWatchLookConnectResult(f func(connectError error, wasSelfConnect bool) (keepRunning bool)) {
 	testHookWatchLookConnectResult = f
 }
 
+func (c *Client) DialNodeUsingProxy(ctx context.Context, n *tailcfg.DERPNode, proxyURL *url.URL) (net.Conn, error) {
+	return c.dialNodeUsingProxy(ctx, n, proxyURL)
+}
+
 // breakConnection breaks the connection, which should trigger a reconnect.
 func (c *Client) BreakConnection(brokenClient *Client) {
 	c.mu.Lock()