userinfo: verify the signature on JWT-encoded composite claims when verification keys are configured - #1395
Conversation
…erification keys are configured oidc_proto_userinfo_composite_decode_source parsed aggregated/distributed (composite) claim JWTs with a bare oidc_jwt_parse and merged their payload unconditionally. When static verification keys are configured (OIDCProviderVerifyCertFiles), the composite-claims JWT is now signature validated against them first, and one that fails validation is discarded. Without configured keys the behavior is unchanged, since OIDC Core 1.0 section 5.6.2 does not require signature validation of these JWTs when the Claims Provider keys cannot be obtained.
zandbelt
left a comment
There was a problem hiding this comment.
Thanks for following up on the composite-claims path — the gap you identified is real.
oidc_proto_userinfo_composite_decode_source does parse the JWT and merge its payload with no
signature check at all, and that should not stand. The patch also builds clean and the whole test
suite passes here.
I don't want to take it in this shape though, because it validates against the wrong party's keys.
The keys belong to the Claims Provider, not the OP
OpenID Connect Core 1.0 section 5.6.2:
An
iss(issuer) Claim SHOULD be included in any JWT issued by a Claims Provider so that the
Claims Provider's keys can be retrieved for signature validation of the JWT. The value of the
Claim is the Claims Provider's Issuer Identifier URL.
The aggregated/distributed claims JWT is signed by the Claims Provider, which is by definition a
different entity from the OpenID Provider. OIDCProviderVerifyCertFiles is the OP's key set — see
auth_openidc.conf, where it is documented as the keys "that can be used for ID Token
verification". Verifying a Claims Provider's JWT against the OP's ID token keys will not line up in
any deployment that uses composite claims the way the spec describes them.
Three things follow from that.
1. It breaks working deployments. Anyone who uses composite claims and has
OIDCProviderVerifyCertFiles configured will have legitimate Claims Provider JWTs discarded from
this release on: the claims silently disappear, authorization that used to succeed starts failing,
and all the operator gets is a line in the error log. Note that the negative case in
test_proto_userinfo_request_composite_claims_verify_keys — a JWT signed with a key that is not in
the configured set — is indistinguishable from a perfectly legitimate third-party Claims Provider.
The test asserts the regression as though it were the fix.
2. It does not address the threat that matters. In the aggregated case the only party that can
place a JWT in _claim_sources is the OP, and the OP can already assert any claim it likes directly
in the UserInfo response. Checking that JWT against the OP's own keys therefore adds nothing over
the TLS channel we already have to the OP. The case actually worth defending against is a
substituted or tampered Claims Provider assertion, and catching that requires the Claims
Provider's key — precisely the one this does not use.
3. Most deployments get no protection anyway. The check is gated on whether an unrelated
directive happens to be set. Installations that configure the provider through
OIDCProviderMetadataURL or OIDCProviderJwksUri — the common case — keep the unverified path
exactly as it is today. So the change leaves the reported gap open for the majority while breaking a
subset of everyone else.
The ChangeLog entry overstates the spec
since OpenID Connect Core 1.0 section 5.6.2 does not require signature validation of these JWTs
when the Claims Provider keys cannot be obtained
Section 5.6.2 does not say this. It says iss SHOULD be included so that the keys can be
retrieved for signature validation. The absence of an explicit requirement is not an explicit
exemption, and this sentence is doing a lot of work to make the opt-in design look sanctioned by the
specification.
Smaller points
- Changing
const oidc_cfg_t *cfgtooidc_cfg_t *cfgon
oidc_proto_userinfo_composite_decode_sourceis a const regression, forced by
oidc_proto_jwt_verifytaking a non-constcfg. I would rather fix the const-correctness at the
callee than propagate it outward. oidc_util_key_symmetric_merge(r->pool, verify_keys, NULL)supplies no symmetric key, unlike the
signed-UserInfo path a few dozen lines above that derives one from the client secret. An HS-signed
composite JWT can therefore never verify.- The negative test signs with HS256 under an unknown
kid, which really only demonstrates
"different algorithm plus unknown key fails". A tampered signature under the same alg and kid
would be the stronger assertion. There is also no test covering the compatibility promise the
change is built on, i.e. that behaviour is unchanged when no verification keys are configured. - I have a ChangeLog entry of my own landing under
08/03/2026, so that section will need a rebase
once it is on master.
Where I would like this to go
The gap is worth closing, so please do not drop it. The shape I would take is one that follows
5.6.2: resolve the Claims Provider's keys from the iss claim in the composite JWT. That needs
configuration that does not exist yet — a JWKS URI or a static key set per Claims Provider issuer —
and the verification should be gated on that new setting, so the behaviour change is opt-in on a
knob that actually means what it does, rather than riding on OIDCProviderVerifyCertFiles.
Happy to discuss the configuration surface for that before you invest in another round.
|
Thanks for the detailed review — you're right on all three points, and reusing Agreed on the way forward: resolve the Claims Provider's verification material from the
I'll also fix the smaller items in the same pass: keep const-correctness at the callee instead of widening the parameter where possible, strengthen the negative test by tampering with the signature/payload while keeping the same algorithm and Happy to wait for your take on the configuration surface before writing the next revision. |
oidc_proto_userinfo_composite_decode_source took the JWT from an aggregated/distributed (composite) claim source apart with a bare oidc_jwt_parse and merged its payload into the userinfo claims unconditionally, without any signature verification.
With this change, when static verification keys are configured (OIDCProviderVerifyCertFiles), the composite-claims JWT is signature validated against them first, and one that fails validation is discarded - its claims are not applied. Without configured verification keys the previous behavior is preserved, since OIDC Core 1.0 section 5.6.2 does not require signature validation of these JWTs when the Claims Provider keys cannot be obtained.