fix: harden GeneralizedTime to whole seconds in cert/CRL generators (#122) - #142
Open
crossi-dev wants to merge 1 commit into
Open
Conversation
…5280 RFC 5280 §4.1.2.5.2 mandates that GeneralizedTime values in X.509 certificates MUST NOT include fractional seconds. When a caller passes a Date with non-zero milliseconds and notAfter falls after 2049 (the GeneralizedTime boundary), asn1js encodes the milliseconds as a fractional suffix (e.g. 20700101000000.500Z), which OpenSSL rejects with 'format error in certificate notAfter field'. Apply truncation to whole seconds at the x509 layer in both X509CertificateGenerator.create() and X509CrlGenerator.create() before constructing ASN.1 Validity/Time objects. Truncation uses Math.floor(d.getTime()/1000)*1000 so the caller's Date is never mutated. Also bump the @peculiar/asn1-x509 minimum from ^2.6.0 to ^2.6.1, the version where the same fix was applied at the dependency level (defense in depth so either layer is sufficient on its own). Closes PeculiarVentures#122
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
1 participant
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.
Re #122
You noted the fix landed in
@peculiar/asn1-x509but thatx509itself hadn't been hardened yet — so this does two things:1. Bump the floor.
@peculiar/asn1-x509^2.6.0→^2.6.1, so a fresh install can't resolve a pre-fix version of the dependency.2. Defense-in-depth in the generators. Truncate
notBefore/notAfter(and the CRL'sthisUpdate/nextUpdate/revocationDate) to whole seconds before building the ASN.1Validity:Per RFC 5280 §4.1.2.5 (
GeneralizedTimeMUST NOT carry fractional seconds), sox509emits compliant DER regardless of whichasn1-x509actually resolves, or whether a caller passes aDatewith non-zero ms. The caller'sDateobjects aren't mutated.Why it matters
A
notAfterafter 2049 is encoded asGeneralizedTime; with non-zero ms the old path produced20700101000000.500Z, which OpenSSL rejects (error 14). Fresh installs already dodge this via the dep, but thex509layer itself stays correct now too.Verified
New
#122regression test:notAfterin 2070 withms=500→ asserts noGeneralizedTimebyte contains., decoded ms is 0, and the caller'sDateis untouched.I bumped the dep floor to
^2.6.1rather than pinning — want it tighter, or is the caret range what you'd prefer here? Either's a one-liner.(I'm with Choreless — we fix and ship for small teams — but no strings, x509 is just a lib I lean on.) — Charles