diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 5d623cf..b3a2a6b 100644 --- a/.github/workflows/ci.yaml +++ b/.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 diff --git a/claims_fuzz_test.go b/claims_fuzz_test.go new file mode 100644 index 0000000..8c18ec9 --- /dev/null +++ b/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) + }) +} diff --git a/keys/fuzz_test.go b/keys/fuzz_test.go new file mode 100644 index 0000000..e7a5a13 --- /dev/null +++ b/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) + }) +} diff --git a/routetable/fuzz_test.go b/routetable/fuzz_test.go new file mode 100644 index 0000000..1997213 --- /dev/null +++ b/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() + }) +} diff --git a/tsnetid/remoteaddr_test.go b/tsnetid/remoteaddr_test.go new file mode 100644 index 0000000..f5676ea --- /dev/null +++ b/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) + } + } +} diff --git a/tsnetid/tsnetid.go b/tsnetid/tsnetid.go index 2a7348f..aba96fb 100644 --- a/tsnetid/tsnetid.go +++ b/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 diff --git a/verifier/fuzz_test.go b/verifier/fuzz_test.go new file mode 100644 index 0000000..5b2e8d2 --- /dev/null +++ b/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) + } + }) +}