QUIC: A UDP-Based Multiplexed and Secure Transport
draft-ietf-quic-transport-34

[[ comments ]]

[ section 8.2.3 ]

* I found this wording a tad odd:

    If the PATH_CHALLENGE frame that resulted in successful path
    validation was sent in a datagram that was not expanded to at least
    1200 bytes, the endpoint can regard the address as valid.

  It seems like whether the frame was padded to 1200 or not, if the response
  data matches the challenge data the address can be considered validated.

  I think the point at the end of the sentence is to say that *only* the
  address, but not the MTU, can be taken as validated.

[ section 9.6.3 ]

* Entirely optional, but I wonder if it's worth noting that in certain
  situations, for example an IPv6-only client and IPv4-only server, the
  client might be required to evaluate use of an alternate address family
  address if, for example, some transition mechanism (a la NAT64) was in
  use.

[ section 9.7 ]

* "as this would enable..." reads to me like the opposite of what's intended.
  Perhaps: "as failure to do so would enable..."?

[ section 14.1 ]

* I think it might be important to note that this strategy places some
  restrictions on the use of things like IPv6 extension headers that can be
  used in these packets.

  For example, on an IPv6-only link with a 1280 MTU, enforcing a 1200 byte
  UDP payload in these packets leaves only 32 bytes of space for any
  extension headers.

  I think this is likely fine for these initial packets (vis. section 8.1 and
  so on ), but as a general requirement for all packets this could
  artificially constrain use of new extension headers.

[ section 19.3.1 ]

* This seems intricate enough that it might be nice if there were an
  Appendix (A.5?) section walking through an example computation or two.

[ section 19.18 ]

