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

Allow rate limiting of Version Negotiation #1891

Merged
merged 2 commits into from
Oct 23, 2018
Merged

Conversation

martinthomson
Copy link
Member

As discussed in #1758, we only need to allow rate limiting.

However, the particular example I draw on will become critical if we
adopt the proposed changes to version negotiation in #1755 (or something
like it), because the only packet a server can confidently respond to
with Version Negotiation in that case is the Initial. If we adopt an
iteration of #1755, then this text would need to be expanded to explain
that also.

Closes #1758.

(I marked this editorial so that we can include it in -16. I do that on the basis that a server can always feign packet loss, so this isn't really a testable requirement. That's only slightly dishonest, I think.)

As discussed in #1758, we only need to allow rate limiting.

However, the particular example I draw on will become critical if we
adopt the proposed changes to version negotiation in #1755 (or something
like it), because the only packet a server can confidently respond to
with Version Negotiation in that case is the Initial.  If we adopt an
iteration of #1755, then this text would need to be expanded to explain
that also.

Closes #1758.
@martinthomson martinthomson added editorial An issue that does not affect the design of the protocol; does not require consensus. -transport labels Oct 19, 2018
@MikeBishop
Copy link
Contributor

MikeBishop commented Oct 19, 2018

I think it's incorrect to mention specific packet types here, since a VN packet implies that the server doesn't speak this version (and therefore doesn't know what packet types exist). Implying otherwise encourages bugs in which servers wait until they see the packet type which matches v1:Initial, which might not be used in vX. #1758 caveats this risky text with "If the server has knowledge of the version," which at least acknowledges the problem.

I think a simple permission to rate-limit the number of VN packets sent in response to the same SCID:DCID tuple, or the same 5-tuple, should be sufficient and also less tempting to wrong assumptions.

@MikeBishop
Copy link
Contributor

The method in #1755 assumes a precondition, which is that the server can speak the proposed version (i.e. doesn't need to send VN), but found a mutually- supported preferable version it would like to switch to. That's a different state than pure VN.

@martinthomson
Copy link
Member Author

This is a for instance. And I think that this will become crucial if we go with #1755. In that case, a server that might read the Initial and change the version as a result cannot send Version Negotiation in response to 0-RTT packets.

Copy link
Contributor

@MikeBishop MikeBishop left a comment

Choose a reason for hiding this comment

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

Still easier to misunderstand than I'd really like, but I agree it's correct now.

@martinthomson martinthomson merged commit 6932dea into master Oct 23, 2018
@martinthomson martinthomson deleted the skip-vn-0rtt branch October 23, 2018 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-transport editorial An issue that does not affect the design of the protocol; does not require consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants