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

From: Erik Kline via Datatracker <[email-masked]>
Date: Mon, 04 Jan 2021 22:18:08 -0800

Erik Kline 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:
----------------------------------------------------------------------

[[ 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"

---

From: Lucas Pardue <[email-masked]>
Date: Wed, 6 Jan 2021 03:41:58 +0000

Hi Erik,

Thanks for the review. I've captured your comments as issues on the QUIC WG
GItHub repository. Links to each are provided as in-line responses.

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

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

>   address if, for example, some transition mechanism (a la NAT64) was in

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

>   Perhaps: "as failure to do so would enable..."?

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

>   artificially constrain use of new extension headers.
https://github.com/quicwg/base-drafts/issues/4516

[ section 19.3.1 ]
>   Appendix (A.5?) section walking through an example computation or two.
https://github.com/quicwg/base-drafts/issues/4517

[ section 19.18 ]
>   (or maybe it's better as some future, entirely separate PATH_INFO frame)
https://github.com/quicwg/base-drafts/issues/4518

>   stance that should be taken ("never trust that lyin' ICMP!")?
https://github.com/quicwg/base-drafts/issues/4519

[ section 10.3 ]
>   but the packet is not intended to be a stateless reset)?
https://github.com/quicwg/base-drafts/issues/4520

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

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

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

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

> * ", as defined Section" -> ", as defined in Section"
https://github.com/quicwg/base-drafts/issues/4523

Cheers,
Lucas
On behalf of QUIC WG Chairs

---

From: "Martin Thomson" <[email-masked]>
Date: Wed, 06 Jan 2021 14:52:40 +1100

Hi Erik,

Thanks for the comments.   I'll answer your questions here, since I don't think they result in document changes.  We'll take the comments to GitHub, where they can be broken down into specific actions.

>   stance that should be taken ("never trust that lyin' ICMP!")?

I don't think we could reach consensus on a single policy for treatment of ICMP.  As you observe, some might regard ICMP as unreliable no matter the circumstances, others might trust but verify, and others might be happy to accept the signals without validation.  Support for RFC 4884 and larger packet samples might shift attitudes.

What I can say is that many implementations are not doing anything with ICMP, other than what the OS might automatically turn into socket errors.  That means that ICMP Unreachable can still have an effect in many stacks, but PTB doesn't enjoy the same privilege (at least to my knowledge, though I guess the OS could apply write size limits on its own, I don't know that any do).

In practice, the exception that follows means that you don't need to perform the check very often.  Most implementations only check when they are unable to decrypt or - more applicable to servers - unable to determine which connection is able to handle the packet.  The relevant text says:

> This comparison can be performed for every inbound datagram. Endpoints MAY skip this check if any packet from a datagram is successfully processed. However, the comparison MUST be performed when the first packet in an incoming datagram either cannot be associated with a connection, or cannot be decrypted.

Those that do check every datagram do run a risk of encountering a collision.  But it is tiny.

To take an extreme example, a server instance that receives ~2^62 packets for every connection (the maximum number any single connection can have, and far more than you can practically send), and you have a large number of connections (2^17 connections is a lot, but maybe plausible), and you check every inbound packet against every connection, the odds of collision are in the same ballpark as the odds that a dedicated and resourceful attacker might have to break encryption[1] at 2^-51 [2].

The Internet is big enough that this will likely happen, but I'm of the opinion that this risk is acceptable.  In any case, avoiding extra cycles has the effect of driving this probability right down.

[1] We allow 2^-57 probability.
[2] Well, maybe a little higher, but I'm too lazy to run the birthday paradox numbers.

---

From: Benjamin Kaduk <[email-masked]>
Date: Fri, 8 Jan 2021 10:42:56 -0800

Hi Martin,

> In practice, the exception that follows means that you don't need to perform the check very often.  Most implementations only check when they are unable to decrypt or - more applicable to servers -…

My reading is that Erik was asking about checking when generating the
packet, but your response seems to be about checking when receiving the
packet.  (I had aa similar question but didn't ask it since, as you note
below, the odds are in our favor.)

-Ben

---

From: "Martin Thomson" <[email-masked]>
Date: Sat, 09 Jan 2021 15:13:24 +1100

> below, the odds are in our favor.)

Most of the same logic applies, but I would look to the probability of collisions and decide that the effort is not worth it.  I am fairly sure no one thought to do so thus far.