-
Notifications
You must be signed in to change notification settings - Fork 203
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
Version numbers #113
Version numbers #113
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, with a few nits.
QUIC versions are identified using a 32-bit value. | ||
|
||
The version 0x00000000 is reserved to represent an invalid version. This | ||
version of the specification is identified by the number 0x00000001. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I note, not as an objection, that I'll need to substantially change my Alt-Svc v= encoding to accommodate this format. Thanks. ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see, you have chosen to represent the version as a string rather than a number. I was just copying what Jana had in his proposal, I didn't think to check what you had in your example. I have a slight preference for the simplicity of numbers here, but there are some aesthetic advantages of what you have. I've put some more comments over there.
number to 0xff000000. For example, draft-ietf-quic-transport-13 would be | ||
identified as 0xff00000D. | ||
|
||
Versions of QUIC that are used for experimentation are coordinated on the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we reserve a range for experimentation as well as suggesting where to coordinate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thought was that the remainder of the space is for experimentation. See also RFC 6648.
If the packet contains a version that is acceptable to the server, the server | ||
proceeds with the handshake ({{handshake}}). All subsequent packets sent by the | ||
server MUST have the VERSION flag unset. This commits the server to the version | ||
that the client selected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems somewhat contradictory to the text about server triggering Version Negotiation if the ALPN token and the selected version don't line up. The QUIC version may be acceptable, but the server and client have no mutual application protocols that allow it, for example.
The "verify that you would have sent that version list if offered this initial version" requirement gets into that, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I have a separate change that removes the text on triggering version negotiation based on the ALPN token. This is simpler: negotiate a quic version, then negotiate an application protocol based on what is permitted within QUIC.
I haven't done a thorough analysis of what effect that has on the downgrade situation though. So there might be some potential avenues of attack on the unauthenticated part of the negotiation. My hope is that the version fields we've got in the transport parameters will detect those sorts of attack, but I'm not sure yet.
abf7dcf
to
136c4c6
Compare
136c4c6
to
c014a0f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this addresses the concerns I had. I assume the "length of the flags" field leaves open the possibility that a MORE_FLAGS bit might indicate two or more flag bytes at the front of the packet, if that's the way we decide to go.
Potentially splitting a > 64 bit conn id is a little strange, but I can deal with it for packets with the version flag.
Nice work.
This does prevent MORE_FLAGS from being used when version negotiation is present unless the flags appear after the version field. |
As a semantic matter, I believe saying "the location and size of the Flags field" can't change defers to whatever we end up putting in the flags field description for the initial version. Although I mildly disagree with your preference, given the wording perhaps this is something to argue out when someone specifies the MORE_FLAGS bit. |
This defines what a version number is and carves out space for us.
I've added a section on what can change between versions. Right now, I've identified what I believe to be the absolute minimum that has to stay constant. This is made easier because it only relates to packets that are exchanged before a connection is setup. Closes #55.
I've aligned the version negotiation section with the recent changes to -tls. That includes the client including a version in all packets prior to the commencement of packet protection.
I'm laying some groundwork for later changes here. I'll build on this when I define the transport parameters, stealing from #99.
Builds on #111.