feat(auth): add DPoP jti replay protection and address review comments

Add a server-side replay cache that rejects reused DPoP proof jti values
within the acceptance window (RFC 9449 §11.1). The check runs as the last
step in validateDPoP so only otherwise-valid proofs populate the cache,
and the cache TTL tracks DPoPSkew since older proofs are already rejected
by the iat freshness check.

Also addresses review nits: move the dpopNonceManager doc comment onto the
correct type and use WarnContext in checkToken.

Signed-off-by: Dave Mihalcik <dmihalcik@virtru.com>

Author: dmihalcik-virtru

service/internal/auth/authn.go

@@ -104,18 +104,22 @@ type Authentication struct {
 	logger *logger.Logger
 	// DPoP nonce management
 	dpopNonceManager *dpopNonceManager
+	// dpopReplayCache rejects replayed DPoP proofs by `jti`
+	dpopReplayCache *dpopReplayCache
 
 	// Used for testing
 	_testCheckTokenFunc func(ctx context.Context, authHeader []string, dpopInfo receiverInfo, dpopHeader []string) (jwt.Token, context.Context, error)
 }
 
-// dpopNonceManager manages server-issued DPoP nonces per RFC 9449 §8
+// nonceState is an immutable snapshot of the active and previous nonces,
+// swapped atomically on rotation.
 type nonceState struct {
 	currentNonce  string
 	previousNonce string
 	rotatedAt     time.Time
 }
 
