search

tintinweb / scapy-ssl_tls

SSL/TLS layers for scapy the interactive packet manipulation tool
GNU General Public License v2.0
419 stars 156 forks source link

Server side support #57

Closed alexmgr closed 8 years ago

alexmgr commented 8 years ago

Finally got a bit of time to add some example code to handle server side support. Also fixed a few bugs along the way the way. Needs a bit more work, but sending in for early review.

@tintinweb The change is quite large, so if you got a sec for some confirmation testing, would appreciate it. I ran most basic tests and all was OK but never know.

Doesn't cover server-side DHE and ECDHE yet, but the change should be easy from here.

alexmgr commented 8 years ago

Rebased off master. Forgot to do that before pushing the previous set.

alexmgr commented 8 years ago

All unittests pass locally. Not sure yet why there failing on the build server.

alexmgr commented 8 years ago

Seems to be introduced by scapy 2.3.1, not related to this particular change.

tintinweb commented 8 years ago

excellent, planned to implement this in the server-automata branch but never found the time to do so. Glad you've helped out! :)

regarding the scapy 2.3.2 issue. Looks like scapy recently introduced some caching and therefore Packet.do_build and Packet.do_dissect changed. This PR is implementing a custom do_dissect which is based on scapy <= 2.3.1 and does not seem to be compatible with v2.3.2 as it does not initialize the caching properties. We should probably try to avoid overriding do_dissect (since implementation changed already and might in the future) and find another way to fix dissection. No clue which side-effects the current fix has. Will investigate.

alexmgr commented 8 years ago

Yeah, I had a quick look a this. It's a bug in the scapy code. They instantiate the static variable raw_packet_cache_fields to None instead of {}. I'll file a bug upstream.

I'll just walk around this and port the 2.3.1 code to do_dissect() inside this PR. Should fix it.

We might want to consider doing this for TLS() class also. Might submit another PR for that if time allows.

tintinweb commented 8 years ago

Not sure if we really need to change SSL.do_dissect at this point as it is highly specific to that one TLSRecord case. Still I'd like to investigate if there's an elegant way to avoid overriding TLSDecryptablePacket.do_dissect for our use-case.

Apart from that, the PR looks good. Tested with old/new scapy and did not see any obvious errors. (/) scapy 2.2.0-dev (unittests, examples) (linux, win) (/) scapy-2.3.2 (unittests, examples)