Fix Dissection of Extensions, Added Ciphertext Sensing, Reworked Record Splitting

[tintinweb]

• all fixes from PR #19 (passed unittests)
• moved record splitting from TLSRecord to SSL. SSL should be responsible for providing the right amount of data for TLSRecord to autodissect
• added TLSCiphertext autosensing. Corrupt sublayers may show up as TLSCiphertext (see TLSFinished, which is encrypted or appdata)
• fixed dissection of TLSExtensions; dissection for client_hello/server_hello extensions was not working in v1.0 due problem with conditional fields (too early checking, see commit msg)
• added unittest: TLSExtensions
• added unittest: dissect pcap
• added unittest: sample files (pcap, server.pem)
• added unittest: auxiliary function "env_local_file()" to access files from the tests/files folder
• renamed some extensions to make them more descriptive

[tintinweb]

Fails unittest: test_fixed_crypto_data_matches_verify_data - need investigation; not sure if I broke something.

======================================================================
FAIL: test_fixed_crypto_data_matches_verify_data (tests.test_ssl_tls_crypto.TestTLSSessionCtx)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/root/scapy-ssl_tls/tests/test_ssl_tls_crypto.py", line 154, in test_fixed_crypto_data_matches_verify_data
    self.assertEqual(binascii.hexlify(tls_ctx.get_verify_data()), verify_data)
AssertionError: '8d585e6d8eb7adacefcacb31' != '12003ac89553b7a233da64b9'

Name                              Stmts   Miss  Cover   Missing
---------------------------------------------------------------
scapy_ssl_tls.py                      0      0   100%
scapy_ssl_tls/pkcs7.py               21      0   100%
scapy_ssl_tls/ssl_tls.py            381     15    96%   28, 58-59, 86, 93-96, 99-103, 106-108, 112
scapy_ssl_tls/ssl_tls_crypto.py     399     64    84%   37, 80, 88-91, 170-246, 286-287, 295-296, 300-303, 348-349, 369, 398-408, 411-412, 475, 478, 484, 636, 639, 709-715
---------------------------------------------------------------
TOTAL                               801     79    90%
----------------------------------------------------------------------
Ran 52 tests in 0.966s

[buildhive]

tintin » scapy-ssl_tls #35 FAILURE Looks like there's a problem with this pull request (what's this?)

[buildhive]

tintin » scapy-ssl_tls #36 FAILURE Looks like there's a problem with this pull request (what's this?)

[buildhive]

tintin » scapy-ssl_tls #38 FAILURE Looks like there's a problem with this pull request (what's this?)

[tintinweb]

extensions break unittest. the approach from v1.0 messes up all extensions. removing conditional fields fixes extensions, but breaks tlsfinished somehow if no extension is present in client_hello. anyhow, examples/full_handshake_rsa_aes_sha_with_sessionctx.py succeeds tlsfinished (extension included) against openssl s_server and passes all tests except the one mentioned above

we also might want to further simplify get_verify:

1. make tls_ctx.packets.history an SSL() object and push all records to SSL.records so we do not have to serialize them anymore
2. here's some simple code to get all relevant handshakes:

    # put all records into one list (might not need that with 1.)
    records = []
    for pkt in (p for p in self.packets.history if p.haslayer(tls.SSL)):
        records += pkt.records

    # grab relevant handshakes
    for handshake in (r[tls.TLSHandshake] for r in records if r.haslayer(tls.TLSHandshake)):
        if not handshake.haslayer(tls.TLSFinished) and not handshake.haslayer(tls.TLSHelloRequest):
            verify_data.append(str(handshake))

[alexmgr]

Haven't reviewed everything, but I like the last idea make tls_ctx.packets.history an SSL() object holder.

Regarding extensions, they need to work in both cases, cause many client/servers out there still do not support extensions. I know it's tricky to fix, and couldn't find a good way to do it. Maybe we could somewhat check the length of the handshake vs our current position. If equal at the end of the hello, then remove the extensions.

Busy weekend, nice work!

I have a pull request which adds automatic decryption of records in SSL. I'll send it out tomorrow, once #19 is merged and I rebase on top of all this. I think we should think of fully fixing the extension problem though.

[tintinweb]

• packets.history is still of type lst but we now only store individual records (from SSL.records) in packets.history
• fixed the ConditionalField stuff. unittests pass :) also interaction with openssl s_server works with and without extensions in client_hello. finally ;) Had to subclass and modify the ConditioanlField .. could not come up with a better name than ConditionalFieldPlusPlus as it just allows us to perform checks on all internal attribs (pkt,s,val)

[alexmgr]

Nice stuff.

On top of that, you can probably remove the gettls* top level functions and tests I put in ssl_tls.py. They shouldn't be needed anymore.

I'm happy for you to merge this. I'll add decryption on top of all the changes.

[buildhive]

tintin » scapy-ssl_tls #42 SUCCESS This pull request looks good (what's this?)

[tintinweb]

sounds good! feel free to add your modifications and merge the PR once unittests pass. no more changes for my side atm :) thanks.

[alexmgr]

Go ahead and merge it. It's cleaner if I keep my change seperate. I'll just rebase off master once this goes in.

[buildhive]

tintin » scapy-ssl_tls #45 SUCCESS This pull request looks good (what's this?)