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

Rewrite text about Version Negotiation #1039

Merged
merged 11 commits into from
Feb 16, 2018
96 changes: 51 additions & 45 deletions draft-ietf-quic-transport.md
Original file line number Diff line number Diff line change
Expand Up @@ -865,8 +865,8 @@ endpoint, as described in {{termination}}.
## Matching Packets to Connections {#packet-handling}

Incoming packets are classified on receipt. Packets can either be associated
with an existing connection, be discarded, or - for servers - potentially create
a new connection.
with an existing connection, or - for servers - potentially create a new
connection.

Packets that can be associated with an existing connection are handled according
to the current state of that connection. Packets are associated with existing
Expand All @@ -876,56 +876,62 @@ Packets without connection IDs and long-form packets for connections that have
incomplete cryptographic handshakes are associated with an existing connection
using the tuple of source and destination IP addresses and ports.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is ambigious: "packets without connection ID and long-form packets". Does it mean packets without connection ID, or packets that are in long form during handshake?

A long form always has a connection ID in the current version. During handshake a tuple is insufficient if the peer uses a non-unique port. So the handshake should always use the initial client connection ID for association. Also because without the connection ID the handshake crypto won't work. For other versions this might be different, but then the current text is not very clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I recall a conversation where we were going to assume that the 4-tuple does not change over the life of the handshake, and that we were willing to accept connection failure if it did.

At the server, there's a little ambiguity as to whether the next packet will contain the original client connection ID or the server-generated one.

Just setting aside connection ID for these purposes seems cleaner, though with a bunch more verbiage we could force endpoints to consider conn ID as well. I dunno.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we agreed that changing 4-tuple during the handshake was not permitted. It's too complicated otherwise. The questions of address validation alone are too complex to consider.

Copy link
Contributor

Choose a reason for hiding this comment

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

My concern is not changing tuple but having two conns with the same. It happens when avoiding ephemeral port exhaustion.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree we should try to write this text in a way that allows two connections on the same 4-tuple.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it too much to ask to limit it to one overloaded handshake at a time? Once the handshake is complete, there's no restriction on using the same tuple for another handshake.

Copy link
Contributor

Choose a reason for hiding this comment

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

One handshake at a time would complicate reverse proxies significantly and force them to use multiple ports or limit rates.

Proposed text:

Clients and servers associate packets with an existing or a new connection by using the connection ID or the tuple of port address pairs. The connection ID may be absent from a packet header in which case the tuple must be unique. The tuple may be non-unique when a client reuses the source port so the connection ID SHOULD be used when available. A server MAY choose to associate handshake packets using a combination of both the tuple and the client connection ID in order to protect against accidental or intentional collisions. Post handshake both the client and the server can rely on just the connection ID when available in the header because of packet protection, otherwise the tuple will be sufficient. Connection migration necessitates also considering other tuples and connection IDs as advertised by the NEW_CONNECTION_ID ({{frame-new-connection-id}}) frame.

Copy link
Contributor

Choose a reason for hiding this comment

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

A few edits to Mikkel's text, but the right ideas are there:

Clients and servers associate packets with an existing or a new connection by using the connection ID if one is present, or the tuple of port-address pairs otherwise. When the connection ID is omitted from short packet headers, correct association requires that the tuple be unique. The tuple might be non-unique when a client reuses the source port, so the connection ID SHOULD be included in the client-to-server direction. A server MAY choose to associate handshake packets using a combination of both the tuple and the client connection ID in order to protect against accidental or intentional collisions. After the handshake completes, the tuple and connection ID might change over the course of the connection; see {{frame-new-connection-id}}) and {{wherever-migration-lives}}.

I'm a little dubious of the "accidental or intentional collisions," though. I think we could reasonably drop that sentence.

Copy link
Contributor

Choose a reason for hiding this comment

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

The collision part originally referred to the handshake which is why the server was singled out, but after the handshake the same idea also applies to the client when the server chooses a new connection ID. I'm not sure how important that sentence is, but if included, the edited version might be better off noting that: "Potential collisions can be avoided by associating connections with both the connection ID and the tuple together.", but this is not 100% fool proof either, so perhaps leave that as an implementation detail.

Copy link
Member

Choose a reason for hiding this comment

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

The lead-in is good, but I think that the point we need to acknowledge is that during a handshake, uniqueness of the connection ID is not guaranteed because it is not chosen by the server. Therefore, new connections might be unsuccessful due to a collision on connection ID. Using the address tuple in addition to the connection ID narrows this enough that it probably isn't going to cause problems.

Presumably the client won't choose a connection ID that it has seen, so the risk is small: a server and client simultaneously choosing a colliding connection ID. For the server this can be either during the handshake or for NEW_CONNECTION_ID. Right now we don't have a use for long headers after the handshake, so those can probably be recognized as new connections, but that's possibly asking too much of servers for this somewhat marginal use case.

A packet that uses the short header could be associated with an existing
connection with an incomplete cryptographic handshake. Such a packet could be a
valid packet that has been reordered with respect to the long-form packets that
will complete the cryptographic handshake. This might happen after the final
set of cryptographic handshake messages from either peer. These packets are
expected to be correlated with a connection using the tuple of IP addresses and
ports. Packets that might be reordered in this fashion SHOULD be buffered in
anticipation of the handshake completing.
Clients SHOULD discard any packet that cannot be associated with an existing
Copy link
Contributor

