Re: [tsvwg] Comments on UDP Options -13

["[e-mail]" <[e-mail]>]

Hi, Tom,

I’ll try to clarify below. Not sure I can dig up all the related threads, but also not sure they’re needed…

—
Joe Touch, temporal epistemologist
www.strayalpha.com

> On Sep 2, 2021, at 6:22 AM, Tom Jones <[e-mail]> wrote:
> 
> Hi Joe,
> 
> I have read draft -13 in advance of the iterim tomorrow. This is after a long
> gap of not reading updated drafts or closely following the mailing list
> discussions. I have read and offer comments on the document up to section 6
> here.
> 
> My comments and suggestions follow.
> 
> I don't want to re litigate old arguments if it can be helped, if
> something here has been discussed please share a link to the relevant
> mailing list thread and I will read and ask again if needed.
> 
> --
> 
> The first mention of ENCR is here:
> 
> 	>> Only the OCS, AUTH, and ENCR options depend on the contents of
> 	   the option area.
> 
> It isn't described until section 5.8. I suggest you clarify that it is a UNSAFE
> sub option, maybe something like:
> 
> 	>> Only the OCS, AUTH and the ENCR (UNSAFE suboption) depend on the contents of
> 	   the options area.
> 
> I think the document should be cleaned up so that references to ENCR make it
> clear that it is hidden away inside an UNSAFE option.

Agreed.

> --
> 
> 	This issue is discussed further in Section 17.
> 	...
> 	(with the exception of NOPs, which should never need to come in sequences of more than seven in a row)
> 
> The text from section 17 doesn't really add a lot, could a sentence be added
> here too, I think it will help readers a lot.
> 
> This padding limit makes UDP Options DPLPMTUD a violation of UDP Options. This
> seems a weird way to start out.
> 
> https://datatracker.ietf.org/doc/html/draft-fairhurst-tsvwg-udp-options-dplpmtud#section-3.2.1

Gorry can help us understand how that padding works. It could be just that the options end (at EOL), but the remaining zeros (which aren’t NOPs because they’re after EOL) are the padding intended.

> -- 
> 
> 	>> The OCS MUST be included when the UDP checksum is nonzero and UDP
> 	   options are present.
> 
> What happens when the checksum is non zero and the OCS is missing? The document
> should be explicit here, maybe:
> 
> 	>> The OCS MUST be included when the UDP checksum is nonzero and UDP
> 	   options are present. If the OCS option is not present and the UDP
> 	   checksum is nonzero then the OCS calculation implicitly fails and
> 	   all options MUST be ignored and the surplus area silently discarded.

This is the thread that Gorry and Mike just commented on today. I think we have a resolution that would update this text.

> --
> 
> 	This Internet checksum is computed over the entire surplus area
> 	prefixed by the surplus length pseudoheader (Figure 8) and then
> 	adjusting the result before storing it into the OCS checksum field.
> 
> I don't understand what is meant by 'adjusting the result', I think this should
> be 'aligning the result', or on reading again, 'coordinated'? I think more
> clarity is required here.

Agreed; it’s alignment.

> --
> 
> 	The OCS covers the UDP option area as formatted for transmission and
> 	immediately upon reception.
> 
> I don't understand what the sentence is for. I understand that the inserting
> the calculated OCS should be the final modification to the packet before
> transmission, I don't see what 'immediately upon reception' is trying to say.

OCS check should be the first step of interpreting the option area at the receiver.

> --
> 
> 	>> If the OCS fails, all options MUST be ignored and the surplus
> 	area silently discarded.
> 
> and 
> 
> 	As a reminder, use of the UDP checksum is optional when the UDP
> 	checksum is zero. When not used, the OCS is assumed to be "correct"
> 	for the purpose of accepting UDP packets at a receiver (see Section
> 	7).
> 
> give the possible results of the OCS as: 'fails' and 'correct'.  I think the OCS
> is 'correct' or 'incorrect' and the OCS calculation 'passes' or 'fails'
> 
> Should the OCS calculation pass in the second case? Maybe this should be:
> 
> 	the OCS is assumed to be "correct" and the OCS calculation passes

Agreed this is vague and will clarify. 

