mirror of
https://github.com/tailscale/tailscale.git
synced 2026-09-22 11:35:12 -04:00
The map response reader now caps a single message at 256 MiB on the wire and 1 GiB after zstd decompression. The server-chosen uint32 size prefix previously let a malicious control server make us allocate up to 4 GiB before reading any body bytes, and the decoded size was unbounded, so a small zstd frame could expand into gigabytes of JSON. A 16 MB cap has been hit by real production traffic before, so both limits sit far above plausible legitimate sizes. The size-prefixed read moved into a readMapResponseMessage helper, and the newer control/tsp path already enforced both kinds of bounds; this brings the long-poll path it replaces in line, with more generous limits for large tailnets. ts2021.Client.Do additionally caps every noise response body with httpbody.LimitSize, so a malicious or buggy control server can't make us buffer an unbounded response. Client.Do shadows the embedded http.Client's Do method, so register, set-dns, set-device-attr, audit-log, all DoNoiseRequest consumers (webclient, tailnet lock, SSH actions, id-token, feature queries), and the debug CLI get the cap without per-call-site changes, and future noise endpoints get it for free. The cap lives in the new util/httpbody package so other HTTP clients can adopt the same convention: LimitSize looks the size limit up from res.Request's context (a Response knows the Request that produced it), falling back to DefaultMaxSize, 1 MiB, when the context carries no override. It is like io.LimitReader except that reads past the limit fail with an error wrapping httpbody.ErrTooLarge instead of silently truncating, and a body of at most the limit, including one of exactly the limit, reads back without error: the wrapper probes for EOF once the limit is exhausted to tell an exactly-at-limit body from an oversize one. The per-request override, httpbody.WithMaxSize, is a context key, so transports pick it up with no API changes; LimitSizeTo applies an explicit limit ignoring any override. Repeated LimitSize or LimitSizeTo calls replace the previous limit rather than compounding it, so a later call can raise or remove the limit an earlier one set. Responses that stream an unbounded number of individually bounded messages disable the cap with httpbody.WithMaxSize(ctx, 0): the /machine/map long-poll and control/tsp's map session, whose messages are already capped per-message (by readMapResponseMessage and decodeMsg, and by tsp's framedReader and boundedReader). Their non-200 error bodies are not message streams, so those are capped with LimitSizeTo instead. The tailnet lock /tka/init/begin, /tka/sync/offer and /tka/affected-sigs responses can carry per-node key signatures or missing AUMs, which at 100,000 peers reach tens of MB, so they raise the cap to 512 MiB. The per-response io.LimitedReader decoders that silently truncated those responses at 1 or 10 MiB are removed: the transport cap is now the single enforcement point, and it reports oversize bodies instead of truncating them. The /key fetch over plain TLS switches from io.LimitReader to httpbody.LimitSizeTo, so an oversized response reports the problem instead of producing a confusing truncated-JSON error. Thanks to Ben Carman for the report! Updates tailscale/corp#48187 Reported-by: Ben Carman Change-Id: Ibf95e1ab9e4f0d7ef8866e8c26e62ed2a514455a Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
370 lines
9.5 KiB
Go
370 lines
9.5 KiB
Go
// Copyright (c) Tailscale Inc & contributors
|
|
// SPDX-License-Identifier: BSD-3-Clause
|
|
|
|
package controlclient
|
|
|
|
import (
|
|
"bytes"
|
|
"encoding/binary"
|
|
"encoding/json"
|
|
"errors"
|
|
"math"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"net/netip"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/klauspost/compress/zstd"
|
|
"tailscale.com/hostinfo"
|
|
"tailscale.com/ipn/ipnstate"
|
|
"tailscale.com/net/netmon"
|
|
"tailscale.com/net/tsdial"
|
|
"tailscale.com/tailcfg"
|
|
"tailscale.com/types/key"
|
|
"tailscale.com/util/eventbus/eventbustest"
|
|
)
|
|
|
|
func TestSetDiscoPublicKey(t *testing.T) {
|
|
initialKey := key.NewDisco().Public()
|
|
|
|
c := &Direct{
|
|
discoPubKey: initialKey,
|
|
}
|
|
|
|
c.mu.Lock()
|
|
if c.discoPubKey != initialKey {
|
|
t.Fatalf("initial disco key mismatch: got %v, want %v", c.discoPubKey, initialKey)
|
|
}
|
|
c.mu.Unlock()
|
|
|
|
newKey := key.NewDisco().Public()
|
|
c.SetDiscoPublicKey(newKey)
|
|
|
|
c.mu.Lock()
|
|
if c.discoPubKey != newKey {
|
|
t.Fatalf("disco key not updated: got %v, want %v", c.discoPubKey, newKey)
|
|
}
|
|
if c.discoPubKey == initialKey {
|
|
t.Fatal("disco key should have changed")
|
|
}
|
|
c.mu.Unlock()
|
|
}
|
|
|
|
func TestNewDirect(t *testing.T) {
|
|
hi := hostinfo.New()
|
|
ni := tailcfg.NetInfo{LinkType: "wired"}
|
|
hi.NetInfo = &ni
|
|
bus := eventbustest.NewBus(t)
|
|
|
|
k := key.NewMachine()
|
|
dialer := tsdial.NewDialer(netmon.NewStatic())
|
|
dialer.SetBus(bus)
|
|
opts := Options{
|
|
ServerURL: "https://example.com",
|
|
Hostinfo: hi,
|
|
GetMachinePrivateKey: func() (key.MachinePrivate, error) {
|
|
return k, nil
|
|
},
|
|
Dialer: dialer,
|
|
Bus: bus,
|
|
}
|
|
c, err := NewDirect(opts)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
if c.serverURL != opts.ServerURL {
|
|
t.Errorf("c.serverURL got %v want %v", c.serverURL, opts.ServerURL)
|
|
}
|
|
|
|
// hi is stored without its NetInfo field.
|
|
hiWithoutNi := *hi
|
|
hiWithoutNi.NetInfo = nil
|
|
if !hiWithoutNi.Equal(c.hostinfo) {
|
|
t.Errorf("c.hostinfo got %v want %v", c.hostinfo, hi)
|
|
}
|
|
|
|
changed := c.SetNetInfo(&ni)
|
|
if changed {
|
|
t.Errorf("c.SetNetInfo(ni) want false got %v", changed)
|
|
}
|
|
ni = tailcfg.NetInfo{LinkType: "wifi"}
|
|
changed = c.SetNetInfo(&ni)
|
|
if !changed {
|
|
t.Errorf("c.SetNetInfo(ni) want true got %v", changed)
|
|
}
|
|
|
|
changed = c.SetHostinfo(hi)
|
|
if changed {
|
|
t.Errorf("c.SetHostinfo(hi) want false got %v", changed)
|
|
}
|
|
hi = hostinfo.New()
|
|
hi.Hostname = "different host name"
|
|
changed = c.SetHostinfo(hi)
|
|
if !changed {
|
|
t.Errorf("c.SetHostinfo(hi) want true got %v", changed)
|
|
}
|
|
|
|
endpoints := fakeEndpoints(1, 2, 3)
|
|
changed = c.newEndpoints(endpoints)
|
|
if !changed {
|
|
t.Errorf("c.newEndpoints want true got %v", changed)
|
|
}
|
|
changed = c.newEndpoints(endpoints)
|
|
if changed {
|
|
t.Errorf("c.newEndpoints want false got %v", changed)
|
|
}
|
|
endpoints = fakeEndpoints(4, 5, 6)
|
|
changed = c.newEndpoints(endpoints)
|
|
if !changed {
|
|
t.Errorf("c.newEndpoints want true got %v", changed)
|
|
}
|
|
}
|
|
|
|
func fakeEndpoints(ports ...uint16) (ret []tailcfg.Endpoint) {
|
|
for _, port := range ports {
|
|
ret = append(ret, tailcfg.Endpoint{
|
|
Addr: netip.AddrPortFrom(netip.Addr{}, port),
|
|
})
|
|
}
|
|
return
|
|
}
|
|
|
|
func TestParseRateLimitError(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
statusCode int
|
|
body string
|
|
retryAfter string // Retry-After header value
|
|
wantMsg string
|
|
wantMin time.Duration // minimum expected retryAfter
|
|
wantMax time.Duration // maximum expected retryAfter
|
|
}{
|
|
{
|
|
name: "retry-after-seconds",
|
|
statusCode: 429,
|
|
body: "too many requests",
|
|
retryAfter: "30",
|
|
wantMsg: "too many requests",
|
|
wantMin: 30 * time.Second,
|
|
wantMax: 30 * time.Second,
|
|
},
|
|
{
|
|
name: "no-retry-after-header",
|
|
statusCode: 429,
|
|
body: "slow down",
|
|
retryAfter: "",
|
|
wantMsg: "slow down",
|
|
wantMin: 5 * time.Second,
|
|
wantMax: 10 * time.Second,
|
|
},
|
|
{
|
|
name: "unparseable-retry-after",
|
|
statusCode: 429,
|
|
body: "rate limited",
|
|
retryAfter: "not-a-number",
|
|
wantMsg: "rate limited",
|
|
wantMin: 5 * time.Second,
|
|
wantMax: 10 * time.Second,
|
|
},
|
|
{
|
|
name: "empty-body",
|
|
statusCode: 429,
|
|
body: "",
|
|
retryAfter: "5",
|
|
wantMsg: "",
|
|
wantMin: 5 * time.Second,
|
|
wantMax: 5 * time.Second,
|
|
},
|
|
{
|
|
name: "body-with-whitespace",
|
|
statusCode: 429,
|
|
body: " too many requests \n",
|
|
retryAfter: "10",
|
|
wantMsg: "too many requests",
|
|
wantMin: 10 * time.Second,
|
|
wantMax: 10 * time.Second,
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
rec := httptest.NewRecorder()
|
|
if tt.retryAfter != "" {
|
|
rec.Header().Set("Retry-After", tt.retryAfter)
|
|
}
|
|
rec.WriteHeader(tt.statusCode)
|
|
rec.Body.WriteString(tt.body)
|
|
res := rec.Result()
|
|
|
|
err := parseRateLimitError(res)
|
|
if err == nil {
|
|
t.Fatal("expected non-nil error")
|
|
}
|
|
|
|
var rle *rateLimitError
|
|
if !errors.As(err, &rle) {
|
|
t.Fatalf("error is not a *rateLimitError: %T", err)
|
|
}
|
|
if rle.msg != tt.wantMsg {
|
|
t.Errorf("msg = %q, want %q", rle.msg, tt.wantMsg)
|
|
}
|
|
if rle.retryAfter < tt.wantMin || rle.retryAfter > tt.wantMax {
|
|
t.Errorf("retryAfter = %v, want between %v and %v", rle.retryAfter, tt.wantMin, tt.wantMax)
|
|
}
|
|
|
|
// Verify the Error() string contains useful information.
|
|
errStr := err.Error()
|
|
if !strings.Contains(errStr, "rate limited") {
|
|
t.Errorf("Error() = %q, want it to contain 'rate limited'", errStr)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestIsRateLimitedResponse(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
statusCode int
|
|
retryAfter string
|
|
want bool
|
|
}{
|
|
{name: "429-no-header", statusCode: 429, want: true},
|
|
{name: "429-with-header", statusCode: 429, retryAfter: "30", want: true},
|
|
{name: "503-with-header", statusCode: 503, retryAfter: "30", want: true},
|
|
{name: "503-no-header", statusCode: 503, want: false},
|
|
{name: "500-with-header", statusCode: 500, retryAfter: "30", want: false},
|
|
{name: "200", statusCode: 200, want: false},
|
|
}
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
rec := httptest.NewRecorder()
|
|
if tt.retryAfter != "" {
|
|
rec.Header().Set("Retry-After", tt.retryAfter)
|
|
}
|
|
rec.WriteHeader(tt.statusCode)
|
|
|
|
if got := isRateLimitedResponse(rec.Result()); got != tt.want {
|
|
t.Errorf("shouldHonorRetryAfter; got %v, want %v", got, tt.want)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestRateLimitErrorIsError(t *testing.T) {
|
|
err := &rateLimitError{msg: "test", retryAfter: 5 * time.Second}
|
|
var target *rateLimitError
|
|
if !errors.As(err, &target) {
|
|
t.Fatal("errors.As should match *rateLimitError")
|
|
}
|
|
if target.retryAfter != 5*time.Second {
|
|
t.Errorf("retryAfter = %v, want 5s", target.retryAfter)
|
|
}
|
|
}
|
|
|
|
func TestTsmpPing(t *testing.T) {
|
|
hi := hostinfo.New()
|
|
ni := tailcfg.NetInfo{LinkType: "wired"}
|
|
hi.NetInfo = &ni
|
|
bus := eventbustest.NewBus(t)
|
|
|
|
k := key.NewMachine()
|
|
dialer := tsdial.NewDialer(netmon.NewStatic())
|
|
dialer.SetBus(bus)
|
|
opts := Options{
|
|
ServerURL: "https://example.com",
|
|
Hostinfo: hi,
|
|
GetMachinePrivateKey: func() (key.MachinePrivate, error) {
|
|
return k, nil
|
|
},
|
|
Dialer: dialer,
|
|
Bus: bus,
|
|
}
|
|
|
|
c, err := NewDirect(opts)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
|
|
pingRes := &tailcfg.PingResponse{
|
|
Type: "TSMP",
|
|
IP: "123.456.7890",
|
|
Err: "",
|
|
NodeName: "testnode",
|
|
}
|
|
|
|
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
defer r.Body.Close()
|
|
body := new(ipnstate.PingResult)
|
|
if err := json.NewDecoder(r.Body).Decode(body); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if pingRes.IP != body.IP {
|
|
t.Fatalf("PingResult did not have the correct IP : got %v, expected : %v", body.IP, pingRes.IP)
|
|
}
|
|
w.WriteHeader(200)
|
|
}))
|
|
defer ts.Close()
|
|
|
|
now := time.Now()
|
|
|
|
pr := &tailcfg.PingRequest{
|
|
URL: ts.URL,
|
|
}
|
|
|
|
err = postPingResult(now, t.Logf, c.httpc, pr, pingRes)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
}
|
|
|
|
func TestReadMapResponseMessage(t *testing.T) {
|
|
// Normal messages round-trip.
|
|
var buf bytes.Buffer
|
|
var siz [4]byte
|
|
binary.LittleEndian.PutUint32(siz[:], 4)
|
|
buf.Write(siz[:])
|
|
buf.WriteString("body")
|
|
msg, err := readMapResponseMessage(&buf, nil)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if string(msg) != "body" {
|
|
t.Fatalf("got message %q, want %q", msg, "body")
|
|
}
|
|
|
|
// The size prefix is a uint32 chosen by the control server. A
|
|
// malicious server must not be able to make us allocate up to 4 GiB
|
|
// before any body bytes are read.
|
|
buf.Reset()
|
|
binary.LittleEndian.PutUint32(siz[:], math.MaxUint32)
|
|
buf.Write(siz[:])
|
|
if _, err := readMapResponseMessage(&buf, msg); err == nil || !strings.Contains(err.Error(), "exceeds max") {
|
|
t.Fatalf("readMapResponseMessage = %v, want size cap error", err)
|
|
}
|
|
}
|
|
|
|
func TestDecodeMsgMaxDecodedSize(t *testing.T) {
|
|
// A zstd frame whose header declares more decoded content than
|
|
// maxDecodedMapResponseSize. The decoder rejects such a frame before
|
|
// decoding any block, so a malicious control server can't make us
|
|
// expand a small frame into an unbounded amount of JSON, and the test
|
|
// doesn't need to allocate the decoded bytes either.
|
|
oversized := []byte{
|
|
0x28, 0xb5, 0x2f, 0xfd, // zstd frame magic
|
|
0xc0, // 8-byte frame content size, no single segment, no checksum, no dict ID
|
|
0x00, // window descriptor: 1 KiB window
|
|
0x01, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, // declared content size: 16 GiB + 1
|
|
0x21, 0x00, 0x00, // block header: last block, raw block, 4 bytes
|
|
'b', 'o', 'm', 'b',
|
|
}
|
|
ms := newMapSession(key.NewNode(), nil, nil)
|
|
var resp tailcfg.MapResponse
|
|
err := ms.decodeMsg(oversized, &resp)
|
|
if !errors.Is(err, zstd.ErrDecoderSizeExceeded) {
|
|
t.Fatalf("decodeMsg(oversized frame) = %v, want zstd.ErrDecoderSizeExceeded", err)
|
|
}
|
|
}
|