Re: [quicwg/base-drafts] Rework Retry packet (#1498)

[Mike Bishop <[e-mail]>]

MikeBishop commented on this pull request.



> @@ -532,6 +532,86 @@ See {{version-negotiation}} for a description of the version negotiation
 process.
 
 
+## Retry Packet {#packet-retry}
+
+A Retry packet uses the invariant portion of the long packet header with a type

"Invariant portion of the long packet header" feels awkward.  The invariants specify what a long header looks like, and this draft specifies additional fields which are in certain types.  The "Long Header" section actually adds additional fields beyond the long header which are type-specific to most (but not all) packet types used by this version.  Maybe the real subdivision is "Retry Packets" and "Connection Setup Packets", along with a reminder than Version Negotiation packets are defined in invariants?

> + 0                   1                   2                   3
+ 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
++-+-+-+-+-+-+-+-+
+|1|    0x7e     |
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|                         Version (32)                          |
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|DCIL(4)|SCIL(4)|
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|               Destination Connection ID (0/32..144)         ...
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|                 Source Connection ID (0/32..144)            ...
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|    ODCIL(8)   |      Original Destination Connection ID (*)   |
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|                        Retry Token (*)                      ...

Technically, you could; just that, like a short-header packet, the Retry would have to come last.  But as you say, I can't imagine a use for it in this version of QUIC, and since these are version-specific packets....

> +|                         Version (32)                          |
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|DCIL(4)|SCIL(4)|
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|               Destination Connection ID (0/32..144)         ...
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|                 Source Connection ID (0/32..144)            ...
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|    ODCIL(8)   |      Original Destination Connection ID (*)   |
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+|                        Retry Token (*)                      ...
++-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+~~~
+{: #retry-format title="Retry Packet"}
+
+A Retry packet (shown in {{retry-format}}) only the invariant portion of the

This is duplicative of the first sentence in this section.  Perhaps condense?

> +
+Retry Token:
+
+: An opaque token that the server can use to validate the client's address.
+
+The server populates the Destination Connection ID with the connection ID that
+the client included in the Source Connection ID of the Initial packet.
+
+The server includes a connection ID of its choice in the Source Connection ID
+field.  The client MUST use this connection ID in the Destination Connection ID
+of subsequent packets that it sends.
+
+A Retry packet does not include a packet number and cannot be explictly
+acknowledged by a client.
+
+A server MUST only send a Retry in response to a client Initial packet.

I'm not even sure it's necessary for it to be a SHOULD; there's no interoperability risk if it sends a Retry in response to a lone 0-RTT packet, is there?  In which case it sounds more like servers MAY omit sending Retry in response to 0-RTT when they haven't seen the Initial.

> +
+A Retry packet does not include a packet number and cannot be explictly
+acknowledged by a client.
+
+A server MUST only send a Retry in response to a client Initial packet.
+
+If the Original Destination Connection ID field does not match the Destination
+Connection ID from most recent the Initial packet it sent, clients MUST discard
+the packet.  This prevents an off-path attacker from injecting a Retry packet
+with a bogus new Source Connection ID.
+
+The client responds to a Retry packet with Initial packet that includes the
+provided Retry Token to continue connection establishment.
+
+A server that might send another Retry packet in response to a subsequent
+Initial packet MUST set the Source Connection ID to new value of at least 8

@nibanks, it's the other way around.  "A server that might send another" => all but the last.  If you don't know you're the last element in the chain, you MUST change the CID.  (Presumably to something that will get the retransmitted packet further down the DDoS chain.)

>  
 A server sends its first Initial packet in response to a client Initial.  A
 server may send multiple Initial packets.  The cryptographic key exchange could
 require multiple round trips or retransmissions of this data.
 
 The payload of an Initial packet includes a CRYPTO frame (or frames) containing
 a cryptographic handshake message, ACK frames, or both. The first CRYPTO frame
-sent always begins at an offset of 0 (see {{handshake}}). The client's complete
-first message MUST fit in a single packet (see {{handshake}}). Note that if the
-server sends a HelloRetryRequest, the client will send a second Initial packet
-with a CRYPTO frame with an offset starting at the end of the CRYPTO stream in
-the first Initial.
+sent always begins at an offset of 0 (see {{handshake}}).
+
+The first packet sent by a client always includes a CRYPTO frame that contains
+the first cryptographic handshake message.  The first packet sent by a client
+MUST fit in a single UDP datagram (see {{handshake}}).

Tautological, since we have no mechanism for a packet to span datagrams.  But the first TLS message must fit in the first packet.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/quicwg/base-drafts/pull/1498#pullrequestreview-133807792