Skip to content

fix: prevent handshake watchdog cancellation from closing successful TLS connections - #572

Open
szybnev wants to merge 1 commit into
projectdiscovery:mainfrom
szybnev:feat/tls-watchdog-cancel
Open

szybnev wants to merge 1 commit into
projectdiscovery:mainfrom
szybnev:feat/tls-watchdog-cancel

Conversation

@szybnev

@szybnev szybnev commented Sep 20, 2026

Copy link
Copy Markdown

Problem

closeAfterTimeout can close a connection after a successful TLS handshake, even though its timeout has not expired. This is a scheduling/logic race, not a Go data race and not specific to Docker, proxies, DNS, or a VM clock.

The helper is unchanged in v0.5.14, v0.5.19 and current main/v0.5.21 (a6c0214c35891524aee4c76ad0f6cb11bbe87f5f). We encountered it through httpx/nuclei builds using the first two versions; the reproducer below uses only the Go standard library and the unchanged helper on native macOS/arm64.

Why it happens

The returned completion function calls both handshakeDoneCancel() and the timeout context's cancel(). If the watchdog reaches its select after both calls, both Done() channels are ready. Selecting ctx.Done() currently calls Close() unconditionally, even when ctx.Err() is context.Canceled, not context.DeadlineExceeded.

The regular TLS, uTLS, ZTLS and ZTLS fallback paths share this helper. In a successful path the connection can therefore be handed back to the caller and then closed by its own watchdog. This can cause subsequent I/O to fail. Concurrency or scheduler delay can expose the interleaving; no actual timeout is required.

The existing cancellation test sleeps before cancelling, allowing the watchdog to start waiting on select. The new test deliberately cancels immediately, covers repeated cancellation, and advances virtual time past the original deadline without real sleeps.

Fix

Check that the timeout context actually expired before closing its resources. Keep the existing ownership and deadline behavior; this does not change TLS verification, retries, timeout values, or dependencies. The separate watchdog-overhead work in #554 is outside this fix; #552 concerned cleanup on handshake failure rather than closure after success.

Reproduction and validation

Environment: go version go1.27.1 darwin/arm64.

On unmodified main with the new test:

go test -race -run '^TestCloseAfterTimeout' -count=1 ./fastdialer
--- FAIL: TestCloseAfterTimeout_ImmediateCancelKeepsConnectionOpen
    successful completion closed the connection 115/256 times

The number is scheduling-dependent, not an expected failure rate for real scans. Three isolated runs of the exact helper plus the same regression test (standard library only) produced 121, 119 and 133 erroneous Close calls, out of 256 cancelled watchdogs per run.

With the fix:

go test -race -run '^TestCloseAfterTimeout' -count=20 ./fastdialer
go test -race -run '^(TestCloseAfterTimeout.*|TestDial|TestDialTLSHandshakeFailureNoGoroutineLeak)$' -count=1 ./...

The focused tests include the existing deadline/cancellation checks and loopback TLS/ZTLS handshake-failure checks. Full unfiltered go test ./... is not claimed: several existing tests contact external hosts. No public targets or product infrastructure are required for this regression test. The test uses testing/synctest, available with the repository's Go 1.25 minimum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant