control/control{client,http}: don't noise dial localhost:443 in http-only tests

1eaad7d3deb regressed some tests in another repo that were starting up
a control server on `http://127.0.0.1:nnn`. Because there was no https
running, and because of a bug in 1eaad7d3deb (which ended up checking
the recently-dialed-control check twice in a single dial call), we
ended up forcing only the use of TLS dials in a test that only had
plaintext HTTP running.

Instead, plumb down support for explicitly disabling TLS fallbacks and
use it only when running in a test and using `http` scheme control
plane URLs to 127.0.0.1 or localhost.

This fixes the tests elsewhere.

Updates #13597

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

control/controlclient/noise.go

@@ -5,6 +5,7 @@ package controlclient
 
 import (
 	"bytes"
+	"cmp"
 	"context"
 	"encoding/json"
 	"errors"
@@ -16,6 +17,7 @@ import (
 
 	"golang.org/x/net/http2"
 	"tailscale.com/control/controlhttp"
+	"tailscale.com/envknob"
 	"tailscale.com/health"
 	"tailscale.com/internal/noiseconn"
 	"tailscale.com/net/dnscache"
@@ -28,6 +30,7 @@ import (
 	"tailscale.com/util/mak"
 	"tailscale.com/util/multierr"
 	"tailscale.com/util/singleflight"
+	"tailscale.com/util/testenv"
 )
 
 // NoiseClient provides a http.Client to connect to tailcontrol over
@@ -56,8 +59,8 @@ type NoiseClient struct {
 	privKey      key.MachinePrivate
 	serverPubKey key.MachinePublic
 	host         string // the host part of serverURL
-	httpPort     string // the default port to call
-	httpsPort    string // the fallback Noise-over-https port
+	httpPort     string // the default port to dial
+	httpsPort    string // the fallback Noise-over-https port or empty if none
 
 	// dialPlan optionally returns a ControlDialPlan previously received
 	// from the control server; either the function or the return value can
@@ -104,6 +107,11 @@ type NoiseOpts struct {
 	DialPlan func() *tailcfg.ControlDialPlan
 }
 
+// controlIsPlaintext is whether we should assume that the controlplane is only accessible
+// over plaintext HTTP (as the first hop, before the ts2021 encryption begins).
+// This is used by some tests which don't have a real TLS certificate.
+var controlIsPlaintext = envknob.RegisterBool("TS_CONTROL_IS_PLAINTEXT_HTTP")
+
 // NewNoiseClient returns a new noiseClient for the provided server and machine key.
 // serverURL is of the form https://<host>:<port> (no trailing slash).
 //
@@ -116,14 +124,17 @@ func NewNoiseClient(opts NoiseOpts) (*NoiseClient, error) {
 	}
 	var httpPort string
 	var httpsPort string
-	if u.Port() != "" {
+	if port := u.Port(); port != "" {
 		// If there is an explicit port specified, trust the scheme and hope for the best
 		if u.Scheme == "http" {
-			httpPort = u.Port()
+			httpPort = port
 			httpsPort = "443"
+			if (testenv.InTest() || controlIsPlaintext()) && (u.Hostname() == "127.0.0.1" || u.Hostname() == "localhost") {
+				httpsPort = ""
+			}
 		} else {
 			httpPort = "80"
-			httpsPort = u.Port()
+			httpsPort = port
 		}
 	} else {
 		// Otherwise, use the standard ports
@@ -340,7 +351,7 @@ func (nc *NoiseClient) dial(ctx context.Context) (*noiseconn.Conn, error) {
 	clientConn, err := (&controlhttp.Dialer{
 		Hostname:        nc.host,
 		HTTPPort:        nc.httpPort,
-		HTTPSPort:       nc.httpsPort,
+		HTTPSPort:       cmp.Or(nc.httpsPort, controlhttp.NoPort),
 		MachineKey:      nc.privKey,
 		ControlKey:      nc.serverPubKey,
 		ProtocolVersion: uint16(tailcfg.CurrentCapabilityVersion),

control/controlhttp/client.go

@@ -86,9 +86,6 @@ func (a *Dialer) getProxyFunc() func(*http.Request) (*url.URL, error) {
 // httpsFallbackDelay is how long we'll wait for a.HTTPPort to work before
 // starting to try a.HTTPSPort.
 func (a *Dialer) httpsFallbackDelay() time.Duration {
-	if a.forceNoise443() {
-		return time.Nanosecond
-	}
 	if v := a.testFallbackDelay; v != 0 {
 		return v
 	}
@@ -323,6 +320,9 @@ func (a *Dialer) dialHost(ctx context.Context, optAddr netip.Addr) (*ClientConn,
 		Host:   net.JoinHostPort(a.Hostname, strDef(a.HTTPSPort, "443")),
 		Path:   serverUpgradePath,
 	}
+	if a.HTTPSPort == NoPort {
+		u443 = nil
+	}
 
 	type tryURLRes struct {
 		u    *url.URL    // input (the URL conn+err are for/from)
@@ -347,15 +347,24 @@ func (a *Dialer) dialHost(ctx context.Context, optAddr netip.Addr) (*ClientConn,
 		}
 	}
 
+	forceTLS := a.forceNoise443()
+
 	// Start the plaintext HTTP attempt first, unless disabled by the envknob.
-	if !a.forceNoise443() {
+	if !forceTLS || u443 == nil {
 		go try(u80)
 	}
 
 	// In case outbound port 80 blocked or MITM'ed poorly, start a backup timer
 	// to dial port 443 if port 80 doesn't either succeed or fail quickly.
-	try443Timer := a.clock().AfterFunc(a.httpsFallbackDelay(), func() { try(u443) })
-	defer try443Timer.Stop()
+	var try443Timer tstime.TimerController
+	if u443 != nil {
+		delay := a.httpsFallbackDelay()
+		if forceTLS {
+			delay = 0
+		}
+		try443Timer = a.clock().AfterFunc(delay, func() { try(u443) })
+		defer try443Timer.Stop()
+	}
 
 	var err80, err443 error
 	for {
@@ -374,7 +383,7 @@ func (a *Dialer) dialHost(ctx context.Context, optAddr netip.Addr) (*ClientConn,
 				// Stop the fallback timer and run it immediately. We don't use
 				// Timer.Reset(0) here because on AfterFuncs, that can run it
 				// again.
-				if try443Timer.Stop() {
+				if try443Timer != nil && try443Timer.Stop() {
 					go try(u443)
 				} // else we lost the race and it started already which is what we want
 			case u443:

control/controlhttp/constants.go

@@ -32,6 +32,11 @@ const (
 	serverUpgradePath = "/ts2021"
 )
 
+// NoPort is a sentinel value for Dialer.HTTPSPort to indicate that HTTPS
+// should not be tried on any port. It exists primarily for some localhost
+// tests where the control plane only runs on HTTP.
+const NoPort = "none"
+
 // Dialer contains configuration on how to dial the Tailscale control server.
 type Dialer struct {
 	// Hostname is the hostname to connect to, with no port number.
@@ -62,6 +67,8 @@ type Dialer struct {
 	// HTTPSPort is the port number to use when making a HTTPS connection.
 	//
 	// If not specified, this defaults to port 443.
+	//
+	// If "none" (NoPort), HTTPS is disabled.
 	HTTPSPort string
 
 	// Dialer is the dialer used to make outbound connections.

tstest/integration/integration_test.go

@@ -1791,6 +1791,7 @@ func (n *testNode) StartDaemonAsIPNGOOS(ipnGOOS string) *Daemon {
 		cmd.Args = append(cmd.Args, "--config="+n.configFile)
 	}
 	cmd.Env = append(os.Environ(),
+		"TS_CONTROL_IS_PLAINTEXT_HTTP=1",
 		"TS_DEBUG_PERMIT_HTTP_C2N=1",
 		"TS_LOG_TARGET="+n.env.LogCatcherServer.URL,
 		"HTTP_PROXY="+n.env.TrafficTrapServer.URL,