feat(x509): expose ASN.1 parse options and raise the default node limit - #2926
feat(x509): expose ASN.1 parse options and raise the default node limit#2926hacdias wants to merge 1 commit into
Conversation
Signed-off-by: Henrique Dias <mail@hacdias.com>
🦋 Changeset detectedLatest commit: c28aed8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@TimoGlastra I'm wondering whether or not we should wait for the linked PRs. The last version addresses the case of parsing certificates, CRLs, etc. However, the x509 package also does some back and forth parsing inside the constructors and generators, and doesn't take any parsing options (I would argue that parsing options don't belong on a creator), so the PRs I opened there bypass the restrictions when creating / generating. This means that with this PR it won't be possible to create CRLs/certificates with more than 10 000 ASN.1 nodes (parser default). I'd rather wait, but I have no idea how long it'll take. The last PRs took longer than 3 months and if we want to wait we need to pin all versions. Another option is to patch the dependency in order to bypass the restrictions for now during the creation/generation code paths, which, in my view, are trusted paths. |
| // `@peculiar/asn1-x509` instead. Each build keeps its own schema registry, so a value created | ||
| // here has no schema over there. Serializing keeps the values and their schema within a single | ||
| // instance, as the other extensions below do. | ||
| const value = AsnConvert.serialize(new CRLDistributionPoints([distributionPoint])) |
| serialNumber: options.serialNumber, | ||
| extensions: [new x509.CRLDistributionPointsExtension(distributionPoints)], | ||
| // Built from DER rather than `x509.CRLDistributionPointsExtension`, see | ||
| // `createCrlDistributionPointsExtension`. |
There was a problem hiding this comment.
This is quite unfortunate and brittle
|
We could wait a bit and see how quickly the PRs get merged. Patches don't really work for published packages, as all dependants om Credo wouldn't have them. |
|
Let's wait a bit then. |
2 participants
Updates
@peculiar/*packages, which enforce the asn1js parse limits. The 10000 node default rejects any CRL over ~3300 entries whilefetchCrldownloads up to 10 MB, and an unparseable CRL counts as reachable-but-invalid, soSoftFaildoes not tolerate it: a CA with a large CRL would hard-fail chain validation in every mode butDisabled.X509ParseOptionson the parsing factories, on theX509Serviceand revocation methods that parse, and asparseOptionson the module.defaultX509ParseOptionswithmaxNodesat 1500000, sized to that 10 MB cap. OnX509ModuleConfigfor what resolves through the config, and as a default parameter on the six factories, which mdoc, sd-jwt, dcql and openid4vc call directly.X509Service.parseCertificateSigningRequestnow takes anagentContextfirst, like every other method on it.X509Apiis unchanged.@peculiar/asn1-x5092.9.4 added anexportsmap and resolves to ESM,@peculiar/x509declares none and resolves to CommonJS, so the two get separate schema registries and the lookup fails.Creating large CRLs is still capped, the generators re-parse their own output under the default limits. That is the trusted path, fixed upstream in PeculiarVentures/x509#150 and PeculiarVentures/asn1-schema#156.