[tsvwg] Intdir early review of draft-ietf-tsvwg-udp-options-19

Carlos Pignataro via Datatracker <noreply@ietf.org> Fri, 10 February 2023 01:11 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: tsvwg@ietf.org
Delivered-To: tsvwg@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 47254C17EE09; Thu, 9 Feb 2023 17:11:07 -0800 (PST)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Carlos Pignataro via Datatracker <noreply@ietf.org>
To: int-dir@ietf.org
Cc: draft-ietf-tsvwg-udp-options.all@ietf.org, tsvwg@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 9.8.0
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <167599146728.42049.10916891372133731811@ietfa.amsl.com>
Reply-To: Carlos Pignataro <cpignata@cisco.com>
Date: Thu, 09 Feb 2023 17:11:07 -0800
Archived-At: <https://mailarchive.ietf.org/arch/msg/tsvwg/r7XaE4ulwCtn76NW3UbyQ6FObSk>
Subject: [tsvwg] Intdir early review of draft-ietf-tsvwg-udp-options-19
X-BeenThere: tsvwg@ietf.org
X-Mailman-Version: 2.1.39
List-Id: Transport Area Working Group <tsvwg.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/tsvwg>, <mailto:tsvwg-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/tsvwg/>
List-Post: <mailto:tsvwg@ietf.org>
List-Help: <mailto:tsvwg-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/tsvwg>, <mailto:tsvwg-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 10 Feb 2023 01:11:07 -0000

Reviewer: Carlos Pignataro
Review result: Ready with Issues

Hi!

I was assigned an early review from the INTDIR of the following two I-Ds, for
an INTAREA review as part of the TSVWG WG Last Call:

https://datatracker.ietf.org/doc/draft-ietf-tsvwg-udp-options/
https://datatracker.ietf.org/doc/draft-ietf-tsvwg-udp-options-dplpmtud/

In general, I found both documents to be descriptive and consistent. They are
also effectively organzied, and not too difficult to follow (in reference to
draft-ietf-tsvwg-udp-options.)

They add meaningful substantive transport-layer capabilities, and from an
Int-Area viewpoint, including tunneling and tunnel encap consdieration, I found
no major issues or blockers.

I do, however, have a few comments and questions for your consideration.

draft-ietf-tsvwg-udp-options & draft-ietf-tsvwg-udp-options-dplpmtud:

Please treat all these as questions and observations/thoughts, hoping they are
clear and a bit useful, with thanks for the request.

A. Transport Options for UDP [draft-ietf-tsvwg-udp-options-19]

As a high-order bit, I would have expected a broader, deeper, and more explicit
discussion of two areas: (1) considerations relating every endpoint, stack,
middle-box, security filter, etc., which might complain in the presence of the
difference in length -- not explicitly defined, and (2) security and privacy
considerations of having effectively a covert channel, and whether it is best
to recommend it is not used versus recommending its use.

For (1), I find the section "Interactions with Legacy Devices" to be "anecdata"
more than "analytical", and (possibly as intended) not comprehensive nor much
useful. For example, "worked through NAT devices" is neither qualitative nor
quantitative, and a single NAT that might drop a UDP optioned packet might be
too much. As an aside, the qualifier "legacy" sounds like a stretch label --
every udp stack as semi-obsolete -- though this might be my misinterpretation
of the nuances of the use and semantics of the word.

For (2), I'd frankly find most useful a discussion on the engineering and
architectural tradeoffs between, (a) e.g., saying "the inconsistency of length
fields is largely a security invitation, and as such, though shall be malformed
packets sent to /dev/null, and if you want options use something else", versus
(b) saying "largely works and is mega useful and read the security
considerations". There seems to be an implied assumption, although frankly I
might have missed text, or it could be documented in the WG mailing list as
discussion had, and not worth for the I-D.

Some more in-lined comments, marked with "CMP":

Abstract

   Transport protocols are extended through the use of transport header
   options. This document extends UDP by indicating the location,
   syntax, and semantics for UDP transport layer options.

CMP: I often find the Abstract setting stage for a whole piece of work. These
could be nits and editorials, though maybe significant to call out: CMP: The
first sentence is an absolute statement. Are *all* transport protocols (or
*all* the time) extended as such? Frankly UDP has not been for decades, still
being a transport protocol, and truly will *not* be either extended with
"header options" (but trailing ones). For these two things, the very first
sentence in the document does not, to me, fully parse.

6. The UDP Surplus Area Structure

   The remainder of the surplus area consists of options defined using
   a TLV (type, length, and optional value) syntax similar to that of
   TCP [RFC9293], as detailed in Section 8. These options continue

CMP: It seems there are some options that do not use the Length either -- are
only Type. As such, is length optional in the first sentence, similar to the
value being optional? Otherwise, the minimum option would be two octets.

   For IPv4, IP Total Length field indicates the total IP datagram
   length (including IP header) and the size of the IP options is
   indicated in the IP header (in 4-byte words) as the "Internet Header
[...]
   UDP options use the entire surplus area, i.e., the contents of the
   IP payload after the last byte of the UDP payload. They commence
   with a 2-byte Option Checksum (OCS) field aligned to the first 2-
   byte boundary (relative to the start of the IP datagram) of that
[...]
   UDP options are typically a minimum of two bytes in length as shown
   in Figure 5, excepting only the one byte options "No Operation"
   (NOP) and "End of Options List" (EOL) described below.

CMP: NB: this is a humble, not presumptuous or arrogant. In the text above, as
well as throughout the document, I read "bytes" which is strictly a
machine-dependent quantity, and always try to use 'octets' or 'bits' instead.
RFC 791 says '32-bit words', not '4-byte words'. Octets instead of bytes?

8. UDP Options

   The Kind field is always one byte. The Length field is one byte for
   all lengths below 255 (including the Kind and Length bytes). A

CMP: It is clear from the examples and subsections under Section 9 that the UDP
Option Length includes the Kind and Length/EL fields themselves. Is that what
is meant in the parenthetical above? Something like this might be more clear:
CMP: "The Length field is one octet when its value is below 255. The value of
the Length  includes the Kind and Length fields, and as such its minimum value
is 2. [...]"

9.4. Fragmentation (FRAG)

CMP: How is FRAG a SAFE UDP Option?

24.1. Normative References

   [Fa22]    Fairhurst, G., T. Jones, "Datagram PLPMTUD for UDP
             Options," draft-ietf-tsvwg-udp-options-dplpmtud, Sep.
             2022.

CMP: Why is this a Normative Reference? I Imagined one I-D defined the base
option spec, while others can make use of them, and add other options. CMP:
That is, a unidirectional Normative reference
draft-ietf-tsvwg-udp-options-dplpmtud -> draft-ietf-tsvwg-udp-options.

~~~

B. Datagram PLPMTUD for UDP Options [draft-ietf-tsvwg-udp-options-dplpmtud-04]

CMP: One question only:

4.1.  Sending Probe Packets using the Echo Request and Response Options

   A Probe Packet includes the UDP Options area containing a RES Option
   and any other required options concluded with an EOL Option followed
   by any padding needed to inflate to the required probe size.

CMP: Is there an advantage or implication of this approach, versus having a
PADDING UDP Option which can be passed to UDP on receipt, and verify that
padding? (modulo the risk of a covert-channel)

Again, I hope these are clear and useful, and thanks for the review request and
entertaining these comments.

Thank you!

Carlos Pignataro