[tram] Late transport review for: draft-ietf-tram-stun-pmtud-10

Gorry Fairhurst <gorry@erg.abdn.ac.uk> Thu, 20 September 2018 18:29 UTC

Return-Path: <gorry@erg.abdn.ac.uk>
X-Original-To: tram@ietfa.amsl.com
Delivered-To: tram@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id D9CFE130DEE; Thu, 20 Sep 2018 11:29:38 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.9
X-Spam-Level:
X-Spam-Status: No, score=-1.9 tagged_above=-999 required=5 tests=[BAYES_00=-1.9] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id GGiDDmlry1gE; Thu, 20 Sep 2018 11:29:35 -0700 (PDT)
Received: from pegasus.erg.abdn.ac.uk (pegasus.erg.abdn.ac.uk [137.50.19.135]) by ietfa.amsl.com (Postfix) with ESMTP id 37099130DD9; Thu, 20 Sep 2018 11:29:34 -0700 (PDT)
Received: from Gs-MacBook-Pro.local (fgrpf.plus.com [212.159.18.54]) by pegasus.erg.abdn.ac.uk (Postfix) with ESMTPSA id B06101B00388; Thu, 20 Sep 2018 19:29:27 +0100 (BST)
Message-ID: <5BA3E707.7050307@erg.abdn.ac.uk>
Date: Thu, 20 Sep 2018 19:29:27 +0100
From: Gorry Fairhurst <gorry@erg.abdn.ac.uk>
Reply-To: gorry@erg.abdn.ac.uk
Organization: University of Aberdeen
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:12.0) Gecko/20120428 Thunderbird/12.0.1
MIME-Version: 1.0
To: tram@ietf.org, "tsv-ads@ietf.org" <tsv-ads@ietf.org>
CC: gonzalo.camarillo@ericsson.com, simon.perreault@logmein.com
References: <5BA3D727.8020805@erg.abdn.ac.uk>
In-Reply-To: <5BA3D727.8020805@erg.abdn.ac.uk>
X-Forwarded-Message-Id: <5BA3D727.8020805@erg.abdn.ac.uk>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: <https://mailarchive.ietf.org/arch/msg/tram/ZXUAXbDgDnZiYjRnj2NobxObx6E>
Subject: [tram] Late transport review for: draft-ietf-tram-stun-pmtud-10
X-BeenThere: tram@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "Discussing the creation of a Turn Revised And Modernized \(TRAM\) WG, which goal is to consolidate the various initiatives to update TURN and STUN." <tram.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/tram>, <mailto:tram-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/tram/>
List-Post: <mailto:tram@ietf.org>
List-Help: <mailto:tram-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/tram>, <mailto:tram-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 20 Sep 2018 18:29:39 -0000

I have just read the latest revison (10) of draft-ietf-tram-stun-pmtud. 
so this is a late review, sorry!

I missed that this document was being put to the IESG, and have some comments and questions. These comments may help in document review, but are copied to the document's authors and WG for their information and to allow them to address any issues.



Best wishes,

Gorry Fairhurst

----

This is important work to define a PL PMTUD method for the STUN protocol. I think this adds capability beyond what is possible using
network-layer IP PMTUD. I can see the value in a specification for adding some PMTU detection to the STUN protocol, and I think this specification addresses this need,
although this review also expresses concerns about whether the current text is ready for publication.

Some text in this draft is over IPv4-Centric:
Here is one example: section 4.2.2 is IPv4 centric, please also note the additional requirements in RFC 8201.

There is a related specification being developed in TSVWG:
https://tools.ietf.org/html/draft-ietf-tsvwg-datagram-plpmtud
- I think there would be significant benefit if the two documents are revised to mention the other document, but possibly it would be useful also to check against the design and see
whether the protocol addresses the same set of requirements?
  

Abstract

I would expect an abstract to describe what is within the document and why someone should read this. I read the current abstract and suggest this is rather
short on detail about what it provides. I think the current text needs work.

Introduction

I disagree with the following statement, and think the editors need to consider restating this:
/ The Packetization Layer Path MTU Discovery (PMTUD) specification
    [RFC4821] describes a method to discover the Path MTU but does not
    describe a practical protocol to do so with UDP./
The abbreviation required is PLPMTUD.
I'm not happy with the words "practical" protocol, that seems wrong, please rephrase.
  
PLPMTUD for datagram protocols is a work item in TSVWG, and the draft on this probably supplies better background to why STUN needs to add a
feature to help discover the path MTU.

The second para of the intro states:
/  Many UDP-based protocols do not implement the Path MTU discovery
    mechanism described in [RFC4821]. /
I disagree. I do not know of any details in RFC4821 that specify a protocol for UDP. In fact, there is no simple way to add PLPMTUD to UDP
applications without integrating this with the protocol using UDP.

