Specify the packets that carry closing frames #1890
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
martinthomson
added
editorial
martinduke
reviewed
Oct 19, 2018
draft-ietf-quic-transport.md
Outdated
| @@ -2302,6 +2302,12 @@ the application requests that the connection be closed. The application | |||
| protocol can use an APPLICATION_CLOSE message with an appropriate error code to | |||
| signal closure. | |||
|
|
|||
| If the endpoint believes that a connection has been successfully established, it | |||
There was a problem hiding this comment.
I find this condition a little vague for a MUST. How about "has received a short header from the peer"?
I think that the second piece to #1786, which was to make allowances for fatal errors arising from Initial packets won't be necessary. I'm happy to see a PR, of course, but the sentiment I've heard expressed is that it is very hard to manage this, and the spec wouldn't expressly prevent someone from trying to build a system that was resilient to maliciously-crafted Initial packets, but we wouldn't take steps in this document to deal with that. As previously agreed, if you are attacked and the attacker was able to modify Initial packets, then they can deny you the ability to connect. Closes #1818, #1786.
martinthomson
force-pushed
the
how-to-protect-close
branch
from
552e087
to
8c2aaa3
Compare
ianswett
reviewed
Oct 23, 2018
draft-ietf-quic-transport.md
Outdated
| @@ -2313,6 +2304,13 @@ the application requests that the connection be closed. The application | |||
| protocol can use an APPLICATION_CLOSE message with an appropriate error code to | |||
| signal closure. | |||
|
|
|||
| If the connection has been successfully established, endpoints MUST send any | |||
| closing frames in a 1-RTT packet. Prior to connection establishment, endpoints | |||
| SHOULD send closing frames in a Handshake packet. An endpoint MAY send closing | |||
There was a problem hiding this comment.
Does this need a caveat like "...SHOULD send closing frames in a Handshake packet if possible."
martinthomson
commented
Oct 23, 2018
martinthomson
force-pushed
the
how-to-protect-close
branch
from
a5b112c
to
d513b05
Compare
janaiyengar
approved these changes
Oct 23, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I decided to reword #1818 after reading it.
I think that the second piece to #1786 , which was to make allowances for
fatal errors arising from Initial packets won't be necessary. See my
comments on #1819 for that.
Closes #1818, #1786.