-
-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix missing TLS packets and recognizing TLS encrypted data #4015
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4015 +/- ##
===========================================
- Coverage 81.93% 52.85% -29.09%
===========================================
Files 328 328
Lines 75404 75425 +21
===========================================
- Hits 61783 39865 -21918
- Misses 13621 35560 +21939
|
Thanks for your interest in Scapy. Could you share an example that triggers the issue? For example by adding a pcap file to a github comment. |
When running the included test with the included pcap file with master, I observe scapy parsing: the client hello, the server hello done, 2 hello requests?, raw handshake data, and finally application data. |
There's indeed an issue with TLS and tcp reassembly, but that should be handled by fixing TCP sessions (which is currently somewhat broken because we dissect then rebuild packets), rather than modifying TLS. |
Perhaps the first issue would be fixed with a refactoring of TCP sessions, but as-is, as far as I can tell, TLS |
I noticed that scapy seemed to skip packets when parsing tlsv1.2 messages from pcap files. I could not find a relevant issue and investigated it myself. It seems that scapy was returning the last tls message in a sequence when tls was fragmented across tcp packets.
Independently, I noticed that tls messages wireshark calls "Encrypted Handshake Messages" were not recognized by scapy. Since these messages aren't indicated by any outside field, I modified tls records to consider the message content encrypted if a message fails to parse.
I'm opening this PR as a draft since I haven't done testing outside of my use case. I'm not sure what implications outside of tls handshakes these changes have.