fwdmachine: verify upstream TLS certificates - #5141
Open
KernelClint wants to merge 1 commit into
Open
Conversation
AI-Assisted: yes (GPT-5.6-Cyber)
Codecov Report❌ Patch coverage is
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
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ForwardMachinein TLS mode terminates the client's connection and opens its own onward connectionto 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-379loads the system certificates and then switches both forms of authentication off:
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:
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 thecertificate cache already uses at
scapy/fwdmachine.py:349(ident = server_name or dest):And because interception against an upstream that cannot be verified is a legitimate use of this
class,
verify_upstream=Trueis 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 previousfour lines did separately.