Skip to content

Conversation

@mrsylerpowers
Copy link
Contributor

Added ability to dissect and use TLS ESNI
fixes #2096

@codecov
Copy link

codecov bot commented Jun 18, 2019

Codecov Report

Merging #2097 into master will decrease coverage by 2.93%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2097      +/-   ##
==========================================
- Coverage   87.07%   84.14%   -2.94%     
==========================================
  Files         201      202       +1     
  Lines       45732    46100     +368     
==========================================
- Hits        39820    38789    -1031     
- Misses       5912     7311    +1399
Impacted Files Coverage Δ
scapy/layers/tls/extensions.py 81.29% <100%> (+0.24%) ⬆️
scapy/contrib/isotp.py 20.64% <0%> (-69.69%) ⬇️
scapy/contrib/cansocket_python_can.py 40% <0%> (-54.55%) ⬇️
scapy/arch/bpf/supersocket.py 27.77% <0%> (-50.01%) ⬇️
scapy/contrib/cansocket_native.py 30.55% <0%> (-50%) ⬇️
scapy/layers/dhcp6.py 64.18% <0%> (-21.56%) ⬇️
scapy/contrib/openflow.py 78.39% <0%> (-10.42%) ⬇️
scapy/layers/inet6.py 81.87% <0%> (-6.42%) ⬇️
scapy/arch/bpf/core.py 81.37% <0%> (-5.89%) ⬇️
scapy/contrib/ethercat.py 93.83% <0%> (-4.33%) ⬇️
... and 33 more

@guedou
Copy link
Member

guedou commented Jun 19, 2019

flake8 is not happy:

scapy/layers/tls/extensions.py:160:33: E128 continuation line under-indented for visual indent
scapy/layers/tls/extensions.py:162:32: E128 continuation line under-indented for visual indent
scapy/layers/tls/extensions.py:162:67: W291 trailing whitespace
scapy/layers/tls/extensions.py:164:32: E128 continuation line under-indented for visual indent
scapy/layers/tls/extensions.py:165:80: E501 line too long (87 > 79 characters)
scapy/layers/tls/extensions.py:167:32: E128 continuation line under-indented for visual indent
scapy/layers/tls/extensions.py:169:32: E128 continuation line under-indented for visual indent
scapy/layers/tls/extensions.py:169:57: E231 missing whitespace after ','
scapy/layers/tls/extensions.py:171:32: E128 continuation line under-indented for visual indent
scapy/layers/tls/extensions.py:205:1: E302 expected 2 blank lines, found 1
scapy/layers/tls/extensions.py:210:80: E501 line too long (81 > 79 characters)
scapy/layers/tls/extensions.py:212:32: E128 continuation line under-indented for visual indent
scapy/layers/tls/extensions.py:214:32: E128 continuation line under-indented for visual indent
scapy/layers/tls/extensions.py:215:80: E501 line too long (87 > 79 characters)
scapy/layers/tls/extensions.py:217:32: E128 continuation line under-indented for visual indent
scapy/layers/tls/extensions.py:219:32: E128 continuation line under-indented for visual indent
scapy/layers/tls/extensions.py:221:32: E128 continuation line under-indented for visual indent

@guedou
Copy link
Member

guedou commented Jun 19, 2019

Also, could you add some unit tests as done here https://github.com/secdev/scapy/blob/master/scapy/contrib/gtp.uts#L57 for disecting and instanciating.

@mrsylerpowers
Copy link
Contributor Author

mrsylerpowers commented Jun 19, 2019

Hey @guedou where should these test be? In the tls1.3 file or the tls or in the contrib without a python file to go with it unlike all of the other files there? The tls one seems kind of messy

@guedou
Copy link
Member

guedou commented Jun 20, 2019

You can add them at the end of this file https://github.com/secdev/scapy/blob/master/test/tls13.uts

Thanks

@mrsylerpowers mrsylerpowers force-pushed the master branch 4 times, most recently from 535f1d5 to 0488123 Compare June 24, 2019 19:29
@mrsylerpowers mrsylerpowers force-pushed the master branch 2 times, most recently from fd19a4e to cbbb247 Compare July 8, 2019 14:24
@mrsylerpowers
Copy link
Contributor Author

@guedou I believe this should be ready for review

@gpotter2
Copy link
Member

We're probably going to wait for #2132 to be merged before merging any TLS-related PRs. Thanks for the PR though !

Scapy only implements TLS 1.3 - draft 18 in its current state. It has diverted a lot from the current RFC, and we probably want to focus into making that work first.

@gpotter2
Copy link
Member

Hi !
Could you rebase against master ?
git pull --rebase upstream master

@mrsylerpowers

This comment has been minimized.

@gpotter2
Copy link
Member

Thanks for your PR & sorry for the delay

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.

Add Encrypted Server Name Indication for TLS 1.3

3 participants