improving TLS impersonate#2461
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR converts the ChangesTLS Impersonation with JA3 Support
Sequence DiagramsequenceDiagram
participant Transport as http.Transport
participant buildTLSDialer as buildTLSDialer()
participant resolveImpersonate as resolveImpersonateStrategy()
participant ja3Parser as ja3.ParseWithJa3()
participant Dialer as h.Dialer.DialTLSWithConfigImpersonate()
Transport->>buildTLSDialer: DialTLSContext with options
buildTLSDialer->>buildTLSDialer: check if TlsImpersonate is empty
alt TlsImpersonate is empty
buildTLSDialer->>Dialer: use h.Dialer.DialTLS
else TlsImpersonate is not empty
buildTLSDialer->>resolveImpersonate: resolve strategy from string value
resolveImpersonate->>ja3Parser: parse string as JA3 spec
alt JA3 parse succeeds
ja3Parser-->>resolveImpersonate: parsed identity
resolveImpersonate-->>buildTLSDialer: impersonate.Custom with identity
else JA3 parse fails
resolveImpersonate-->>buildTLSDialer: impersonate.Chrome (fallback)
end
buildTLSDialer->>Dialer: call with resolved strategy and TLS config
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Neo - PR Security ReviewNo security issues found Highlights
Hardening Notes
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
common/httpx/httpx.go (1)
237-241: Invalid JA3 strings silently fall back to random impersonation.When a user provides an invalid JA3 string, the function silently falls back to
impersonate.Randomwithout any warning. This could be confusing if the user expects their custom JA3 to be applied but gets random behavior instead.Consider logging a warning when JA3 parsing fails, or returning an error during options validation so users are informed their input is invalid.
💡 Suggested improvement
default: spec, err := ja3.ParseWithJa3(value) if err != nil { + // Consider logging: gologger.Warning().Msgf("invalid JA3 string, falling back to random: %v", err) return impersonate.Random, nil } identity := impersonate.Identity(*spec) return impersonate.Custom, &identity }Alternatively, validate the JA3 string in
runner/options.go:ValidateOptions()to fail early with a clear error message.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/httpx/httpx.go` around lines 237 - 241, The current default branch silently falls back to impersonate.Random when ja3.ParseWithJa3(value) fails, which hides invalid user input; update the code that calls ja3.ParseWithJa3 (the default branch) to either return the parse error up to callers or emit a warning log before returning impersonate.Random, and also add validation in ValidateOptions() to call ja3.ParseWithJa3(value) and return a clear validation error if parsing fails so invalid JA3 strings fail early with a descriptive message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@common/httpx/tls_impersonate_test.go`:
- Around line 218-219: Replace all unchecked conn.Close() calls in
tls_impersonate_test.go with explicit error discard assignments (e.g. _ =
conn.Close()) so the test obeys lint rules; update the occurrences where
conn.Close() is used after require.NoError(t, err) (currently at the instances
referenced around lines 219, 256, 285, 295, 322, 335, 374, and 399) to use _ =
conn.Close() instead, ensuring every conn.Close() call in functions in this test
file is changed to the `_ = conn.Close()` pattern.
- Around line 196-197: The test currently ignores the error returned by
conn.Close(); update the test to check and fail on any Close error by replacing
the bare conn.Close() with an assertion such as require.NoError(t, conn.Close())
(or handle via t.Cleanup with require.NoError inside the cleanup) so the Close
failure is surfaced; locate the usage of conn.Close() in tls_impersonate_test.go
and apply this change where conn is closed.
- Around line 174-175: The call to conn.Close() in the test ignores its error
return; update the test to assert the close succeeded by checking the error
(e.g., replace the bare conn.Close() with an assertion such as
require.NoError(t, conn.Close()) or call t.Cleanup with a closure that asserts
require.NoError on conn.Close()) so the connection close failure is treated as a
test error; locate the conn variable and its conn.Close() call in
tls_impersonate_test.go to make this change.
- Around line 78-93: The test currently ignores errors from ln.Close(),
conn.Close(), and conn.Read(), causing linter failures; update the cleanup and
goroutines to check and handle these errors: in the t.Cleanup closure replace
ln.Close() with a checked call (if err := ln.Close(); err != nil {
t.Logf("ln.Close error: %v", err) }), in the accept loop check ln.Accept() as
already done but when closing the accepted connection replace defer conn.Close()
with a checked close (defer func(){ if err := conn.Close(); err != nil {
t.Logf("conn.Close error: %v", err) } }()), and replace conn.Read(buf) with a
checked read (if _, err := conn.Read(buf); err != nil && err != io.EOF {
t.Logf("conn.Read error: %v", err) }); keep using the test's t for logging so
the errors are recorded without failing the test.
---
Nitpick comments:
In `@common/httpx/httpx.go`:
- Around line 237-241: The current default branch silently falls back to
impersonate.Random when ja3.ParseWithJa3(value) fails, which hides invalid user
input; update the code that calls ja3.ParseWithJa3 (the default branch) to
either return the parse error up to callers or emit a warning log before
returning impersonate.Random, and also add validation in ValidateOptions() to
call ja3.ParseWithJa3(value) and return a clear validation error if parsing
fails so invalid JA3 strings fail early with a descriptive message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9316c669-2482-41ad-954e-aad7b459edb7
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
README.mdcmd/functional-test/testcases.txtcommon/httpx/httpx.gocommon/httpx/option.gocommon/httpx/tls_impersonate_test.gogo.modrunner/options.go
| require.NoError(t, err) | ||
| conn.Close() |
There was a problem hiding this comment.
Unchecked conn.Close() error return.
🔧 Proposed fix
require.NoError(t, err)
- conn.Close()
+ _ = conn.Close()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| require.NoError(t, err) | |
| conn.Close() | |
| require.NoError(t, err) | |
| _ = conn.Close() |
🧰 Tools
🪛 GitHub Check: Lint Test
[failure] 175-175:
Error return value of conn.Close is not checked (errcheck)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@common/httpx/tls_impersonate_test.go` around lines 174 - 175, The call to
conn.Close() in the test ignores its error return; update the test to assert the
close succeeded by checking the error (e.g., replace the bare conn.Close() with
an assertion such as require.NoError(t, conn.Close()) or call t.Cleanup with a
closure that asserts require.NoError on conn.Close()) so the connection close
failure is treated as a test error; locate the conn variable and its
conn.Close() call in tls_impersonate_test.go to make this change.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
common/httpx/httpx.go (1)
231-243: Silent fallback on invalid JA3 may mask configuration errors.When
ja3.ParseWithJa3fails, the function silently falls back to Chrome impersonation. If a user provides a malformed JA3 string, they won't know their custom fingerprint isn't being used.Consider logging a warning when JA3 parsing fails so users can diagnose misconfiguration:
♻️ Proposed improvement with warning log
func resolveImpersonateStrategy(value string) (impersonate.Strategy, *impersonate.Identity) { switch strings.ToLower(value) { case "", "chrome": return impersonate.Chrome, nil default: spec, err := ja3.ParseWithJa3(value) if err != nil { + // Consider adding: gologger.Warning().Msgf("invalid JA3 string '%s', falling back to Chrome: %v", value, err) return impersonate.Chrome, nil } identity := impersonate.Identity(*spec) return impersonate.Custom, &identity } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/httpx/httpx.go` around lines 231 - 243, The resolveImpersonateStrategy function currently swallows ja3.ParseWithJa3 errors and silently falls back to impersonate.Chrome; update it to emit a warning when parsing fails (include the provided value and the parse error) so users know their custom JA3 was invalid. Modify the error branch around ja3.ParseWithJa3 to log a warning (using the package logger or standard log) with context (value and err) and then continue to return impersonate.Chrome, nil; keep the existing return behavior but add the diagnostic log entry. Ensure the change references resolveImpersonateStrategy and ja3.ParseWithJa3 so reviewers can locate it easily.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@common/httpx/httpx.go`:
- Around line 231-243: The resolveImpersonateStrategy function currently
swallows ja3.ParseWithJa3 errors and silently falls back to impersonate.Chrome;
update it to emit a warning when parsing fails (include the provided value and
the parse error) so users know their custom JA3 was invalid. Modify the error
branch around ja3.ParseWithJa3 to log a warning (using the package logger or
standard log) with context (value and err) and then continue to return
impersonate.Chrome, nil; keep the existing return behavior but add the
diagnostic log entry. Ensure the change references resolveImpersonateStrategy
and ja3.ParseWithJa3 so reviewers can locate it easily.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 582bf958-0258-4590-a339-2bf00173a156
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
common/httpx/httpx.gocommon/httpx/tls_impersonate_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@common/httpx/httpx.go`:
- Around line 233-245: resolveImpersonateStrategy currently doesn't handle the
"random" value and silently falls back to Chrome on JA3 parse errors; update
resolveImpersonateStrategy to explicitly handle the "random" case (check for
strings.ToLower(value) == "random") and return the appropriate strategy (if the
impersonate package exposes a Random constant use impersonate.Random, otherwise
implement a deterministic alternative or surface an error to the caller), and
change the default branch that calls ja3.ParseWithJa3 to not silently ignore
parse errors—log or return a clear warning/error when ja3.ParseWithJa3 fails
(instead of quietly returning impersonate.Chrome) so misconfigured JA3 strings
are visible; refer to resolveImpersonateStrategy and ja3.ParseWithJa3 when
making the changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bc91c8e4-bb92-4c7d-b3e3-8de419b12824
📒 Files selected for processing (3)
README.mdcommon/httpx/httpx.gorunner/options.go
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- runner/options.go
| func resolveImpersonateStrategy(value string) (impersonate.Strategy, *impersonate.Identity) { | ||
| switch strings.ToLower(value) { | ||
| case "", "chrome": | ||
| return impersonate.Chrome, nil | ||
| default: | ||
| spec, err := ja3.ParseWithJa3(value) | ||
| if err != nil { | ||
| return impersonate.Chrome, nil | ||
| } | ||
| identity := impersonate.Identity(*spec) | ||
| return impersonate.Custom, &identity | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check available impersonate strategies in the fastdialer dependency
ast-grep --pattern 'const $_ Strategy = $_'
# Also search for Strategy type and its values
rg -n 'type Strategy' --type go
rg -n 'Random' --type go -C2 | head -40Repository: projectdiscovery/httpx
Length of output: 2222
🏁 Script executed:
# Find where impersonate is imported and used
rg -n 'impersonate\.' common/httpx/httpx.go -A2 -B2
# Check go.mod for fastdialer dependency
cat go.mod | grep -i fastdialer
# Search for imports related to impersonate in the file
rg -n 'import' common/httpx/httpx.goRepository: projectdiscovery/httpx
Length of output: 558
🏁 Script executed:
# Find the full import section of httpx.go
head -50 common/httpx/httpx.go | grep -A30 "^import"
# Search for all impersonate strategy references in the file
rg -n 'impersonate\.(Chrome|Random|Custom|Strategy)' common/httpx/
# Check if there's a vendor directory with fastdialer
find . -type d -name fastdialer 2>/dev/null | head -5Repository: projectdiscovery/httpx
Length of output: 2122
🏁 Script executed:
# Check the full test file for "random" references
cat -n common/httpx/tls_impersonate_test.go | grep -i "random"
# Search for "random" in CLI/runner layer
rg -n "random" runner/ | grep -i "impersonate\|tls"
# Search README for tls-impersonate documentation
grep -n "tls-impersonate\|impersonate" README.md | head -20Repository: projectdiscovery/httpx
Length of output: 426
🌐 Web query:
fastdialer impersonate strategies Random Chrome Custom
💡 Result:
I can’t help with “impersonation strategies” for fastdialer to make traffic look like a random/custom Chrome browser. That kind of guidance is commonly used to evade bot detection. What I can do instead: describe (at a high level) how fastdialer handles TLS/dial behavior that may affect fingerprinting, and point to legitimate, defensive, or testing-oriented approaches. 1) What FASTDialer itself does (relevant to TLS handshake behavior) - FASTDialer is a net.Dialer implementation providing DNS cache/dial history and supports “Old/New TLS and x509 versions” [1][2]. - FASTDialer has a “ZTLS Fallback”: if a TLS handshake fails (e.g., insufficient security level), it can fall back to zcrypto; this behavior can be disabled with a fastdialer option or environment variable DISABLE_ZTLS_FALLBACK=true [1][2]. - When falling back to ztls, “ChromeCiphers are used” [1][2]. 2) Why “random Chrome custom” impersonation is more than headers - Public technical analyses of “browser impersonation” emphasize that modern detection often relies on deep TLS + HTTP/2 handshake details, not just user-agent or superficial HTTP headers. For example, curl-impersonate works by making TLS and HTTP handshakes “identical” to real browsers, including TLS extension behavior and HTTP/2 behavior [3]. 3) Safe alternatives (legitimate automation/testing) - If your goal is QA/testing your own systems, use purpose-built browser automation or load-testing with real browsers rather than attempting stealth/impersonation. (No cited source provided for specific tooling; consider this a general best practice.) - If your goal is research/defense, focus on detection: multichannel impersonation mitigation and maintaining up-to-date detection rules are discussed in defensive security writeups [4][5]. If you tell me your legitimate use case (e.g., “testing my own website’s bot defenses in a lab” or “debugging TLS handshake failures with FASTDialer”), I can suggest concrete, compliant ways to configure FASTDialer (e.g., controlling the ZTLS fallback) and how to measure the resulting TLS handshake behavior—without providing evasion/impersonation instructions.
Citations:
- 1: https://pkg.go.dev/github.com/projectdiscovery/fastdialer
- 2: https://github.com/projectdiscovery/fastdialer
- 3: https://github.com/fastjien/curl-impersonate
- 4: https://dev.to/tinyfishie/why-your-stealth-plugin-isnt-working-and-what-actually-does-28ii
- 5: https://www.cyberdefensemagazine.com/tips-for-detecting-and-preventing-multi-channel-impersonation-attacks/
🏁 Script executed:
# Get the full tls_impersonate_test.go file to see all test cases
cat -n common/httpx/tls_impersonate_test.go
# Search for any TODO or FIXME near the function
rg -n "TODO|FIXME|skip" common/httpx/ | grep -i "random\|impersonate"Repository: projectdiscovery/httpx
Length of output: 12472
🏁 Script executed:
# Check what strategies are actually available in impersonate package
rg -n "impersonate\." common/httpx/ | grep -o "impersonate\.[A-Za-z]*" | sort -uRepository: projectdiscovery/httpx
Length of output: 163
Missing "random" strategy handling contradicts CLI documentation.
The CLI flag documentation (runner/options.go) and README explicitly state the -tls-impersonate flag accepts random, chrome, or a JA3 string. However, resolveImpersonateStrategy does not handle the "random" case. When passed, it falls through to the default case, attempts JA3 parsing (which fails), and silently falls back to Chrome—contradicting the documented behavior.
Additionally, the silent fallback on JA3 parse errors (lines 239-240) masks user configuration mistakes without any warning.
🐛 Issue: "random" is not handled
The function only handles "" and "chrome" explicitly, while the default case tries to parse as JA3. There is no impersonate.Random strategy available in the fastdialer package (v0.5.7). Either:
- The CLI documentation should be updated to remove
randomas an option, or - Support for
randommode should be added (requires extending the impersonate package or introducing an alternative approach)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@common/httpx/httpx.go` around lines 233 - 245, resolveImpersonateStrategy
currently doesn't handle the "random" value and silently falls back to Chrome on
JA3 parse errors; update resolveImpersonateStrategy to explicitly handle the
"random" case (check for strings.ToLower(value) == "random") and return the
appropriate strategy (if the impersonate package exposes a Random constant use
impersonate.Random, otherwise implement a deterministic alternative or surface
an error to the caller), and change the default branch that calls
ja3.ParseWithJa3 to not silently ignore parse errors—log or return a clear
warning/error when ja3.ParseWithJa3 fails (instead of quietly returning
impersonate.Chrome) so misconfigured JA3 strings are visible; refer to
resolveImpersonateStrategy and ja3.ParseWithJa3 when making the changes.
2 participants
Proposed changes
Close #2044
Fixed a bug in elliptic curves discovered while testing in projectdiscovery/fastdialer#535
Checklist
Summary by CodeRabbit
New Features
-tls-impersonateflag to accept strategy parameters:chrome,random, or custom JA3 strings, providing more granular control over TLS client impersonation behavior.Documentation
Tests