Repository navigation
Conversation
This was referenced Oct 17, 2026
This branch has not been deployed
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.
Fixes #200.
What was wrong.
iq_record_subjectkept a client certificate's CN whenever it was non-empty, fitpeer_cn, and held no NUL. It had no control-character check.TlsSession.ExtractCommonNamerefuses any control character on TCP. So the same certificate had no name over TCP but was named over QUIC, andQuicEngineConnection.PeerCommonName, the value documented for authorizing on, could carry a CR/LF into whatever the handler logged next.What changed. In
iq_record_subject, a newiq_has_controlcheck replaces the NUL test.How the rule matches TCP's:
ASN1_STRING_to_UTF8, in the same BoringSSL (5b83e5d). That function decodes every string type with a validating decoder (a T61String is read as Latin-1) and re-encodes it, so both stacks see the same valid UTF-8.char.IsControl, which is exactly U+0000-U+001F and U+007F-U+009F.memchrwent.peer_subjectis unchanged:X509_NAME_onelineescapes these bytes.PeerCommonName's doc now lists control characters among the reasons it is null.Native build.
libioxide_ngtcp2.sowas rebuilt withscripts/build-ngtcp2-native.shat its pinned refs (ngtcp2 7f7e24b, picotls f07f1c8, BoringSSL 5b83e5d), using the script's defaultWORK..textgrew by 64 bytes and.eh_frameshrank by 8.Tests. New, in E2E
QuicIdentityCapTests, next to the CN-cap tests:mtls/quic: a common name carrying CRLF is refused, and the subject escapes it. This is the Tls suite'sIdentitySubjectTeststest of the same name, run over QUIC. The CN isalice\r\nauthorized=root.PeerCommonNameis null, andPeerSubjectcarriesalice\x0D\x0Aauthorized=root.mtls/quic: a common name is refused for exactly the characters TCP refuses. Eight CNs, on both sides of each edge:The expected verdict comes from
char.IsControl, TCP's own predicate.The ordinary-CN control is the existing
mtls/quic: control - a client whose name fits is named by both the subject and the CN.Helpers:
SignedByTestCaalso takes anX500DistinguishedName.CommonNamedbuilds a UTF8String CN that a subject string could not spell.Confirmed both ways:
FAIL mtls/quic: a common name carrying CRLF is refused, and the subject escapes it: a CN carrying CR/LF must not be reported as an identity, got: alice\u000D\u000Aauthorized=root.FAIL ... refused for exactly the characters TCP refuses: tab\u0009here: the TCP rule gives no name, QUIC read tab\u0009here; .... The same failure appears for ESC, DEL, U+0085 and U+009F. The three rows that should be read back already matched.Suites.
Overlap with #376 (the fix for #349). That PR also rebuilds
libioxide_ngtcp2.so.git merge-treeof the two branches auto-merges the shim (the result carries both fixes and compiles), and only the .so conflicts, since it is binary. Whichever PR merges second needsscripts/build-ngtcp2-native.shrerun on the merged shim, and the resulting .so committed.All eight together. This batch's PRs (#376 to #383) were merged onto main 71aab91 in one scratch tree. The only conflict is the binary
libioxide_ngtcp2.sobetween #376 and #377, rebuilt there from the merged shim with the script's defaults. Everything builds, and Unit 102, E2E 309, Http 88, Chaos 80, Tls 169 (14 kTLS cases skipped) and File 5 pass, with 0 failed and 0 pending. The repo's only pending test left is the Redis suite's (#336).Noticed, not changed.