[Web] Fix non-deterministic TLS bridge failures from non-minimal DER serial numbers - #3550
Merged
Conversation
The TLS-over-fetch bridge synthesizes X.509 certs in the browser. The ASN.1 INTEGER encoder prepended a 0x00 byte when needed to keep an integer positive, but never stripped redundant leading 0x00 bytes. With a 4-byte random serial number, roughly 1 session in 256 generated a serial starting with 0x00 followed by a byte under 0x80 — invalid DER per X.690 §8.3.2. OpenSSL bails out with "asn1 encoding routines:c2i_ibuf:illegal padding", which surfaces in PHP as cURL error 35 on every HTTPS request for the rest of the session. Strip the unnecessary leading zeros before applying the existing sign-bit prepend, and add a regression test that pins a problematic serial.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes intermittent TLS bridge failures caused by invalid DER encoding of synthesized X.509 certificate serial numbers (non-minimal ASN.1 INTEGER encoding).
Changes:
- Update ASN.1 INTEGER encoding to strip redundant leading
0x00bytes while preserving positive sign encoding rules. - Add an integration-style regression test that generates a cert with an “offending” serial number and verifies OpenSSL can parse it.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/php-wasm/web/src/lib/tls/certificates.ts | Ensures DER-minimal INTEGER encoding for serial numbers by removing redundant leading zero padding. |
| packages/php-wasm/web/src/lib/tls/certificates.spec.ts | Adds an OpenSSL round-trip regression test for the previously failing serial-number byte pattern. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Sync the inline comment in the new spec with the actual openssl command (-noout -serial).
adamziel
changed the title
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.
What
The TLS-over-fetch bridge synthesizes X.509 certs in the browser so PHP's libcurl can talk to real HTTPS hosts through
fetch(). The ASN.1 INTEGER encoder incertificates.tswas prepending a0x00byte when needed to keep a value positive, but it never stripped redundant leading0x00bytes — so the encoded serial number sometimes wasn't valid DER.With a 4-byte random serial, about 1 session in 256 produced something like
[0x00, 0x12, 0x34, 0x56], encoded as02 04 00 12 34 56. That violates DER's minimal-length rule (X.690 §8.3.2), and OpenSSL refuses to parse the cert with:Once a session generated a bad cert, every HTTPS request through the bridge surfaced as
cURL error (35)in PHP until the page was reloaded. The original report blamed wpcomstaging.com, but it was a coincidence — any host could trigger it.Fix
In
ASN1Encoder.integer(), strip leading0x00bytes whose successor has the high bit clear (they're unnecessary), then apply the existing high-bit prepend for sign disambiguation.Test
A new case in
certificates.spec.tspins a serial number with the offending shape and round-trips the cert throughopenssl x509. Failed before the fix, passes after.