fixed a runtime bug when the cert attribute doesn't exists
When running this test: https://gist.github.com/Frank-Buss/c27c663f79ccd1eeb6d74f7795f8bdde it generates this error:
File "/home/frank/tmp/scapy/scapy/packet.py", line 1883, in getfield_and_val
raise AttributeError(attr)
AttributeError: cert
This PR fixes it. Not sure about the underlying reason, the TLS packets might be broken. Maybe better to fix it elsewhere or show a warning message as well, but this works at least.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 79.14%. Comparing base (
d23dda3) to head (aaeb1ce). Report is 15 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #4494 +/- ##
==========================================
- Coverage 81.67% 79.14% -2.54%
==========================================
Files 355 330 -25
Lines 84830 79663 -5167
==========================================
- Hits 69289 63050 -6239
- Misses 15541 16613 +1072
| Files with missing lines | Coverage Δ | |
|---|---|---|
| scapy/layers/tls/handshake.py | 87.05% <100.00%> (-1.28%) |
:arrow_down: |
PS: would be also nice if parse_tls_records could be written better, is it possible to get individual TLS packets without low-level splitting the TLS packets myself? I tried it with the full stream, but then I get one TLS packet, with nested TLS packets as payload, and I couldn't extract the underlying layers of one packet.
Thanks for the PR. Could you share a small snippet that reproduces the issue ?Sent from my iPhoneOn 6 Aug 2024, at 08:41, Frank Buss @.***> wrote:When running this test: https://gist.github.com/Frank-Buss/c27c663f79ccd1eeb6d74f7795f8bdde it generates this error: File "/home/frank/tmp/scapy/scapy/packet.py", line 1883, in getfield_and_val raise AttributeError(attr) AttributeError: cert
This PR fixes it. Not sure about the underlying reason, the TLS packets might be broken. Maybe better to fix it elsewhere or show a warning message as well, but this works at least.
You can view, comment on, or merge this pull request online at: https://github.com/secdev/scapy/pull/4494
Commit Summary
aaeb1ce fixed a runtime bug when the cert attribute doesn't exists
File Changes (1 file)
M
scapy/layers/tls/handshake.py
(4)
Patch Links:
https://github.com/secdev/scapy/pull/4494.patch https://github.com/secdev/scapy/pull/4494.diff
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>
The gist I linked reproduces the error. Couldn't make it shorter, because I couldn't find a way to display the decrypted data to create just one TLS packet which shows the bug. If you show me how to show the decrypted data of a TLS packet, I can try to make the snippet shorter. But not sure if this would work anyway because of the session object.
PS: would be also nice if parse_tls_records could be written better, is it possible to get individual TLS packets without low-level splitting the TLS packets myself? I tried it with the full stream, but then I get one TLS packet, with nested TLS packets as payload, and I couldn't extract the underlying layers of one packet.
You can try to use pkt[TLS].iterpayloads().
You'll still see the payloads in each one, but you can simply ignore them.
One note though: in your example the data field seems to include TLS data from both the client and server. I'm not sure how you got there but Scapy won't be able to handle that nicely. (You did well splitting it into the client part and the server part, the passing the tls_session. This is more likely to work.)
Ideally you'd need to keep track of whether the incoming data is from the client or server in the parent parser of your application, and call mirror() accordingly.
Hi again, Could you ellaborate a tiny bit about where that TLS data is coming from? There certificate appears to be malformed
Yes, it is probably malformed, it is created by a program on the fly. Can't tell much more about it, as it might be security relevant. But I think scapy shouldn't throw an exception in this case, given the fact that it is used to debug such traffic. But an additional warning message would be probably a good idea.