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.

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

authenticate connection IDs #2567

Merged
merged 11 commits into from
May 31, 2020
Merged

Conversation

marten-seemann
Copy link
Member

Fixes #2529.

@codecov-commenter
Copy link

codecov-commenter commented May 25, 2020

Codecov Report

Merging #2567 into master will decrease coverage by 0.00%.
The diff coverage is 92.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2567      +/-   ##
==========================================
- Coverage   86.25%   86.25%   -0.00%     
==========================================
  Files         122      122              
  Lines        9645     9717      +72     
==========================================
+ Hits         8319     8381      +62     
- Misses        988      997       +9     
- Partials      338      339       +1     
Impacted Files Coverage Δ
session.go 75.30% <76.09%> (-0.19%) ⬇️
internal/wire/transport_parameters.go 89.80% <98.39%> (+0.42%) ⬆️
internal/handshake/crypto_setup.go 66.52% <100.00%> (ø)
internal/handshake/token_generator.go 78.57% <100.00%> (+0.79%) ⬆️
qlog/event.go 95.98% <100.00%> (+0.07%) ⬆️
qlog/qlog.go 95.93% <100.00%> (+0.10%) ⬆️
server.go 84.02% <100.00%> (+0.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe85b52...4f19b15. Read the comment docs.

@marten-seemann marten-seemann force-pushed the authenticate-connection-ids branch 4 times, most recently from 3af5f01 to bfab263 Compare May 28, 2020 05:52
@marten-seemann marten-seemann mentioned this pull request May 28, 2020
@marten-seemann marten-seemann force-pushed the authenticate-connection-ids branch from bfab263 to 4f19b15 Compare May 29, 2020 12:57
return
// check the initial_source_connection_id
if !params.InitialSourceConnectionID.Equal(s.handshakeDestConnID) {
return qerr.NewError(qerr.TransportParameterError, fmt.Sprintf("expected initial_source_connection_id to equal %s, is %s", s.handshakeDestConnID, params.InitialSourceConnectionID))
Copy link
Member

Choose a reason for hiding this comment

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

Could exposing the expected id leak info to an attacker?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really. The connection ID has already been used on the Initial packet, so it's known to a MITM.

@marten-seemann marten-seemann merged commit ce40a7e into master May 31, 2020
@marten-seemann marten-seemann deleted the authenticate-connection-ids branch May 31, 2020 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

authenticate connection IDs
3 participants