From fb84ccd82d21b23f1831590c5b009f789bf67832 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Wed, 8 Feb 2023 18:24:30 +0000
Subject: [PATCH] control/controlhttp: don't require valid TLS cert for Noise
 connection

We don't require any cert at all for Noise-over-plaintext-port-80-HTTP,
so why require a valid cert chain for Noise-over-HTTPS? The reason we use
HTTPS at all is to get through firewalls that allow tcp/443 but not tcp/80,
not because we need the security properties of TLS.

Updates #3198

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
---
 control/controlhttp/client.go    | 20 +++++++++++---
 control/controlhttp/constants.go |  6 ++---
 control/controlhttp/http_test.go | 46 ++++++++++++++++----------------
 3 files changed, 43 insertions(+), 29 deletions(-)

diff --git a/control/controlhttp/client.go b/control/controlhttp/client.go
index dd8e0057a..4eebbee9a 100644
--- a/control/controlhttp/client.go
+++ b/control/controlhttp/client.go
@@ -410,10 +410,24 @@ func (a *Dialer) tryURLUpgrade(ctx context.Context, u *url.URL, addr netip.Addr,
 	tr.TLSClientConfig.NextProtos = []string{}
 	tr.TLSNextProto = map[string]func(string, *tls.Conn) http.RoundTripper{}
 	tr.TLSClientConfig = tlsdial.Config(a.Hostname, tr.TLSClientConfig)
-	if a.insecureTLS {
-		tr.TLSClientConfig.InsecureSkipVerify = true
-		tr.TLSClientConfig.VerifyConnection = nil
+	if !tr.TLSClientConfig.InsecureSkipVerify {
+		panic("unexpected") // should be set by tlsdial.Config
 	}
+	verify := tr.TLSClientConfig.VerifyConnection
+	if verify == nil {
+		panic("unexpected") // should be set by tlsdial.Config
+	}
+	// Demote all cert verification errors to log messages. We don't actually
+	// care about the TLS security (because we just do the Noise crypto atop whatever
+	// connection we get, including HTTP port 80 plaintext) so this permits
+	// middleboxes to MITM their users. All they'll see is some Noise.
+	tr.TLSClientConfig.VerifyConnection = func(cs tls.ConnectionState) error {
+		if err := verify(cs); err != nil && a.Logf != nil && !a.omitCertErrorLogging {
+			a.Logf("warning: TLS cert verificication for %q failed: %v", a.Hostname, err)
+		}
+		return nil // regardless
+	}
+
 	tr.DialTLSContext = dnscache.TLSDialer(dialer, dns, tr.TLSClientConfig)
 	tr.DisableCompression = true
 
diff --git a/control/controlhttp/constants.go b/control/controlhttp/constants.go
index 9a7da22f6..045eeba7f 100644
--- a/control/controlhttp/constants.go
+++ b/control/controlhttp/constants.go
@@ -78,9 +78,9 @@ type Dialer struct {
 	proxyFunc func(*http.Request) (*url.URL, error) // or nil
 
 	// For tests only
-	drainFinished     chan struct{}
-	insecureTLS       bool
-	testFallbackDelay time.Duration
+	drainFinished        chan struct{}
+	omitCertErrorLogging bool
+	testFallbackDelay    time.Duration
 }
 
 func strDef(v1, v2 string) string {
diff --git a/control/controlhttp/http_test.go b/control/controlhttp/http_test.go
index 128421a25..5090b830a 100644
--- a/control/controlhttp/http_test.go
+++ b/control/controlhttp/http_test.go
@@ -194,16 +194,16 @@ func testControlHTTP(t *testing.T, param httpTestParam) {
 	}
 
 	a := &Dialer{
-		Hostname:          "localhost",
-		HTTPPort:          strconv.Itoa(httpLn.Addr().(*net.TCPAddr).Port),
-		HTTPSPort:         strconv.Itoa(httpsLn.Addr().(*net.TCPAddr).Port),
-		MachineKey:        client,
-		ControlKey:        server.Public(),
-		ProtocolVersion:   testProtocolVersion,
-		Dialer:            new(tsdial.Dialer).SystemDial,
-		Logf:              t.Logf,
-		insecureTLS:       true,
-		testFallbackDelay: 50 * time.Millisecond,
+		Hostname:             "localhost",
+		HTTPPort:             strconv.Itoa(httpLn.Addr().(*net.TCPAddr).Port),
+		HTTPSPort:            strconv.Itoa(httpsLn.Addr().(*net.TCPAddr).Port),
+		MachineKey:           client,
+		ControlKey:           server.Public(),
+		ProtocolVersion:      testProtocolVersion,
+		Dialer:               new(tsdial.Dialer).SystemDial,
+		Logf:                 t.Logf,
+		omitCertErrorLogging: true,
+		testFallbackDelay:    50 * time.Millisecond,
 	}
 
 	if proxy != nil {
@@ -646,19 +646,19 @@ func TestDialPlan(t *testing.T) {
 
 			drained := make(chan struct{})
 			a := &Dialer{
-				Hostname:          host,
-				HTTPPort:          httpPort,
-				HTTPSPort:         httpsPort,
-				MachineKey:        client,
-				ControlKey:        server.Public(),
-				ProtocolVersion:   testProtocolVersion,
-				Dialer:            dialer.Dial,
-				Logf:              t.Logf,
-				DialPlan:          tt.plan,
-				proxyFunc:         func(*http.Request) (*url.URL, error) { return nil, nil },
-				drainFinished:     drained,
-				insecureTLS:       true,
-				testFallbackDelay: 50 * time.Millisecond,
+				Hostname:             host,
+				HTTPPort:             httpPort,
+				HTTPSPort:            httpsPort,
+				MachineKey:           client,
+				ControlKey:           server.Public(),
+				ProtocolVersion:      testProtocolVersion,
+				Dialer:               dialer.Dial,
+				Logf:                 t.Logf,
+				DialPlan:             tt.plan,
+				proxyFunc:            func(*http.Request) (*url.URL, error) { return nil, nil },
+				drainFinished:        drained,
+				omitCertErrorLogging: true,
+				testFallbackDelay:    50 * time.Millisecond,
 			}
 
 			conn, err := a.dial(ctx)