Gorry,

> On Mar 24, 2017, at 8:02 AM, Gorry Fairhurst <[email protected]> wrote:
> 
> On 17/03/2017, 21:56, Bob Hinden wrote:
>> 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
> Thanks Bob,
> 
> I note  the question of status, but in this case, there's a lot happened in 
> the Internet since this PS was first published, and much of this was in the 
> transport area. Please see replies to comments below. I'm happy to discuss or 
> refine text, if this helps.

Comments below.  Let’s get together and go over the document while we are in 
Chicago.  I want to make sure the changes are made correctly and in context.

Thanks,
Bob

> 
> Gorry
>>> On Feb 14, 2017, at 9:23 AM, Gorry Fairhurst<[email protected]>  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.
> Thanks. Seems better, although I would suggest to revise this in this way:
> 
>   A packetization layer layer (e.g., TCP) 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, except to probe during PMTU discovery.
>   A simple implementation could ask the IP layer
>   for this value each time it created a new segment, but this could be
>   inefficient.  An implementation typically caches other values
>   derived from the PMTU.  It may be simpler to receive asynchronous
>   notification when the PMTU changes, so that these variables may be
>   also updated.
> 
> - This would fix typos, and also make the text relevant to transport 
> protocols,
> rather than only TCP. Which is I beleive the correct interpretation.

I think that’s good.  I will require some other changes to the Section that is 
titled "TCP layer action”, let’s discuss this later.

> 
>>> 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.
> Good. Seems much better.

Good.

>> 
>>> —
>>> 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.
>> 
> I'd personally remove this and the notes (you decide) - more because it says 
> think that it opens a can of worms about transport details that are largely 
> beyond the scope of the document (such as whether to trust ICMPv6 as 
> indications of actual loss, and how this interacts with re-packaging to a 
> different segment size, congetsuon control, etc).
> 
> If the WG thinks retaining some text is good, I'd say (as a cautious 
> transport person) that we should call out a warning to people doing 
> transmissions...
> 
> OLD:
>     If the new estimated PMTU is
>      still wrong, the process repeats, and there is an exponential
>      growth in the number of superfluous segments sent.
> NEW:
>     If the new estimated PMTU is
>      still wrong, the process repeats, and there is an exponential
>      growth in the number of superfluous segments sent.
>      Retransmissions can increase network load in
>      response to congestion, worsening that congestion.  Any
>      packetization layer that uses retransmission is responsible for
>     congestion control of its retransmissions.
> 
> If you wish, cite RFC8085 and RFC4821 to let people figure this out? (these 2 
> sentences come directly from RFC8085).

Ok, I think that works.

>>> ---
>>> 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:
> The protocol changed though - so it's not just a replacement reference.
> 
> I think text should note that RFC7530 says NFS must be able to run over a 
> congestion-controlled transport, such as TCP.
>>    It is recommended that NFS running over TCP utilize the MTU discovery
>>    provided by TCP.
>> 
>> Other suggestions?
> Maybe, you could say something like:
> "RFC191 used NFS as an example of a UDP-based application that benefits from 
> PMTU discovery. Since then RFC7530, states the supported transport layer 
> between NFS
> and IP must be an IETF standardized transport protocol that is specified to 
> avoid
> network congestion; such transports include TCP and the Stream Control 
> Transmission Protocol (SCTP). In this case, the transport is itself 
> responsible for determining and using an effective Path MTU, including 
> implementing PMTU discovery when this is needed.”


> 
> RFC8085 may well be a good place to point to say what is needed/desirable for 
> PMTU for UDP applications, if you wish to say something about UDP.

Thanks, I think that works.  Not sure if the other paragraphs on NFS should 
remain.


>>> 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 agree that new mechanisms are out of scope, I was hoping that a simple note 
> on tunnels would be in the security considerations.
> 
> RFC8085 or draft-ietf-intarea-tunnels could be pointed to describe the 
> implications of tunnels.
>>> ---
>>> I think the security consideration is naive!
>> Suggestions?
> "   In the first attack, the false message indicates a PMTU much smaller
>   than reality.  This should not entirely stop data flow, since the
>   victim node should never set its PMTU estimate below the IPv6 minimum
>   link MTU.  It will, however, result in suboptimal performance."
> 
> So... In the case of the first attack, I still question whether this choice 
> of words misrepresents the attack.
> What if, the false message advertises ~1280 bytes, and the path uses a tunnel 
> encapsulation transporting IPv6, then the PTB message can be basis for a 
> straight DoS attack that does stop data flow?
> 
> I would recommend revising the security considerations (at least to something 
> like):
> "  In the first attack, the false message indicates a PMTU much smaller
>   than reality.  In response, the
>   victim node should never set its PMTU estimate below the IPv6 minimum
>   link MTU. A sender that falsely reduces to this MTU would observe 
> suboptimal performance."
> 
> - Although this skirts the problem.

OK, I agree this is better.

>>> 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?
> See below - and above.
>>> 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.
> 
> That certainly is a nod in what I think is really the correct direction.
> Probably /UDP/ should be /IPv6 packet/ given the context of this ID.

Good, I think that makes sense.

>> 
>>> ——
>>> 
>>> 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.
> 
> I appreciate people have spent time trying to get the IPv6 specifications 
> progressed to standards status. This is a good effort, but I think just 
> adding a nod in the introduction is rather feeble for soemthing now 
> recommended. I think at least the security considerations should motivate the 
> benefits of this approach (see next).
>>> "   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.
> Could the WG consider to also add the first two sentences to the security 
> considerations?

Sorry, which two sentences?

> 
> Gorry
> 
> P.S. A few more out-dated statements abiut TCP, based on the way specific 
> stacks work when the original was written:
> 
> - The phrase below needs to be removed also, since not all TCP stacks behave 
> in this way anymore:
> ", which is always a multiple of the segment size"
> 
> - There was very specific detailis relating to 4.2BSD TCP implementations, 
> which would have been better not being here.
> 
> 

OK, I can remove both.




Attachment: signature.asc
Description: Message signed with OpenPGP

Reply via email to