Skip to content

utils: restore TLS settings when a pcapng reader closes - #5137

Open
KernelClint wants to merge 1 commit into
secdev:masterfrom
KernelClint:pcapng-restore-tls-settings
Open

utils: restore TLS settings when a pcapng reader closes#5137
KernelClint wants to merge 1 commit into
secdev:masterfrom
KernelClint:pcapng-restore-tls-settings

Conversation

@KernelClint

Copy link
Copy Markdown
Contributor

When a pcapng capture carries a Decryption Secrets Block, Scapy loads the keys and switches on TLS
session processing so the encrypted traffic in that file can be read
(scapy/utils.py:1994-2016).
Both are process-global settings on conf.

Closing the reader restores neither. rdpcap() closes it (scapy/utils.py:1303-1314), but the
inherited close only closes the file. The keys and the enable flag outlive the capture that supplied
them, so a later unrelated capture is read with the first file's keys still installed. A
key-carrying capture with no packets in it, read first, was enough to make a second encrypted
capture decrypt.

The change remembers the previous values the first time a reader installs keys, and puts them back
when it closes:

+        self._tls_state = None  # type: Optional[Tuple[Dict[str, bytes], bool]]
...
+                    if self._tls_state is None:
+                        self._tls_state = (
+                            conf.tls_nss_keys,
+                            conf.tls_session_enable,
+                        )
...
+    def close(self):
+        # type: () -> None
+        if self._tls_state is not None:
+            conf.tls_nss_keys, conf.tls_session_enable = self._tls_state
+            self._tls_state = None
+        RawPcapReader.close(self)

Settings the operator configured themselves are preserved: only what this reader changed is undone.
Decryption within the capture that carries the keys is unaffected.

The added regression reads a key-carrying capture, closes it, and asserts conf.tls_nss_keys and
conf.tls_session_enable are back to what they were. Without the source change it fails.

Performance was measured on one computer, before and after the fix: reading a capture took
1,668.8 ns before and 1,808.2 ns after. Repeat runs of this test moved by about 7%, which is wide
enough that only a large change would show — this is not one.

@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.64%. Comparing base (b3bbcc8) to head (880e6cf).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5137   +/-   ##
=======================================
  Coverage   80.63%   80.64%           
=======================================
  Files         390      390           
  Lines       96936    96944    +8     
=======================================
+ Hits        78168    78180   +12     
+ Misses      18768    18764    -4     
Files with missing lines Coverage Δ
scapy/utils.py 72.79% <100.00%> (+0.20%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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