contrib/mysql: add classic protocol support#4967
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4967 +/- ##
==========================================
+ Coverage 80.31% 80.35% +0.03%
==========================================
Files 381 384 +3
Lines 93630 95107 +1477
==========================================
+ Hits 75202 76420 +1218
- Misses 18428 18687 +259
🚀 New features to boost your workflow:
|
|
Please try to increase the test coverage, since this looks like AI-generated code. |
|
Thanks, that is fair feedback. I will add more tests to exercise the currently uncovered branches in \scapy.contrib.mysql. The current version already includes UTScapy regression tests and validation against real MySQL pcaps, but I agree that the patch coverage can be improved further. |
|
I added more UTScapy tests to exercise uncovered branches, including auth-response variants, helper edge cases, fallback/error paths, and incomplete stream reassembly. |
|
Please fix the unit test. |
|
Thanks, fixed. I reproduced the failure with \conf.debug_dissector=True. The fallback test was using a short EOF-like server payload (\e00), which could raise while being parsed as an incomplete EOF packet in strict dissection mode. I changed the test to use an unambiguous unknown server payload for the Raw fallback case and re-ran the MySQL UTScapy tests locally, including the -K scanner\ run and strict dissection mode. |
|
@gpotter2 What do you think? |
| if len(remain) < 4: | ||
| return None | ||
| payload_length = struct.unpack("<I", remain[:3] + b"\x00")[0] | ||
| payload = remain[4:4 + payload_length] |
There was a problem hiding this comment.
I am a bit confused by this code. It calls 3 or 4 functions once that all iterate through a list of stuff. Couldn't this be merged in a single iteration?
There was a problem hiding this comment.
I simplified that part as well and collapsed the server-side state handling into a more direct single-pass dispatch flow.
|
Thanks for the feedback. I cleaned up the module structure to reduce one-off helpers and make the stream dispatch/reassembly logic more direct and easier to follow, without changing the supported protocol scope. I re-ran the MySQL UTScapy tests, strict dissection mode, flake8, mypy, the WSL CI-like runs, and the real-PCAP smoke tests used during development. Everything is still passing. |
|
Thanks for the update. There are still comments you haven't addressed :) |
Summary
This PR adds a new
scapy.contrib.mysqlmodule implementing support for the MySQL classic protocol over TCP.The current scope covers a first usable subset of the protocol for dissection and packet building, including:
payload_length+sequence_id)Protocol::HandshakeV10Protocol::SSLRequestProtocol::HandshakeResponse41OldAuthSwitchRequestAuthSwitchRequestAuthSwitchResponseAuthMoreDataOK_PacketERR_PacketEOF_PacketCOM_QUERYCOM_STMT_PREPARE_OKRegression tests are added in
test/contrib/mysql.uts.Scope
This PR is intentionally limited to a usable MVP for the classic protocol.
Not implemented in this first version:
SSLRequestValidation
UTScapy
Added regression tests for:
COM_QUERYNULLOKwhenCLIENT_DEPRECATE_EOFappliesCOM_STMT_PREPARE_OKmetadata flowsResult:
UTScapy:15/15passingLint / typing
Validated locally with:
tox -e flake8on Windows: OKtox -e flake8on WSL Ubuntu / Python 3.9: OKtox -e mypyon Windows: OKOn WSL Ubuntu / Python 3.9,
tox -e mypyreports existing repository-level typing issues outside this module.Real PCAP validation
The contrib was also tested against these public MySQL captures:
umitproject/packet-manipulator/audits/pcap-tests/mysql.pcapcolinnewell/pcap2mysql-log/test/captures/big-data.pcaparkime/tests/pcap/mysql-allow.pcapIn local validation, the MySQL messages in these 3 captures were successfully parsed with the current implementation.
Notes
This PR follows the earlier discussion in #4954.
I tried to keep names close to the MySQL protocol naming where possible, while avoiding collisions with Scapy internals when needed.
Feedback is welcome on:
scapy.contrib