Skip to content

Conversation

@adamjanovsky
Copy link

extms snads for Extended Master Secret, see RFC 7627.

I just needed to do this for myself, so created PR by the way. If it does not comply to the rules or is screwed in any way, I'm probably not spending more time on this...

Did not created unit test, however tested it on few real handshakes. It works. My assumptions:

  • The ClientHello offers using this extension via extension field. But whether it is actually going to be used is decided by the server, in a similar field inside the Server Hello record.
  • Scapy is already gathering the handshake messages, so they can be used easily.

Based on this, I did the following adjustments:

  • tls_session_update on TLSServerHello record looks for the extension and notes its usage into the session object
  • tls_session_update on ClientKeyExchange gathers all handshake messages into one binary string and computes the hash on it. By that time, all should be available.
  • Then, the master secret is computed with the updated compute_master_secret method.

@gpotter2
Copy link
Member

Thanks a lot for your PR,

However, we cannot merge it if does not passes the tests. You are failing the PEP8 one (formatting of your code). It’s really fast to fix

scapy/layers/tls/handshake.py:32:80: E501 line too long (86 > 79 characters)
scapy/layers/tls/handshake.py:991:1: E303 too many blank lines (5)
scapy/layers/tls/handshake.py:995:1: E302 expected 2 blank lines, found 5
scapy/layers/tls/crypto/prf.py:210:80: E501 line too long (94 > 79 characters)

The PR needs this and a test. If you don’t find the time to provide a test, could you simply give us a small code example of its usage, so that I can add it myself ?

@adamjanovsky
Copy link
Author

adamjanovsky commented Sep 1, 2018 via email

@guedou
Copy link
Member

guedou commented Oct 10, 2018

@adamjanovsky any progress on the requested fixes?

@adamjanovsky
Copy link
Author

adamjanovsky commented Oct 10, 2018 via email

@guedou
Copy link
Member

guedou commented Jan 6, 2020

An improved version was submitted in #2403

@guedou guedou closed this Jan 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants