test(fuzz): fuzz the claims, key-store, route-file, and JWKS parsers

.github/workflows/ci.yaml

@@ -40,6 +40,17 @@ jobs:
       - name: fuzz the token parser
         run: go test ./jwt/ -run '^$' -fuzz FuzzParseAndVerify -fuzztime 30s
 
+      # The token parser is the first surface, but every parser that reads
+      # attacker- or peer-controlled bytes must also never panic: the claim set,
+      # the shared key store, the rendered route file, and a fetched JWKS. A
+      # short live fuzz on each catches a regression a seed corpus would not.
+      - name: fuzz the parsers
+        run: |
+          go test ./ -run '^$' -fuzz '^FuzzClaimsUnmarshal$' -fuzztime 20s
+          go test ./keys/ -run '^$' -fuzz '^FuzzUnmarshalEntries$' -fuzztime 20s
+          go test ./routetable/ -run '^$' -fuzz '^FuzzRouteFileParse$' -fuzztime 20s
+          go test ./verifier/ -run '^$' -fuzz '^FuzzJWKSToPublic$' -fuzztime 20s
+
       # The core must never grow a dependency. A backend that only verifies
       # tokens takes on nothing, and that is a property worth enforcing
       # rather than documenting.
@@ -78,7 +89,11 @@ jobs:
       - name: no tailnet addresses
         run: |
           set -euo pipefail
-          if git grep -nE '\b100\.(6[4-9]|[7-9][0-9]|1[01][0-9]|12[0-7])\.[0-9]{1,3}\.[0-9]{1,3}\b' -- ':!.github/workflows/ci.yaml'; then
+          # tsnetid is excluded: its job is tailnet addressing, so it legitimately
+          # names the CGNAT range and uses address literals in its tests. The check
+          # still guards examples, manifests and docs, where a real node address
+          # (rather than an example hostname) does not belong.
+          if git grep -nE '\b100\.(6[4-9]|[7-9][0-9]|1[01][0-9]|12[0-7])\.[0-9]{1,3}\.[0-9]{1,3}\b' -- ':!.github/workflows/ci.yaml' ':!tsnetid'; then
             echo "::error::a tailnet CGNAT address (the 100.64/10 range) is present; use an example host"
             exit 1
           fi

claims_fuzz_test.go

@@ -0,0 +1,31 @@
+package tsjwt
+
+import (
+	"encoding/json"
+	"testing"
+)
+
+// A token payload is attacker-controlled, and Claims.UnmarshalJSON has custom
+// logic (it collects unknown members into Extra and rejects reserved-name
+// collisions on marshal). Neither decode nor a re-marshal of what decoded may
+// panic on any input.
+func FuzzClaimsUnmarshal(f *testing.F) {
+	for _, s := range []string{
+		`{}`,
+		`{"iss":"x","aud":"y","sub":"s","jti":"j","iat":1,"nbf":0,"exp":2}`,
+		`{"extra":{"a":[1,2,{"b":null}]},"tenant":"t","roles":["a"]}`,
+		`{"aud":123}`, `{"exp":"nope"}`, `{"tenants":null}`,
+		`{"iat":9223372036854775807}`,
+		`[]`, `null`, `"str"`, `{`, ``,
+	} {
+		f.Add([]byte(s))
+	}
+	f.Fuzz(func(t *testing.T, data []byte) {
+		var c Claims
+		if err := json.Unmarshal(data, &c); err != nil {
+			return
+		}
+		// Whatever decoded must also re-marshal without panicking.
+		_, _ = json.Marshal(c)
+	})
+}

keys/fuzz_test.go

@@ -0,0 +1,27 @@
+package keys_test
+
+import (
+	"testing"
+
+	"github.com/bitcomplete/tsjwt/keys"
+)
+
+// UnmarshalEntries parses the shared key store, which a peer replica (or a
+// hostile store writer) controls. It must not panic on any input, and a value
+// it accepts must re-marshal without panicking.
+func FuzzUnmarshalEntries(f *testing.F) {
+	for _, s := range []string{
+		``, `{}`, `{"entries":[]}`,
+		`{"entries":[{"kid":"k","x":"AA","y":"AA","replica":"r"}]}`,
+		`{"entries":null}`, `{"entries":123}`, `[`, `{"entries":[{"lease":"bad"}]}`,
+	} {
+		f.Add([]byte(s))
+	}
+	f.Fuzz(func(t *testing.T, data []byte) {
+		entries, err := keys.UnmarshalEntries(data)
+		if err != nil {
+			return
+		}
+		_, _ = keys.MarshalEntries(entries)
+	})
+}

routetable/fuzz_test.go

@@ -0,0 +1,29 @@
+package routetable_test
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/bitcomplete/tsjwt/routetable"
+)
+
+// The rendered route file is read by the data plane from a mounted ConfigMap.
+// Parsing it — JSON decode plus RouteFile.Parse, which url.Parse's each
+// upstream — must never panic on a malformed or hostile file.
+func FuzzRouteFileParse(f *testing.F) {
+	for _, s := range []string{
+		`{}`, `{"routes":[]}`,
+		`{"revision":1,"gateway":"g","routes":[{"name":"n","upstream":"http://x.svc:80","audience":"a"}]}`,
+		`{"routes":[{"upstream":"://"}]}`, `{"routes":[{"upstream":""}]}`,
+		`{"routes":null}`, `[`, ``,
+	} {
+		f.Add([]byte(s))
+	}
+	f.Fuzz(func(t *testing.T, data []byte) {
+		var rf routetable.RouteFile
+		if err := json.Unmarshal(data, &rf); err != nil {
+			return
+		}
+		_, _ = rf.Parse()
+	})
+}