* I'm idly wondering if there would be any debugging value in the response
  including the IP & port to which the response is being sent (i.e. from
  which the path challenge was received) ... assuming the packet with the
  PATH_RESPONSE frame is protected.

  Not important though, and perhaps it was already discussed and rejected.
  (or maybe it's better as some future, entirely separate PATH_INFO frame)


[[ questions ]]

[ section 8.2.4 ]

* To be clear, this document is effectively saying that it takes no position
  on the interpretation of any ICMP errors received?  Is it up to the
  implementer to decide if "validated" (in as much as ICMP messages can be
  validated) Admin Prohibited messages, for example, should constitute a
  positive confirmation of path failure?  Or is there some very specific
  stance that should be taken ("never trust that lyin' ICMP!")?

[ section 10.3 ]

* Does this "datagram ends with stateless reset token" scheme mean that
  implementations must check the output of every packet, including post
  encryption, and take some action if a (very low probability) collision
  occurs (meaning the output accidentally produces this 16 byte value
  but the packet is not intended to be a stateless reset)?


[[ nits ]]

[ section 7 ]

* There seem to be two paragraphs with the same text about how an endpoint
  validates ECN support.  Seems like maybe only the first paragraph is really
  necessary (or, put another way: I can't see what new information the second
  paragraph adds).

  (the paragraph below Figure 4 seems to be repeated information)

[ section 8.1.1 ]

* "a NEW_TOKEN frames" -> "a NEW_TOKEN frame" or "NEW_TOKEN frames"

[ section 17.2.3 ]

* ", as defined Section" -> ", as defined in Section"

This is really great technical and written work.  Kudos to everyone involved, and I'm happy to join the "Yes" pile.

There were some SHOULDs in the document that had me wondering why one might legitimately do something other than what's being recommended, since SHOULD does leave the implementer a choice.  For instance, in Section 10.2.2 there's this:

   An endpoint MAY enter the draining state from the closing state if it
   receives a CONNECTION_CLOSE frame, which indicates that the peer is
   also closing or draining.  In this case, the draining state SHOULD
   end when the closing state would have ended.  In other words, the
   endpoint uses the same end time, but ceases transmission of any
   packets on this connection.

This left me wondering why the endpoint would ever do something other than what's specified as SHOULD here.  There might be a good reason, but the prose doesn't say what that might be (or I managed to miss it).  Or maybe "SHOULD end" should simply be "ends"?

Also, I have the same question as Ben about Section 22.1.4.  I think it's appropriate that a permanent registration refers to a specification that is also likely to be permanent, as we do with Standards Track RFCs.

Lastly, also on 22.1.4: I believe (though my colleagues can correct me if I'm wrong) that the IESG's current preference is to list the IESG, and not a working group, as the contact email address for registrations over which the IETF has change control.

Thank you to the WG and implementation community for this document.

** Section 3.  Per “The state machines shown in this section are largely informative ”, why the qualifier of “largely”?

** Section 8.1.  Per “Servers SHOULD ensure that tokens sent in Retry packets are only accepted for a short time”, is there any guidance on what a short time is here?

** Section 21.5.  Per “QUIC servers SHOULD NOT be deployed in networks that also have inadequately secured UDP endpoints”, I was wondering if this caution is realistic.

Thanks for a clear and complete document.

Section 17.4: For someone coming to this new, it might not be obvious why requiring the disabling of the spin bit on a fraction of connections is useful. This may be worth a sentence of explanation.

Thanks for the great work on this important protocol, and for a very well written document!  Just some very minor editorial comments:


— Section 3.5 —

   An endpoint SHOULD copy the error code from the STOP_SENDING frame to
   the RESET_STREAM frame it sends, but MAY use any application error
   code.

— Section 9.6.2 —
   It SHOULD drop packets
   for this connection received on the old IP address, but MAY continue
   to process delayed packets.

Do as you think best with cases such as these, but for my part, I dislike the “SHOULD... but MAY” formulation, as I find it contradictory when it’s read strictly.  In general, I prefer to simply avoid the BCP 14 key word for the second part (“SHOULD do x, but may make other choices”).  In both cases here, I’d probably just leave off the second part, as it doesn’t seem to add anything.  At the least, I encourage making it “may” (or “can”).  But again, as you think best.

— Section 4 —

   It is necessary to limit the amount of data that a receiver could
   buffer, to prevent a fast sender from overwhelming a slow receiver,
   or to prevent a malicious sender from consuming a large amount of
   memory at a receiver.

You’re not talking about limiting the ability of the receiver (“could buffer”), but limiting the potential buffering requirement on the client (“has to buffer”), yes?

— Section 4.1 —

   Once a receiver advertises a limit for the connection or a stream, it
   MAY advertise a smaller limit, but this has no effect.

I don’t think this really fits the spirit of “MAY”.  I would say, “it is not an error to advertise a smaller limit, but....”

— Section 7 —

   Once completed, endpoints are able to exchange
   application data.

The antecedent to “once completed” is dangling, and the previous sentence talks about exchanging application data earlier.  I suggest, “Once the handshake is completed, endpoints are able to exchange application data freely.”

Thank you for addressing my Discuss point!

My previous comments and the associated github issues are available at
https://mailarchive.ietf.org/arch/msg/quic/yiSeJb2bgM4Aeef-ut5K1PhVNrw/
with thanks to Lucas for doing the format conversion.

IANA is now happy. So waiting for updated draft of that before approving.

I'm proud of the IETF for producing this document. I have a few minor comments and a bunch of nits.:

COMMENTS:

17.2.1 I believe it is correct that there will be no negative consequences from not having Retry-like integrity protection on VN packets. But I ask the editors to take one more careful look at it, as the VN format is one of those things we really cannot fix later.

21.13 "This means that client-controlled fields, such as the initial Destination Connection ID used on Initial and 0-RTT packets SHOULD NOT be used by themselves to make routing decisions." There was a lot of discussion in the QUIC-LB design team about whether this was an attack to be worried about or not, and we came down in favor of "not". 

More importantly, I don't see how this is practical advice. If we're to use Retry SCIDs to route subsequent packets, then load balancers have to use the DCID of Initials. Without validating the token, which most LBs will not do, they have no way of distinguishing between attacker-generated DCIDs with a bogus token and those that originally came from the server. One option is to simply remove this recommendation.

Alternatively, you could leave this section unaltered and delete the bit in 8.1.2 about using Retry to reroute packets. In practice, keeping 21.13 would require a revision of QUIC-LB to just use 4-tuple routing for long header packets or make it less robust for new versions.

22 I am unclear about the status of these registries (except the version registry) for new versions. QUICv2 might have entirely new frame, TP, and error registries, right? Is it worthwhile to point that out? Or that it's heavily discouraged, or forbidden?

NITS:

3.1 An endpoint shouldn't "generate STREAM_DATA_BLOCKED frames" if it is suffering from connection flow control limits.

8.1.2 I am not sure what you mean by the phrase "that can be unprotected"

13.3 I believe MAX_STREAM_DATA retransmissions should cease in state RESET_RECVD.

13.3 "it is not forbidden to retransmit copies of frames from lost packets" Is this true for PATH_CHALLENGE? I thought this quite explicitly shouldn't be copied.

14 "Thus, modern IPv4 and all IPv6 network paths will be able to support QUIC." Generally true, but should be qualified for the presence of arbitrary numbers of tunnels.

16 The CID length field is another exception to varint encoding.

17.2.2 Please include a reference for HelloRetryRequest for those unfamiliar with TLS.

17.2.5.3 "A client MUST use the same cryptographic handshake message it included in this packet. A server MAY treat a packet that contains a different cryptographic handshake message as a connection error or discard it." If the client hello is large, the Retry Token itself might affect what part of it fits in the packet. The language here doesn't contradict that, but a naive server implementation of the server check might not catch that corner case (e.g. including a hash of the CHLO in the Retry token)

[BTW the very next paragraph redundantly repeats part of this requirement].

Hello,

thank you for this document.

NITs
This appears twice:
   This document defines version 1 of QUIC, which conforms to the version-independent properties of QUIC defined in [QUIC-INVARIANTS].
First time at te beginning of 1 and then at the end of 1.1