fix: verify refreshed id_token signature before storing claims in session

[Pyolar]

oidc_refresh_token_grant_apply_id_token calls oidc_jwt_parse to parse the id_token from a refresh grant response, but oidc_jwt_parse only decrypts — it never validates the JWS signature. If the token endpoint returns an unsigned or improperly signed id_token, the unverified claims are stored directly in the session.

This adds a oidc_proto_jwt_verify call (matching the pattern already used in oidc_proto_idtoken_parse) between parsing and storing, so a token that fails signature validation is discarded.

[zandbelt]

Thanks for looking at this — the observation is correct and worth acting on. oidc_refresh_token_grant_apply_id_token really is the only place that takes an id_token apart without validating it: it calls oidc_jwt_parse with no keys and pushes the payload straight into oidc_session_set_idtoken_claims, which is what backs OIDCPassClaimsAs and require claim. Every other consumer (oidc_proto_idtoken_parse, oidc_proto_userinfo_request, the logout token path) verifies.

A note on the framing first: this isn't quite the vulnerability the description makes it out to be. The token arrives over the TLS-protected back-channel from the configured token endpoint under client authentication, and OpenID Connect Core 1.0 section 3.1.3.7 item 6 explicitly allows TLS server validation to stand in for signature validation in that case — which is why oidc_proto_idtoken_parse deliberately makes that exception for the code flow (see the comment above the is_code_flow test). To exploit this the OP itself has to be hostile, and a hostile OP can inject the same claims at login. So this is spec conformance and defence in depth rather than a fix for a privilege boundary that is currently crossable. Still worth doing.

On the patch itself, a few things:

1. It doesn't prevent the store it says it prevents. oidc_session_set_idtoken() runs before the parse and isn't moved, so with OIDCStoreIDToken On the unverified serialized token is still persisted in the session and handed out through OIDCPassIDTokenAs and the session info hook. Only the claims are dropped.

2. It regresses unsigned id_tokens. oidc_proto_jwt_verify refuses "alg":"none" outright when no algorithm is pinned, and cannot succeed when OIDCIDTokenSignedResponseAlg none is configured. oidc_proto_idtoken_parse skips verification entirely in exactly that case, for the back-channel reason above — and a refresh response is the same back-channel. As written, a deployment against an OP that legitimately issues unsigned id_tokens keeps working at login and silently loses its refreshed claims and its session-expiry update.

3. Encrypted id_tokens stay broken. The NULL decryption-keys argument to oidc_jwt_parse is left in place, so a JWE id_token still fails to parse here, where the login path builds the key set from the client secret, oidc_cfg_private_keys_get and the configured client keys.

4. Signature alone isn't what section 12.2 asks for. There's no iss, aud/azp, exp or iat check, and — the one I'd care about most — nothing checks that the sub is the same as the one in the id_token issued at login. Section 12.2 requires that, and a refreshed id_token silently moving an established session to a different subject is a more interesting failure than an unsigned one.

5. (oidc_cfg_t *)c drops const. The caller oidc_refresh_token_grant already has a non-const oidc_cfg_t *, so the parameter can just lose its const rather than casting it away — we try to keep const-drop casts out of the tree.

Rather than a partial copy of the block from id_token.c, I think this collapses into the function that already gets all of it right:

if (oidc_proto_idtoken_parse(r, c, provider, s_id_token, NULL /* nonce */, &id_token_jwt,
                             TRUE /* back-channel */) == FALSE) {
        oidc_warn(r, "refreshed id_token could not be validated, retaining the claims obtained earlier");
        return;
}

nonce == NULL is handled (it belongs to the authentication request, not to a refresh), and that one call brings the decryption keys, the none exception, signature verification and the section 12.2 payload checks. On top of that a sub comparison against the claims already in the session, and moving the oidc_session_set_idtoken() call behind the validation, covers points 1 through 4.

[Pyolar]

Thanks for the review and the follow-up. Reusing oidc_proto_idtoken_parse is clearly the right shape, and the section 12.2 sub check was the piece I missed. Good to see this land with tests.