TLS Session Context refactoring #73
Conversation
- Introducing a keystore to abstract asymetrical keys from TLS context - Encapsulated RSA utility functions into a new module: ssl_tls_keystore - Added a test module to make keystore UT simpler and independant from crypto layer - Refactored ssl_tls_crypto to use the asymetrical keystore layer, as well as various examples and tests
- Removed all references to rsa.pubkey and rsa.privkey. Everything uses the RSA keystore - Moved TLSSessionCtx string templating to the RSAKeystore class. Each sub element can be responsible for it's formating. Looks weird right now, until I have completed the refactoring of other elements
- Use double quotes consistently - Moved camel casing to lower_case_with_underscores casing
- Moved DH Kex parameter handling into the keystore - Refactored the TLSSessionCtx to use the new DH keystore - Refactored UT
|
definitely the way to go! that will make it pretty straight forward to implement the idea of #64 - so thats a big like from my side. I can probably review this next week as I am currently travelling an plan to release/push v1.2.3 to pypi by the end of this week (fixing lots of nasties) |
- Made keystore naming consistent with kex_keystore - Print the size of RSA keys in use
- UT needed to work with RSA keys for the size calculation to work
- Refactored the TLSSessionCtx to use the new ECDH keystore
- Client side and server side keys are maintained in seperate key stores
- TLScontext holds all crypto and session material for one end of the connection - TLSSessionCtx holds a reference to 2 TLSContexts
|
This was tedious, but I think it's in a decent state now. |
- Preserved the stateless nature of __process. Just moved state handlers to seperate private functions
|
New output from session ctx looks something like this: Formatting should be super easy to tweak. Should also be much easier to refactor further. I'll put this through a PEP8 formatter and call it a day. After painfully rebasing #72 on top of this ;) |
- Replaced remaining single quotes with double quotes - Normalize the resulting files with: autopep8 -a -a -i - Added pep8 section to setup.cfg to allow usage of long lines
|
thats looking pretty! |
- Resolved conflicts from session resumption into the refactoring work
|
Just saw your comment. Has this almost done yesterday, so pushing this after merge. I'll put the whole project through autopep8 and we can merge this. @tintinweb, would you mind giving this a quick test? I think all is good, but just to make sure. I ran most of the examples, including the automata stuff, so this should probably ll be OK. An integration script would be nice at some stage to, to test all the examples @ commit time. Will see if I can do this later. |
- Ran "autopep8 -a -a -i -r ." on project. Formatting should be more standardized now
|
sure, will check if my scripts have any issues and report back. I can also take care of the integration suite if you want. it's on my todo list for some time and I keep getting reminded on this when manually checking our scripts for upcoming releases. |
|
Sounds good. Let me know how this goes. For the integration scripts, sounds good also. Feel free to send to master directly if you wish. |
TLSExtSignatureAndHashAlgorithm; hash_algorithm -> hash_alg; signature_algorithm -> sig_alg fixes security_scanner SLOTH checks due to attrib name change in TLSExtSignatureAndHashAlgorithm adds TLS_1_4
nosetests shows an "Unknown client key exchange" warning, will have to investigate. |
That's "normal". It was a happening in the background before. I just added an |
- RSA key loading should happen in the TLS context, not the session context. Avoids the `if client` logic - Added a warning for unknown server kex
|
This concludes it for me ;). Feel free to merge if you're happy with the testing and changes. |
- resumption property was printed as compression
|
this is supernice! finished testing. I also see the warning for the full_Rsa_connection_with_application_data example but I agree it does not look like a regression. |
I'm looking at adding TLS 1.3 to scapy_ssl_tls. But before that, I think that TLSSessionCtx needs some refactoring.
It's quite difficult to integrate new things now. Here is my grand masterplan ;). @tintinweb, let me know if that works for you:
self.crypto.client.rsa.privkeyand co to use the keystore. Mostly donecrypto.server.dh.pand co to use the keystore.crypto.session.key.client.macand co to use the keystore.This has the advantage that all seperate components are in charge of loading/saving/printing as they which, and makes iterative changes (and UT) easier.
Let me know what you think. I'll try and conserve the external interfaces to minimize changes, but references to tls_ctx variables from outside will break.