derp/derphttp: fix netcheck HTTPS probes

The netcheck client, when no UDP is available, probes distance using
HTTPS.

Several problems:

* It probes using /derp/latency-check.
* But cmd/derper serves the handler at /derp/probe
* Despite the difference, it work by accident until c8f4dfc8c0
  which made netcheck's probe require a 2xx status code.
* in tests, we only use derphttp.Handler, so the cmd/derper-installed
  mux routes aren't preesnt, so there's no probe. That breaks
  tests in airplane mode. netcheck.Client then reports "unexpected
  HTTP status 426" (Upgrade Required)

This makes derp handle both /derp/probe and /derp/latency-check
equivalently, and in both cmd/derper and derphttp.Handler standalone
modes.

I notice this when wgengine/magicsock TestActiveDiscovery was failing
in airplane mode (no wifi). It still doesn't pass, but it gets
further.

Fixes #11989

Change-Id: I45213d4bd137e0f29aac8bd4a9ac92091065113f
pull/11995/head
Brad Fitzpatrick 7 months ago committed by Brad Fitzpatrick
parent 1fe0983f2d
commit 15fc6cd966

@ -191,7 +191,12 @@ func main() {
http.Error(w, "derp server disabled", http.StatusNotFound) http.Error(w, "derp server disabled", http.StatusNotFound)
})) }))
} }
mux.HandleFunc("/derp/probe", probeHandler)
// These two endpoints are the same. Different versions of the clients
// have assumes different paths over time so we support both.
mux.HandleFunc("/derp/probe", derphttp.ProbeHandler)
mux.HandleFunc("/derp/latency-check", derphttp.ProbeHandler)
go refreshBootstrapDNSLoop() go refreshBootstrapDNSLoop()
mux.HandleFunc("/bootstrap-dns", tsweb.BrowserHeaderHandlerFunc(handleBootstrapDNS)) mux.HandleFunc("/bootstrap-dns", tsweb.BrowserHeaderHandlerFunc(handleBootstrapDNS))
mux.Handle("/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { mux.Handle("/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@ -370,17 +375,6 @@ func isChallengeChar(c rune) bool {
c == '.' || c == '-' || c == '_' c == '.' || c == '-' || c == '_'
} }
// probeHandler is the endpoint that js/wasm clients hit to measure
// DERP latency, since they can't do UDP STUN queries.
func probeHandler(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case "HEAD", "GET":
w.Header().Set("Access-Control-Allow-Origin", "*")
default:
http.Error(w, "bogus probe method", http.StatusMethodNotAllowed)
}
}
var validProdHostname = regexp.MustCompile(`^derp([^.]*)\.tailscale\.com\.?$`) var validProdHostname = regexp.MustCompile(`^derp([^.]*)\.tailscale\.com\.?$`)
func prodAutocertHostPolicy(_ context.Context, host string) error { func prodAutocertHostPolicy(_ context.Context, host string) error {

@ -20,6 +20,16 @@ const fastStartHeader = "Derp-Fast-Start"
func Handler(s *derp.Server) http.Handler { func Handler(s *derp.Server) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// These are installed both here and in cmd/derper. The check here
// catches both cmd/derper run with DERP disabled (STUN only mode) as
// well as DERP being run in tests with derphttp.Handler directly,
// as netcheck still assumes this replies.
switch r.URL.Path {
case "/derp/probe", "/derp/latency-check":
ProbeHandler(w, r)
return
}
up := strings.ToLower(r.Header.Get("Upgrade")) up := strings.ToLower(r.Header.Get("Upgrade"))
if up != "websocket" && up != "derp" { if up != "websocket" && up != "derp" {
if up != "" { if up != "" {
@ -58,3 +68,14 @@ func Handler(s *derp.Server) http.Handler {
s.Accept(r.Context(), netConn, conn, netConn.RemoteAddr().String()) s.Accept(r.Context(), netConn, conn, netConn.RemoteAddr().String())
}) })
} }
// ProbeHandler is the endpoint that clients without UDP access (including js/wasm) hit to measure
// DERP latency, as a replacement for UDP STUN queries.
func ProbeHandler(w http.ResponseWriter, r *http.Request) {
switch r.Method {
case "HEAD", "GET":
w.Header().Set("Access-Control-Allow-Origin", "*")
default:
http.Error(w, "bogus probe method", http.StatusMethodNotAllowed)
}
}

@ -10,6 +10,7 @@ import (
"fmt" "fmt"
"net" "net"
"net/http" "net/http"
"net/http/httptest"
"net/netip" "net/netip"
"sync" "sync"
"testing" "testing"
@ -464,3 +465,24 @@ func TestLocalAddrNoMutex(t *testing.T) {
t.Errorf("got error %q; want %q", got, want) t.Errorf("got error %q; want %q", got, want)
} }
} }
func TestProbe(t *testing.T) {
h := Handler(nil)
tests := []struct {
path string
want int
}{
{"/derp/probe", 200},
{"/derp/latency-check", 200},
{"/derp/sdf", http.StatusUpgradeRequired},
}
for _, tt := range tests {
rec := httptest.NewRecorder()
h.ServeHTTP(rec, httptest.NewRequest("GET", tt.path, nil))
if got := rec.Result().StatusCode; got != tt.want {
t.Errorf("for path %q got HTTP status %v; want %v", tt.path, got, tt.want)
}
}
}

Loading…
Cancel
Save