validate key attestation x5c chain against configured truststore - #50400
validate key attestation x5c chain against configured truststore#50400naruto-lgtm wants to merge 2 commits into
Conversation
Signed-off-by: Nayyar <naruto-lgtm@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes OID4VCI key attestation / JWT proof x5c chain validation so that certificate trust is anchored in Keycloak’s configured TruststoreProvider (matching other X.509 validation paths), instead of implicitly trusting the JVM-wide default truststore (cacerts).
Changes:
- Route
x5cvalidation through trust anchors derived fromTruststoreProvider(with a JVM-default fallback path). - Update
JwtProofValidatorto passKeycloakSessionintox5cresolution so realm/server truststore configuration can be applied. - Add unit tests covering accepted/rejected
x5cchains relative to supplied trust anchors.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtil.java | Derives PKIX trust anchors from TruststoreProvider and validates x5c chains against them. |
| services/src/main/java/org/keycloak/protocol/oid4vc/issuance/keybinding/JwtProofValidator.java | Passes KeycloakSession so x5c validation uses configured trust anchors. |
| services/src/test/java/org/keycloak/protocol/oid4vc/issuance/keybinding/AttestationValidatorUtilX5cTest.java | Adds tests for trust-anchor-scoped x5c acceptance/rejection behavior. |
| TruststoreProvider truststoreProvider = session == null ? null : session.getProvider(TruststoreProvider.class); | ||
| if (truststoreProvider != null && truststoreProvider.getTruststore() != null) { | ||
| addAnchors(anchors, truststoreProvider.getRootCertificates()); | ||
| addAnchors(anchors, truststoreProvider.getIntermediateCertificates()); | ||
| if (!anchors.isEmpty()) { | ||
| return anchors; | ||
| } | ||
| } |
There was a problem hiding this comment.
Good catch. Tightened it: when a truststore is configured but yields no anchors, it now rejects the chain instead of falling through to cacerts, so an empty or misconfigured truststore can't silently re-broaden trust back to every public CA. The JVM-default fallback now only happens when no truststore is configured at all, which matches the Javadoc/intent.
|
IMHO it should actually validate against configured |
…ling back Signed-off-by: Nayyar <naruto-lgtm@users.noreply.github.com>
|
@dominikschlosser agreed the source of attestation trust should be pluggable so it can move to ETSI trust lists / EUDI later. One snag with routing the x5c path straight through TrustMaterialIdentityProvider: resolveKeys hands back JWKs keyed by kid/issuer, which is a leaf-key trust model. The x5c path needs PKIX anchors (CA roots/intermediates) to validate the full chain, and the SPI doesn't expose those today. Matching on a resolved leaf key would collapse the chain to a single-key check, which is effectively what the kid-based TrustedAttestationKeyResolver already does and drops the CA delegation that x5c is there for. My suggestion: land this as the truststore-anchoring fix to close the immediate hole (any publicly-trusted CA satisfying an attestation), then extend TrustMaterialIdentityProvider to expose trust anchors and switch the x5c path onto that in a follow-up. I'm happy to do the SPI change once we settle the shape. Were you picturing something like a resolveTrustAnchors(request) on the provider, or the leaf-key match? |
|
@naruto-lgtm Yes the interface intentionally only covers what it is used for until now and probably has to be extended for that use case. As OID4VCI is still experimental, these kind of breaking changes should still be possible and i don't see the value in fixing it in a different way at first. But ultimately, @mposolda should answer that question. |
|
Works for me. Since it's still experimental I'm fine making the breaking SPI change directly rather than landing this as a stopgap and doing the follow-up later. The one thing that still needs a decision is the shape: a resolveTrustAnchors(request) on the provider returning PKIX anchors vs matching a resolved leaf key. Those give different chain semantics (full CA-delegation validation vs collapsing to a single-key check), so I'd rather not rework it on a guess. Happy to implement whichever direction @mposolda prefers once he weighs in. |
|
any update? |
mposolda
left a comment
There was a problem hiding this comment.
@naruto-lgtm @dominikschlosser Thanks for the PR and for the feedback!
There was the other PR #51042 , which was merged in the meantime. See also the underlying issue #50518 . (CC @forkimenjeckayang )
For security reasons, there is no use of Java cacerts for figuring the list of valid CA certificates for validation of attestation proofs. Instead, the valid CAs are obtained from the configured trust-material identity provider(s), which needs to be set on the client, which access token is used for OID4VCI credential request. The identity provider based mechanism is used for obtaining the keys from proofs from kid header as well as from x5c header. See the documentation for more details: https://www.keycloak.org/docs/nightly/server_admin/index.html#_oid4vci_proofs . The challenges you mentioned in the previous comment should be addressed now.
Considering this, I wonder that this PR might be closed?
Repro: sign an OID4VCI key attestation (or a
jwtproof carryingx5c) with any certificate that chains to a public CA shipped in the JVMcacerts(an ordinary publicly-issued TLS certificate works).AttestationValidatorUtilaccepts it as a valid attestation signer.Cause: the
x5cchain was anchored in the JVM-wide default truststore, so trust in an attestation key collapsed to "issued by some public CA" rather than the attestation issuers an operator actually accepts.Fix: anchor the chain in the realm's configured truststore, the same source
CertificateValidatoruses for X.509 client-certificate trust. The JVM default is used only when no truststore is configured, so existing deployments keep working. Applies to both the attestation headerx5cpath and theJwtProofValidatorx5cpath.Closes #50399