diff --git a/derp/derphttp/derphttp_client.go b/derp/derphttp/derphttp_client.go index 3360d436f..376f55519 100644 --- a/derp/derphttp/derphttp_client.go +++ b/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() } } diff --git a/net/netcheck/netcheck.go b/net/netcheck/netcheck.go index 18a87f4d8..dd2afe55b 100644 --- a/net/netcheck/netcheck.go +++ b/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 { - c.logf("probing %s: %v", node.HostName, err) + if r, err := http.DefaultClient.Do(req); err != nil || r.StatusCode > 299 { + 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 { - c.logf("probing %s: %v", node.HostName, err) + if r, err := http.DefaultClient.Do(req); err != nil || r.StatusCode > 299 { + 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