tailcfg: break DERPNode.DERPTestPort into DERPPort & InsecureForTests

The DERPTestPort int meant two things before: which port to use, and
whether to disable TLS verification. Users would like to set the port
without disabling TLS, so break it into two options.

Updates #1264

Signed-off-by: Brad Fitzpatrick <[email protected]>

derp/derphttp/derphttp_client.go

@@ -410,9 +410,7 @@ func (c *Client) dialRegion(ctx context.Context, reg *tailcfg.DERPRegion) (net.C
 func (c *Client) tlsClient(nc net.Conn, node *tailcfg.DERPNode) *tls.Conn {
 	tlsConf := tlsdial.Config(c.tlsServerName(node), c.TLSConfig)
 	if node != nil {
-		if node.DERPTestPort != 0 {
-			tlsConf.InsecureSkipVerify = true
-		}
+		tlsConf.InsecureSkipVerify = node.InsecureForTests
 		if node.CertName != "" {
 			tlsdial.SetConfigExpectedCert(tlsConf, node.CertName)
 		}
@@ -511,8 +509,8 @@ func (c *Client) dialNode(ctx context.Context, n *tailcfg.DERPNode) (net.Conn, e
 				dst = n.HostName
 			}
 			port := "443"
-			if n.DERPTestPort != 0 {
-				port = fmt.Sprint(n.DERPTestPort)
+			if n.DERPPort != 0 {
+				port = fmt.Sprint(n.DERPPort)
 			}
 			c, err := c.dialContext(ctx, proto, net.JoinHostPort(dst, port))
 			select {

tailcfg/derpmap.go

@@ -130,10 +130,15 @@ type DERPNode struct {
 	// server.
 	STUNOnly bool `json:",omitempty"`
 
-	// DERPTestPort is used in tests to override the port, instead
-	// of using the default port of 443. If non-zero, TLS
-	// verification is skipped.
-	DERPTestPort int `json:",omitempty"`
+	// DERPPort optionally provides an alternate TLS port number
+	// for the DERP HTTPS server.
+	//
+	// If zero, 443 is used.
+	DERPPort int `json:",omitempty"`
+
+	// InsecureForTests is used by unit tests to disable TLS verification.
+	// It should not be set by users.
+	InsecureForTests bool `json:",omitempty"`
 
 	// STUNTestIP is used in tests to override the STUN server's IP.
 	// If empty, it's assumed to be the same as the DERP server.

tailcfg/tailcfg_clone.go

@@ -327,16 +327,17 @@ func (src *DERPNode) Clone() *DERPNode {
 // A compilation failure here means this code must be regenerated, with command:
 //   tailscale.com/cmd/cloner -type User,Node,Hostinfo,NetInfo,Login,DNSConfig,DNSResolver,RegisterResponse,DERPRegion,DERPMap,DERPNode
 var _DERPNodeNeedsRegeneration = DERPNode(struct {
-	Name         string
-	RegionID     int
-	HostName     string
-	CertName     string
-	IPv4         string
-	IPv6         string
-	STUNPort     int
-	STUNOnly     bool
-	DERPTestPort int
-	STUNTestIP   string
+	Name             string
+	RegionID         int
+	HostName         string
+	CertName         string
+	IPv4             string
+	IPv6             string
+	STUNPort         int
+	STUNOnly         bool
+	DERPPort         int
+	InsecureForTests bool
+	STUNTestIP       string
 }{})
 
 // Clone duplicates src into dst and reports whether it succeeded.

tstest/integration/integration.go

@@ -145,14 +145,15 @@ func RunDERPAndSTUN(t testing.TB, logf logger.Logf, ipAddress string) (derpMap *
 				RegionCode: "test",
 				Nodes: []*tailcfg.DERPNode{
 					{
-						Name:         "t1",
-						RegionID:     1,
-						HostName:     ipAddress,
-						IPv4:         ipAddress,
-						IPv6:         "none",
-						STUNPort:     stunAddr.Port,
-						DERPTestPort: httpsrv.Listener.Addr().(*net.TCPAddr).Port,
-						STUNTestIP:   stunAddr.IP.String(),
+						Name:             "t1",
+						RegionID:         1,
+						HostName:         ipAddress,
+						IPv4:             ipAddress,
+						IPv6:             "none",
+						STUNPort:         stunAddr.Port,
+						DERPPort:         httpsrv.Listener.Addr().(*net.TCPAddr).Port,
+						InsecureForTests: true,
+						STUNTestIP:       stunAddr.IP.String(),
 					},
 				},
 			},

wgengine/magicsock/magicsock_test.go

@@ -95,14 +95,15 @@ func runDERPAndStun(t *testing.T, logf logger.Logf, l nettype.PacketListener, st
 				RegionCode: "test",
 				Nodes: []*tailcfg.DERPNode{
 					{
-						Name:         "t1",
-						RegionID:     1,
-						HostName:     "test-node.unused",
-						IPv4:         "127.0.0.1",
-						IPv6:         "none",
-						STUNPort:     stunAddr.Port,
-						DERPTestPort: httpsrv.Listener.Addr().(*net.TCPAddr).Port,
-						STUNTestIP:   stunIP.String(),
+						Name:             "t1",
+						RegionID:         1,
+						HostName:         "test-node.unused",
+						IPv4:             "127.0.0.1",
+						IPv6:             "none",
+						STUNPort:         stunAddr.Port,
+						DERPPort:         httpsrv.Listener.Addr().(*net.TCPAddr).Port,
+						InsecureForTests: true,
+						STUNTestIP:       stunIP.String(),
 					},
 				},
 			},