From c098f1742bb04fbe41fc6cf492cd334efef734eb Mon Sep 17 00:00:00 2001 From: srdusr <99972264+srdusr@users.noreply.github.com> Date: Tue, 26 May 2026 09:16:00 +0200 Subject: Add deeper TLS (ServerHello, ALPN); fix a QUIC/TCP false-positive bug TLS: ServerHello now reports the negotiated version and cipher suite alongside the existing ClientHello SNI support, plus ClientHello's ALPN extension. ServerHello's version prefers the supported_versions extension over legacy_version when present - TLS 1.3 always sets legacy_version to 0x0303 for middlebox compatibility, so reading only that field would misreport every real TLS 1.3 connection as 1.2. Cipher suite names are hardcoded only for TLS 1.3's five suites (a small closed set); everything else reports as raw hex rather than a guessed name from a "common suites" list. Live-verifying that against a real Cloudflare TLS 1.3 handshake surfaced a real, unrelated bug in the QUIC dissector added earlier: it was also being tried against TCP port-443 payloads (a side effect of the earlier L7Registry port-sharing fix), and produced false "QUIC" labels on TLS ciphertext continuation fragments - large encrypted records split across multiple TCP segments, each fed to the parser independently since this project doesn't reassemble by default, so a later fragment's effectively random bytes occasionally passed as a plausible QUIC header. Fixed in two layers: parse_quic() now enforces RFC 9000's real 20-byte cap on connection ID lengths, closing most of the long-header false- positive surface; and L7Dissector gained a transport() method (defaulting to kAny, so every other dissector's behavior is unchanged) so QuicDissector can declare itself UDP-only - necessary because the length cap alone can't touch QUIC's short-header form, which by design has no structural signal beyond one bit once header protection can't be removed without connection state. Re-verified against the identical live scenario afterward: zero false QUIC labels on the same Cloudflare TCP handshake, and a repeat of the earlier real HTTP/3 capture confirmed genuine QUIC still decodes correctly on UDP. --- tests/test_quic.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'tests/test_quic.cpp') diff --git a/tests/test_quic.cpp b/tests/test_quic.cpp index 2366176..4b02be5 100644 --- a/tests/test_quic.cpp +++ b/tests/test_quic.cpp @@ -78,6 +78,25 @@ TEST_CASE("parse_quic rejects a long-header packet whose DCID length exceeds the CHECK_FALSE(parse_quic(bytes).has_value()); } +TEST_CASE("parse_quic rejects a DCID/SCID length past RFC 9000's 20-byte cap even with room in " + "the buffer") { + // A real protocol bound, not a buffer-size check: plenty of bytes + // are available here, the claimed length is just illegal for this + // QUIC version. This is what actually stops a mid-record TLS + // ciphertext continuation fragment (effectively random bytes to + // this parser, since this project doesn't reassemble TCP by + // default) from occasionally passing as a plausible QUIC header -- + // found via live capture against real cloudflare.com traffic, not + // by inspection. + std::vector bytes = {0xC0, 0x00, 0x00, 0x00, 0x01, 21}; + bytes.resize(bytes.size() + 21, 0xAA); // plenty of room for a 21-byte DCID + CHECK_FALSE(parse_quic(bytes).has_value()); + + std::vector scid_bytes = {0xC0, 0x00, 0x00, 0x00, 0x01, 0, 21}; + scid_bytes.resize(scid_bytes.size() + 21, 0xAA); // plenty of room for a 21-byte SCID + CHECK_FALSE(parse_quic(scid_bytes).has_value()); +} + TEST_CASE("QuicDissector claims port 443 and formats an Initial packet") { QuicDissector dissector; CHECK(dissector.port() == kQuicPort); -- cgit v1.2.3