bneradt commented on code in PR #13919:
URL: https://github.com/apache/trafficserver/pull/13919#discussion_r4428365549
##########
plugins/experimental/ja4_fingerprint/plugin.cc:
##########
@@ -268,6 +270,18 @@ log_fingerprint(JA4_data const *data)
}
}
+// Only QUIC sends the quic_transport_parameters extension (RFC 9001, section
8.2).
+JA4::Protocol
+get_protocol(TSClientHello ch)
+{
+ unsigned char const *buf{};
+ std::size_t buflen{};
+ if (TS_SUCCESS == TSClientHelloExtensionGet(ch,
EXT_QUIC_TRANSPORT_PARAMETERS, &buf, &buflen)) {
Review Comment:
Good point, fixed. JA4's transport now comes from the connection, not from
the ClientHello. Both `ja4_fingerprint` and `jax_fingerprint` check
`TSVConnProtocolStackContains(vconn, "quic")`, a new API that mirrors
`TSHttpSsnClientProtocolStackContains()` for a `TSVConn`. It's needed because
there is no session yet during the TLS handshake hooks. The QUIC NetVC reports
`quic,tls/1.3,udp,ipv4`, and the API is documented in
`TSClientProtocolStack.en.rst` and the release notes.
On the spoofing case: OpenSSL leaves `quic_transport_parameters` out of a
TCP ClientHello's extension list, so a TCP client couldn't get a `q` on OpenSSL
builds anyway. BoringSSL lists every extension on the wire, though, so the
extension check could be spoofed there, and checking the connection type is the
right fix either way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
