Land the remaining audit-fix stack on main (#7,#9,#10,#11,#12)

.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)
+	})
+}

gateway/perlistener_test.go

@@ -0,0 +1,66 @@
+package gateway_test
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/bitcomplete/tsjwt/gateway"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	gwapi "sigs.k8s.io/gateway-api/apis/v1"
+)
+
+// allowedRoutes must be evaluated per matching listener, not per Gateway. On a
+// Gateway with an open listener (From=All) and a strict one (From=Same) that own
+// different hostnames, a route from another namespace is admitted only by the
+// open listener and so may serve only the open listener's hostname — it must not
+// borrow the strict listener's hostname.
+//
+// Regression for adversarial finding B (per-listener allowedRoutes).
+func TestAllowedRoutesEvaluatedPerListener(t *testing.T) {
+	t.Parallel()
+	all := gwapi.NamespacesFromAll
+	same := gwapi.NamespacesFromSame
+	g := &gwapi.Gateway{ObjectMeta: metav1.ObjectMeta{Name: "edge", Namespace: "infra"}}
+	g.Spec.Listeners = []gwapi.Listener{
+		{
+			Name: "open", Port: 443, Protocol: gwapi.HTTPSProtocolType,
+			Hostname:      ptr(gwapi.Hostname("open.example")),
+			AllowedRoutes: &gwapi.AllowedRoutes{Namespaces: &gwapi.RouteNamespaces{From: &all}},
+		},
+		{
+			Name: "strict", Port: 443, Protocol: gwapi.HTTPSProtocolType,
+			Hostname:      ptr(gwapi.Hostname("strict.example")),
+			AllowedRoutes: &gwapi.AllowedRoutes{Namespaces: &gwapi.RouteNamespaces{From: &same}},
+		},
+	}
+
+	// A foreign-namespace route may attach (the open listener is From=All), but
+	// requesting the strict listener's hostname must not be served.
+	foreignStrict := route("gstrict", "other", "edge", []string{"strict.example"}, rule("guest-svc", 80, "/"))
+	foreignStrict.Spec.ParentRefs[0].Namespace = ptr(gwapi.Namespace("infra"))
+	foreignOpen := route("gopen", "other", "edge", []string{"open.example"}, rule("guest-svc", 80, "/"))
+	foreignOpen.Spec.ParentRefs[0].Namespace = ptr(gwapi.Namespace("infra"))
+	localStrict := route("lstrict", "infra", "edge", []string{"strict.example"}, rule("app-svc", 80, "/"))
+
+	svcs := []corev1.Service{svc("guest-svc", "other", 80), svc("app-svc", "infra", 80)}
+	got, _ := gateway.Translate(g, []gwapi.HTTPRoute{foreignStrict, foreignOpen, localStrict}, svcs)
+
+	var openServed, localServed bool
+	for _, r := range got {
+		switch {
+		case strings.HasPrefix(r.Name, "other/gstrict"):
+			t.Errorf("foreign-namespace route served strict.example, which only a From=Same listener owns: %s", r.Name)
+		case strings.HasPrefix(r.Name, "other/gopen"):
+			openServed = true
+		case strings.HasPrefix(r.Name, "infra/lstrict"):
+			localServed = true
+		}
+	}
+	if !openServed {
+		t.Error("foreign route on the From=All listener's hostname should be served")
+	}
+	if !localServed {
+		t.Error("same-namespace route on the From=Same listener's hostname should be served")
+	}
+}

gateway/reconcile.go

@@ -179,6 +179,9 @@ func (r *GatewayReconciler) render(ctx context.Context, gw *gwapi.Gateway) (stri
 	}
 
 	table, errs := Translate(gw, routes.Items, services.Items)
+	// Report acceptance back on each attached route's status before rendering
+	// the table, so a rejected route is visible to its author, not only logged.
+	r.applyRouteStatus(ctx, gw, routes.Items, errs)
 	// The generation is a usable revision: it changes when the Gateway
 	// does, and the data plane only uses it for logs and readiness.
 	file := routetable.Render(gw.Namespace+"/"+gw.Name, gw.Generation, table)
