Subject: Re: [Last-Call] Tsvart last call review of draft-ietf-6man-icmp-limits-07

From: Tom Herbert <[email-masked]>
Date: Thu, 5 Mar 2020 11:12:25 -0800

Hi Bernard,

Thanks for the review. Comments inline.

On Tue, Feb 18, 2020 at 1:03 PM Bernard Aboba via Datatracker
<[email-masked]> wrote:
>
> Reviewer: Bernard Aboba
> Review result: Ready with Issues
>
> 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.
>
We can provide a reference to PMTU discovery as one example. Probing
with different features in packets would also be a way to determine
what is causing packet less (something akin to Happy Eyeballs).

> 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?
>
The thirty-bit Pointer field is from RFC4443. We will add text that
the pointer field must be validated (I don't believe RFC4443 mentions
that).

> 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?

I think it's like any other use case of ICMP errors that might also be
spoofed. One mitigation against spoofing in the ICMP protocol is that
the packet that caused the error is set in the ICMP data so that a
source host should validate that it sent the packet that caused the
error. Generally, though for ICMP there is no inherent security so the
sources need to take the messages as advisory and any response needs
to be done in accordance with security policies in effect.

>
> 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?

I don't think the risk is any greater than any other ICMP errors. I
suspect switches and intermediate nodes will probably not bother
sending the errors since they mostly don't send existing ICMP errors
anyway. In host implementations, such as Linux, I don't believe there
will be much concern with supporting these errors. The errors are
still subject to general policies that limit ICMP errors.
>
>
>
> --------------------------------------------------------------------
> IETF IPv6 working group mailing list
> [email-masked]
> Administrative Requests: https://www.ietf.org/mailman/listinfo/ipv6
> --------------------------------------------------------------------