Skip to content

Conversation

@gpotter2
Copy link
Member

@gpotter2 gpotter2 commented Dec 2, 2018

There is way over my head to do, here's what I could get so far:

  • (this PR includes Adds Extended Master Secret to TLS (RFC 7627) #1574)
  • fixes small IP bug when ihl < 5
  • merges TLS13ServerHello and TLSServerHello, as there is now no way to differentiate them apart from their extensions
  • updates some constants, especially the default TLS version in the packets. It used to be set to 0x0301 in some drafts, but is now 0x0303 (TLS 1.2)
  • Fix TLSServerHello dissection + Implement the new (changed) HelloRetry
  • updated the pack (2/2) of tests according to the latest draft. Some still don't pass

TODO: (feel free to contribute)

@gpotter2
Copy link
Member Author

gpotter2 commented Dec 2, 2018

Current issue is that TLS13 fails to decode the TLS-encrypted content using the key provided in ServerHello, after packet 5

@gpotter2 gpotter2 added the tls label Apr 13, 2019
@codecov
Copy link

codecov bot commented Apr 15, 2019

Codecov Report

Merging #1726 into master will decrease coverage by 35.22%.
The diff coverage is 43.92%.

@@             Coverage Diff             @@
##           master    #1726       +/-   ##
===========================================
- Coverage    87.2%   51.97%   -35.23%     
===========================================
  Files         197      119       -78     
  Lines       44484    29619    -14865     
===========================================
- Hits        38793    15395    -23398     
- Misses       5691    14224     +8533
Impacted Files Coverage Δ
scapy/layers/inet.py 27.34% <ø> (-45.41%) ⬇️
scapy/layers/tls/record_tls13.py 29.46% <0%> (-30.9%) ⬇️
scapy/layers/tls/basefields.py 60.4% <0%> (-19.47%) ⬇️
scapy/layers/tls/automaton_cli.py 32.14% <0%> (-44.98%) ⬇️
scapy/layers/tls/keyexchange.py 40.86% <0%> (-42.12%) ⬇️
scapy/layers/tls/crypto/prf.py 37.03% <0%> (-47.15%) ⬇️
scapy/layers/tls/crypto/cipher_aead.py 28.63% <20%> (-53.31%) ⬇️
scapy/layers/tls/session.py 55.77% <33.33%> (-21.94%) ⬇️
scapy/layers/tls/handshake.py 42.62% <45.45%> (-38.46%) ⬇️
scapy/layers/tls/keyexchange_tls13.py 42.93% <71.42%> (-11.72%) ⬇️
... and 169 more

@gpotter2
Copy link
Member Author

@guedou @p-l- If you have some time available, it could be worth looking into this.

I'm still figuring out how to fix the issue mentioned above: the tests in tls13.uts fail when the payload becomes encrypted... (starting with packet t5). I managed to fix the TLS 1.3 detection and TLSHelloRetryRequest

@guedou
Copy link
Member

guedou commented Jun 23, 2019

Thanks for this work. TLS 1.3 is a though protocol =)

I will try to find time to review it carefully. According to the diffs, it seems that you updated the current test vectors using RC8448. It is likely not useful for our tests, but an errata fixes some vectors (see https://www.rfc-editor.org/errata_search.php?rfc=8448).

I do not recall which version of the draft the original author used, but I will be that it is 18 or 20. I wonder if changes in the key scheduling algorithm were made in between (https://tools.ietf.org/rfcdiff?url1=draft-ietf-tls-tls13-18.txt&url1=rfc8446 might help us investigating). However, as far as I recall, the last revisions of the drafts were related to zero RTT and not the core of TLS 1.3.

@gpotter2
Copy link
Member Author

The version in place is the draft 18. The goal of this PR was to upgrade it to the version that got accepted (we won't support all drafts..). This meant updating the vectors.

One of the biggest (and breaking difference) since draft 18 is that it used a TLS version of 0x304, whereas the last revision uses 0x303 and specifies a TLS extension that says 0x304. This breaks all previous tests.

@guedou guedou changed the title [WIP] Tls 1.3 [WIP] TLS 1.3 Jul 4, 2019
@guedou
Copy link
Member

guedou commented Jul 4, 2019

RFC8448 also defines the expected results for key computation. I started adding some checks. See the attached patch: tls13.check_keys.patch.txt

This could help us narrowing the issue.

@gpotter2
Copy link
Member Author

gpotter2 commented Jul 4, 2019

(I'm not actively working on this, but as a maintainer, you have write access on this branch. You can
git remote add gpotter2 git@github.com:gpotter2/scapy.git
git checkout gpotter2/tls-1.3
and push in the branch)

@gpotter2
Copy link
Member Author

gpotter2 commented Jul 7, 2019

Overwritten by #2132

@gpotter2 gpotter2 closed this Jul 7, 2019
@gpotter2 gpotter2 deleted the tls-1.3 branch July 19, 2020 09:19
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.

Unable to parse TLS 1.3 Server Hello

2 participants