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

@@ -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 {
+	if !tr.TLSClientConfig.InsecureSkipVerify {
-		tr.TLSClientConfig.InsecureSkipVerify = true
+		panic("unexpected") // should be set by tlsdial.Config
-		tr.TLSClientConfig.VerifyConnection = nil
 	}
+	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
 

control/controlhttp/constants.go

@@ -79,7 +79,7 @@ type Dialer struct {
 
 	// For tests only
 	drainFinished        chan struct{}
-	insecureTLS       bool
+	omitCertErrorLogging bool
 	testFallbackDelay    time.Duration
 }
 

control/controlhttp/http_test.go

@@ -202,7 +202,7 @@ func testControlHTTP(t *testing.T, param httpTestParam) {
 		ProtocolVersion:      testProtocolVersion,
 		Dialer:               new(tsdial.Dialer).SystemDial,
 		Logf:                 t.Logf,
-		insecureTLS:       true,
+		omitCertErrorLogging: true,
 		testFallbackDelay:    50 * time.Millisecond,
 	}
 
@@ -657,7 +657,7 @@ func TestDialPlan(t *testing.T) {
 				DialPlan:             tt.plan,
 				proxyFunc:            func(*http.Request) (*url.URL, error) { return nil, nil },
 				drainFinished:        drained,
-				insecureTLS:       true,
+				omitCertErrorLogging: true,
 				testFallbackDelay:    50 * time.Millisecond,
 			}