Skip to content

fix: preserve external ca issuer encoding - #208

Closed
PekingSpades wants to merge 1 commit into
httptoolkit:mainfrom
PekingSpades:fix/ca-issuer-encoding
Closed

PekingSpades wants to merge 1 commit into
httptoolkit:mainfrom
PekingSpades:fix/ca-issuer-encoding

Conversation

@PekingSpades

Copy link
Copy Markdown

An external CA can issue leaf certificates whose issuer text matches the CA subject but whose ASN.1 encoding differs. Go then rejects the chain as signed by an unknown authority. Preserve the parsed CA subject Name when generating the leaf issuer.

Fixes #207.

Complete reproduction: #207. Based on v4.6.3 (10e33d5).

Changes

  • Pass the original subjectName to the certificate generator.
  • Add deterministic regressions using mixed PrintableString/UTF8String names, with CT both enabled and disabled.

Actual before/after comparison

The same newly generated OpenSSL CA and Go 1.27.0 verifier were used for the published 4.6.3 package and this branch.

Check Published 4.6.3 / unmodified source This branch
Go chain verification FAIL: x509: certificate signed by unknown authority PASS: certificate chain verified
OpenSSL verification OK OK
New issuer-encoding regressions 0 passing, 2 failing 2 passing

Using the files from the linked report and a built checkout at ../mockttp:

node certificate-issuer.cjs '' leaf-before.pem
node certificate-issuer.cjs ../mockttp leaf-after.pem
go run verify-certificate.go leaf-before.pem
go run verify-certificate.go leaf-after.pem

Validation

  • npm run build: passed.
  • Browser suite: 340 passing.
  • Full Node suite: 827 passing, 6 pending, 1 failing.
  • Unmodified baseline full Node suite: 823 passing, 6 pending, 3 failing.

The remaining failure was the existing live TLS passthrough test skips the server for non-matching HTTPS requests, with a TLS connection disconnect. The same case also failed on the unmodified baseline.

All validation used Node 24.19.0, with freshly installed upstream dependencies. Browser checks used Chrome Headless 151.0.7922.34. The full Node suite used a local pkimetal service and a 30-second Mocha timeout. Existing live-network outcomes vary between runs; the deterministic regression and loopback comparisons isolate these changes.

Keep the CA's encoded subject when issuing leaf certificates so
clients can link the leaf to trusted CAs with UTF8String names.
Cover both ordinary and certificate-transparency issuance.
@CLAassistant

CLAassistant commented Sep 15, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@pimterry

Copy link
Copy Markdown
Member

Hi @PekingSpades. Opening 3x issues like this and then 3x PRs within 5 minutes on a project where you've never engaged before is spam.

I'm going to block you from the org now, but I'll add some explanation first which maybe you'll find helpful.

Looking at both the speed and the content, these PRs & issues are all clearly AI generated. Using AI is fine (imo) when you're driving it and actively working to fully explore a good solution, carefully reviewing the output, and basically using it as a tool to write better code faster. They can be very useful tools like this.

Finding a few bugs, and then just dropping the descriptions into an agent and opening a PR immediately from the result is not a useful contribution though. Maintainers are quite capable of pasting a bug description into an agent all by themselves. The resulting workflow for a maintainer to prompt an agent themselves is much more efficient than prompting an agent via reviews on a contribution like this. It's not helpful to just act as a proxy for an agent without bringing some substantial work, practical context or deeper understanding that adds value to the process.

Similarly for the issues: I'm not sure if you found these through your own use of Mockttp, or just scanning for issues directly. In the former case, that is valuable feedback, and I would really suggest doing this and sharing more details about your use case and how and why the issue affects you. On the other hand though, blindly scanning for bugs is not really useful. Maintainers are quite capable of scanning for bugs by themselves. Not all bugs are important (it's tradeoffs all the way down) and user bug reports are in large part a feedback mechanism about which bugs are important in practice and why. Reporting issues via scanning & reporting bugs without context loses this information.

I do appreciate some of this: the certificate issue here is a clear bug that wasn't previously known, and I've just fixed that now (see 7acf136). Note that this fix is not the same as this PR though, which only covers one small case as a quick patch rather than a proper solution (e.g. it doesn't fix the line directly above, which has exactly the same shape of bug in a harder to hit form, or the similar issues in SKI/AKI re-derivation).

For the other 2 issues around message bodies, these already known, and there's a larger restructuring of the entire body streaming/buffering mechanism throughout the codebase already happening, so those aren't useful unfortunately.

In future, if you want to get involved with a project I'd suggest opening an issue or two, including context about why it matters, and waiting for discussion first. Alternatively, if you have a good fix and want to open a PR: make sure you've really explored the problem and you're confident the PR is more useful than the maintainers just copying the bug report into an agent themselves. The key is to to engage with projects before dumping a batch of issues & PRs like this. If your issue or PR is just copy-pasting from an agent, don't do it at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

External CA certificates fail Go verification after issuer name re-encoding

3 participants