Skip to content

fwdmachine: verify upstream TLS certificates - #5141

Open
KernelClint wants to merge 1 commit into
secdev:masterfrom
KernelClint:fwdmachine-verify-upstream-tls
Open

fwdmachine: verify upstream TLS certificates#5141
KernelClint wants to merge 1 commit into
secdev:masterfrom
KernelClint:fwdmachine-verify-upstream-tls

Conversation

@KernelClint

Copy link
Copy Markdown
Contributor

ForwardMachine in TLS mode terminates the client's connection and opens its own onward connection
to the real server. The downstream side is an interception by design — it generates lookalike
certificates — but the upstream side is an ordinary TLS client connection.

scapy/fwdmachine.py:365-379
loads the system certificates and then switches both forms of authentication off:

clisslcontext = ssl.SSLContext(ssl.PROTOCOL_TLS)
clisslcontext.load_default_certs()
clisslcontext.check_hostname = False
clisslcontext.verify_mode = ssl.CERT_NONE

Anything on the path between the machine and the origin can therefore present any certificate and
have its data relayed onward. A self-signed upstream and a validly signed one for a different
hostname both went through.

The change uses Python's default client policy, which requires a trusted chain and checks the
hostname:

-            clisslcontext = ssl.SSLContext(ssl.PROTOCOL_TLS)
-            clisslcontext.load_default_certs()
-            clisslcontext.check_hostname = False
-            clisslcontext.verify_mode = ssl.CERT_NONE
+            if self.verify_upstream:
+                clisslcontext = ssl.create_default_context()
+            else:
+                clisslcontext = ssl.SSLContext(ssl.PROTOCOL_TLS)
+                clisslcontext.load_default_certs()
+                clisslcontext.check_hostname = False
+                clisslcontext.verify_mode = ssl.CERT_NONE

Two details matter for not breaking working setups.

The hostname checked comes from the downstream ClientHello's SNI, and a client may send none. The
name passed to wrap_socket() therefore falls back to the destination, matching the idiom the
certificate cache already uses at scapy/fwdmachine.py:349 (ident = server_name or dest):

-                ss = clisslcontext.wrap_socket(ss, server_hostname=server_name)
+                ss = clisslcontext.wrap_socket(
+                    ss, server_hostname=server_name or dest[0]
+                )

And because interception against an upstream that cannot be verified is a legitimate use of this
class, verify_upstream=True is a constructor option rather than a hard change of behaviour.

The added regressions assert the verifying context is used by default, the opt-out still reaches
the old one, and a connection with no SNI is checked against the destination host. Reverting the
source with the tests in place fails them.

Performance was measured on one computer, before and after the fix: building the context took
758.7 ns before and 542.2 ns after — 28.5% faster. Repeat runs moved by about 3%, so this is
larger than the test's own variation. create_default_context() does in one step what the previous
four lines did separately.

AI-Assisted: yes (GPT-5.6-Cyber)
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.72%. Comparing base (b3bbcc8) to head (d9cb53b).

Files with missing lines Patch % Lines
scapy/fwdmachine.py 50.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5141      +/-   ##
==========================================
+ Coverage   80.63%   80.72%   +0.09%     
==========================================
  Files         390      390              
  Lines       96936    96939       +3     
==========================================
+ Hits        78168    78258      +90     
+ Misses      18768    18681      -87     
Files with missing lines Coverage Δ
scapy/fwdmachine.py 31.43% <50.00%> (+31.43%) ⬆️

... and 6 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