From 6e522fd212b25e9fcda7b3a0656ffc0e2eb01d11 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Tue, 3 Nov 2026 01:15:45 +0000 Subject: [PATCH] fix(security): scope the verified-token cache to its middleware instance The verified-token cache is one process-wide cache keyed by the raw token. A verdict stored by one middleware instance satisfied any other instance, so a token valid for one client or audience skipped the issuer, audience and signature checks on another route. Each instance now prefixes keys with its provider, client ID and audience. Dynamic client registration updates the prefix. --- helpers.go | 25 ++++++++++++++-- main.go | 7 +++++ token_cache_namespace_test.go | 56 +++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 token_cache_namespace_test.go diff --git a/helpers.go b/helpers.go index c3d6ced..454e939 100644 --- a/helpers.go +++ b/helpers.go @@ -13,6 +13,7 @@ import ( "net/http/cookiejar" "net/url" "strings" + "sync/atomic" "time" ) @@ -293,10 +294,28 @@ func extractClaims(tokenString string) (map[string]interface{}, error) { // TokenCache provides a specialized cache for JWT tokens and their parsed claims. // It wraps the UniversalCache with token-specific operations. type TokenCache struct { + // ns scopes entries to one middleware instance (issuer|client|audience) + // so a verdict cached by one instance cannot satisfy another. + ns atomic.Value // cache is the underlying universal cache implementation cache *UniversalCache } +// SetNamespace sets the verification context that scopes this cache's keys. +func (tc *TokenCache) SetNamespace(ns string) { + if tc == nil { + return + } + tc.ns.Store(ns) +} + +func (tc *TokenCache) key(token string) string { + if ns, _ := tc.ns.Load().(string); ns != "" { + return "t-" + ns + "|" + token + } + return "t-" + token +} + // NewTokenCache creates and initializes a new TokenCache. // It uses the global cache manager to ensure singleton behavior. func NewTokenCache() *TokenCache { @@ -313,7 +332,7 @@ func NewTokenCache() *TokenCache { // - claims: Parsed claims from the token // - expiration: The duration for which the cache entry should be valid func (tc *TokenCache) Set(token string, claims map[string]interface{}, expiration time.Duration) { - token = "t-" + token + token = tc.key(token) _ = tc.cache.Set(token, claims, expiration) // Safe to ignore: cache failures are non-critical } @@ -325,7 +344,7 @@ func (tc *TokenCache) Set(token string, claims map[string]interface{}, expiratio // - map[string]interface{}: The cached claims if found // - A boolean indicating whether the token was found in the cache (true if found, false otherwise) func (tc *TokenCache) Get(token string) (map[string]interface{}, bool) { - token = "t-" + token + token = tc.key(token) value, found := tc.cache.Get(token) if !found { return nil, false @@ -338,7 +357,7 @@ func (tc *TokenCache) Get(token string) (map[string]interface{}, bool) { // Parameters: // - token: The raw token string to remove from the cache func (tc *TokenCache) Delete(token string) { - token = "t-" + token + token = tc.key(token) tc.cache.Delete(token) } diff --git a/main.go b/main.go index 6a593ab..93254eb 100644 --- a/main.go +++ b/main.go @@ -548,6 +548,7 @@ func NewWithContext(ctx context.Context, config *Config, next http.Handler, name logger.Debugf("TraefikOidc.New: logoutURLPath configured as: %q", t.logoutURLPath) t.providerURL = config.ProviderURL + t.tokenCache.SetNamespace(t.tokenCacheNamespace()) // Use singleton resource manager for metadata initialization rm := GetResourceManager() @@ -944,6 +945,7 @@ func (t *TraefikOidc) performDynamicClientRegistration() { if t.audience == "" { t.audience = resp.ClientID // Default audience to client ID } + t.tokenCache.SetNamespace(t.tokenCacheNamespace()) // R162: DCR can provision a client whose method is private_key_jwt // (RFC 7591 ClientMetadata.TokenEndpointAuthMethod) even when the // static config did not set ClientAuthMethod=private_key_jwt. In that @@ -1083,3 +1085,8 @@ func buildFullURL(scheme, host, path string) string { return fmt.Sprintf("%s://%s%s", scheme, host, path) } + +// tokenCacheNamespace identifies the verification context of this instance. +func (t *TraefikOidc) tokenCacheNamespace() string { + return t.providerURL + "|" + t.clientID + "|" + t.audience +} diff --git a/token_cache_namespace_test.go b/token_cache_namespace_test.go new file mode 100644 index 0000000..b7632fd --- /dev/null +++ b/token_cache_namespace_test.go @@ -0,0 +1,56 @@ +package traefikoidc + +import ( + "testing" + "time" +) + +func newNSCache(t *testing.T) *UniversalCache { + t.Helper() + c := NewUniversalCache(UniversalCacheConfig{Type: CacheTypeToken, MaxSize: 100, DefaultTTL: time.Hour, CleanupInterval: time.Hour}) + t.Cleanup(func() { _ = c.Close() }) + return c +} + +// A verdict stored by one middleware instance must not satisfy another +// instance that verifies against a different client or audience. +func TestTokenCacheIsolatedPerInstanceNamespace(t *testing.T) { + shared := newNSCache(t) + a := &TokenCache{cache: shared} + b := &TokenCache{cache: shared} + a.SetNamespace("https://idp|app-a|app-a") + b.SetNamespace("https://idp|app-b|app-b") + + a.Set("tok", map[string]interface{}{"sub": "u"}, time.Minute) + if _, ok := b.Get("tok"); ok { + t.Fatal("instance B hit a verdict cached by instance A") + } + if _, ok := a.Get("tok"); !ok { + t.Fatal("instance A lost its own verdict") + } + a.Delete("tok") + if _, ok := a.Get("tok"); ok { + t.Fatal("Delete did not remove the namespaced entry") + } +} + +func TestTokenCacheNamespaceFollowsDCR(t *testing.T) { + shared := newNSCache(t) + tc := &TokenCache{cache: shared} + tc.SetNamespace("p|old|old") + tc.Set("tok", map[string]interface{}{"sub": "u"}, time.Minute) + tc.SetNamespace("p|new|new") + if _, ok := tc.Get("tok"); ok { + t.Fatal("verdict cached under the old client survived a namespace change") + } +} + +func TestTokenCacheEmptyNamespaceKeepsLegacyKey(t *testing.T) { + shared := newNSCache(t) + a := &TokenCache{cache: shared} + b := &TokenCache{cache: shared} + a.Set("tok", map[string]interface{}{"sub": "u"}, time.Minute) + if _, ok := b.Get("tok"); !ok { + t.Fatal("namespace-less caches must keep sharing the legacy key") + } +}