@@ -314,4 +317,61 @@ func (conditionHelpers) setCondition(conds *[]metav1.Condition, c metav1.Conditi
 	*conds = append(*conds, c)
 }
 
+// applyRouteStatus writes this controller's Accepted condition onto every route
+// attached to gw, so an author learns from the route's own status whether it
+// was accepted or why it was rejected — not only from the controller log. A
+// status write that fails is logged, never fatal to the reconcile.
+func (r *GatewayReconciler) applyRouteStatus(ctx context.Context, gw *gwapi.Gateway, routes []gwapi.HTTPRoute, errs []RouteError) {
+	l := log.FromContext(ctx)
+	ref := parentRefToGateway(gw)
+	for i := range routes {
+		route := &routes[i]
+		if !attachedTo(gw, route) {
+			continue
+		}
+		if !upsertRouteParentCondition(route, ref, routeAcceptedCondition(route, errs)) {
+			continue // unchanged; do not churn the status
+		}
+		if err := r.Status().Update(ctx, route); err != nil {
+			l.Error(err, "update HTTPRoute status", "route", route.Namespace+"/"+route.Name)
+		}
+	}
+}
+
+// upsertRouteParentCondition merges cond into the RouteParentStatus for this
+// controller and ref, and reports whether anything changed.
+func upsertRouteParentCondition(route *gwapi.HTTPRoute, ref gwapi.ParentReference, cond metav1.Condition) bool {
+	for i := range route.Status.Parents {
+		p := &route.Status.Parents[i]
+		if string(p.ControllerName) != ControllerName || !parentRefEqual(p.ParentRef, ref) {
+			continue
+		}
+		for j := range p.Conditions {
+			c := p.Conditions[j]
+			if c.Type == cond.Type {
+				if c.Status == cond.Status && c.Reason == cond.Reason &&
+					c.Message == cond.Message && c.ObservedGeneration == cond.ObservedGeneration {
+					return false // identical; leave it, and its transition time, alone
+				}
+				break
+			}
+		}
+		meta.setCondition(&p.Conditions, cond)
+		return true
+	}
+	route.Status.Parents = append(route.Status.Parents, gwapi.RouteParentStatus{
+		ParentRef:      ref,
+		ControllerName: gwapi.GatewayController(ControllerName),
+		Conditions:     []metav1.Condition{cond},
+	})
+	return true
+}
+
+func parentRefEqual(a, b gwapi.ParentReference) bool {
+	name := a.Name == b.Name
+	ns := (a.Namespace == nil && b.Namespace == nil) ||
+		(a.Namespace != nil && b.Namespace != nil && *a.Namespace == *b.Namespace)
+	return name && ns
+}
+
 var _ = apierrors.IsNotFound

gateway/reconcile_test.go

