Subject: Martin Duke's Yes on draft-ietf-quic-transport-33: (with COMMENT)

From: Martin Duke via Datatracker <[email-masked]>
Date: Mon, 21 Dec 2020 16:09:01 -0800

Martin Duke has entered the following ballot position for
draft-ietf-quic-transport-33: Yes

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-quic-transport/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

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].

---

From: Lucas Pardue <[email-masked]>
Date: Tue, 22 Dec 2020 18:15:08 +0000

Hi Martin,

Thanks for the comments. I've captured each comment as a new issue on the
QUIC WG base-drafts Github repository and tagged you in each one. The
document editors will follow up on each issue and may create Pull Requests
if needs be. See in-line response for more details. We'll be tracking all
IESG comments on the Transport document under
https://github.com/quicwg/base-drafts/milestone/23

> https://datatracker.ietf.org/doc/draft-ietf-quic-transport/

https://github.com/quicwg/base-drafts/issues/4450

> packets or make it less robust for new versions.

https://github.com/quicwg/base-drafts/issues/4453

> discouraged, or forbidden?

https://github.com/quicwg/base-drafts/issues/4451

> suffering from connection flow control limits.

https://github.com/quicwg/base-drafts/issues/4452

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

https://github.com/quicwg/base-drafts/issues/4454

> 13.3 I believe MAX_STREAM_DATA retransmissions should cease in state

https://github.com/quicwg/base-drafts/issues/4455

> this true for PATH_CHALLENGE? I thought this quite explicitly shouldn't be

https://github.com/quicwg/base-drafts/issues/4456

> Generally true, but should be qualified for the presence of arbitrary
https://github.com/quicwg/base-drafts/issues/4457

> 16 The CID length field is another exception to varint encoding.
https://github.com/quicwg/base-drafts/issues/4458

> with TLS.

 https://github.com/quicwg/base-drafts/issues/4459

> https://github.com/quicwg/base-drafts/issues/4460

Cheers,
Lucas
On behalf of QUIC WG Chairs