diff --git a/jwt.go b/jwt.go index b7588c7..bd7133d 100644 --- a/jwt.go +++ b/jwt.go @@ -6,9 +6,7 @@ import ( "crypto" "crypto/ecdsa" "crypto/rsa" - "crypto/x509" "encoding/base64" - "encoding/pem" "fmt" "math/big" "strings" @@ -513,31 +511,9 @@ func verifyNotBefore(notBefore float64) error { return verifyTimeConstraint(notBefore, "nbf", false) } -// verifySignature verifies the JWT signature using the provided public key. -// Supports RSA (RS256/384/512, PS256/384/512) and ECDSA (ES256/384/512) algorithms. -// Parameters: -// - tokenString: The complete JWT token string -// - publicKeyPEM: The public key in PEM format -// - alg: The signing algorithm specified in the JWT header -// -// Returns: -// - An error if the key parsing fails, the algorithm is unsupported, -// or the signature verification fails -func verifySignature(tokenString string, publicKeyPEM []byte, alg string) error { - block, _ := pem.Decode(publicKeyPEM) - if block == nil { - return fmt.Errorf("failed to parse PEM block containing the public key") - } - pubKey, err := x509.ParsePKIXPublicKey(block.Bytes) - if err != nil { - return fmt.Errorf("failed to parse public key: %w", err) - } - return verifySignatureWithKey(tokenString, pubKey, alg) -} - // verifySignatureWithKey verifies a JWT signature using an already-parsed -// public key, skipping the PEM-encode/decode round trip that verifySignature -// performs. This is the hot path used by VerifyJWTSignatureAndClaims. +// public key, skipping the PEM-encode/decode round trip. This is the hot +// path used by VerifyJWTSignatureAndClaims and verifyLogoutTokenSignature. func verifySignatureWithKey(tokenString string, pubKey crypto.PublicKey, alg string) error { parts := strings.Split(tokenString, ".") if len(parts) != 3 { diff --git a/logout.go b/logout.go index 51c56e9..4ffd099 100644 --- a/logout.go +++ b/logout.go @@ -30,6 +30,7 @@ type LogoutTokenClaims struct { Subject string `json:"sub,omitempty"` Audience interface{} `json:"aud"` // Can be string or []string IssuedAt int64 `json:"iat"` + ExpiresAt int64 `json:"exp,omitempty"` JTI string `json:"jti"` Events map[string]interface{} `json:"events"` SessionID string `json:"sid,omitempty"` @@ -194,7 +195,8 @@ func (t *TraefikOidc) validateLogoutToken(tokenString string) (*LogoutTokenClaim } } - // Verify signature only (not standard claims - logout tokens don't have 'exp') + // Verify signature (standard logout-token claims like exp are validated + // below; exp IS present per OIDC Back-Channel Logout 1.0 §2.4). if err := t.verifyLogoutTokenSignature(jwt, tokenString); err != nil { return nil, fmt.Errorf("signature verification failed: %w", err) } @@ -238,6 +240,20 @@ func (t *TraefikOidc) validateLogoutToken(tokenString string) (*LogoutTokenClaim return nil, fmt.Errorf("logout token issued in the future: %v", iatTime) } + // Validate exp. OIDC Back-Channel Logout 1.0 §2.4 marks exp REQUIRED + // and §2.6 REQUIREDs validating it the same way as ID Tokens; the + // spec's security note also recommends short exp precisely so a + // captured logout token can't be replayed later. Without this an + // expired logout token was still accepted and still invalidated live + // sessions. + if claims.ExpiresAt == 0 { + return nil, fmt.Errorf("missing exp claim") + } + expTime := time.Unix(claims.ExpiresAt, 0) + if time.Now().After(expTime.Add(5 * time.Minute)) { + return nil, fmt.Errorf("logout token expired at %v", expTime) + } + // Validate events claim - must contain the logout event if claims.Events == nil { return nil, fmt.Errorf("missing events claim") @@ -294,33 +310,15 @@ func (t *TraefikOidc) validateLogoutTokenAudience(aud interface{}) bool { return false } -// verifyLogoutTokenSignature verifies only the signature of a logout token. -// Unlike VerifyJWTSignatureAndClaims, this does NOT validate standard claims like 'exp' -// because logout tokens don't have an expiration claim per OIDC Back-Channel Logout spec. -// -// Parameters: -// - jwt: The parsed JWT structure -// - tokenString: The raw token string for signature verification -// -// Returns: -// - An error if signature verification fails +// verifyLogoutTokenSignature verifies the signature of a logout token. +// Like every other token path it resolves the key via GetPublicKey, +// which on a kid miss performs a bounded live JWKS refresh - so a logout +// token signed with a freshly rotated key is accepted immediately +// instead of failing for up to the 1h JWKS TTL (which would leave the +// user's session alive after the IdP logged them out). func (t *TraefikOidc) verifyLogoutTokenSignature(jwt *JWT, tokenString string) error { t.logger.Debug("Verifying logout token signature") - // Read jwksURL with RLock - t.metadataMu.RLock() - jwksURL := t.jwksURL - t.metadataMu.RUnlock() - - jwks, err := t.jwkCache.GetJWKS(context.Background(), jwksURL, t.httpClient) - if err != nil { - return fmt.Errorf("failed to get JWKS: %w", err) - } - - if jwks == nil { - return fmt.Errorf("JWKS is nil, cannot verify token") - } - kid, ok := jwt.Header["kid"].(string) if !ok || kid == "" { return fmt.Errorf("missing key ID in token header") @@ -331,25 +329,19 @@ func (t *TraefikOidc) verifyLogoutTokenSignature(jwt *JWT, tokenString string) e return fmt.Errorf("missing algorithm in token header") } - // Find the matching key in JWKS - var matchingKey *JWK - for i := range jwks.Keys { - if jwks.Keys[i].Kid == kid { - matchingKey = &jwks.Keys[i] - break - } - } + // GetPublicKey bounds a live JWKS refresh when kid is absent from the + // cached keyset, matching the normal token-verification path and + // keeping logout recovery robust across key rotation. + t.metadataMu.RLock() + jwksURL := t.jwksURL + t.metadataMu.RUnlock() - if matchingKey == nil { - return fmt.Errorf("no matching public key found for kid: %s", kid) - } - - publicKeyPEM, err := jwkToPEM(matchingKey) + pubKey, err := t.jwkCache.GetPublicKey(context.Background(), jwksURL, kid, t.httpClient) if err != nil { - return fmt.Errorf("failed to convert JWK to PEM: %w", err) + return fmt.Errorf("failed to get public key for logout token: %w", err) } - if err := verifySignature(tokenString, publicKeyPEM, alg); err != nil { + if err := verifySignatureWithKey(tokenString, pubKey, alg); err != nil { return fmt.Errorf("signature verification failed: %w", err) } diff --git a/logout_test.go b/logout_test.go index 16df599..889f87d 100644 --- a/logout_test.go +++ b/logout_test.go @@ -726,6 +726,7 @@ func TestBackchannelLogoutIntegration(t *testing.T) { "iss": "https://provider.example.com", "aud": "test-client", "iat": now, + "exp": time.Now().Add(time.Hour).Unix(), "jti": "unique-id-123", "events": map[string]interface{}{ "http://schemas.openid.net/event/backchannel-logout": map[string]interface{}{}, @@ -1644,6 +1645,12 @@ func createSignedLogoutToken(t *testing.T, privateKey *ecdsa.PrivateKey, claims headerJSON, _ := json.Marshal(header) headerB64 := base64.RawURLEncoding.EncodeToString(headerJSON) + // OIDC Back-Channel Logout requires exp; default-inject a valid future + // one unless the caller supplies its own (e.g. an expired-exp test). + if _, ok := claims["exp"]; !ok { + claims["exp"] = time.Now().Add(time.Hour).Unix() + } + claimsJSON, _ := json.Marshal(claims) claimsB64 := base64.RawURLEncoding.EncodeToString(claimsJSON) diff --git a/review_r73_test.go b/review_r73_test.go index b7535e4..1fa2eae 100644 --- a/review_r73_test.go +++ b/review_r73_test.go @@ -76,6 +76,7 @@ func TestBackchannelLogout_RejectsReplayedJTI(t *testing.T) { "iss": "https://provider.example.com", "aud": "test-client", "iat": time.Now().Unix(), + "exp": time.Now().Add(time.Hour).Unix(), "jti": "unique-jti-123", "events": map[string]interface{}{ "http://schemas.openid.net/event/backchannel-logout": map[string]interface{}{}, diff --git a/review_r87_test.go b/review_r87_test.go new file mode 100644 index 0000000..5efa7ad --- /dev/null +++ b/review_r87_test.go @@ -0,0 +1,123 @@ +package traefikoidc + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + "time" +) + +// signR87LogoutToken builds an ES256 logout token with the exact given claims +// (so a test can omit or age the exp claim). +type r87LogoutHarness struct { + oidc *TraefikOidc + tok func(map[string]interface{}) string +} + +func newR87LogoutHarness(t *testing.T) *r87LogoutHarness { + t.Helper() + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("keygen: %v", err) + } + x := base64.RawURLEncoding.EncodeToString(priv.PublicKey.X.Bytes()) + y := base64.RawURLEncoding.EncodeToString(priv.PublicKey.Y.Bytes()) + oidc := &TraefikOidc{ + logger: NewLogger("error"), + enableBackchannelLogout: true, + backchannelLogoutPath: "/backchannel-logout", + sessionInvalidationCache: &mockCacheInterface{data: map[string]interface{}{}}, + clientID: "test-client", + issuerURL: "https://provider.example.com", + jwkCache: &staticJWKCache{jwks: &JWKSet{Keys: []JWK{{ + Kty: "EC", Crv: "P-256", X: x, Y: y, Kid: "test-key-1", Use: "sig", Alg: "ES256", + }}}}, + jwksURL: "https://provider.example.com/.well-known/jwks.json", + } + tok := func(claims map[string]interface{}) string { + h, _ := json.Marshal(map[string]interface{}{"alg": "ES256", "typ": "logout+jwt", "kid": "test-key-1"}) + hb := base64.RawURLEncoding.EncodeToString(h) + c, _ := json.Marshal(claims) + cb := base64.RawURLEncoding.EncodeToString(c) + hash := sha256.Sum256([]byte(hb + "." + cb)) + r, s, _ := ecdsa.Sign(rand.Reader, priv, hash[:]) + sig := make([]byte, 64) + rb, sb := r.Bytes(), s.Bytes() + copy(sig[32-len(rb):32], rb) + copy(sig[64-len(sb):], sb) + return hb + "." + cb + "." + base64.RawURLEncoding.EncodeToString(sig) + } + return &r87LogoutHarness{oidc: oidc, tok: tok} +} + +func (h *r87LogoutHarness) post(claims map[string]interface{}) int { + req := httptest.NewRequest(http.MethodPost, "/backchannel-logout", + strings.NewReader("logout_token="+url.QueryEscape(h.tok(claims)))) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + rw := httptest.NewRecorder() + h.oidc.handleBackchannelLogout(rw, req) + return rw.Code +} + +func r87BaseClaims() map[string]interface{} { + return map[string]interface{}{ + "iss": "https://provider.example.com", + "aud": "test-client", + "iat": time.Now().Unix(), + "jti": fmt.Sprintf("r87-%d", time.Now().UnixNano()), // unique per call + "events": map[string]interface{}{ + "http://schemas.openid.net/event/backchannel-logout": map[string]interface{}{}, + }, + "sid": "session-1", + } +} + +// TestBackchannelLogout_RequiresExp is a regression: OIDC Back-Channel +// Logout 1.0 §2.4 makes exp REQUIRED and §2.6 requires it to be +// validated. The code previously accepted an exp-less logout token (the +// comment claimed "logout tokens don't have exp", which is false) and +// still invalidated the session, letting an unbounded-lifetime captured +// token be replayed. +func TestBackchannelLogout_RequiresExp(t *testing.T) { + h := newR87LogoutHarness(t) + claims := r87BaseClaims() + delete(claims, "exp") // ensure absent + if code := h.post(claims); code != http.StatusBadRequest { + t.Fatalf("expected 400 for logout token missing exp, got %d", code) + } +} + +// TestBackchannelLogout_RejectsExpiredExp is a regression: a logout token +// whose exp is already past must be rejected even when iat is fresh, as +// the spec's validation-is-like-ID-Tokens rule requires. Previously only +// iat was capped (15 min), so an expired exp was silently accepted and the +// session was still invalidated. +func TestBackchannelLogout_RejectsExpiredExp(t *testing.T) { + h := newR87LogoutHarness(t) + claims := r87BaseClaims() + claims["iat"] = time.Now().Add(-2 * time.Minute).Unix() // fresh (within 15 min) + claims["exp"] = time.Now().Add(-10 * time.Minute).Unix() // expired beyond the 5-min skew + if code := h.post(claims); code != http.StatusBadRequest { + t.Fatalf("expected 400 for logout token with expired exp, got %d", code) + } +} + +// TestBackchannelLogout_AcceptsValidExp is the control: an in-window +// exp must still be accepted and must invalidate the session. +func TestBackchannelLogout_AcceptsValidExp(t *testing.T) { + h := newR87LogoutHarness(t) + claims := r87BaseClaims() + claims["exp"] = time.Now().Add(time.Hour).Unix() + if code := h.post(claims); code != http.StatusOK { + t.Fatalf("expected 200 for valid logout token, got %d", code) + } +}