ietf
[Top] [All Lists]

Re: Last Call: <draft-ietf-6man-rfc1981bis-04.txt> (Path MTU Discovery for IP version 6) to Internet Standard

2017-03-17 16:57:26
Gorry,

Thanks for the detailed review.

Several of the reviews have suggested significant changes to this document.  
The working group was trying to make a few changes to bring it into alignment 
with some changes to rfc2460bis (based on updating documents).  It was not 
attempting a major rewrite when advancing it to Internet Standard.  The 
alternative that the working group discussed was to file errata on RFC1981 and 
leave it to a future document update.

I don’t think many of these changes can be done and still advance it to 
Internet Standard. If we can’t advance the current document, then the w.g. may 
want to just do the errata and withdraw the advancement request.  Of course, if 
people want to work on a revision of IPv6 Path MTU Discovery, it would be 
welcomed, but it won’t be able to advanced to Internet Standard.

Comments below.

Thanks,
Bob



On Feb 14, 2017, at 9:23 AM, Gorry Fairhurst 
<gorry(_at_)erg(_dot_)abdn(_dot_)ac(_dot_)uk> wrote:


I have some late transport comments on this ID. The update seems to retain a 
lot of thinking that is really historical and I'd really encourage people to 
look again to making the document uptodate.

Detailed comments follow.

Best wishes,

Gorry

----
The following text strikes me as a little odd in an update:
" Moreover, TCP implementations that follow the "slow-
  start" congestion-avoidance algorithm [CONG] typically calculate and
  cache several other values derived from the PMTU.  It may be simpler
  to receive asynchronous notification when the PMTU changes, so that
  these variables may be updated.”
- A modern TCP caches at least some path information in the TCB, why start 
with this clause at all:
"Moreover, TCP implementations that follow the "slow start" 
congestion-avoidance algorithm [CONG] typically calculate and”
and simply replace this with something like:
"TCP implementations”?
——

To clarify, you are suggesting to replace it with:

   TCP implementations typically cache several other values derived
   from the PMTU.  It may be simpler to receive asynchronous notification when
   the PMTU changes, so that these variables may be updated.

I think that is editorial, and OK to change.  The paragraph will read:

   The TCP layer must track the PMTU for the path(s) in use by a
   connection; it should not send segments that would result in packets
   larger than the PMTU.  A simple implementation could ask the IP layer
   for this value each time it created a new segment, but this could be
   inefficient.  TCP implementation typically cache several other values
   derived from the PMTU.  It may be simpler to receive asynchronous
   notification when the PMTU changes, so that these variables may be
   updated.

The following text also seems to not reflect a modern TCP stack:
" It is sufficient
  to treat this as any other dropped segment, and wait until the
  retransmission timer expires to cause retransmission of the segment.”
(and following 3 paras).
Could this be replaced by text that does not exclude modern retransmission 
methods:
" It is sufficient
  to treat this in the same way as any other dropped segment, and
  will be recovered by normal retransmission methods.”

Yes, resulting in:

   When a Packet Too Big message is received, it implies that a packet
   was dropped by the node that sent the ICMP message.  It is sufficient
   to treat this in the same way as any other dropped segment, and will
   be recovered by normal retransmission methods.  If the Path MTU
   Discovery process requires several steps to find the PMTU of the full
   path, this could delay the connection by many round-trip times.


—
There is a block of text that describes retransmission triggered by ICMPv6.
Has this code been implemented in modern releases of TCP?:
"   Alternatively, the retransmission could be done in immediate response
  to a notification that the Path MTU has changed, but only for the
  specific connection specified by the Packet Too Big message.”
- It seems to expose a number of attack vectors that really should not be 
exposed!!

Are you suggesting remove this text?  Following this text, there are two notes.


---
The discussion of NFS may still be a reasonable historic example, but to be 
current it should really refer also to NFSv4/TCP as utlising the MTU 
discovery provided by TCP, since UDP-based NFS is no longer a key application.
—

I was planning to update the reference to NFS to RFC7530.   I could add text 
that says something like:

   It is recommended that NFS running over TCP utilize the MTU discovery
   provided by TCP.

Other suggestions?

There is no mention that paths including tunnels can eat ICMPv6 PTB messages 
on the tunnel segment, blackholing them, which prevents reaching the 
destination.

As noted above, I think adding discussion about tunnels is out of scope of this 
effort.

---
I think the security consideration is naive!

Suggestions?


This statement in particular seems to open DOS vulnerability:
"
 When a node receives a Packet Too Big message, it MUST reduce its
  estimate of the PMTU for the relevant path, based on the value of the
  MTU field in the message."
- Introdueces a significant vulnerability.  A rogue PTB message that reduces 
the PMTU to a minimum, can result in a path too small to carry an 
encapsulated packet. (Recently noted by Fernando Gont).

Suggestions?


Moreover, other layers view ICMP messages with suspicion and have long noted 
the need to check ICMP payload and match only packets that relate to actual 
5-tuples in use (effectively reducing vulnerability to off-path attacks). For 
example, the Guidelines for UDP, rfc5405bis, state:

" Applications SHOULD appropriately validate the payload of ICMP
  messages to ensure these are received in response to transmitted
  traffic (i.e., a reported error condition that corresponds to a UDP
  datagram actually sent by the application). …“
- clearly handling this in IP-layer tunnels can be troublesome, but that's a 
problem that should be described, not obscured.

I could add something like the following as a new second paragraph in Section 4 
Protocol Requirements:

  Protocols SHOULD appropriately validate the payload of ICMPv6 PTB
  messages to ensure these are received in response to transmitted
  traffic (i.e., a reported error condition that corresponds to a UDP
  datagram actually sent by the application).

or similar.


——

I’d finally  like to add my concerns about the understatement of the value of 
PLPMTUD, which seems to not reflect the recommendations to use this method:
“  It defines a method for Packetization Layer Path
  MTU Discovery (PLPMTUD) designed for use over paths where delivery of
  ICMP messages to a host is not assured.”
This  seems under-stating the value and recommendations to deploy PLMTUD, 
compared with current transport-area recommendations, for instance, the UDP 
Guidelines provide much more on this important design consideration:


The w.g. spent a lot of time on this paragraph.  The intent was to point to it, 
but not into detail of how it worked, nor make it a new requirement.  There was 
some discussion that adding more detail about the relationship between PTMUD 
and PLPMTUD in the ongoing update to IPv6 Node Requirements would be helpful.



"   Packetization Layer Path MTU Discovery (PLPMTUD) [RFC4821] does not
  rely upon network support for ICMP messages and is therefore
  considered more robust than standard PMTUD.  It is not susceptible to
  "black holing" of ICMP message.  To operate, PLPMTUD requires changes
  to the way the transport is used, both to transmit probe packets, and
  to account for the loss or success of these probes.  This updates not
  only the PMTU algorithm, it also impacts loss recovery, congestion
  control, etc.  These updated mechanisms can be implemented within a
  connection-oriented transport (e.g., TCP, SCTP, DCCP), but are not a
  part of UDP, but this type of feedback is not typically present for
  unidirectional applications."

----

The examples used in the definition of  "upper layer" and "link" also makes 
this document appear as historic, rather than a new RFC!

As noted above, I think there would be support for a rewrite if some folks 
wanted to take that on.



Attachment: signature.asc
Description: Message signed with OpenPGP