fix: prevent indefinite hangs during TLS operations (#819)

[Gengyscan]

Summary

Fixes #819 — tlsx hangs indefinitely when scanning large target lists (30k+ hosts).

Root Cause

Two concurrency bugs in the ztls client:

Bug 1: tlsHandshakeWithTimeout never times out

select {
case <-ctx.Done():
    return errorutil.NewWithTag("ztls", "timeout while attempting handshake")
case errChan <- tlsConn.Handshake():  // ← evaluated BEFORE select
}

Go evaluates the send-expression (tlsConn.Handshake()) before entering the select statement. This means the function blocks on the handshake synchronously, and the ctx.Done() branch can never fire. If a remote host is slow or unresponsive, the goroutine hangs forever.

Bug 2: EnumerateCiphers uses context.TODO()

if err := c.tlsHandshakeWithTimeout(conn, context.TODO()); err == nil {

context.TODO() has no deadline, so even if tlsHandshakeWithTimeout worked correctly, cipher enumeration would never time out.

Fix

Fix 1: Run handshake in a goroutine

errChan := make(chan error, 1)  // buffered to prevent goroutine leak
go func() {
    errChan <- tlsConn.Handshake()
}()
select {
case err := <-errChan:
    // handle result
case <-ctx.Done():
    _ = tlsConn.Close()  // unblock pending I/O in the goroutine
    return timeout error
}

• Handshake runs in a separate goroutine so select can race it against the context deadline
• errChan is buffered (capacity 1) so the goroutine won't leak if the timeout fires first
• On timeout, tlsConn.Close() is called to unblock any pending Read/Write in the handshake goroutine

Fix 2: Replace context.TODO() with proper timeout

handshakeCtx := context.Background()
var cancel context.CancelFunc
if c.options.Timeout != 0 {
    handshakeCtx, cancel = context.WithTimeout(handshakeCtx,
        time.Duration(c.options.Timeout)*time.Second)
}

Uses the same Timeout != 0 guard pattern already present in ConnectWithOptions (line 118) to avoid creating a context that expires instantly when no timeout is configured.

Testing

• The fix is minimal and contained to pkg/tlsx/ztls/ztls.go
• Both changes follow existing patterns in the codebase (ConnectWithOptions already uses the same timeout guard)
• The goroutine pattern follows standard Go concurrency idioms for timeout-aware I/O

Summary by CodeRabbit

• Bug Fixes
  • Improved reliability of TLS handshake timeout handling to prevent connection failures during certificate validation.
  • Enhanced error handling to better manage edge cases during TLS operations.

[neo-by-projectdiscovery-dev]

Neo - PR Security Review

No security issues found

Highlights

• Fixes goroutine blocking issue by running TLS handshake in separate goroutine, allowing timeout to work correctly
• Replaces context.TODO() with proper timeout context in EnumerateCiphers to enable timeout protection
• Uses buffered channel (capacity 1) to prevent goroutine leaks when timeout fires first

Hardening Notes

• Consider adding validation in clients.Options to reject negative or excessively large Timeout values (e.g., > 300 seconds) to prevent configuration errors
• In EnumerateCiphers at line 266, consider adding nil check for h1.ServerHello before accessing CipherSuite to prevent potential panics in edge cases

_{Comment @pdneo help for available commands. · Open in Neo}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 399409c4-f106-4beb-b1ee-53efb43a287e

📥 Commits

Reviewing files that changed from the base of the PR and between d13b67f and 761a872.

📒 Files selected for processing (1)

• pkg/tlsx/ztls/ztls.go

---

Walkthrough

The changes modify TLS handshake timeout handling in ztls.go. EnumerateCiphers now uses a configurable timeout from options instead of context.TODO(), while tlsHandshakeWithTimeout refactors the handshake into a goroutine-based approach with channel-based result communication and enforced timeout via context cancellation.

Changes

Cohort / File(s)	Summary
Timeout Handling Refactor pkg/tlsx/ztls/ztls.go	Modified EnumerateCiphers to apply contextual timeout from options with proper cancellation. Refactored tlsHandshakeWithTimeout to execute handshake in a separate goroutine via channel communication, handle tls.ErrCertsOnly as success, and enforce timeout by canceling/closing connections when context deadline is reached.

Sequence Diagram(s)

sequenceDiagram
    participant Caller as EnumerateCiphers
    participant TimeoutCtx as Context Manager
    participant Goroutine as Handshake Goroutine
    participant Channel as Result Channel
    participant Conn as TLS Connection
    
    Caller->>TimeoutCtx: Create context with timeout
    Caller->>Goroutine: Spawn goroutine
    Goroutine->>Conn: Perform TLS handshake
    
    par Handshake Path
        Conn-->>Goroutine: Success or Error
        Goroutine->>Channel: Send result
    and Timeout Path
        TimeoutCtx->>TimeoutCtx: Deadline reached
        TimeoutCtx->>Conn: Cancel/Close connection
    end
    
    Caller->>Channel: Receive result
    alt Result received
        Caller->>Caller: Process handshake result
    else Timeout triggered
        Caller->>Caller: Handle timeout error
    end
    
    Caller->>TimeoutCtx: Cancel context cleanup

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A goroutine hops where contexts bloom,
No more hangs lurking in the gloom!
With channels singing timeouts clear,
The TLS handshake's path is near.
Fixed and freed, resources aligned—
Happy hops for all humankind! 🌟

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: preventing indefinite hangs during TLS operations by implementing proper timeout handling in the tlsx library.
Linked Issues check	✅ Passed	The changes directly address the root causes identified in issue #819 by implementing timeout-aware handshake and cipher enumeration operations as required.
Out of Scope Changes check	✅ Passed	All changes are contained to pkg/tlsx/ztls/ztls.go and are directly related to fixing timeout handling in tlsHandshakeWithTimeout and EnumerateCiphers functions.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

📝 Coding Plan

• Generate coding plan for human review comments

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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