#2403 improvement: EXT MS TLS 1.2

[gpotter2]

• rebase of PR#1574 improved - Adds Extended Master Secret to TLS (RFC 7627) #2403
• adds a test provided by TLS does not support Encrypt-then-MAC \ Extended Master Secret extensions #2784
• fixes part of TLS does not support Encrypt-then-MAC \ Extended Master Secret extensions #2784
• fix PR#1574 improved - Adds Extended Master Secret to TLS (RFC 7627) #2403

[codecov]

Codecov Report

Merging #2797 into master will increase coverage by 0.34%.
The diff coverage is 92.68%.

@@            Coverage Diff             @@
##           master    #2797      +/-   ##
==========================================
+ Coverage   88.02%   88.36%   +0.34%     
==========================================
  Files         252      252              
  Lines       53671    53701      +30     
==========================================
+ Hits        47242    47455     +213     
+ Misses       6429     6246     -183     

Impacted Files	Coverage Δ
scapy/layers/tls/session.py	84.43% <75.00%> (+0.10%)	⬆️
scapy/layers/tls/handshake.py	83.82% <87.50%> (+0.31%)	⬆️
scapy/layers/tls/crypto/prf.py	84.56% <100.00%> (+0.39%)	⬆️
scapy/layers/tls/keyexchange.py	86.93% <100.00%> (+0.17%)	⬆️
scapy/arch/windows/__init__.py	70.60% <0.00%> (-0.48%)	⬇️
scapy/layers/inet.py	74.13% <0.00%> (+0.08%)	⬆️
scapy/utils.py	80.81% <0.00%> (+0.33%)	⬆️
scapy/layers/tls/basefields.py	85.90% <0.00%> (+0.67%)	⬆️
... and 8 more

[strayge]

Is it correctly fallback to MD5+SHA1 for TLS 1.0 - TLS 1.1 (as said chapter 3 of RFC 7627)?

I've got traffic examples for Extended-Master-Secret for TLS 1.0 & TLS 1.1 only with Encrypt-then-MAC.
So can't check it right now.

[gpotter2]

I'm not a 100% sure, will check that tonight. Should be defined over

scapy/scapy/layers/tls/session.py

Line 36 in 54da22f

class connState(object):

but this is usually pretty alright. This fallback is implemented for some parts like the PRF

[strayge]

Checked. Current implementation works only for TLS 1.2.
Here fix: https://github.com/strayge/scapy/commit/b6a97dccb880c0285c893a644fc3e57a74f89192

Also for testing I wrote POC of Encrypt-Then-MAC: https://github.com/strayge/scapy/commit/aee5f5712f287256a94ea34a46f577b9434374ae

with both this commits successfully decrypted: TLS 1.0 - TLS 1.2 with EMS + ETM extensions
(checked on AES-CBC & AES-GCM suites)

[gpotter2]

Thanks a lot for the PoC ! Looking good.
There are a few things that require changes (I'll go over that tommorow), such as the case where the server doesn't support ExtMS or EncryptThenMac, despite the client request, and everything related to building those packets, but it's a solid start.

[strayge]

Extensions detection placed in TLSServerHello class, so we detect it only if server support it (and client requested it).

Also in PoC missed implementation of Encrypt-Then-MAC for stream ciphers (for AEAD ciphers server must not send this extension, according to RFC).

[p-l-]

@gpotter2 can you rebase against current master, since #2792 has been merged?

[strayge]

@gpotter2 should I made separated PR for Encrypt-then-MAC?

[gpotter2]

That would probably be a good thing, thanks for the suggestion !

[strayge]

Also I found that current implementation of EMS does not work with TLS 1.2 + RC4.
Don't know why.
TLS 1.0 and TLS 1.1 works well with RC4 + extended_master_secret.

openssl 1.1.1g for client and server.
First Finished in 3rd packet is not decrypted.
Here is example. Key is the same.
tls12_rc4_ems.zip

load_layer("tls")
a = rdpcap("./tls12_rc4.pcap")
t1 = TLS(a[0].load)
t1.tls_session.server_rsa_key = PrivKey("./ssl.key")
t2 = TLS(a[1].load, tls_session=t1.tls_session.mirror())
t3 = TLS(a[2].load, tls_session=t2.tls_session.mirror())

[gpotter2]

The RC4 example your provided has encrypt_then_mac so that's probably the issue.

[strayge]

The RC4 example your provided has encrypt_then_mac so that's probably the issue.

ServerHello does not contains encrypt_then_mac, so it should not be used.

P.S. But it's minor issue, as RC4 is not widely used nowadays. I'm happy with this PR and without RC4.

[gpotter2]

I've provided a fix, feel free to try it out.

[strayge]

Thanks! Now it works for all my test cases.