Skip to content

Comments

fix: tlsx hangs indefinitely for some hosts#920

Open
SolariSystems wants to merge 2 commits intoprojectdiscovery:mainfrom
SolariSystems:solari/fix-819-1771853510
Open

fix: tlsx hangs indefinitely for some hosts#920
SolariSystems wants to merge 2 commits intoprojectdiscovery:mainfrom
SolariSystems:solari/fix-819-1771853510

Conversation

@SolariSystems
Copy link

@SolariSystems SolariSystems commented Feb 23, 2026

Summary

Fixes #819

Fix the ztls tlsHandshakeWithTimeout() function which has a fundamental design flaw: Handshake() is called synchronously inside a select case expression, meaning the select NEVER evaluates ctx.Done() if the handshake blocks. The fix is to run the handshake in a goroutine so the select can properly race between handshake completion and context timeout. Additionally, fix cipher enumeration in both tls and ztls modes which lack proper timeouts (bare Handshake() and context.TODO()).

Changes Made

The diff correctly fixes the root cause of the indefinite hang (synchronous Handshake() in select case expression) and adds proper timeouts to all three affected code paths.

Verification

  • Build: PASS
  • Tests: PASS
  • Lint: PASS

Summary by CodeRabbit

  • New Features

    • TLS cipher enumeration now uses per-handshake timeouts (default 5s), improving reliability during scanning.
  • Bug Fixes

    • Handshake operations that exceed the timeout are cleanly aborted to prevent hangs and ensure enumeration completes.
    • Certain certificate-only responses are now handled as successful handshakes to improve cipher detection accuracy.

/claim #819

@neo-by-projectdiscovery-dev
Copy link

neo-by-projectdiscovery-dev bot commented Feb 23, 2026

Neo Security Audit

No security issues found

Highlights

  • The tlsHandshakeWithTimeout goroutine properly closes the connection on timeout (line 341 in ztls.go), preventing goroutine accumulation
  • Buffered error channel (size 1) prevents goroutine leaks when timeout occurs before handshake completes
Hardening Notes
  • The tlsHandshakeWithTimeout goroutine properly closes the connection on timeout (line 341 in ztls.go), preventing goroutine accumulation
  • Buffered error channel (size 1) prevents goroutine leaks when timeout occurs before handshake completes
  • Cipher enumeration now includes proper timeout contexts in both tls and ztls implementations, with 5-second default timeout

Comment @neo help for available commands. · Open in Neo

@coderabbitai
Copy link

coderabbitai bot commented Feb 23, 2026

Walkthrough

Replaces blocking TLS handshakes with context-aware, per-call timeouts during cipher enumeration in both the standard tls and custom ztls paths; ztls uses a goroutine-based handshake wrapper to enforce timeouts and treat tls.ErrCertsOnly as success.

Changes

Cohort / File(s) Summary
Standard TLS Timeout Context
pkg/tlsx/tls/tls.go
Replaced blocking Handshake with HandshakeContext using a timeout context derived from options.Timeout (defaults to 5s); cancels context and closes connection after each attempt.
Custom TLS Goroutine-Based Timeout
pkg/tlsx/ztls/ztls.go
Added per-cipher timeout handling and tlsHandshakeWithTimeout that runs tlsConn.Handshake() in a goroutine, selects on ctx.Done() or handshake result, treats tls.ErrCertsOnly as success, and closes the connection on timeout to unblock the goroutine.

Sequence Diagram(s)

sequenceDiagram
    participant Enum as EnumerateCiphers
    participant Ctx as Context (timeout)
    participant TLS as TLS Conn
    participant Gor as Goroutine

    Enum->>Ctx: create timeout context (options.Timeout || 5s)
    alt Standard TLS path
        Enum->>TLS: HandshakeContext(ctx)
        TLS-->>Enum: success / error / ctx.Done
    else ztls path
        Enum->>Gor: start goroutine -> tlsConn.Handshake()
        Gor->>TLS: execute Handshake()
        par
            TLS-->>Gor: handshake result
        and
            Ctx-->>Enum: ctx.Done (timeout)
            Enum->>TLS: Close() on timeout
        end
        Gor-->>Enum: handshake result (or blocked until Close)
    end
    Enum->>Ctx: cancel context
    Enum->>Enum: record cipher on success / skip on error
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A rabbit hops with careful ticks,

I wrap the handshakes in quick sweet licks,
Five seconds, then off you go — no more freeze,
Goroutines twirl and timeouts appease,
Scans proceed like springtime breeze. 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: tlsx hangs indefinitely for some hosts' directly addresses the primary issue fixed in this changeset - preventing hangs by implementing proper timeout handling.
Linked Issues check ✅ Passed The pull request addresses issue #819 by replacing synchronous handshakes with context-aware ones, adding per-call timeouts (defaulting to 5 seconds), ensuring context cancellation, and closing connections on timeout to prevent goroutine leaks and indefinite hangs.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the hang issue: timeout handling in tls/ztls handshake functions and proper context cancellation in cipher enumeration, with no unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Address security hardening suggestions from Neo review:

- Close tlsConn on context timeout in tlsHandshakeWithTimeout() to
  unblock the goroutine stuck in Handshake() and prevent goroutine
  accumulation under sustained timeout conditions
- Add defer enumCancel() after context creation in both tls and ztls
  EnumerateCiphers to ensure context resources are released even if
  early returns occur

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tlsx hangs indefinitely for some hosts

1 participant