+// dpopNonceManager manages server-issued DPoP nonces per RFC 9449 §8
 type dpopNonceManager struct {
 	state        atomic.Pointer[nonceState]
 	mu           sync.Mutex // serializes rotation only
@@ -180,11 +184,20 @@ func (nm *dpopNonceManager) validateNonce(nonce string) bool {
 
 // Creates new authN which is used to verify tokens for a set of given issuers
 func NewAuthenticator(ctx context.Context, cfg Config, logger *logger.Logger, wellknownRegistration func(namespace string, config any) error) (*Authentication, error) {
+	// Remember DPoP `jti` values for the acceptance window so a captured proof
+	// cannot be replayed; a proof older than DPoPSkew is rejected by the `iat`
+	// check, so the cache TTL tracks that window.
+	replayTTL := cfg.DPoPSkew
+	if replayTTL <= 0 {
+		replayTTL = time.Hour
+	}
+
 	a := &Authentication{
 		enforceDPoP:      cfg.EnforceDPoP,
 		strictDPoPHTU:    cfg.DPoP.StrictHTU,
 		logger:           logger,
 		dpopNonceManager: newDPoPNonceManager(cfg.DPoP.RequireNonce, cfg.DPoP.NonceExpiration),
+		dpopReplayCache:  newDPoPReplayCache(replayTTL),
 	}
 
 	tokenVerifier, oidcConfig, err := newTokenVerifier(ctx, cfg.AuthNConfig, a.logger)
@@ -384,7 +397,8 @@ func (a Authentication) MuxHandler(handler http.Handler) http.Handler {
 				http.Error(w, "unauthenticated", http.StatusUnauthorized)
 				return
 			}
-			log.WarnContext(ctxWithAuthX,
+			log.WarnContext(
+				ctxWithAuthX,
 				"unauthenticated",
 				slog.Any("error", err),
 				slog.Any("dpop", dp),
@@ -623,7 +637,8 @@ func permissionDeniedLogAttrs(token jwt.Token, casbinAuthz CasbinAuthzLog, err e
 	attrs := []any{slog.String("azp", token.Subject())}
 
 	if casbinAuthz.ConfiguredGroupsClaim != "" || casbinAuthz.SubjectGroups != nil {
-		attrs = append(attrs, slog.Group("casbin_authz",
+		attrs = append(attrs, slog.Group(
+			"casbin_authz",
 			slog.String("configured_groups_claim", casbinAuthz.ConfiguredGroupsClaim),
 			slog.Any("subject_groups", casbinAuthz.SubjectGroups),
 		))
@@ -799,7 +814,7 @@ func (a *Authentication) checkToken(ctx context.Context, authHeader []string, dp
 		if errors.As(err, &nonceErr) {
 			a.logger.DebugContext(ctx, "dpop nonce challenge issued", slog.String("reason", nonceErr.Message))
 		} else {
-			a.logger.Warn("failed to validate dpop", slog.Any("err", err))
+			a.logger.WarnContext(ctx, "failed to validate dpop", slog.Any("err", err))
 		}
 		return nil, nil, err
 	}
@@ -948,21 +963,38 @@ func (a Authentication) validateDPoP(accessToken jwt.Token, acessTokenRaw string
 		}
 	}
 
+	// Replay protection (RFC 9449 §11.1): reject a proof whose `jti` has already
+	// been seen within the acceptance window. Checked last so only otherwise-valid
+	// proofs populate the cache, preventing an attacker from poisoning it.
+	jtiI, ok := dpopToken.Get("jti")
+	if !ok {
+		return nil, errors.New("missing `jti` claim in DPoP JWT")
+	}
+	jti, ok := jtiI.(string)
+	if !ok {
+		return nil, errors.New("`jti` claim invalid format in DPoP JWT")
+	}
+	if a.dpopReplayCache.observe(jti, time.Now()) {
+		return nil, errors.New("DPoP proof replay detected; `jti` already used")
+	}
+
 	return dpopKey, nil
 }
 
 func (a Authentication) isPublicRoute(path string) func(string) bool {
 	return func(route string) bool {
 		matched, err := doublestar.Match(route, path)
 		if err != nil {
-			a.logger.Warn("error matching route",
+			a.logger.Warn(
+				"error matching route",
 				slog.String("route", route),
 				slog.String("path", path),
 				slog.Any("error", err),
 			)
 			return false
 		}
-		a.logger.Trace("matching route",
+		a.logger.Trace(
+			"matching route",
 			slog.String("route", route),
 			slog.String("path", path),
 			slog.Bool("matched", matched),

service/internal/auth/authn_test.go

@@ -809,6 +809,51 @@ func (s *AuthSuite) Test_CheckToken_AcceptsDPoP_GET() {
 	s.Require().NoError(err)
 }
 
+// Test_CheckToken_RejectsReplayedDPoP verifies that reusing the same DPoP proof
+// (identical `jti`) within the acceptance window is rejected per RFC 9449 §11.1,
+// even though the first presentation succeeds.
+func (s *AuthSuite) Test_CheckToken_RejectsReplayedDPoP() {
+	dpopRaw, err := rsa.GenerateKey(rand.Reader, 2048)
+	s.Require().NoError(err)
+	dpopKey, err := jwk.FromRaw(dpopRaw)
+	s.Require().NoError(err)
+	s.Require().NoError(dpopKey.Set(jwk.AlgorithmKey, jwa.RS256))
+	dpopPublic, err := dpopKey.PublicKey()
+	s.Require().NoError(err)
+
+	tok := jwt.New()
+	s.Require().NoError(tok.Set(jwt.ExpirationKey, time.Now().Add(time.Hour)))
+	s.Require().NoError(tok.Set("iss", s.server.URL))
+	s.Require().NoError(tok.Set("aud", "test"))
+	s.Require().NoError(tok.Set("cid", "client-replay"))
+	thumbprint, err := dpopKey.Thumbprint(crypto.SHA256)
+	s.Require().NoError(err)
+	cnf := map[string]string{"jkt": base64.URLEncoding.WithPadding(base64.NoPadding).EncodeToString(thumbprint)}
+	s.Require().NoError(tok.Set("cnf", cnf))
+	signedTok, err := jwt.Sign(tok, jwt.WithKey(jwa.RS256, s.key))
+	s.Require().NoError(err)
+
+	dpopToken := makeDPoPToken(s.T(), dpopTestCase{
+		key:              dpopPublic,
+		actualSigningKey: dpopKey,
+		accessToken:      signedTok,
+		alg:              jwa.RS256,
+		typ:              "dpop+jwt",
+		htm:              http.MethodPost,
+		htu:              "/a/path",
+		iat:              time.Now(),
+	})
+
+	ri := receiverInfo{u: []string{"/a/path"}, m: []string{http.MethodPost}}
+
+	_, _, err = s.auth.checkToken(context.Background(), []string{"DPoP " + string(signedTok)}, ri, []string{dpopToken})
+	s.Require().NoError(err, "first presentation of the proof should succeed")
+
+	_, _, err = s.auth.checkToken(context.Background(), []string{"DPoP " + string(signedTok)}, ri, []string{dpopToken})
+	s.Require().Error(err, "replaying the same proof should be rejected")
+	s.Contains(err.Error(), "replay")
+}
+
 // Test_ConnectAuthNInterceptor_PropagatesHTTPMethod verifies that the Connect
 // interceptor forwards the actual HTTP method (from req.HTTPMethod()) into
 // receiverInfo.m, instead of hardcoding POST. This is what makes DPoP
@@ -1027,9 +1072,10 @@ func (s *AuthSuite) Test_Allowing_Auth_With_No_DPoP() {
 	}
 	config := Config{}
 	config.AuthNConfig = authnConfig
-	auth, err := NewAuthenticator(context.Background(), config, &logger.Logger{
-		Logger: slog.New(slog.Default().Handler()),
-	},
+	auth, err := NewAuthenticator(
+		context.Background(), config, &logger.Logger{
+			Logger: slog.New(slog.Default().Handler()),
+		},
 		func(_ string, _ any) error { return nil },
 	)
 

service/internal/auth/dpop_replay.go

@@ -0,0 +1,55 @@
+package auth
+
+import (
+	"sync"
+	"time"
+)
+
+// dpopReplayCache rejects replayed DPoP proofs by their `jti` value within the
+// acceptance window, per RFC 9449 §11.1. Entries expire after ttl (set to the
+// DPoP `iat` skew): a proof older than that window is already rejected by the
+// `iat` freshness check, so the `jti` need not be remembered any longer.
+type dpopReplayCache struct {
+	mu     sync.Mutex
+	seen   map[string]time.Time // jti -> expiry
+	ttl    time.Duration
+	lastGC time.Time
+}
+
+func newDPoPReplayCache(ttl time.Duration) *dpopReplayCache {
+	return &dpopReplayCache{
+		seen: make(map[string]time.Time),
+		ttl:  ttl,
+	}
+}
+
+// observe records jti and reports whether it is a replay (already seen and not
+// yet expired). The first time a jti is seen it returns false and is remembered
+// until now+ttl; any subsequent call within that window returns true.
+func (c *dpopReplayCache) observe(jti string, now time.Time) bool {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
+	c.gc(now)
+
+	if expiry, ok := c.seen[jti]; ok && expiry.After(now) {
+		return true
+	}
+	c.seen[jti] = now.Add(c.ttl)
+	return false
+}
+
+// gc prunes expired entries to keep the map bounded. It is throttled to run at
+// most once per ttl so the common (cache-hot) path stays O(1). Callers must hold
+// c.mu.
+func (c *dpopReplayCache) gc(now time.Time) {
+	if c.ttl <= 0 || now.Sub(c.lastGC) < c.ttl {
+		return
+	}
+	for jti, expiry := range c.seen {
+		if !expiry.After(now) {
+			delete(c.seen, jti)
+		}
+	}
+	c.lastGC = now
+}

service/internal/auth/dpop_replay_test.go

@@ -0,0 +1,53 @@
+package auth
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDPoPReplayCache_FirstUseThenReplay(t *testing.T) {
+	c := newDPoPReplayCache(time.Hour)
+	now := time.Now()
+
+	require.False(t, c.observe("jti-1", now), "first use of a jti is not a replay")
+	require.True(t, c.observe("jti-1", now), "second use of the same jti within the window is a replay")
+	require.False(t, c.observe("jti-2", now), "a different jti is not a replay")
+}
+
+func TestDPoPReplayCache_ExpiredEntryIsReusable(t *testing.T) {
+	ttl := time.Minute
+	c := newDPoPReplayCache(ttl)
+	now := time.Now()
+
+	require.False(t, c.observe("jti", now))
+	// After the TTL elapses the proof would already be rejected by the iat check,
+	// so the jti is allowed to be re-recorded rather than reported as a replay.
+	require.False(t, c.observe("jti", now.Add(ttl+time.Second)), "entry past its TTL should not count as a replay")
+}
+
+func TestDPoPReplayCache_GCPrunesExpiredEntries(t *testing.T) {
+	ttl := time.Minute
+	c := newDPoPReplayCache(ttl)
+	now := time.Now()
+
+	for _, jti := range []string{"a", "b", "c"} {
+		require.False(t, c.observe(jti, now))
+	}
+	assert.Len(t, c.seen, 3)
+
+	// A later observe past the GC interval prunes the expired entries, leaving
+	// only the freshly recorded one.
+	require.False(t, c.observe("d", now.Add(ttl+time.Second)))
+	assert.Len(t, c.seen, 1, "expired entries should be pruned, leaving only the new jti")
+}
+
+func TestDPoPReplayCache_ZeroTTLDoesNotPanic(t *testing.T) {
+	c := newDPoPReplayCache(0)
+	now := time.Now()
+	// With a non-positive TTL entries expire immediately, so nothing is ever a replay.
+	require.False(t, c.observe("jti", now))
+	require.False(t, c.observe("jti", now))
+}