diff options
| author | srdusr <[email protected]> | 2026-05-26 09:16:00 +0200 |
|---|---|---|
| committer | srdusr <[email protected]> | 2026-05-26 09:16:00 +0200 |
| commit | c098f1742bb04fbe41fc6cf492cd334efef734eb (patch) | |
| tree | 8af2c79ec321357d2766fdab03d6a870ff7ceb6c /tests | |
| parent | 2d010c9f851ea4eb851179db012c8977ce6e4bd5 (diff) | |
| download | packeteer-c098f1742bb04fbe41fc6cf492cd334efef734eb.tar.gz packeteer-c098f1742bb04fbe41fc6cf492cd334efef734eb.zip | |
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.
Diffstat (limited to 'tests')
| -rw-r--r-- | tests/test_dissector.cpp | 37 | ||||
| -rw-r--r-- | tests/test_quic.cpp | 19 | ||||
| -rw-r--r-- | tests/test_tls.cpp | 115 |
3 files changed, 165 insertions, 6 deletions
diff --git a/tests/test_dissector.cpp b/tests/test_dissector.cpp index 9dd4135..4517ae7 100644 --- a/tests/test_dissector.cpp +++ b/tests/test_dissector.cpp @@ -23,9 +23,10 @@ private: class AlwaysMatchesDissector : public L7Dissector { public: - AlwaysMatchesDissector(std::uint16_t port, std::string label) - : port_(port), label_(std::move(label)) {} + AlwaysMatchesDissector(std::uint16_t port, std::string label, Transport transport = Transport::kAny) + : port_(port), label_(std::move(label)), transport_(transport) {} std::uint16_t port() const override { return port_; } + Transport transport() const override { return transport_; } std::optional<std::string> summarize(std::span<const unsigned char>) const override { return label_; } @@ -33,6 +34,7 @@ public: private: std::uint16_t port_; std::string label_; + Transport transport_; }; } // namespace @@ -46,7 +48,7 @@ TEST_CASE("L7Registry falls through to a later dissector on the same port " registry.add(&tls_like); registry.add(&quic_like); - auto result = registry.dissect(443, {}); + auto result = registry.dissect(443, Transport::kTcp, {}); REQUIRE(result.has_value()); CHECK(*result == "QUIC something"); } @@ -59,7 +61,7 @@ TEST_CASE("L7Registry still returns the first dissector to succeed, not the last registry.add(&first); registry.add(&second); - auto result = registry.dissect(80, {}); + auto result = registry.dissect(80, Transport::kTcp, {}); REQUIRE(result.has_value()); CHECK(*result == "first"); } @@ -70,5 +72,30 @@ TEST_CASE("L7Registry returns nullopt when no dissector on the port matches") { L7Registry registry; registry.add(&only); - CHECK_FALSE(registry.dissect(443, {}).has_value()); + CHECK_FALSE(registry.dissect(443, Transport::kTcp, {}).has_value()); +} + +TEST_CASE("L7Registry skips a dissector whose declared transport doesn't match, even on the " + "right port") { + // The actual fix for QUIC's false positives on TCP:443: a + // dissector that only claims UDP must never be tried against a + // TCP payload on the same port, regardless of what its own + // summarize() would have returned. + AlwaysMatchesDissector udp_only(443, "UDP thing", Transport::kUdp); + + L7Registry registry; + registry.add(&udp_only); + + CHECK_FALSE(registry.dissect(443, Transport::kTcp, {}).has_value()); + CHECK(registry.dissect(443, Transport::kUdp, {}).has_value()); +} + +TEST_CASE("L7Registry's default kAny transport matches either TCP or UDP") { + AlwaysMatchesDissector any(53, "DNS-like"); // default transport: kAny + + L7Registry registry; + registry.add(&any); + + CHECK(registry.dissect(53, Transport::kTcp, {}).has_value()); + CHECK(registry.dissect(53, Transport::kUdp, {}).has_value()); } 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<unsigned char> 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<unsigned char> 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); diff --git a/tests/test_tls.cpp b/tests/test_tls.cpp index 7a3ad27..bdb5300 100644 --- a/tests/test_tls.cpp +++ b/tests/test_tls.cpp @@ -22,7 +22,8 @@ void append_be16(std::vector<unsigned char>& out, std::uint16_t v) { // length instead of the real one (computed here, not via post-hoc // offset math into the finished buffer - equally fragile). std::vector<unsigned char> build_client_hello(const std::string& sni, - bool corrupt_sni_ext_len = false) { + bool corrupt_sni_ext_len = false, + std::vector<std::string> alpn_protocols = {}) { std::vector<unsigned char> body; body.push_back(0x03); body.push_back(0x03); // client_version: TLS 1.2 @@ -52,6 +53,20 @@ std::vector<unsigned char> build_client_hello(const std::string& sni, append_be16(extensions, ext_len); extensions.insert(extensions.end(), sni_ext_data.begin(), sni_ext_data.end()); } + if (!alpn_protocols.empty()) { + std::vector<unsigned char> protocol_list; + for (const auto& proto : alpn_protocols) { + protocol_list.push_back(static_cast<unsigned char>(proto.size())); + protocol_list.insert(protocol_list.end(), proto.begin(), proto.end()); + } + std::vector<unsigned char> alpn_ext_data; + append_be16(alpn_ext_data, static_cast<std::uint16_t>(protocol_list.size())); + alpn_ext_data.insert(alpn_ext_data.end(), protocol_list.begin(), protocol_list.end()); + + append_be16(extensions, kTlsExtensionAlpn); + append_be16(extensions, static_cast<std::uint16_t>(alpn_ext_data.size())); + extensions.insert(extensions.end(), alpn_ext_data.begin(), alpn_ext_data.end()); + } append_be16(body, static_cast<std::uint16_t>(extensions.size())); body.insert(body.end(), extensions.begin(), extensions.end()); @@ -73,6 +88,47 @@ std::vector<unsigned char> build_client_hello(const std::string& sni, return record; } +// Builds a real, well-formed TLS record containing a ServerHello. +// `negotiated_version`, when set, adds a supported_versions extension +// carrying that single 2-byte version - how a TLS 1.3 ServerHello +// actually signals its real version, since legacy_version is always +// 0x0303 there for middlebox compatibility. +std::vector<unsigned char> build_server_hello(std::uint16_t cipher_suite, + std::optional<std::uint16_t> negotiated_version = {}) { + std::vector<unsigned char> body; + append_be16(body, 0x0303); // legacy_version: always TLS 1.2 on the wire + body.insert(body.end(), 32, 0x00); // random + body.push_back(0x00); // legacy_session_id_echo length: 0 + append_be16(body, cipher_suite); + body.push_back(0x00); // legacy_compression_method: null + + std::vector<unsigned char> extensions; + if (negotiated_version) { + append_be16(extensions, kTlsExtensionSupportedVersions); + append_be16(extensions, 2); + append_be16(extensions, *negotiated_version); + } + append_be16(body, static_cast<std::uint16_t>(extensions.size())); + body.insert(body.end(), extensions.begin(), extensions.end()); + + std::vector<unsigned char> handshake; + handshake.push_back(kTlsHandshakeTypeServerHello); + std::uint32_t hs_len = static_cast<std::uint32_t>(body.size()); + handshake.push_back(static_cast<unsigned char>((hs_len >> 16) & 0xFF)); + handshake.push_back(static_cast<unsigned char>((hs_len >> 8) & 0xFF)); + handshake.push_back(static_cast<unsigned char>(hs_len & 0xFF)); + handshake.insert(handshake.end(), body.begin(), body.end()); + + std::vector<unsigned char> record; + record.push_back(kTlsContentTypeHandshake); + record.push_back(0x03); + record.push_back(0x03); + append_be16(record, static_cast<std::uint16_t>(handshake.size())); + record.insert(record.end(), handshake.begin(), handshake.end()); + + return record; +} + } // namespace TEST_CASE("parse_tls_client_hello extracts a real SNI extension") { @@ -130,3 +186,60 @@ TEST_CASE("TlsSniDissector claims port 443 and its summary matches parse_tls_cli CHECK(summary->substr(0, 3) == "TLS"); CHECK(summary->find("SNI=packeteer.test") != std::string::npos); } + +TEST_CASE("parse_tls_client_hello extracts ALPN protocols in order") { + auto record = build_client_hello("example.com", false, {"h2", "http/1.1"}); + auto hello = parse_tls_client_hello(record); + REQUIRE(hello.has_value()); + REQUIRE(hello->alpn_protocols.has_value()); + REQUIRE(hello->alpn_protocols->size() == 2); + CHECK((*hello->alpn_protocols)[0] == "h2"); + CHECK((*hello->alpn_protocols)[1] == "http/1.1"); +} + +TEST_CASE("parse_tls_client_hello leaves alpn_protocols unset when the extension is absent") { + auto record = build_client_hello("example.com"); + auto hello = parse_tls_client_hello(record); + REQUIRE(hello.has_value()); + CHECK_FALSE(hello->alpn_protocols.has_value()); +} + +TEST_CASE("parse_tls_server_hello extracts the negotiated cipher suite") { + auto record = build_server_hello(0x1301); // TLS_AES_128_GCM_SHA256 + auto hello = parse_tls_server_hello(record); + REQUIRE(hello.has_value()); + CHECK(hello->cipher_suite == 0x1301); +} + +TEST_CASE("parse_tls_server_hello uses legacy_version when there's no supported_versions extension") { + auto record = build_server_hello(0xC02F); // a TLS 1.2-era cipher suite + auto hello = parse_tls_server_hello(record); + REQUIRE(hello.has_value()); + CHECK(hello->negotiated_version == 0x0303); // TLS 1.2 +} + +TEST_CASE("parse_tls_server_hello prefers supported_versions over legacy_version for TLS 1.3") { + auto record = build_server_hello(0x1301, std::uint16_t{0x0304}); + auto hello = parse_tls_server_hello(record); + REQUIRE(hello.has_value()); + CHECK(hello->negotiated_version == 0x0304); // TLS 1.3, not the legacy 0x0303 on the wire +} + +TEST_CASE("parse_tls_server_hello rejects a ClientHello handshake type") { + auto record = build_client_hello("example.com"); + CHECK_FALSE(parse_tls_server_hello(record).has_value()); +} + +TEST_CASE("tls_cipher_suite_name names all five TLS 1.3 suites and falls back to hex otherwise") { + CHECK(tls_cipher_suite_name(0x1301) == "TLS_AES_128_GCM_SHA256"); + CHECK(tls_cipher_suite_name(0x1305) == "TLS_AES_128_CCM_8_SHA256"); + CHECK(tls_cipher_suite_name(0xC02F) == "0xc02f"); +} + +TEST_CASE("TlsSniDissector formats a ServerHello with version and cipher") { + TlsSniDissector dissector; + auto record = build_server_hello(0x1301, std::uint16_t{0x0304}); + auto summary = dissector.summarize(record); + REQUIRE(summary.has_value()); + CHECK(*summary == "TLS ServerHello version=TLS1.3 cipher=TLS_AES_128_GCM_SHA256"); +} |