| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
`selfsigned` derives the serial number from 9 random bytes and runs them through its own `toPositiveHex()`, which clears the sign bit but does not re-minimise the resulting DER INTEGER. node-forge's encoder strips only one of the redundant leading zero bytes, so roughly 1 in 65536 certificates goes out with a positive INTEGER carrying illegal padding. node-forge parses that back fine, so the `verifyCertificateChain()` check `selfsigned` runs before returning passes and the pair looks valid. OpenSSL refuses it, so it only fails later as ERR_OSSL_ASN1_ILLEGAL_PADDING from the middle of a TLS handshake. Every serial number is drawn independently, so `generate()` now validates the certificate with `X509Certificate` and generates again when OpenSSL will not load it. Three attempts bring the odds down to about 1 in 2.8e14, and it throws rather than ever returning an unusable pair. `install.js` goes through the same path, so the pair written on install cannot be a broken one either. Refs: nodejs/undici#5245
Fixes nodejs#5245. `selfsigned` builds the certificate serial number from 9 random bytes and runs them through its own `toPositiveHex()`, which clears the sign bit but does not re-minimise the resulting DER INTEGER. When those bytes start with 0x80 (or 0x00) followed by 0x00, the serial keeps two redundant leading zero bytes. node-forge strips only one of them -- the "should all leading bytes be stripped vs just one?" TODO in its asn1.js -- so a positive INTEGER with a leading 0x00 followed by a byte under 0x80 reaches OpenSSL, which rejects it with ERR_OSSL_ASN1_ILLEGAL_PADDING. It happens for roughly 1 in 65536 certificates, and `selfsigned`'s own `verifyCertificateChain()` does not catch it because node-forge's parser accepts the non-minimal encoding. The certificate only fails once OpenSSL reads it, so every test that builds a TLS/HTTP2 server from a freshly generated pair was flaky, most visibly test/http2-abort.js. The fix landed upstream in metcoder95/https-pem#2: the package now validates each certificate with X509Certificate and generates again when OpenSSL refuses it, for both the async generator and the pair written by the postinstall hook. Nothing is needed on undici's side beyond the bump.
Fixes nodejs#5245. `selfsigned` builds the certificate serial number from 9 random bytes and runs them through its own `toPositiveHex()`, which clears the sign bit but does not re-minimise the resulting DER INTEGER. When those bytes start with 0x80 (or 0x00) followed by 0x00, the serial keeps two redundant leading zero bytes. node-forge strips only one of them -- the "should all leading bytes be stripped vs just one?" TODO in its asn1.js -- so a positive INTEGER with a leading 0x00 followed by a byte under 0x80 reaches OpenSSL, which rejects it with ERR_OSSL_ASN1_ILLEGAL_PADDING. It happens for roughly 1 in 65536 certificates, and `selfsigned`'s own `verifyCertificateChain()` does not catch it because node-forge's parser accepts the non-minimal encoding. The certificate only fails once OpenSSL reads it, so every test that builds a TLS/HTTP2 server from a freshly generated pair was flaky, most visibly test/http2-abort.js. The fix landed upstream in metcoder95/https-pem#2: the package now validates each certificate with X509Certificate and generates again when OpenSSL refuses it, for both the async generator and the pair written by the postinstall hook. Nothing is needed on undici's side beyond the bump.
| Back | FazBrowse Home | New Git URL |
Follow-up to your review on nodejs/undici#5800 — moving the fix here instead of keeping the workaround in undici's test suite.
The bug
selfsigned derives the certificate serial number from 9 random bytes and runs them through its own toPositiveHex():
That makes the integer positive, but it does not re-minimise it, and DER requires minimal encoding. When the 9 random bytes start with 0x80 (or 0x00) followed by 0x00, the serial keeps two redundant leading zero bytes.
node-forge would normally strip those, except it only strips one, and says so in lib/asn1.js:
So 00 00 01 … becomes 00 01 …, still non-minimal. A positive INTEGER with a leading 0x00 followed by a byte under 0x80 is illegal padding, and OpenSSL rejects it:
It takes two or more zero bytes followed by a byte under 0x80, which needs b0 ∈ {0x00, 0x80}, b1 == 0x00 and b2 < 0x80: roughly 1 in 65536 certificates.
The reason it gets out the door is that selfsigned runs verifyCertificateChain() before returning and it passes — node-forge's own parser accepts the non-minimal encoding it just produced. The certificate only fails once OpenSSL reads it, from the middle of a TLS handshake, which is why it showed up as an unexplained flake in undici (nodejs/undici#5245) rather than as a generation error.
What this changes
New generate.js wraps selfsigned: it validates the certificate with X509Certificate and generates again if OpenSSL refuses it. Each attempt draws an independent serial number, so three attempts bring it down to about 1 in 2.8e14, and it throws rather than ever returning an unusable pair.
Both entry points go through it:
selfsigned is still required lazily, so require('@metcoder95/https-pem') stays as cheap as it was.
Tests
tests/index.js stubs forge.random.getBytesSync with the pathological seed, so the failure is deterministic instead of 1 in 65536. Four of the new tests fail on main and pass here; the node-forge devDependency is for that stub only.
The first test asserts the upstream behaviour and reports a diagnostic instead of failing if selfsigned ever stops emitting these, so it will say when generate.js can be deleted.
Upstream
The real fix belongs in selfsigned, emitting a minimally encoded serial:
Finishing the node-forge TODO so it strips every redundant leading byte would fix the whole class of it. Happy to open either of those too — this is the workaround in the meantime.