Comments on QUIC transport - draft-ietf-quic-transport-20

[Gorry Fairhurst <[e-mail]>]

I have just read QUIC transport, prior to doing a detailed read of the 
congestion control spec) and have a number of comments on the text and 
one more involved question on the decision on what to do with congestion 
control when there is a path change. I 'll send the latter issue later. 
(The editors can break these up into issues in the git if this seems 
useful to them).

Gorry

—————
The following comments/questions mainly relate to text:
—
NiT: Section 4:
“ It is necessary to limit the amount of data that a receiver could
    buffer,”
- Is this correct? (i.e. do we wish to limit what it can do or what it 
needs to buffer - is this a maximum limit or a minimal requirement? (I 
suspect the latter).
---
Question: Section 4:
  “The implementation SHOULD provide an interface to QUIC to tell it
    about its buffering limits so that there is not excessive buffering
    at multiple layers.”
- This seems an odd place to me to make a subtle new requirement on the 
IP-layer/interface: This lower-layer implementation interface 
requirement is buried in the spec. This seems that it could go 
unnoticed, and isn’t really a requirement of the protocol, but of the 
layer below.
—
Nit: Section 4:
   “that the sender receives an update before running out of flow control
    credit, even if one of the packets is lost.”
- the requirement could be misread as ONLY one packet is lost, whereas I 
think it means if one or more packets are lost.
—
Nit: Section 4:
  “That is, once a receiver
    advertises an offset, it MAY advertise a smaller offset, but this has
    no effect.”
- No effect on what? - presumably something at the sender?
—
Nit: Section 4:
   “Implementations might choose to increase limits as
    streams close to keep the number of streams available to peers
    roughly consistent.”
- could this be changed to “are closed” or something to avoid misreading 
of the word “close”.
—
NiT: Section 5:
“A Version Negotiation (Section 17.2.1) packet echoes the connection
    IDs selected by the client, both to ensure correct routing “:
- not strictly routing? Isn’t this “forwarding”. (The word routing is 
used again in the section and in section 7.2
—
Section 5:
NiT: “different IP or port”
- could you say:
“different IP address or transport port”
---
Section 5.1.1:
NiT: “The connection
    ID randomly selected by the client in the Initial packet and any
    connection ID provided by a Retry packet are not assigned sequence
    numbers unless a server opts to retain them as its initial connection
    ID.”
- could this sentence be simplified, it is hard to parse and long. (Or 
insert a comma)
—
Section 5.2:
NiT: “  or - for servers -“
- what is the meaning of the minus signs? does it imply “particularly” 
or “only”, I was unsure.
—
Section 5.2.1:
Nit: “Packets that don't match an existing connection are discarded.”
- can we remove “don’t” and explain what does match?
- Is this something like: “Packets with a Connection ID that does not 
match an existing
    connection are discarded.”
---
Section 5.2.1:
Nit: “Due to packet reordering or loss, clients might receive packets for a
    connection that are encrypted with a key it has not yet computed.”
“
- Is it possible to say something like: “Packet reordering or loss, 
might cause a client  to receive packets for a connection that were 
encrypted with a key that it has not yet computed.”
—
Section 8.1.2:
- The title seems a little odd to me? Future?
—
Section 9.3.3.:
?: “ An off-path attacker that can observe packets might forward copies of
    genuine packets to endpoints.  “
- This may make total sense to some people, but to me an “off path” 
observer has no access to the data on the path. Maybe someone could work 
this text in a way that doesn’t cause this confusion.
—
  Section 9.3.3.:
NiT: “If the copied packet arrives before
    the genuine packet, this will appear as a NAT rebinding.”
I think the pathology is the same as a NAT rebinding, but it isn’t one. 
The text could be clear that this appears as if there had been a NAT 
rebidding?
—
Section 9.3.3.:
?: “The attack is more reliable “
- The term reliable does have certain implications in transport 
documents, if this isn’t intended could this phrased as a higher 
likelihood of success or something like that?
- similarly “attack is reliably faster “
- explain how the path can be reliable, or please choose another word?
—
Issue: Section 10.4.2.:
?" “using a second iteration of a preimage-resistant function.
- Maybe I am alone, but I  did not understand what was intended by this, 
and where this was defined: is this I-D:
---
Section 19.1:
Question: “Padding can be used to
    increase an initial client packet to the minimum required size, or to
    provide protection against traffic analysis for protected packets.”
- I suggest this also can be used to perform path packet probing in 
DPLPMTUD, and the text should somehow be improved to permit this.
—
Section 9.4:
“The capacity available on the new path might not be the same as the
    old path.”
- I disagree in the words here, because I think the normal IETF 
transport assumption is much stronger than “might”, and the draft should 
say something like:
“The endpoint should not assume the capacity available on the new path 
isbe the same as the
    old path.”
—
Section 9.4:
NiT: “On confirming a peer's ownership of its new address, an endpoint
    SHOULD immediately reset the congestion controller and round-trip
    time estimator for the new path.”
- To be clear, does this reset require setting the CC to the initial 
value for a connection?
—
RFC2119:  “A sender can make exceptions for probe packets”
- Is that “MAY” - that would seem entirely reasonable.
—
“   While multiple paths might be used during connection migration, a
    single congestion control context and a single loss recovery context
    (as described in [QUIC-RECOVERY]) may be adequate. “
- I think the present I-D should not make these claims, because this 
seems like a CC decision. I actually do not understand this specific 
claim as currently presented.
—
Section 9.7:
Ref: “The IPv6 flow label SHOULD be a pseudo-random function of the source
    and destination addresses, source and destination UDP ports, and the
    destination CID. “
- Please cite RFC3976 (which contains details on how to set flow labels).—
—
Section 12.3:
NiT: “and acknowledged in packets which are also Initial packets”
- Change /which/ to /that/.
—
Section 14.2:
Ref: “An endpoint MUST NOT increase PMTU based on ICMP messages.”
- please add citation to  [RFC1191] [RFC8201], so the reader knows this 
is true of any protocol.
—
Section 21.2:
Question: “   Once a connection is established QUIC endpoints might 
accept some
    unauthenticated ICMP packets (see Section 14.2), but the use of these
    packets is extremely limited. “
- Why is this a good idea, please explain what these would be used for?
—
Section 21.5:
“   An adversarial sender might intentionally send fragments of stream
    data in order to cause disproportionate receive buffer memory
    commitment and/or creation of a large and inefficient data structure.”
- Is this memory commitment at the target? (I assume so, but it would be 
easier to read if it said this).
-