[tsvwg] Comments on draft-ietf-tsvwg-udp-options-21 and -22

["C. M. Heard" <[e-mail]>]

On Mon, May 22, 2023 at 6:21 AM Gorry Fairhurst wrote:
> I have just read version 20, and I have some questions as a working
> group member. I hope this helps.
[ ... ]
>
> 12. "Values should "increase" (allowing for rollover) according to a
>        typical 'tick' time"
> - explain rollover in terms of the modulo the field size?

and in draft-ietf-tsvwg-udp-options-22 Section 9.8 we now see:

   o  Values should "increase" (allowing for rollover, i.e., modulo the
      field size) according to a typical 'tick' time

where "modulo the field size" is new text. But later on (in text that
has been around for several versions) we see:

   Rollover can be handled as a special case or more completely using
   sequence number extension [RFC9187], however zero values need to be
   avoided explicitly.

   >> TIME values MUST NOT use zeros as valid time values, because they
   are used as indicators of requests and responses.

Since the value zero is not a value time value, it is incorrect to say
that values should increment modulo the field size, i.e., modulo 2^32.
At most they can increment modulo 2^32 - 1. In fact, though, if time
values are incremented modulo 2^32 - 1, using output values in the range
1 through 2^31-1, what you have is nothing more than conventional ones
complement arithmetic, and the difference between an echoed time stamp
received in a response and the current time value when it that response
is received can be reliably calculated through a rollover event by using
ones complement arithmetic. I've commented on this point before, and
I've received resistance on the grounds that such an interpretation is
unnecessarily constraining. Well, I don't agree. I think being explicit
on this point will enhance interoperability and predictability with
insignificant constraints. If there is disagreement, I'd like to hear
of an alternative scheme that accomplishes the same things within the
constraints of a 32-bit field width and avoidance of the value zero. In
any case, "modulo the field size" is not correct

=======================================================================
In draft-ietf-tsvwg-udp-options-22 Section 9.1 we see

   >> Bytes after EOL in the surplus area MAY be checked as being zero
   on receipt, but MUST be treated as zero regardless of their content
   and are not passed to the user (e.g., as part of the surplus area).

Do the words "MUST be treated as zero regardless of their content" mean
that an implementation that does not verify that bytes after EOL are in
fact equal to zero have to exclude them from the OCS validation? Doing
so would not play well checksum offload (see
https://mailarchive.ietf.org/arch/msg/tsvwg/9_1YWp-TApI9mcCuia8U6jfCwTA/).

I therefore propose this replacement text:

   >> Bytes after EOL in the surplus area MAY be checked as being zero
   on receipt but MUST NOT otherwise be processed (except for OCS
   calculation) or passed to the user.

This comment was made originally against -19 on 27-Jan-2023 but was not
addressed.
=======================================================================
In draft-ietf-tsvwg-udp-options-22 Section 9.4 we see:

   >> Similar to IPv6 reassembly [RFC8200], if any of the fragments
   being reassembled overlap with any other fragments being reassembled
   for the same UDP packet, reassembly of that UDP packet MUST be
   abandoned and all the fragments that have been received for that UDP
   packet must be discarded, and no ICMP error messages should be sent
   in this case (to avoid a potential DOS attack turning into an ICMP
   storm in the reverse direction).

and

   >> UDP reassembly expiration MAY generate an ICMP error, but this
   MUST NOT use the existing IP reassembly timeout error type and code.

   [TBD: ?? Should we define a new code for this purpose?]

In my opinion, any mention of sending ICMP errors for UDP reassembly
failures is totally out of place, and all mention of this should be
removed from Section 9.4. Sending ICMP messages is a function of the
IP layer, not of the UDP layer (or any other transport layer).

My apologies for not commenting on this when it first showed up in -14.
=======================================================================
In draft-ietf-tsvwg-udp-options-22 Section 12 we see:

           if the UDP checksum passes or is zero then
               if ((OCS != 0 and fails or OCS == 0) and UDP CS != 0)
               or ((OCS != 0 and passes) and UDP CS == 0) then
                   deliver the UDP user data but ignore other options
                   (this is required to emulate legacy behavior)
               if OCS != 0 and passes or OCS == 0 when UDP CS != 0 then
                   deliver the UDP user data after parsing
                   and processing the rest of the options,
                   regardless of whether each is supported or succeeds
                   (again, this is required to emulate legacy behavior)

Corrected:

           if the UDP checksum passes or is zero then
               if OCS != 0 and fails or OCS == 0 when UDP CS != 0 then
                   deliver the UDP user data but ignore other options
                   (this is required to emulate legacy behavior)
               if OCS != 0 and passes or OCS == 0 when UDP CS == 0 then
                   deliver the UDP user data after parsing
                   and processing the rest of the options,
                   regardless of whether each is supported or succeeds
                   (again, this is required to emulate legacy behavior)

This comment was made originally against -19 on 27-Jan-2023 but was not
addressed.
=======================================================================
In draft-ietf-tsvwg-udp-options-22 Section 22, 2nd paragraph, we see:

   Note that any user application that considers UDP options to affect
   security need not enable them. However, their use does not impact
   security in a way substantially different than UDP options; both
   enable the use of a control channel that has the potential for
   abuse. Similar to TCP, there are many options that, if unprotected,
   could be used by an attacker to interfere with communication.

The phrase "substantially different than UDP options" should be changed
to "substantially different than TCP options."

This comment was made originally against -19 on 27-Jan-2023 but was not
addressed.
=======================================================================
In draft-ietf-tsvwg-udp-options-22 Section 22, 3rd paragraph, we see:

   UDP options create new potential opportunities for DDOS attacks,
   notably through the use of fragmentation. Except when enabled, UDP
   options cause no additional work at the receiver. At most, the
   required options (if enabled) result in a responding option in the
   next transmitted packet, but no options (including ECHO) ever
   initiate UDP responses in the absence of user transmission.

Corrected:

   UDP options create new potential opportunities for DDOS attacks,
   notably through the use of fragmentation. When enabled, UDP options
   may cause some additional work at the receiver; however, of the
   must-support options, only REQ (when used on a DLPMTUD probe [Fa22])
   will initiate a UDP response in the absence of a user transmission.

Note that [Fa22] needs to be updated to [Fa23].

This comment was made originally against -19 on 27-Jan-2023 but was not
addressed.
=======================================================================

The unaddressed comments mentioned above can be found in
https://mailarchive.ietf.org/arch/msg/tsvwg/aGOHvmGfSTRs4ATgEEpqHrWAln4/.

I also commented there on the need to replace turn [To18]
(draft-touch-tcp-ao-encrypt) by a stable, normative reference, but I
won't belabor that, since it's part of a larger ongoing discussion.

Thanks and regards,

Mike Heard