tsnetid/remoteaddr_test.go

@@ -0,0 +1,32 @@
+package tsnetid
+
+import "testing"
+
+// Identity is derived from the WireGuard peer, so a request whose source is not
+// a tailnet address did not arrive directly and must be refused. Guards against
+// a fronting L7 proxy or Tailscale Funnel presenting a non-tailnet source.
+func TestIsTailnetAddr(t *testing.T) {
+	cases := []struct {
+		addr string
+		want bool
+	}{
+		{"100.64.0.1:443", true},
+		{"100.100.100.100:9", true},
+		{"100.127.255.255:1", true},       // top of 100.64.0.0/10
+		{"[fd7a:115c:a1e0::1]:443", true}, // Tailscale ULA
+		{"100.128.0.1:1", false},          // just outside 100.64.0.0/10
+		{"10.0.0.5:80", false},            // cluster / private
+		{"172.16.0.1:80", false},          // private
+		{"1.2.3.4:80", false},             // public (e.g. via Funnel)
+		{"127.0.0.1:8080", false},         // loopback
+		{"[::1]:80", false},               // v6 loopback
+		{"[fd00::1]:80", false},           // some other ULA, not Tailscale's
+		{"not-an-addr", false},
+		{"", false},
+	}
+	for _, c := range cases {
+		if got := isTailnetAddr(c.addr); got != c.want {
+			t.Errorf("isTailnetAddr(%q) = %v, want %v", c.addr, got, c.want)
+		}
+	}
+}

tsnetid/tsnetid.go

@@ -15,6 +15,8 @@ package tsnetid
 import (
 	"context"
 	"fmt"
+	"net"
+	"net/netip"
 	"sort"
 	"strings"
 	"time"
@@ -25,6 +27,29 @@ import (
 	"tailscale.com/tailcfg"
 )
 
+// Tailscale addresses every node from CGNAT space (IPv4) and its own ULA range
+// (IPv6). A request whose source is outside both did not arrive from a direct
+// tailnet peer.
+var (
+	tailnetV4 = netip.MustParsePrefix("100.64.0.0/10")
+	tailnetV6 = netip.MustParsePrefix("fd7a:115c:a1e0::/48")
+)
+
+// isTailnetAddr reports whether remoteAddr ("host:port", as in
+// http.Request.RemoteAddr) is a Tailscale node address.
+func isTailnetAddr(remoteAddr string) bool {
+	host := remoteAddr
+	if h, _, err := net.SplitHostPort(remoteAddr); err == nil {
+		host = h
+	}
+	ip, err := netip.ParseAddr(host)
+	if err != nil {
+		return false
+	}
+	ip = ip.Unmap()
+	return tailnetV4.Contains(ip) || tailnetV6.Contains(ip)
+}
+
 // CapGroups is the default capability name read from a peer's grants to
 // discover its groups. An operator declares it in the tailnet policy file.
 //
@@ -76,6 +101,18 @@ func (s *Source) Identify(remoteAddr string) (tsjwt.Identity, error) {
 	if s.Local == nil {
 		return zero, fmt.Errorf("%w: no tailnet local client", tsjwt.ErrNoIdentity)
 	}
+	// Identity is the WireGuard peer, so the source must be a tailnet address.
+	// A non-tailnet source means something terminated the connection in front
+	// of this node — an L7 proxy, or Tailscale Funnel — and this process can no
+	// longer see the real caller. Refuse rather than attribute the request to
+	// whatever fronted it. (This does not catch a fronting node that is itself
+	// on the tailnet; the design's rule that the proxy must be its own node,
+	// not sit behind one, still stands — see SECURITY.md.)
+	if !isTailnetAddr(remoteAddr) {
+		return zero, fmt.Errorf("%w: %s is not a tailnet address; the proxy must receive "+
+			"connections directly from tailnet peers, not from a fronting proxy or Funnel",
+			tsjwt.ErrNoIdentity, remoteAddr)
+	}
 	timeout := s.Timeout
 	if timeout <= 0 {
 		timeout = 5 * time.Second

verifier/fuzz_test.go

@@ -0,0 +1,31 @@
+package verifier
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/bitcomplete/tsjwt/keys"
+)
+
+// A backend fetches a JWKS from a signer it does not fully control; a hostile
+// or broken endpoint can return any bytes. Decoding the set and converting each
+// JWK to a public key must not panic on any input.
+func FuzzJWKSToPublic(f *testing.F) {
+	for _, s := range []string{
+		`{"keys":[]}`,
+		`{"keys":[{"kty":"EC","crv":"P-256","kid":"k","use":"sig","alg":"ES256","x":"AA","y":"AA"}]}`,
+		`{"keys":[{"kty":"RSA"}]}`, `{"keys":[{"kty":"EC","crv":"P-256","x":"!!","y":"!!"}]}`,
+		`{"keys":null}`, `[`, ``,
+	} {
+		f.Add([]byte(s))
+	}
+	f.Fuzz(func(t *testing.T, data []byte) {
+		var set keys.JWKS
+		if err := json.Unmarshal(data, &set); err != nil {
+			return
+		}
+		for _, j := range set.Keys {
+			_, _ = jwkToPublic(j)
+		}
+	})
+}