Skip to content
New issue

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

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

Already on GitHub? # to your account

enforce that a connection ID is not retired in a packet that uses that connection ID #2651

Merged
merged 2 commits into from
Jul 8, 2020

Conversation

marten-seemann
Copy link
Member

The draft says:

The sequence number specified in a RETIRE_CONNECTION_ID frame MUST NOT refer to the Destination Connection ID field of the packet in which the frame is contained. The peer MAY treat this as a connection error of type FRAME_ENCODING_ERROR.

@marten-seemann marten-seemann force-pushed the enforce-conn-id-retirement-packet-conn-id branch from 48c6b97 to 1fa5a80 Compare July 6, 2020 09:32
@@ -76,6 +76,9 @@ func (m *connIDGenerator) Retire(seq uint64) error {
if !ok {
return nil
}
if connID.Equal(sentWithDestConnID) {
return qerr.NewError(qerr.FrameEncodingError, fmt.Sprintf("tried to retire connection ID %d (%s), which was used as the Destination Connection ID on this packet", seq, connID))
Copy link
Member Author

Choose a reason for hiding this comment

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

This should be a PROTOCOL_VIOLATION, see quicwg/base-drafts#3860.

@marten-seemann marten-seemann force-pushed the enforce-conn-id-retirement-packet-conn-id branch from 1fa5a80 to 25db216 Compare July 8, 2020 05:07
@codecov
Copy link

codecov bot commented Jul 8, 2020

Codecov Report

Merging #2651 into master will increase coverage by 0.00%.
The diff coverage is 55.56%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2651   +/-   ##
=======================================
  Coverage   86.44%   86.44%           
=======================================
  Files         122      122           
  Lines        9761     9763    +2     
=======================================
+ Hits         8437     8439    +2     
  Misses        989      989           
  Partials      335      335           
Impacted Files Coverage Δ
session.go 76.67% <33.33%> (ø)
conn_id_generator.go 90.00% <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 5227fef...33fc464. Read the comment docs.

@marten-seemann marten-seemann merged commit 2def5ab into master Jul 8, 2020
@marten-seemann marten-seemann deleted the enforce-conn-id-retirement-packet-conn-id branch July 8, 2020 07:58
@aschmahmann aschmahmann mentioned this pull request Sep 22, 2020
72 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants