fix(pcapwriter): serialize DSB keylog writes and fix race condition in Close()#971
Conversation
dosubot
Bot
added
size:L
|
🔧 Debug Build Complete (PR #971) 📦 Download Links: ⏰ Files will be retained for 7 days, please download and test promptly. This build includes debug binaries for: android/linux (arm64/amd64) |
✅ E2E Test Results: PASSEDTest Run: #23708683640 Tests Executed:
✅ All e2e tests passed successfully! The TLS capture functionality is working correctly. Automated e2e test results for commit f8392de |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Serialize WriteKeyLog() through Serve() goroutine via keylogChan to avoid concurrent access to NgWriter (not thread-safe) - Add serveDone channel so Close() waits for Serve() to finish before closing channels and flushing, eliminating shutdown race condition - Drain both packetChan and keylogChan on context cancellation in Serve() so queued data is not silently dropped at shutdown - Set keylogChan to nil on channel close to prevent CPU spin loop (receiving from closed channel returns zero value immediately) - Make Flush() safe for concurrent use: no-op while Serve() is running (Serve handles flushing internally), direct flush only after Serve exits - Remove racy Flush() call in PcapHandler.Close() that accessed NgWriter while Serve() was still running; PcapWriter.Close() handles this safely - Flush pending packets before each DSB write to maintain correct PCAPNG block order (packets -> DSB -> packets) - Add Truncate option to FileWriter; use it in openssl/gotls probes to overwrite stale pcapng files on new capture instead of appending - Remove unused masterKeyBuffer, tcPacketLocker fields and imports - Clean up stale comments and empty defers in savePcapng()
cfc4n
force-pushed
the
bugfix/pcapwriter-thread-safety
branch
from
c00c77e to
a95897b
Compare
|
🔧 Debug Build Complete (PR #971) 📦 Download Links: ⏰ Files will be retained for 7 days, please download and test promptly. This build includes debug binaries for: android/linux (arm64/amd64) |
✅ E2E Test Results: PASSEDTest Run: #23709062031 Tests Executed:
✅ All e2e tests passed successfully! The TLS capture functionality is working correctly. Automated e2e test results for commit 0484949 |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Root cause: Wireshark/tshark processes pcapng blocks sequentially and needs the Decryption Secrets Block (DSB) to appear BEFORE the encrypted packet blocks. ecapture was writing packets first, then DSB, placing the keylog after all packets where Wireshark couldn't use it. Verified: the DSB content was correct (format, secrets_type, keylog data all valid), but its position in the file made it invisible to Wireshark's TLS dissector. The same keylog extracted and passed via tls.keylog_file worked perfectly. Changes: - Reverse DSB/packet ordering in Serve(): write DSB first, then flush buffered packets, so DSB precedes packets in the pcapng file - Add dsbGraceDeadline (10s): delay the first timer-based packet flush until the first DSB arrives, preventing the timer from flushing packets to disk before the keylog has been captured - Add firstDSBWritten flag to track DSB state for the grace period - Fix drainOnShutdown(): write DSBs before packets on shutdown too
|
🔧 Debug Build Complete (PR #971) 📦 Download Links: ⏰ Files will be retained for 7 days, please download and test promptly. This build includes debug binaries for: android/linux (arm64/amd64) |
✅ E2E Test Results: PASSEDTest Run: #23709723072 Tests Executed:
✅ All e2e tests passed successfully! The TLS capture functionality is working correctly. Automated e2e test results for commit bf85b9b |
Uh oh!
There was an error while loading. Please reload this page.