diff --git a/cmd/derper/derper.go b/cmd/derper/derper.go index 11dabfc51..6b1b5e1f6 100644 --- a/cmd/derper/derper.go +++ b/cmd/derper/derper.go @@ -325,11 +325,31 @@ func main() { } } +const ( + noContentChallengeHeader = "X-Tailscale-Challenge" + noContentResponseHeader = "X-Tailscale-Response" +) + // For captive portal detection func serveNoContent(w http.ResponseWriter, r *http.Request) { + if challenge := r.Header.Get(noContentChallengeHeader); challenge != "" { + badChar := strings.IndexFunc(challenge, func(r rune) bool { + return !isChallengeChar(r) + }) != -1 + if len(challenge) <= 64 && !badChar { + w.Header().Set(noContentResponseHeader, "response "+challenge) + } + } w.WriteHeader(http.StatusNoContent) } +func isChallengeChar(c rune) bool { + // Semi-randomly chosen as a limited set of valid characters + return ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z') || + ('0' <= c && c <= '9') || + 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) { diff --git a/cmd/derper/derper_test.go b/cmd/derper/derper_test.go index 4adab4e8b..bf3a0ebce 100644 --- a/cmd/derper/derper_test.go +++ b/cmd/derper/derper_test.go @@ -7,6 +7,9 @@ package main import ( "context" "net" + "net/http" + "net/http/httptest" + "strings" "testing" "tailscale.com/net/stun" @@ -67,3 +70,57 @@ func BenchmarkServerSTUN(b *testing.B) { } } + +func TestNoContent(t *testing.T) { + testCases := []struct { + name string + input string + want string + }{ + { + name: "no challenge", + }, + { + name: "valid challenge", + input: "input", + want: "response input", + }, + { + name: "invalid challenge", + input: "foo\x00bar", + want: "", + }, + { + name: "whitespace invalid challenge", + input: "foo bar", + want: "", + }, + { + name: "long challenge", + input: strings.Repeat("x", 65), + want: "", + }, + } + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + req, _ := http.NewRequest("GET", "https://localhost/generate_204", nil) + if tt.input != "" { + req.Header.Set(noContentChallengeHeader, tt.input) + } + w := httptest.NewRecorder() + serveNoContent(w, req) + resp := w.Result() + + if tt.want == "" { + if h, found := resp.Header[noContentResponseHeader]; found { + t.Errorf("got %+v; expected no response header", h) + } + return + } + + if got := resp.Header.Get(noContentResponseHeader); got != tt.want { + t.Errorf("got %q; want %q", got, tt.want) + } + }) + } +} diff --git a/net/netcheck/netcheck.go b/net/netcheck/netcheck.go index d1a55b091..66da47d6f 100644 --- a/net/netcheck/netcheck.go +++ b/net/netcheck/netcheck.go @@ -1116,13 +1116,20 @@ func (c *Client) checkCaptivePortal(ctx context.Context, dm *tailcfg.DERPMap, pr if err != nil { return false, err } + + chal := "tailscale " + node.HostName + req.Header.Set("X-Tailscale-Challenge", chal) r, err := noRedirectClient.Do(req) if err != nil { return false, err } - c.logf("[v2] checkCaptivePortal url=%q status_code=%d", req.URL.String(), r.StatusCode) + defer r.Body.Close() + + expectedResponse := "response " + chal + validResponse := r.Header.Get("X-Tailscale-Response") == expectedResponse - return r.StatusCode != 204, nil + c.logf("[v2] checkCaptivePortal url=%q status_code=%d valid_response=%v", req.URL.String(), r.StatusCode, validResponse) + return r.StatusCode != 204 || !validResponse, nil } // runHTTPOnlyChecks is the netcheck done by environments that can