I think the current text is expressed imprecisely: PMTUD is an IP-layer function. When applied, this does represent a standards tarck protocol that can be
used with UDP - there is no reason why an ICMP packet could not be used to set the IP size for fragmenting UDP or any other transport. Please so
note the update to PMTUD published as RFC 8201 IPv6 Path MTU Discovery in July 2017. This also has hints at the problems and challenges of
doing PMTUD at the network level.


Please clarify what is intended by the following text:
  / These probing mechanisms are  implemented with Session Traversal Utilities for NAT (STUN), but their usage is not limited to STUN-based protocols./
- I am concerned that this overlaps the work done in tsvwg and to me
this spec should much clearer about the applicability it is defining.
There is useful work here for STUN, but I am not sure what other protocols should use this particular approach. I would prefer to remove this extra claim.

The following text needs more clear text:
  / permits proper operations of UDP-based applications in the network. /
- I'd prefer to see an explanation of what this draft solves, not a general statement that it allows things to be correct. This needs to be rewritten.

There are references to non-adopted IDs in the introduction. What is the intention of the WG here:
[I-D.martinsen-tram-stuntrace] and
[I-D.martinsen-tram-turnbandwidthprobe]
- it seems weird to endorse these in a standards track document, even if informative, - are these references strictly needed (or could these be removed)?

Specification

Path Reordering:
The document seems to suggest keeping time-ordered information. How is the method impacted by reordering of probes (e.g. by alternate paths)
-  I am concerned that this method does not have text about how to not mis-interpret serioulsy delayed segments from
the network layer. I think the set of cached Probe Indication identifiers could do this, but I did not see this explicitly stated.

Loss:
I have a protocol question from Section 2:
/ which increase in size until transactions timeout, indicating that the
Path MTU has been exceeded./
- I wonder if the working group has thought about whether this is robust to loss?

All loss:
What happens when no probes are answered? Does the method reduce to the minimum IPv4 or IPv6 packet size?  or some other value?

Interface to network and application:
I'd like to see more information about what the following text means:
/It then uses that information to update the Path MTU. /
- Does that mean the updated value is then used by the sender. So does the sender drop packets larger? does it endpoint fragment these as the sender?

Host PMTU cache:
- Does this mean the host cache in the end system is changed downwards when there is a detected lower PMTU. Is there an API used for this, because this seems to me to raise questions about how to deal with
entries for paths - and possible aggregation of information from the same endpoint or not?

Raising the PMTU:
PLPMTUD has various ways it can be used, and this is not clear in the text - specifically is this more than black-hole detection or is
this the only service?

Specificaly (i) does the method increase the PMTU that has been used, or does it simply provide black hole detection? (If not, does this
mean that the method can be driven down by path changes, but never increase?)
or (ii) does it probe to increase the PMTU? - In which case, does this spec cause the sender transmit UDP segments larger than the PMTU in the host
cache?

Checksums:
In section 4.2.5 there is text on "Using Checksums as Packet Identifiers"
Text: /It could have been possible to use the checksum generated in the UDP
    checksum for this, but this value is generally not accessible to
    applications.  Also, sometimes the checksum is not calculated or is
    off-loaded to network hardware./
- Actually, this seems like a method that is impacted by NAPT and way these checksums would be chamged
in an incremental checksum update. It also seems to assume that the checksum is non-zero. I think this is good, but should be explicit.
Therefore, unless my concerns are wrong, I would rather urge that these two sentences avoid suggesting that the UDP checksum would work.

Document status:

This statement suggest to me that the specification could be  informational or experimental, since the use of the methods proposed is
entirely at the discretion of the reader, I'm not really sure what is intended, because there are multiple posisbilities suggested in the
document. I'm alos concerned there are two methods specified, is this necessary?:
/ These protocols can make use of  the probing mechanisms described in
this document instead of designing their own adhoc extension. /

Security Considerations and Protocol design:

/If an ICMP packet "Fragmentation needed" is received then this is interpreted as a
Probe Failure, as defined in [RFC4821] Section 7.6.2./
  - It seems like it needs more text to verify that a PTB message originates from a device on the path, as per RFC 8201, which points to the need to validate that PTB messages
originate on-path. This is a specific transport issue, but I don't see that mentioned. The intent is to close a vulnerability, so I think thsi should be discussed and addressed.

I do not agree this really does not introduce any specific
security considerations beyond those described in [RFC4821]. At least, draft-ietf-tsvwg-datagram-plpmtud has a growing list of concerns, I'd  like to check if these also apply here.

I expect some text nees to explain why the probe packets do not contribute to congestion. This may be taken from other RFCs, but the method clearly
sends packets additional to the networ traffic that is sent by the transport and this needs to be considered.

Is there implementation and operational experience of using this method? (I'm interested - and it may help answer some of the above).