jax_fingerprint: Address user arg slot exhaustion#13082
jax_fingerprint: Address user arg slot exhaustion#13082bneradt wants to merge 1 commit intoapache:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes ATS user-arg slot exhaustion when loading multiple jax_fingerprint.so instances by sharing a single user-arg slot per container type (vconn / txn) and storing per-method JAxContext objects in a shared ContextMap.
Changes:
- Rework user-arg handling to reserve one shared slot per type and multiplex contexts by method name via
ContextMap. - Update close-hook cleanup to remove per-method context and delete the shared map when empty.
- Add a gold test + replay YAML that loads JA3/JA4/JA4H simultaneously and verifies all fingerprint headers are produced.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/gold_tests/pluginTest/jax_fingerprint/jax_fingerprint.test.py | Adds an “all methods loaded” integration test run. |
| tests/gold_tests/pluginTest/jax_fingerprint/jax_fingerprint_all_methods.replay.yaml | New replay verifying all method headers are present on a TLS request. |
| plugins/experimental/jax_fingerprint/userarg.h | Adds cleanup API declaration (currently duplicated). |
| plugins/experimental/jax_fingerprint/userarg.cc | Implements shared-slot reservation and ContextMap-backed set/get/cleanup. |
| plugins/experimental/jax_fingerprint/plugin.cc | Switches close handlers to the new cleanup function. |
| plugins/experimental/jax_fingerprint/context_map.h | New shared map container keyed by method name. |
When multiple jax_fingerprint.so instances are loaded (e.g., one per fingerprinting method), each instance was reserving its own user arg slot. ATS has a limited number of slots (~4 per type), causing methods like JA3 and JA4 to fail silently when slots were exhausted. Solution: Share a single user arg slot per type (TS_USER_ARGS_VCONN or TS_USER_ARGS_TXN) across all jax_fingerprint instances. A ContextMap stores JAxContext instances keyed by method name.
bneradt
force-pushed
the
jax-user-arg-exhaustion-fix-asf
branch
from
0002ae0 to
ff9dd8e
Compare
| ContextStorage::iterator | ||
| find_context(std::string_view method_name) | ||
| { | ||
| #ifdef __cpp_lib_generic_unordered_lookup | ||
| return m_contexts.find(method_name); | ||
| #else | ||
| return m_contexts.find(std::string{method_name}); | ||
| #endif |
There was a problem hiding this comment.
ContextMap uses __cpp_lib_generic_unordered_lookup to choose the heterogeneous unordered_map::find fast-path, but this header doesn't include <version>, so that feature-test macro may be undefined even when the library supports it (causing the fallback path to be used unconditionally). Include <version> here (or avoid the macro and rely on transparent hash/equal + overload availability) to make the intent deterministic.
| slots via a ContextMap. This test verifies that all methods produce | ||
| fingerprints when sharing the SSL_CLIENT_HELLO_HOOK. |
There was a problem hiding this comment.
The docstring claims all methods share the SSL_CLIENT_HELLO_HOOK, but JA4H is a request-based method (no client-hello hook). Consider rewording to reflect that the test exercises multiple methods sharing user-arg slots (vconn for JA3/JA4, txn for JA4H) rather than implying a single shared hook.
| slots via a ContextMap. This test verifies that all methods produce | |
| fingerprints when sharing the SSL_CLIENT_HELLO_HOOK. | |
| slots via a ContextMap. This test verifies that JA3/JA4 and JA4H can | |
| coexist and produce fingerprints while sharing that context machinery | |
| across their respective vconn and txn storage. |
| # Load multiple methods - all share the same user arg slot via ContextMap. | ||
| self._ts.Disk.plugin_config.AddLines( | ||
| [ | ||
| 'jax_fingerprint.so --method JA3 --header x-ja3 --standalone', | ||
| 'jax_fingerprint.so --method JA4 --header x-ja4 --standalone', | ||
| 'jax_fingerprint.so --method JA4H --header x-ja4h --standalone', | ||
| ]) |
There was a problem hiding this comment.
This test is intended to validate the user-arg slot sharing fix, but as written it only asserts that headers are present when multiple methods are loaded. That would also pass even if each method still reserved its own slot (as long as slots aren’t exhausted). Consider adding an assertion that the shared slot reservation happens once per type (e.g., via debug log expectations), or otherwise constructing the test so it would fail when additional per-method slot reservations occur.
| # The jax_fingerprint plugin is loaded multiple times with different methods. | ||
| # All TLS-based fingerprint headers should be present and non-empty. | ||
|
|
There was a problem hiding this comment.
The header comment says "All TLS-based fingerprint headers" but this replay asserts x-ja4h as well, which is generated by the JA4H request-based method (not TLS ClientHello based). Consider adjusting the wording to just "all fingerprint headers" (and, if you truly want non-empty, make that explicit in the assertions).
|
[approve ci autest 1] |
When multiple jax_fingerprint.so instances are loaded (e.g., one per fingerprinting method), each instance was reserving its own user arg slot. ATS has a limited number of slots (~4 per type), causing methods like JA3 and JA4 to fail silently when slots were exhausted.
Solution: Share a single user arg slot per type (TS_USER_ARGS_VCONN or TS_USER_ARGS_TXN) across all jax_fingerprint instances. A ContextMap stores JAxContext instances keyed by method name.