> --
> 
> 	UDP fragmentation relies on a fragment expiration timer, which can
> 	be preset or could use a value computed using the UDP Timestamp
> 	option.
> 
> I am not happy suggesting the use of the UDP Timestamp to control fragment
> lifetime. I think this invites security based implementation issues.

Can you clarify? UDP isn’t a “secure” protocol per se. It seems reasonable for UDP reassembly to take into account UDP measurements of the RTT, the same way that TCP uses its timestamps for its time-dependent features.

> --
> 
> 	   >> UDP fragments MUST NOT overlap.
> 
> I think you need to be explicit.
> 
> 	>> UDP fragments MUST NOT overlap. On receipt of an overlapping
> 	   fragment, the fragment and all stored fragments matching the
> 	   Identification MUST be dropped.

I favor copying the text from 8200 on this point:
      o  If any of the fragments being reassembled overlap with any
         other fragments being reassembled for the same packet,
         reassembly of that packet must be abandoned and all the
         fragments that have been received for that packet must be
         discarded, and no ICMP error messages should be sent.

         It should be noted that fragments may be duplicated in the
         network.  Instead of treating these exact duplicate fragments
         as overlapping fragments, an implementation may choose to
         detect this case and drop exact duplicate fragments while
         keeping the other fragments belonging to the same packet.

> --
> 	
> 	>> The default UDP reassembly SHOULD be no more than 2 minutes.
> 
> This should be 'The default UDP timer'
> 
> What happens after 2 minutes? As above, all fragments stored for the expired
> fragment chain should be discarded.

Agreed (same as for IP frags, e.g., as per RFC8200 as well).

> --
> 
> 	Implementers are advised to limit the space available for UDP
> 	reassembly.
> ...
> 	>> UDP reassembly space SHOULD be limited to reduce the impact of
> 	DOS attacks on resource use.
> 
> The first of these feels redundant. 
> 
> I think you should note for implementers. The use of many small fragments,
> specifically ordered has been used in fragmentation and tcp segmentation
> attacks in the past to create DOS by increasing processing cost of fragments.
> 
> Care should be taken when designing reassembly algorithms and selecting data
> structures. see:
> 	https://www.swascan.com/segmentsmack/

I would prefer a stable reference if one were available.

> --
> 
> I think section 5.5 would benefit from a diagram.

Can you let me know what you’re thinking would help? I.e., do you want to see before/after frag?

> --
> 
> Section 5.6 doesn't adhere to the convention described in section 2 (the use of
> '>>' for sections with RFC2119 key words).

I plan on catching this more consistently in the final doc passes.

> --
> 
> I will admit that I wasn't following any discussion around UNSAFE and its
> introduction. I cannot say that I like this design, it feels excessively
> complicated in an already complicated specification.
> 
> I struggle to see why this is being included in this form when there isn't yet
> any case driving its use.

UDP options are generally in the same spirit as TCP options - ignore the ones you don’t support. There are some we don’t want to have that behavior, i.e., we want them to abort option processing rather than be silently ignored, esp. because we can’t negotiate their use with hard state the way TCP does.

Options that modify the UDP user data fall into this category.

> --
> 
> Could you suggest some application uses of the TSval and TSecr options? 
> 
> It is clear to me how my timestamp (and its reflection) are useful to me for
> calculating RTT, but that doesn't require the TSval be forward to the
> application.

You can always look at the smallest value you get reflected back; the difference between that and your current time is RTT.

> It is clear not how my peers timestamp is useful at all. I would prefer that
> the peers timestamp be treated as a monotonically increasing token.

The peer’s TS is needed because it’s what you return to help them compute RTT.

> I guess that the values could be used to detect reordering, is this the
> intention of passing the values to the application?

Also to let them do things like compute RTT max, RTT std dev, etc.

> 
> --
> 
> 	One such use is described as part of the UDP variant of packetization layer
> 	path MTU discovery
> 
> I think this should say 'UDP Options variant', to me, a UDP variant would be an
> application layer protocol as described in section 6.1 of RFC8899.

Agreed.

> https://www.rfc-editor.org/rfc/rfc8899.html#name-application-support-for-dpl

Sure.