mirror of
https://github.com/lukaszraczylo/traefikoidc.git
synced 2026-10-17 04:37:48 +00:00
R92: validate all time claims on lenient-audience access tokens
On the lenient-audience path (validateStandardTokensRS), jwt.Verify short-circuits at the aud check before reaching exp/iat/nbf validation, leaving every time claim unvalidated. accessTokenUnexpired previously re-patched only 'exp' (the R66 fix), so a token with a not-yet-reached 'nbf' but a valid exp was trusted for authorization before it was valid. accessTokenUnexpired now re-applies the full main-path time validation: verifyExpiration, verifyIssuedAt (iat required), and verifyNotBefore when present — all clock-skew-aware. Regression TestR92LenientPathRejectsFutureNbf (plus valid control) fails on old code (future-nbf token accepted), passes on new. R92 clean/declined: bearer auth (aud enforced, full claims — clean); refresh rotation (correctly bound to session; no local reuse detection = design); logout cookie completeness (all chunk names cleared, no revive; two P3 non-security notes declined).
This commit is contained in:
@@ -0,0 +1,54 @@
|
|||||||
|
package traefikoidc
|
||||||
|
|
||||||
|
import (
|
||||||
|
"encoding/base64"
|
||||||
|
"encoding/json"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
)
|
||||||
|
|
||||||
|
// makeTestJWT builds a syntactically valid 3-part JWT with the given claims.
|
||||||
|
// The signature segment is not cryptographically validated by the code under
|
||||||
|
// test (accessTokenUnexpired only parses claims), so a static value is fine.
|
||||||
|
func makeTestJWT(t *testing.T, claims map[string]interface{}) string {
|
||||||
|
t.Helper()
|
||||||
|
hdr, err := json.Marshal(map[string]string{"alg": "RS256", "typ": "JWT"})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("marshal header: %v", err)
|
||||||
|
}
|
||||||
|
pl, err := json.Marshal(claims)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("marshal payload: %v", err)
|
||||||
|
}
|
||||||
|
return base64.RawURLEncoding.EncodeToString(hdr) + "." +
|
||||||
|
base64.RawURLEncoding.EncodeToString(pl) + ".c2ln"
|
||||||
|
}
|
||||||
|
|
||||||
|
// R92 regression: on the lenient-audience access-token path,
|
||||||
|
// accessTokenUnexpired previously re-checked only 'exp' after jwt.Verify
|
||||||
|
// short-circuited at the aud check, leaving 'nbf' (and 'iat') unvalidated.
|
||||||
|
// A token with a not-yet-reached nbf (valid exp) was therefore trusted for
|
||||||
|
// authorization before it was valid. It must now reject not-yet-valid
|
||||||
|
// tokens just like the main verification path (verifyNotBefore).
|
||||||
|
func TestR92LenientPathRejectsFutureNbf(t *testing.T) {
|
||||||
|
tm := &TraefikOidc{}
|
||||||
|
now := time.Now()
|
||||||
|
|
||||||
|
notYetValid := makeTestJWT(t, map[string]interface{}{
|
||||||
|
"exp": float64(now.Add(time.Hour).Unix()),
|
||||||
|
"iat": float64(now.Add(-time.Minute).Unix()),
|
||||||
|
"nbf": float64(now.Add(10 * time.Minute).Unix()), // not yet valid
|
||||||
|
})
|
||||||
|
if tm.accessTokenUnexpired(notYetValid) {
|
||||||
|
t.Error("access token with future nbf was accepted on the lenient-audience path (not-yet-valid token trusted)")
|
||||||
|
}
|
||||||
|
|
||||||
|
// Control: a token with no nbf and a valid exp must still be accepted.
|
||||||
|
valid := makeTestJWT(t, map[string]interface{}{
|
||||||
|
"exp": float64(now.Add(time.Hour).Unix()),
|
||||||
|
"iat": float64(now.Add(-time.Minute).Unix()),
|
||||||
|
})
|
||||||
|
if !tm.accessTokenUnexpired(valid) {
|
||||||
|
t.Error("token with valid exp and no nbf should be accepted")
|
||||||
|
}
|
||||||
|
}
|
||||||
+33
-9
@@ -38,21 +38,44 @@ func (t *TraefikOidc) validateGoogleTokensRS(rs *requestState) (bool, bool, bool
|
|||||||
return t.validateStandardTokensRS(rs)
|
return t.validateStandardTokensRS(rs)
|
||||||
}
|
}
|
||||||
|
|
||||||
// accessTokenUnexpired reports whether a signature-verified access token's exp
|
// accessTokenUnexpired reports whether a signature-verified access
|
||||||
// claim is still in the future. Used on the lenient-audience path where
|
// token's time claims still hold. Used on the lenient-audience path where
|
||||||
// jwt.Verify short-circuits at the aud check before validating exp, so expiry
|
// jwt.Verify short-circuits at the aud check BEFORE reaching exp/iat/nbf
|
||||||
// must be (re)checked here before the token's claims are trusted. Honors the
|
// validation, leaving all time claims unvalidated. Re-apply the same
|
||||||
// same clock-skew tolerance as normal expiration verification.
|
// clock-skew-aware checks the main path applies (verifyExpiration,
|
||||||
|
// verifyIssuedAt with iat required, and verifyNotBefore when present) so a
|
||||||
|
// token that is expired, used before its issue time, or not yet valid (nbf)
|
||||||
|
// is not trusted for authorization here.
|
||||||
func (t *TraefikOidc) accessTokenUnexpired(token string) bool {
|
func (t *TraefikOidc) accessTokenUnexpired(token string) bool {
|
||||||
parsed, err := parseJWT(token)
|
parsed, err := parseJWT(token)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
exp, ok := parsed.Claims["exp"].(float64)
|
claims := parsed.Claims
|
||||||
|
|
||||||
|
exp, ok := claims["exp"].(float64)
|
||||||
if !ok {
|
if !ok {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
return time.Now().Before(time.Unix(int64(exp), 0).Add(ClockSkewToleranceFuture))
|
if err := verifyExpiration(exp); err != nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
iat, ok := claims["iat"].(float64)
|
||||||
|
if !ok {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
if err := verifyIssuedAt(iat); err != nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
if nbf, ok := claims["nbf"].(float64); ok {
|
||||||
|
if err := verifyNotBefore(nbf); err != nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
// validateTokenExpiryRS is the requestState-aware variant of validateTokenExpiry.
|
// validateTokenExpiryRS is the requestState-aware variant of validateTokenExpiry.
|
||||||
@@ -235,8 +258,9 @@ func (t *TraefikOidc) validateStandardTokensRS(rs *requestState) (bool, bool, bo
|
|||||||
// there is no ID token to compare against. jwt.Verify returns at
|
// there is no ID token to compare against. jwt.Verify returns at
|
||||||
// the aud check BEFORE reaching the exp check, so expiry was never
|
// the aud check BEFORE reaching the exp check, so expiry was never
|
||||||
// validated on this path — an expired, wrong-audience token would
|
// validated on this path — an expired, wrong-audience token would
|
||||||
// otherwise be accepted as authenticated. Require the token to be
|
// otherwise be accepted as authenticated. Require the token's time
|
||||||
// still unexpired before trusting its claims for authorization.
|
// claims (exp, iat, nbf) to all still hold before trusting its
|
||||||
|
// claims for authorization.
|
||||||
if !t.accessTokenUnexpired(rs.accessToken) {
|
if !t.accessTokenUnexpired(rs.accessToken) {
|
||||||
if rs.refreshToken != "" {
|
if rs.refreshToken != "" {
|
||||||
return false, true, false
|
return false, true, false
|
||||||
|
|||||||
Reference in New Issue
Block a user