derp/derphttp,net/netcheck: improve netcheck behavior under MITM proxies

In cases where tailscale is operating behind a MITM proxy, we need to consider
that a lot more of the internals of our HTTP requests are visible and may be
used as part of authorization checks. As such, we need to 'behave' as closely
as possible to ideal.

 - Some proxies do authorization or consistency checks based the on Host header
   or HTTP URI, instead of just the IP/hostname/SNI. As such, we need to
   construct a `*http.Request` with a valid URI everytime HTTP is going to be
   used on the wire, even if its over TLS.
   Aside from the singular instance in net/netcheck, I couldn't find anywhere
   else a http.Request was constructed incorrectly.

 - Some proxies may deny requests, typically by returning a 403 status code. We
   should not consider these requests as a valid latency check, so netcheck
   semantics have been updated to consider >299 status codes as a failed probe.

Signed-off-by: Tom DNetto <tom@tailscale.com>

derp/derphttp/derphttp_client.go

@@ -538,12 +538,17 @@ func (c *Client) tlsClient(nc net.Conn, node *tailcfg.DERPNode) *tls.Conn {
 	return tls.Client(nc, tlsConf)
 }
 
-func (c *Client) DialRegionTLS(ctx context.Context, reg *tailcfg.DERPRegion) (tlsConn *tls.Conn, connClose io.Closer, err error) {
+// DialRegionTLS returns a TLS connection to a DERP node in the given region.
+//
+// DERP nodes for a region are tried in sequence according to their order
+// in the DERP map. TLS is initiated on the first node where a socket is
+// established.
+func (c *Client) DialRegionTLS(ctx context.Context, reg *tailcfg.DERPRegion) (tlsConn *tls.Conn, connClose io.Closer, node *tailcfg.DERPNode, err error) {
 	tcpConn, node, err := c.dialRegion(ctx, reg)
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
-	done := make(chan bool) // unbufferd
+	done := make(chan bool) // unbuffered
 	defer close(done)
 
 	tlsConn = c.tlsClient(tcpConn, node)
@@ -556,13 +561,13 @@ func (c *Client) DialRegionTLS(ctx context.Context, reg *tailcfg.DERPRegion) (tl
 	}()
 	err = tlsConn.Handshake()
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, nil, err
 	}
 	select {
 	case done <- true:
-		return tlsConn, tcpConn, nil
+		return tlsConn, tcpConn, node, nil
 	case <-ctx.Done():
-		return nil, nil, ctx.Err()
+		return nil, nil, nil, ctx.Err()
 	}
 }
 

net/netcheck/netcheck.go

@@ -979,13 +979,21 @@ func (c *Client) runHTTPOnlyChecks(ctx context.Context, last *Report, rs *report
 			// One warm-up one to get HTTP connection set
 			// up and get a connection from the browser's
 			// pool.
-			if _, err := http.DefaultClient.Do(req); err != nil {
+			if r, err := http.DefaultClient.Do(req); err != nil || r.StatusCode > 299 {
-				c.logf("probing %s: %v", node.HostName, err)
+				if err != nil {
+					c.logf("probing %s: %v", node.HostName, err)
+				} else {
+					c.logf("probing %s: unexpected status %s", node.HostName, r.Status)
+				}
 				return
 			}
 			t0 := c.timeNow()
-			if _, err := http.DefaultClient.Do(req); err != nil {
+			if r, err := http.DefaultClient.Do(req); err != nil || r.StatusCode > 299 {
-				c.logf("probing %s: %v", node.HostName, err)
+				if err != nil {
+					c.logf("probing %s: %v", node.HostName, err)
+				} else {
+					c.logf("probing %s: unexpected status %s", node.HostName, r.Status)
+				}
 				return
 			}
 			d := c.timeNow().Sub(t0)
@@ -1005,7 +1013,7 @@ func (c *Client) measureHTTPSLatency(ctx context.Context, reg *tailcfg.DERPRegio
 	var ip netaddr.IP
 
 	dc := derphttp.NewNetcheckClient(c.logf)
-	tlsConn, tcpConn, err := dc.DialRegionTLS(ctx, reg)
+	tlsConn, tcpConn, node, err := dc.DialRegionTLS(ctx, reg)
 	if err != nil {
 		return 0, ip, err
 	}
@@ -1036,7 +1044,7 @@ func (c *Client) measureHTTPSLatency(ctx context.Context, reg *tailcfg.DERPRegio
 	}
 	hc := &http.Client{Transport: tr}
 
-	req, err := http.NewRequestWithContext(ctx, "GET", "https://derp-unused-hostname.tld/derp/latency-check", nil)
+	req, err := http.NewRequestWithContext(ctx, "GET", "https://" + node.HostName + "/derp/latency-check", nil)
 	if err != nil {
 		return 0, ip, err
 	}
@@ -1047,6 +1055,13 @@ func (c *Client) measureHTTPSLatency(ctx context.Context, reg *tailcfg.DERPRegio
 	}
 	defer resp.Body.Close()
 
+	// DERPs should give us a nominal status code, so anything else is probably
+	// an access denied by a MITM proxy (or at the very least a signal not to
+	// trust this latency check).
+	if resp.StatusCode > 299 {
+		return 0, ip, fmt.Errorf("unexpected status code: %d (%s)", resp.StatusCode, resp.Status)
+	}
+
 	_, err = io.Copy(ioutil.Discard, io.LimitReader(resp.Body, 8<<10))
 	if err != nil {
 		return 0, ip, err