Choose a reason for hiding this comment

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

This paragraph is very odd right here. This seems to imply connections are never created. Possibly remove this paragraph?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The key word here is "Clients". This finishes up instructions for clients, and the rest of the section is about servers. That said, a week after I wrote this it took me a minute to notice that. Any suggestions on how to make the word "clients" and its precise meaning more salient?

connection. Discarded packets MAY be logged for diagnostic or security
purposes.

0-RTT packets might be received prior to a Client Initial packet at a server.
If the version of these packets is acceptable to the server, it MAY buffer these
packets in anticipation of receiving a reordered Client Initial packet.
If a server receives a packet not associated with an existing connection, it
executes the following steps, in order:

The server MUST check if the packet uses a short form header, or is not long
Copy link
Member

Choose a reason for hiding this comment

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

Let's go with a numbered list here.

enough for the size required for the initial packet of any QUIC version that
Copy link
Member

Choose a reason for hiding this comment

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

Let's avoid using "initial" and instead use "first" here. It could be confused with the packet type.

the server supports. See {{packet-size}} for the definition of packet size
and the minimum size in this version of QUIC. If either condition is true,
Copy link
Member

Choose a reason for hiding this comment

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

"and the minimum size of the first client packet in this version of QUIC"

the packet cannot create a new connection. In this case, the server MUST
either buffer the packet (see {{handshake-buffer}}), send a stateless reset
({{stateless-reset}}), or silently drop it.

Otherwise, the server checks the version field in the long header. If the
server does not support the chosen version, it MUST send a Version
Negotiation packet as described in {{send-vn}}.

If the server supports the version, the server operates in accordance with
the specification of that version. For the version described in this
specification, it checks if the packet is a correctly formatted Initial packet.
If so, the server MUST proceed with the handshake ({{handshake}}). This
commits the server to the version that the client selected. If not an Initial
packet, the server MUST either buffer the packet or silently drop it.

### Handshake Buffering {{#handshake-buffer}}
Copy link
Member

Choose a reason for hiding this comment

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

Use one curly brace and a hash for anchors: {#handshake-buffer}.


Due to packet reordering or loss, subsequent packets for a handshake might
arrive at the server prior to the intended Initial packet. As described above,
Copy link
Member

Choose a reason for hiding this comment

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

Remove "intended" here.

servers MAY buffer these packets in anticipation of the Initial packet
arriving later. This is especially useful for 0-RTT packets that routinely
accompany Initial packets.

Buffering ensures that data is not lost, which improves performance; conversely,
discarding these packets could create false loss signals for the congestion
controllers. However, limiting the number and size of buffered packets might be
needed to prevent exposure to denial of service.

For clients, any packet that cannot be associated with an existing connection
SHOULD be discarded if it is not buffered. Discarded packets MAY be logged for
diagnostic or security purposes.

For servers, packets that aren't associated with a connection potentially create
a new connection. A series of tests dictate the allowed actions.

If the packet uses a short form header, or is not long enough for the size
required for the initial packet of any of its supported versions (as defined
in {{packet-size}}), it cannot create a new connection. In this case, the server
MUST either buffer the packet in case more packets arrive, send a stateless
reset ({{stateless-reset}}), or silently drop it.

If the packet could create a new connection, the server checks the version
field in the packet header. If a supported version, but not a valid Initial
packet for that version, the server MUST either drop the packet or buffer it in
anticipation of additional packets.

If a supported version, and a valid Initial packet for that version, the server
proceeds with the handshake ({{handshake}}). This commits the server to the
version that the client selected.

If an unsupported version, the server MUST send a version negotiation packet
as described in {{send-vn}}.

Versions of QUIC that define smaller minimum initial packet sizes need to be
aware that initial packets will be discarded without action by servers that only
support versions with larger minimums. Clients that support multiple QUIC
versions can avoid this problem by ensuring that they increase the size of their
initial packets to the largest minimum size across all of the QUIC versions they
support. Servers need to recognize initial packets that are the minimum size of
all QUIC versions they support.
Servers MUST NOT send packets in response to these buffered packets until
the initial packet arrives.

In this version of QUIC, clients cannot send short form headers prior to a
response from the server. Servers that do not support a version of QUIC that
allows early short headers MUST either respond with a stateless reset or drop
the packet silently.

## Version Negotiation

{{packet-handling}} describes the conditions under which endpoints negotiate
versions.
Copy link
Member

Choose a reason for hiding this comment

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

I think that we could keep some of the exposition here. Maybe: "Version negotiation ensures that client and server agree to a QUIC version that is mutually supported. A server sends a Version Negotiation packet in response to a packet that might initiate a new connection, see {{packet-handling}} for details."


Clients that support multiple QUIC versions SHOULD pad their Initial packets
Copy link
Member

Choose a reason for hiding this comment

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

Needs a bit more context. "The size of the first packet sent by a client will determine whether a server sends a Version Negotiation packet."

to reflect the largest minimum Initial packet size of all their versions.
This ensures that that the server respond if there are any mutually supported
Copy link
Member

Choose a reason for hiding this comment

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

responds

versions.

### Sending Version Negotiation Packets {#send-vn}

Expand Down