Re: [quicwg/base-drafts] Proposal to make Version Negotiation more like Retry to punt VN to QUICv2 (#2313)

[Martin Thomson <[e-mail]>]

martinthomson requested changes on this pull request.

This needs an addition to the security considerations explaining that version downgrade protection is not provided.  While you might just excise the text and say that this is obvious, having downgrade protection is an expectation of a secure protocol.  That this does not provide that is - as we believe - acceptable so long as this is the only version in existence, or the worst version in existence, but it is nonetheless a possible surprise.

> @@ -1143,40 +1125,30 @@ expectation that it will eventually receive an Initial packet.
 
 ## Handling Version Negotiation Packets {#handle-vn}
 
-When the client receives a Version Negotiation packet, it first checks that the
-Destination and Source Connection ID fields match the Source and Destination
-Connection ID fields in a packet that the client sent.  If this check fails, the
-packet MUST be discarded.
+How a client reacts to receiving a Version Negotiation packet is left as future
+work defined by future versions of QUIC.  Future versions of QUIC that define

This isn't right.  This version needs to define what to do.

I recommend having clients abandon a connection attempt in the absence of better advice.

>  
-A client MUST NOT change the version it uses unless it is in response to a
-Version Negotiation packet from the server.  Once a client receives a packet
-from the server which is not a Version Negotiation packet, it MUST discard other
-Version Negotiation packets on the same connection.  Similarly, a client MUST
-ignore a Version Negotiation packet if it has already received and acted on a
-Version Negotiation packet.
+When a draft implementation receives a Version Negotiation packet, it MAY use
+it to attempt a new connection with one of the supported versions.

```suggestion
it to attempt a new connection with one of the versions listed in the packet.
```

> @@ -4010,14 +3903,6 @@ language from Section 3 of {{!TLS13=RFC8446}}.
    } TransportParameter;
 
    struct {
-      select (Handshake.msg_type) {
-         case client_hello:
-            QuicVersion initial_version;

You missed the definition of QuicVersion at the top of this block.

> @@ -4010,14 +3903,6 @@ language from Section 3 of {{!TLS13=RFC8446}}.
    } TransportParameter;
 
    struct {
-      select (Handshake.msg_type) {
-         case client_hello:
-            QuicVersion initial_version;
-
-         case encrypted_extensions:
-            QuicVersion negotiated_version;
-            QuicVersion supported_versions<4..2^8-4>;
-      };
       TransportParameter parameters<0..2^16-1>;
    } TransportParameters;

This can now be a straight typedef: `TransportParameter TransportParameters<0..2^16-1>;`

-- 
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/2313#pullrequestreview-201941919