Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[TLS13] Add tls13 resume session #2625

Merged
merged 3 commits into from
May 7, 2020
Merged

[TLS13] Add tls13 resume session #2625

merged 3 commits into from
May 7, 2020

Conversation

gpotter2
Copy link
Member

@gpotter2 gpotter2 commented May 3, 2020

@gpotter2 gpotter2 added the tls label May 3, 2020
@romain-perez romain-perez mentioned this pull request May 3, 2020
Closed
9 tasks
@gpotter2 gpotter2 force-pushed the romain-perez-add-tls13-resume-session branch from a866eac to e620149 Compare May 3, 2020 12:29
@codecov
Copy link

codecov bot commented May 3, 2020 •
edited
Loading

Codecov Report

Merging #2625 into master will decrease coverage by 0.49%.
The diff coverage is 17.45%.

@@            Coverage Diff             @@
##           master    #2625      +/-   ##
==========================================
- Coverage   88.41%   87.92%   -0.50%     
==========================================
  Files         247      247              
  Lines       52178    52453     +275     
==========================================
- Hits        46135    46118      -17     
- Misses       6043     6335     +292
Impacted Files Coverage Δ
scapy/layers/tls/handshake.py 82.65% <0.00%> (-0.62%) ⬇️
scapy/layers/tls/automaton_srv.py 69.19% <16.39%> (-7.71%) ⬇️
scapy/layers/tls/automaton_cli.py 74.91% <18.18%> (-4.52%) ⬇️
scapy/layers/tls/extensions.py 82.91% <75.00%> (-0.16%) ⬇️
scapy/contrib/automotive/obd/scanner.py 6.74% <0.00%> (-89.89%) ⬇️
scapy/contrib/isotp.py 85.98% <0.00%> (-5.58%) ⬇️
scapy/contrib/automotive/obd/services.py 93.10% <0.00%> (-3.45%) ⬇️
scapy/contrib/automotive/gm/gmlanutils.py 89.69% <0.00%> (-2.43%) ⬇️
scapy/contrib/automotive/obd/obd.py 94.44% <0.00%> (-1.86%) ⬇️
scapy/layers/tls/record.py 88.52% <0.00%> (-1.71%) ⬇️
... and 12 more

guedou
guedou reviewed May 6, 2020
View reviewed changes
Copy link
Member

@guedou guedou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool PR. I made some simple comments.

scapy/layers/tls/automaton_cli.py Outdated Show resolved Hide resolved
scapy/layers/tls/automaton_cli.py Outdated Show resolved Hide resolved
scapy/layers/tls/automaton_srv.py Outdated Show resolved Hide resolved
scapy/layers/tls/automaton_cli.py Outdated Show resolved Hide resolved
scapy/layers/tls/automaton_cli.py Outdated Show resolved Hide resolved
scapy/layers/tls/automaton_srv.py

s = self.cur_session
with open(self.session_ticket_file, "ab") as f:
# ticket;ticket_nonce;obfuscated_age;start_time;resumption_secret
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not using a comma to delimit the fields?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's clearer ? It matters really little, this is only used internally

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that it is better to use a well-known format even for internal usages =)

@guedou guedou merged commit e38a46d into secdev:master May 7, 2020
@gpotter2 gpotter2 deleted the romain-perez-add-tls13-resume-session branch May 7, 2020 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guedou guedou guedou left review comments

Assignees
No one assigned
Labels
tls
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gpotter2 @guedou