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

Clarify PATH_RESPONSE lack of retransmission in Path Validation #2724

Closed
ianswett opened this issue May 20, 2019 · 3 comments · Fixed by #2729
Closed

Clarify PATH_RESPONSE lack of retransmission in Path Validation #2724

ianswett opened this issue May 20, 2019 · 3 comments · Fixed by #2729
Assignees
@erickinnear
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.
Projects
Late Stage Processing

Comments

@ianswett
Copy link
Contributor

We have a must for PATH_CHALLENGE "The endpoint MUST use unpredictable data in every PATH_CHALLENGE frame so that it can associate the peer's response with the corresponding PATH_CHALLENGE."

"On receiving a PATH_CHALLENGE frame, an endpoint MUST respond immediately by echoing the data contained in the PATH_CHALLENGE frame in a PATH_RESPONSE frame."

This text should clarify that an endpoint MUST respond exactly once as well. Saving PATH_RESPONSEs for later retransmission could be a memory attack. If one wanted to be particularly careful you could also limit how many you buffer if you're congestion control limited.

https://tools.ietf.org/html/draft-ietf-quic-transport-20#section-8.2
@erickinnear
Copy link
Contributor

Good call. Note that this is already indicated in Section 13.2, way down at the bottom of the bulleted list, but I agree that we should have this be MUST NOT save for retransmission/MUST response exactly once.

@martinthomson
Copy link
Member

This is technically design, but trivial. Do you want to take this @erickinnear ?

@erickinnear
Copy link
Contributor

Yup, I'll put up a PR for this

erickinnear pushed a commit to ekinnear/quic-base-drafts that referenced this issue May 20, 2019
@ekinnear
quicwg#2724: Don't store or retransmit PATH_RESPOSNE frames, avoid bu…
6b587ac 
…ffering too many
@erickinnear erickinnear mentioned this issue May 20, 2019
Merged
@mnot mnot added this to Triage in Late Stage Processing May 22, 2019
@mnot mnot added the design An issue that affects the design of the protocol; resolution requires consensus. label May 22, 2019
@mnot mnot moved this from Triage to Design Issues in Late Stage Processing May 22, 2019
@martinthomson martinthomson added the proposal-ready An issue which has a proposal that is believed to be ready for a consensus call. label Jun 14, 2019
@MikeBishop MikeBishop moved this from Design Issues to Consensus Emerging in Late Stage Processing Jun 18, 2019
facebook-github-bot pushed a commit to facebook/mvfst that referenced this issue Jun 27, 2019
@JunqiWang @facebook-github-bot
Do not retransmit PATH_RESPONSE
0cc3eab 
Summary: quicwg/base-drafts#2724

Reviewed By: mjoras

Differential Revision: D15719978

fbshipit-source-id: 1c57f3e2b497f1abfcd5fa3b5643d76aa51d11d9
@mnot mnot moved this from Consensus Emerging to Consensus Call issued in Late Stage Processing Jul 1, 2019
@mnot mnot added has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list. and removed proposal-ready An issue which has a proposal that is believed to be ready for a consensus call. labels Jul 8, 2019
@mnot mnot moved this from Consensus Call issued to Consensus Declared in Late Stage Processing Jul 8, 2019
Late Stage Processing automation moved this from Consensus Declared to Text Incorporated Jul 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@erickinnear erickinnear

Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.
Projects
Late Stage Processing
  
Issue Handled
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't store or retransmit PATH_RESPONSE frames, avoid buffering ekinnear/quic-base-drafts
4 participants
@martinthomson @mnot @ianswett @erickinnear