fix(verifier): bound how long RemoteKeys serves a stale key when the JWKS endpoint is down

verifier/remote.go

@@ -38,6 +38,14 @@ type RemoteKeys struct {
 	// load onto the signer. Default 30 seconds.
 	MinRefetch time.Duration
 
+	// MaxStale is an absolute ceiling on how long a cached key set is served
+	// while the JWKS endpoint is unreachable. Within TTL a set is served
+	// freely; past TTL it is served only until it reaches MaxStale, then
+	// refused — so a rotated or revoked key stops being trusted even if the
+	// endpoint stays down. It must not exceed the signer's key overlap window.
+	// Default 1 hour.
+	MaxStale time.Duration
+
 	// Now replaces the time source. For tests.
 	Now func() time.Time
 
@@ -56,6 +64,10 @@ func (r *RemoteKeys) Key(ctx context.Context, kid string) (*ecdsa.PublicKey, err
 	if ttl <= 0 {
 		ttl = 5 * time.Minute
 	}
+	maxStale := r.MaxStale
+	if maxStale <= 0 {
+		maxStale = time.Hour
+	}
 	now := r.now()
 
 	r.mu.RLock()
@@ -69,7 +81,19 @@ func (r *RemoteKeys) Key(ctx context.Context, kid string) (*ecdsa.PublicKey, err
 		// The key is known but the set is stale. Serve it and let the
 		// refresh below happen; a known kid is not a rotation signal.
 		if err := r.refresh(ctx, false); err != nil {
-			return k, nil // stale but usable beats failing closed here
+			// A failed refresh does not advance fetchedAt, so serve the cached
+			// key only until it reaches the absolute staleness ceiling. Past
+			// it, refuse even though the endpoint is down: a rotated or revoked
+			// key must not stay trusted indefinitely while the signer is
+			// unreachable.
+			r.mu.RLock()
+			age := r.now().Sub(r.fetchedAt)
+			r.mu.RUnlock()
+			if age <= maxStale {
+				return k, nil // stale but within the ceiling
+			}
+			return nil, fmt.Errorf("%w: cached key set is %s old (ceiling %s) and %s is unreachable: %v",
+				tsjwt.ErrNoKey, age.Round(time.Second), maxStale, r.URL, err)
 		}
 		r.mu.RLock()
 		defer r.mu.RUnlock()

verifier/remote_test.go

@@ -0,0 +1,102 @@
+package verifier_test
+
+import (
+	"context"
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	"crypto/rand"
+	"crypto/sha256"
+	"encoding/base64"
+	"encoding/json"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/bitcomplete/tsjwt"
+	"github.com/bitcomplete/tsjwt/verifier"
+)
+
+// A cached remote key must stop being trusted once it exceeds the absolute
+// staleness ceiling, even while the JWKS endpoint keeps erroring — otherwise a
+// rotated or revoked key stays valid for as long as the signer is unreachable.
+//
+// Regression for the RemoteKeys indefinite-stale-serve gap.
+func TestRemoteKeysStalenessCeiling(t *testing.T) {
+	t.Parallel()
+	key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	enc := func(b []byte) string {
+		buf := make([]byte, 32)
+		copy(buf[32-len(b):], b)
+		return base64.RawURLEncoding.EncodeToString(buf)
+	}
+	// The kid is the RFC 7638 thumbprint (computed inline so this test does not
+	// depend on an unexported/other-branch helper, and stays valid once the
+	// verifier enforces kid == thumbprint).
+	canon, _ := json.Marshal(struct {
+		Crv string `json:"crv"`
+		Kty string `json:"kty"`
+		X   string `json:"x"`
+		Y   string `json:"y"`
+	}{"P-256", "EC", enc(key.PublicKey.X.Bytes()), enc(key.PublicKey.Y.Bytes())})
+	sum := sha256.Sum256(canon)
+	kid := base64.RawURLEncoding.EncodeToString(sum[:])
+
+	jwks, _ := json.Marshal(map[string]any{"keys": []map[string]string{{
+		"kty": "EC", "crv": "P-256", "kid": kid, "use": "sig", "alg": "ES256",
+		"x": enc(key.PublicKey.X.Bytes()), "y": enc(key.PublicKey.Y.Bytes()),
+	}}})
+
+	var up atomic.Bool
+	up.Store(true)
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		if !up.Load() {
+			http.Error(w, "down", http.StatusInternalServerError)
+			return
+		}
+		w.Header().Set("Content-Type", "application/jwk-set+json")
+		_, _ = w.Write(jwks)
+	}))
+	defer srv.Close()
+
+	t0 := time.Unix(1_000_000, 0)
+	now := t0
+	rk := &verifier.RemoteKeys{
+		URL: srv.URL, TTL: time.Minute, MaxStale: time.Hour,
+		Now: func() time.Time { return now },
+	}
+	ctx := context.Background()
+
+	// Initial fetch succeeds and caches the key.
+	if _, err := rk.Key(ctx, kid); err != nil {
+		t.Fatalf("initial fetch: %v", err)
+	}
+
+	// The endpoint goes down.
+	up.Store(false)
+
+	// Past TTL but within MaxStale: the stale key is still served.
+	now = t0.Add(30 * time.Minute)
+	if _, err := rk.Key(ctx, kid); err != nil {
+		t.Fatalf("within the staleness ceiling the stale key should still serve: %v", err)
+	}
+
+	// Past MaxStale with the endpoint still down: the key is refused.
+	now = t0.Add(2 * time.Hour)
+	if _, err := rk.Key(ctx, kid); err == nil {
+		t.Fatal("a cached key past MaxStale was still served while the endpoint was down")
+	} else if !errors.Is(err, tsjwt.ErrNoKey) {
+		t.Errorf("want ErrNoKey, got %v", err)
+	}
+
+	// The endpoint recovers: fetching works again and the ceiling resets.
+	up.Store(true)
+	if _, err := rk.Key(ctx, kid); err != nil {
+		t.Fatalf("after recovery: %v", err)
+	}
+}