@@ -51,7 +51,7 @@ func newReconciler(t *testing.T, objs ...client.Object) (*gateway.GatewayReconci
 	c := fake.NewClientBuilder().
 		WithScheme(reconcileScheme(t)).
 		WithObjects(objs...).
-		WithStatusSubresource(&gwapi.Gateway{}, &gwapi.GatewayClass{}).
+		WithStatusSubresource(&gwapi.Gateway{}, &gwapi.GatewayClass{}, &gwapi.HTTPRoute{}).
 		Build()
 	return &gateway.GatewayReconciler{
 		Client: c,

gateway/routes.go

@@ -307,12 +307,11 @@ func attachedTo(gw *gwapi.Gateway, r *gwapi.HTTPRoute) bool {
 			ns = string(*p.Namespace)
 		}
 		if string(p.Name) == gw.Name && ns == gw.Namespace {
-			// Naming the Gateway is necessary but not sufficient: the
-			// Gateway's listeners decide which namespaces may attach, via
-			// allowedRoutes.namespaces (Gateway API, default From=Same).
-			// Without this check a route in any namespace could bind to a
-			// Gateway it does not own and shadow another tenant's routes.
-			return listenerAllowsNamespace(gw, r.Namespace)
+			// Naming the Gateway is necessary but not sufficient: at least one
+			// listener's allowedRoutes.namespaces must admit the route's
+			// namespace (Gateway API, default From=Same). hostnamesFor then
+			// restricts what the route may serve to those same listeners.
+			return len(admittingListeners(gw, r.Namespace)) > 0
 		}
 	}
 	return false
@@ -323,25 +322,37 @@ func attachedTo(gw *gwapi.Gateway, r *gwapi.HTTPRoute) bool {
 // when unset is From=Same (only the Gateway's own namespace). From=Selector
 // needs the route namespace's labels, which this controller does not read, so
 // it fails closed until that lookup is wired in.
-func listenerAllowsNamespace(gw *gwapi.Gateway, routeNS string) bool {
+func listenerAdmits(l gwapi.Listener, gwNamespace, routeNS string) bool {
+	from := gwapi.NamespacesFromSame
+	if l.AllowedRoutes != nil && l.AllowedRoutes.Namespaces != nil && l.AllowedRoutes.Namespaces.From != nil {
+		from = *l.AllowedRoutes.Namespaces.From
+	}
+	switch from {
+	case gwapi.NamespacesFromAll:
+		return true
+	case gwapi.NamespacesFromSame:
+		return routeNS == gwNamespace
+	default:
+		// Selector needs the route namespace's labels, which this controller
+		// does not list; None admits nothing. Fail closed rather than guess.
+		return false
+	}
+}
+
+// admittingListeners returns the Gateway's listeners whose allowedRoutes admit a
+// route from routeNS. A route may attach through, and — crucially — may serve
+// the hostnames of, only these listeners. Evaluating this per listener rather
+// than per Gateway is what stops a route admitted by a permissive listener
+// (From=All) from serving a hostname that only a stricter listener (From=Same)
+// owns.
+func admittingListeners(gw *gwapi.Gateway, routeNS string) []gwapi.Listener {
+	var out []gwapi.Listener
 	for _, l := range gw.Spec.Listeners {
-		from := gwapi.NamespacesFromSame
-		if l.AllowedRoutes != nil && l.AllowedRoutes.Namespaces != nil && l.AllowedRoutes.Namespaces.From != nil {
-			from = *l.AllowedRoutes.Namespaces.From
-		}
-		switch from {
-		case gwapi.NamespacesFromAll:
-			return true
-		case gwapi.NamespacesFromSame:
-			if routeNS == gw.Namespace {
-				return true
-			}
-		case gwapi.NamespacesFromSelector:
-			// Selector needs the route namespace's labels, which this
-			// controller does not list; fail closed rather than guess.
+		if listenerAdmits(l, gw.Namespace, routeNS) {
+			out = append(out, l)
 		}
 	}
-	return false
+	return out
 }
 
 // hostnamesFor intersects the route's hostnames with the Gateway's listeners,
@@ -353,8 +364,11 @@ func listenerAllowsNamespace(gw *gwapi.Gateway, routeNS string) bool {
 // True with an empty list means neither side named any, so the route answers
 // for whatever reaches the Gateway.
 func hostnamesFor(gw *gwapi.Gateway, r *gwapi.HTTPRoute) ([]string, bool) {
+	// Only listeners that admit this route's namespace count. A route admitted
+	// through a From=All listener must not borrow the hostname of a From=Same
+	// listener that would not have admitted it.
 	var listener []string
-	for _, l := range gw.Spec.Listeners {
+	for _, l := range admittingListeners(gw, r.Namespace) {
 		if l.Hostname != nil && *l.Hostname != "" {
 			listener = append(listener, string(*l.Hostname))
 		}
@@ -432,6 +446,30 @@ func dedupe(in []string) []string {
 	return out
 }
 
+// parentRefToGateway builds a ParentReference identifying gw, for a route's
+// parent status.
+func parentRefToGateway(gw *gwapi.Gateway) gwapi.ParentReference {
+	group := gwapi.Group("gateway.networking.k8s.io")
+	kind := gwapi.Kind("Gateway")
+	ns := gwapi.Namespace(gw.Namespace)
+	return gwapi.ParentReference{Group: &group, Kind: &kind, Namespace: &ns, Name: gwapi.ObjectName(gw.Name)}
+}
+
+// routeAcceptedCondition reports this controller's Accepted condition for a
+// route attached to gw, given the translation errors for the whole table: True
+// when every rule translated, or False with the first rejection's reason. It
+// lets a route author see acceptance (or why not) on the route's own status,
+// as Gateway API expects, rather than only in the controller log.
+func routeAcceptedCondition(route *gwapi.HTTPRoute, errs []RouteError) metav1.Condition {
+	key := route.Namespace + "/" + route.Name
+	for _, e := range errs {
+		if e.Route == key || strings.HasPrefix(e.Route, key+"#") {
+			return routeStatusCondition(route.Generation, false, e.Reason, e.Detail)
+		}
+	}
+	return routeStatusCondition(route.Generation, true, gwapi.RouteReasonAccepted, "accepted by tsjwt")
+}
+
 // routeStatusCondition builds the Accepted condition for a route.
 func routeStatusCondition(gen int64, accepted bool, reason gwapi.RouteConditionReason, msg string) metav1.Condition {
 	status := metav1.ConditionTrue

gateway/routestatus_test.go

@@ -0,0 +1,64 @@
+package gateway_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/bitcomplete/tsjwt/gateway"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	ctrl "sigs.k8s.io/controller-runtime"
+	gwapi "sigs.k8s.io/gateway-api/apis/v1"
+)
+
+// The controller must report acceptance on each attached route's own status —
+// Accepted=True for a route it serves, Accepted=False with the reason for one
+// it rejects — so an author sees it without reading controller logs.
+//
+// Regression for the missing per-HTTPRoute status conditions.
+func TestReconcileWritesRouteStatus(t *testing.T) {
+	t.Parallel()
+	gc := gatewayClass("tsjwt", gateway.ControllerName)
+	g := gatewayWithClass("edge", "infra", "tsjwt", "app.ts.net")
+
+	good := route("good", "infra", "edge", []string{"app.ts.net"}, rule("app-svc", 80, "/"))
+	// A cross-namespace backendRef is rejected with RefNotPermitted.
+	bad := route("bad", "infra", "edge", []string{"app.ts.net"}, rule("payments", 8080, "/x"))
+	bad.Spec.Rules[0].BackendRefs[0].Namespace = ptr(gwapi.Namespace("other"))
+	s := svc("app-svc", "infra", 80)
+
+	r, c := newReconciler(t, gc, g, &good, &bad, &s)
+	ctx := context.Background()
+	if _, err := r.Reconcile(ctx, ctrl.Request{
+		NamespacedName: types.NamespacedName{Namespace: "infra", Name: "edge"}}); err != nil {
+		t.Fatalf("reconcile: %v", err)
+	}
+
+	accepted := func(name string) metav1.Condition {
+		t.Helper()
+		var hr gwapi.HTTPRoute
+		if err := c.Get(ctx, types.NamespacedName{Namespace: "infra", Name: name}, &hr); err != nil {
+			t.Fatalf("get route %s: %v", name, err)
+		}
+		for _, p := range hr.Status.Parents {
+			if string(p.ControllerName) != gateway.ControllerName {
+				continue
+			}
+			for _, cnd := range p.Conditions {
+				if cnd.Type == string(gwapi.RouteConditionAccepted) {
+					return cnd
+				}
+			}
+		}
+		t.Fatalf("route %s has no Accepted condition from this controller", name)
+		return metav1.Condition{}
+	}
+
+	if g := accepted("good"); g.Status != metav1.ConditionTrue {
+		t.Errorf("good route: Accepted=%s, want True", g.Status)
+	}
+	if b := accepted("bad"); b.Status != metav1.ConditionFalse ||
+		b.Reason != string(gwapi.RouteReasonRefNotPermitted) {
+		t.Errorf("bad route: Accepted=%s reason=%q, want False/RefNotPermitted", b.Status, b.Reason)
+	}
+}