Repository navigation
fix(server)!: refuse more attester signing roles and re-issued anchors in x5c - #765
Merged
Merged
Conversation
DefaultAttesterLeafPolicy refuses a signing certificate issued for another role that signs things, but its list of extended key usages crypto/x509 doesn't name held only the ISO/IEC 18013-5 mDL document signer and mdoc reader authentication purposes. It missed the mDL JWS signer, which signs JWTs just as an attester does, so a key certified for that role under a shared anchor could sign Wallet Attestations. The policy now also refuses these purposes, in a signing certificate and in a CA certificate that lists nothing else: - ISO/IEC 18013-5 id-mdl-kp-mdlJWS (1.0.18013.5.1.3) and id-mdl-kp-mdlVICAL (1.0.18013.5.1.8) - ISO/IEC TS 23220-4 mdoc document signer (1.0.23220.4.1.2) and mdoc reader authentication (1.0.23220.4.1.6) - RFC 9336 id-kp-documentSigning (1.3.6.1.5.5.7.3.36) - RFC 6187 id-kp-secureShellClient and id-kp-secureShellServer (1.3.6.1.5.5.7.3.21 and .22) Purposes it doesn't list, such as an ecosystem's own Wallet Provider purpose, stay accepted. The fuzz target's independent oracle and OID set cover the new list. Sources for the OIDs: - RFC 9336 section 3 and RFC 6187 section 2.2.2 (id-kp arc 1.3.6.1.5.5.7.3). - ISO/IEC 18013-5 Annex B and C values as implemented in django-ca (ca/django_ca/constants.py, mdlDS .2 and mdlJWS .3), SUNET/vc pkg/mdoc/ISO_18013_5_SUMMARY.md (id-mdl-kp-mdlDS .2, id-mdl-kp-mdlJWS .3, id-mdl-kp-mdlReaderAuth .6), spruceid/isomdl (DS .2, reader .6, VICAL .8) and the OpenID conformance suite's ValidateVicalSignerCertificateProfile (id-mdl-kp-mdlVICAL .8, Annex C.1.7.2). - ISO/IEC TS 23220-4 Annex B.2.5 values as implemented in openwallet-foundation/multipaz (asn1/OID.kt) and spruceid/isomdl (MdocProfile::ISO_23220). BREAKING CHANGE: X5CAttesterChain with the default LeafPolicy refuses an attestation whose signing certificate lists the ISO/IEC 18013-5 mDL JWS or VICAL signer, ISO/IEC TS 23220-4 mdoc document signer or reader, RFC 9336 document signing, or RFC 6187 SSH extended key usage, or that chains only through a CA restricted to such roles. Issue attesters a dedicated certificate, or set LeafPolicy to AnyAttesterLeaf or your own AttesterLeafPolicyFunc. See UPGRADING.md for v0.68.0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
checkX5CExcludesAnchors kept HAIP 1.0's rule that x5c leaves out the trust anchor by comparing each x5c certificate's DER with the anchor each verified chain ends at. For an anchor that isn't self-signed, such as an intermediate configured as an anchor, another certificate for the same CA (re-issued by the anchor's own issuer, or cross-signed by a different root) has different bytes and got through, although path building treats it as the anchor itself. The check now compares subject and SubjectPublicKeyInfo instead, so any certificate for the anchor's CA is refused. A certificate with the anchor's subject but another key is still an ordinary intermediate. The fuzz target's oracle for the rule compares the same identity. BREAKING CHANGE: X5CAttesterChain refuses an attestation whose x5c carries, below the signing certificate, a certificate with the subject and public key of the anchor a verified chain ends at, even when it is not byte-identical to the configured anchor. Send x5c as the signing certificate and its intermediates only. See UPGRADING.md for v0.68.0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two tightenings of
X5CAttesterChain's certificate checks. Both refuse attestations that were accepted before, so both are breaking; UPGRADING.md gets a## v0.68.0section for each.DefaultAttesterLeafPolicyrefuses more signing rolesThe default leaf policy refuses a certificate issued for another role that signs things. crypto/x509 doesn't name some of these roles, and for those the policy checked only the ISO/IEC 18013-5 mDL document signer (
1.0.18013.5.1.2) and mdoc reader authentication (1.0.18013.5.1.6) purposes. It missed the mDL JWS signer, which signs JWTs just as an attester does. A key certified for that role under a shared anchor could therefore sign Wallet Attestations.The policy now also refuses the following, both in a signing certificate and in a CA certificate that lists nothing else:
1.0.18013.5.1.31.0.18013.5.1.81.0.23220.4.1.21.0.23220.4.1.61.3.6.1.5.5.7.3.361.3.6.1.5.5.7.3.21/.22Purposes the policy doesn't list stay accepted. That includes an ecosystem's own Wallet Provider purpose and unassigned values on the ISO arcs. Deployments that need the old behaviour can use
AnyAttesterLeafor their ownAttesterLeafPolicyFunc.OID sources:
id-kp 36) and RFC 6187 §2.2.2 (id-kp 21,22). Both sit under id-kp,1.3.6.1.5.5.7.3.ca/django_ca/constants.py(mdlDS.2, mdlJWS.3)pkg/mdoc/ISO_18013_5_SUMMARY.md(id-mdl-kp-mdlDS.2, id-mdl-kp-mdlJWS.3, id-mdl-kp-mdlReaderAuth.6)extended_key_usage.rs(DS.2, reader.6, VICAL.8)ValidateVicalSignerCertificateProfile(id-mdl-kp-mdlVICAL.8, Annex C.1.7.2)asn1/OID.ktand spruceid/isomdlMdocProfile::ISO_23220.The x5c anchor refusal compares subject and public key
checkX5CExcludesAnchorsenforces HAIP 1.0's rule thatx5cleaves out the trust anchor, but it compared exact DER. Some anchors aren't self-signed, such as an intermediate configured as an anchor. For those, another certificate for the same CA got through: one re-issued by the anchor's own issuer, or one cross-signed by a different root. Path building treats such a certificate as the anchor itself. The check now comparesRawSubjectandRawSubjectPublicKeyInfo. A certificate with the anchor's subject but a different key is still treated as an ordinary intermediate.Tests
TestX5CAttesterChainDefaultLeafPolicy,TestAnyAttesterLeafAcceptsOtherRolesandTestAttesterLeafPolicyFuncAddsRequirementnow include each new role, plus an unlisted ISO arc value that must still be accepted.TestDefaultAttesterLeafPolicyRolesadds a CA restricted to document signing.TestX5CAttesterChainRefusesReissuedAnchorInX5C(re-issued and cross-signed copies refused; same subject with another key accepted).FuzzAttesterLeafPolicy's independent oracle and OID set cover the new list and the subject+key anchor rule.🤖 Generated with Claude Code