Last Call Review of draft-ietf-6man-icmp-limits-07
review-ietf-6man-icmp-limits-07-tsvart-lc-aboba-2020-02-18-00

Request Review of draft-ietf-6man-icmp-limits
Requested rev. no specific revision (document currently at 08)
Type Last Call Review
Team Transport Area Review Team (tsvart)
Deadline 2020-02-25
Requested 2020-02-11
Authors Tom Herbert
Draft last updated 2020-02-18
Completed reviews Genart Last Call review of -07 by Pete Resnick (diff)
Tsvart Last Call review of -07 by Bernard Aboba (diff)
Assignment Reviewer Bernard Aboba 
State Completed
Review review-ietf-6man-icmp-limits-07-tsvart-lc-aboba-2020-02-18
Posted at https://mailarchive.ietf.org/arch/msg/tsv-art/WXAYUXE72oGfpTtzTwwqYno16KA
Reviewed rev. 07 (document currently at 08)
Review result Ready with Issues
Review completed: 2020-02-18

Review
review-ietf-6man-icmp-limits-07-tsvart-lc-aboba-2020-02-18

TSV-ART Review of draft-ietf-6man-icmp-limits
Bernard Aboba

Result: Ready with Issues

This document specifies several new ICMPv6 errors that can be sent
when a node discards a packet due to it being unable to process the
necessary protocol headers because of processing constraints or
limits.  Reasons include:

      Code (pertinent to this specification)
         1 - Unrecognized Next Header type encountered
         TBA - Extension header too big
         TBA - Extension header chain too long
         TBA - Too many options in extension header
         TBA - Option too big

ICMP Reliability

Section 5.2 states: 

"  ICMP is fundamentally an unreliable protocol and in real deployment
   it may consistently fail over some paths. As with any other use of
   ICMP, it is assumed that the errors defined in this document are only
   best effort to be delivered. No protocol should be implemented that
   relies on reliable delivery of ICMP messages. If necessary,
   alternative or additional mechanisms may used to augment the
   processes used to to deduce the reason that packets are being
   discarded. Such alternative mechanisms are out of scope of this
   specification."

[BA] The last sentence is a bit vague. My assumption is that this is
referring to techniques such as are used in Path MTU discovery (e.g.
tweaking of packets so as to determine potential reasons why packets
are being discarded).  However, a reference might be helpful.

Security Concerns

Pointer field

In Section 3.1, the description of the Pointer field states: 

"      Pointer
         Identifies the octet offset within the invoking packet where
         the problem occurred.

         The pointer will point beyond the end of the ICMPv6 packet if
         the field having a problem is beyond what can fit in the
         maximum size of an ICMPv6 error message."

[BA] I worry about attackers using the Pointer field for
mischief, such as buffer overflows.  The draft currently
does not provide advice to implementers about validating
the Pointer field (e.g. checking it against the length of
the offending packet). Do we really need a 32-bit Pointer field? 

Use by attackers

Section 4.2 states: 

"  A host MAY modify its usage of protocol headers in subsequent packets
   to avoid repeated occurrences of the same error."

[BA] While this response is optional, I do worry about the potential for
spoofed ICMP packets to modify the host's security posture. Are there any
types of usage modifications that a host should be particularly wary
of?

Section 6 states: 

"  In some circumstances, the sending of ICMP errors might conceptually
   be exploited for denial of service attack or as a means to covertly
   deduce processing capabilities of nodes. As such, an implementation
   SHOULD allow configurable policy to withhold sending of the ICMP
   errors described in this specification in environments where security
   of ICMP errors is a concern."

[BA] This concern seems quite realistic to me, and as a result, it would
not surprise me if implementations either do not send these ICMP errors,
or withhold them by default. Do you have a position on that?