tls_inspector: add optional protocol parameter to JA4Fingerprinter::create - #46659
Open
dangle1 wants to merge 3 commits into
Open
tls_inspector: add optional protocol parameter to JA4Fingerprinter::create#46659dangle1 wants to merge 3 commits into
dangle1 wants to merge 3 commits into
Conversation
…reate Per the JA4 spec, the first character of a JA4 fingerprint identifies the transport carrying the handshake: `t` for TLS, `q` for QUIC, and `d` for DTLS. Envoy's implementation currently hard-codes `t` with a comment noting that only TLS is handled. Add a public `JA4Fingerprinter::Protocol` enum and a defaulted second argument to `JA4Fingerprinter::create` so callers can select the transport they observed. The default (`Protocol::TLS`) preserves the exact prior behavior of every existing caller, including the built-in TLS inspector listener filter, whose call site is deliberately left byte-identical -- that filter only ever sees stream TLS handshakes today, so it has no new information to convey. A downstream deployment that terminates QUIC (or a future in-tree QUIC listener filter that wants JA4) can now pass `Protocol::QUIC` and produce spec-conformant `q...` fingerprints without touching the JA4 library. Adds a unit test that exercises all three protocol values plus the no-arg overload, and asserts that the protocol argument controls only the first character of the fingerprint. Spec reference: https://github.com/FoxIO-LLC/ja4/blob/main/technical_details/JA4.md Note: this change was drafted with AI assistance and reviewed by the author. Signed-off-by: dangle <dangle@pinterest.com>
dangle1
had a problem deploying
to
external-contributors
August 11, 2026 22:15 — with 
GitHub Actions
Error
GitHub Actions
Error
|
Hi @dangle1, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
agrawroh
reviewed
Aug 11, 2026
agrawroh
left a comment
agrawroh
left a comment
Member
There was a problem hiding this comment.
Could you please also add some more integration tests for this? We have a suite which replays the TCP handshake captures and make sure that we see expected JA4 values.
…eter
Extend the ``JA4Fingerprinter::Protocol`` parameter change with an
integration test that drives the full wire-bytes -> parse -> fingerprint
pipeline:
Decode Browser-1's captured Chrome-stable ClientHello (the same hex used
by tls_inspector_ja4_test.cc's JA4_TEST_VECTORS baseline) through
BoringSSL's SSL_parse_client_hello, then call JA4Fingerprinter::create
with each Protocol value and assert:
- the no-arg default and Protocol::TLS produce the pinned Browser-1
fingerprint from tls_inspector_ja4_test.cc, byte-identical, proving
the default preserves prior behavior end-to-end;
- Protocol::QUIC and Protocol::DTLS change only the first character
(to 'q' and 'd' respectively) and leave every other byte of the
fingerprint unchanged, per the JA4 spec.
Requires adding an ssl external dep and //source/common/ssl:ssl_lib
to ja4_fingerprint_test's BUILD deps for SSL_parse_client_hello and the
SSL_SELECT macro.
Note: this change was drafted with AI assistance and reviewed by the author.
Signed-off-by: dangle <dangle@pinterest.com>
dangle1
had a problem deploying
to
external-contributors
August 13, 2026 18:35 — with
GitHub Actions
Error
GitHub Actions
Error
Reviewers asked for integration coverage in the suite that replays captured TCP-handshake ClientHellos and asserts expected JA4 values -- namely `tls_inspector_ja4_test.cc`. This commit adds a second parameterized suite, `TlsInspectorJA4ProtocolArgTest`, that iterates over the same `JA4_TEST_VECTORS` corpus as `TlsInspectorJA4Test` and, for each captured ClientHello: - decodes the wire bytes and parses the ClientHello via `SSL_parse_client_hello` (the same parse path the listener filter drives via BoringSSL's ssl-early callback), - calls `JA4Fingerprinter::create` with the no-arg default and with each `Protocol` value, and - asserts that the default and `Protocol::TLS` match the corpus's pinned expected fingerprint byte-for-byte, and that `Protocol::QUIC` and `Protocol::DTLS` change only the first character. This reuses the reviewer-approved capture corpus without wiring the new argument into `tls_inspector.cc`, which is deliberately kept byte-identical per the PR's contract (the listener filter has no way to distinguish transports today, so any per-transport selection would be speculative). Note: this change was drafted with AI assistance and reviewed by the author. Signed-off-by: dangle <dangle@pinterest.com>
dangle1
requested a deployment
to
external-contributors
August 13, 2026 18:41 — with
GitHub Actions
Waiting
GitHub Actions
Waiting
Author
|
Hi @agrawroh, I've added integration tests. Please let me know if they are sufficient. |
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
3 participants
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.
Summary
Per the JA4 spec, the first character of a JA4 fingerprint identifies the transport carrying the handshake:
tfor TLS,qfor QUIC,dfor DTLS. Envoy's implementation currently hard-codestwith a comment noting that only TLS is handled.This change adds a public
JA4Fingerprinter::Protocolenum and a defaulted second argument toJA4Fingerprinter::createso callers can select the transport they observed.Protocol::TLS) preserves the exact prior behavior of every existing caller, including the built-in TLS inspector listener filter — its call site is deliberately left byte-identical, since that filter only sees stream TLS handshakes today and has no new information to convey.Protocol::QUICand produce spec-conformantq...fingerprints without touching the JA4 library.Design notes
Protocolis nested insideJA4Fingerprinterand scopedenum class : char. This is a static utility class, so nesting keeps the enum tightly bound to its only consumer.switchin an anonymous namespace rather than treating the enum value as a rawchar. That's easier to read and makes any future non-single-character protocol identifier a compile-time obligation rather than an implicit reinterpretation.Test plan
TEST(JA4Fingerprinter, ProtocolParameterControlsFirstCharacter)inja4_fingerprint_test.ccthat:create(&hello)is byte-identical tocreate(&hello, Protocol::TLS).Protocol::QUICandProtocol::DTLSproduce fingerprints starting withqanddrespectively.bazel test //test/extensions/filters/listener/tls_inspector:ja4_fingerprint_testpasses locally.git diff origin/main -- source/extensions/filters/listener/tls_inspector/tls_inspector.ccis empty — no existing call site changed.Notes
tls_inspector_ja4_test.ccwere computed under the currentt-only default; since the default is preserved they remain valid without modification.