Re: Benjamin Kaduk's Discuss on draft-ietf-6man-icmp-limits-07: (with DISCUSS and COMMENT)

[Benjamin Kaduk <[e-mail]>]

On Wed, Mar 18, 2020 at 05:05:44PM -0700, Tom Herbert wrote:
> On Mon, Mar 9, 2020 at 4:44 PM Benjamin Kaduk via Datatracker
> <[e-mail]> wrote:
> >
> > Benjamin Kaduk has entered the following ballot position for
> > draft-ietf-6man-icmp-limits-07: Discuss
> >
> > When responding, please keep the subject line intact and reply to all
> > email addresses included in the To and CC lines. (Feel free to cut this
> > introductory paragraph, however.)
> >
> >
> > Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> > for more information about IESG DISCUSS and COMMENT positions.
> >
> >
> > The document, along with other ballot positions, can be found here:
> > https://datatracker.ietf.org/doc/draft-ietf-6man-icmp-limits/
> >
> >
> >
> > ----------------------------------------------------------------------
> > DISCUSS:
> > ----------------------------------------------------------------------
> >
> > I recognize that this document is trying to take the track of "we
> > recognize that some implementations/deployments violate RFC 8200, and
> > while we don't condone that behavior, we want to provide diagnostics to
> > try to mitigate the fallout from them", but I'm not sure it succeeds in
> > all cases.  The discussion in Section 5.1 that is quite clear about
> > "does not advocate behaviors that might be considered nonconformant",
> > but there is another usage in Section 2.2 ("[t]his code SHOULD be sent by
> > an intermediate node that discards a packet because it encounters a Next
> > Header type that is unknown in its examination") that seems worth
> > revisiting.  Perhaps we can reword to be more clear that this is the
> > recommended behavior subject to the predicate that an RFC 8200 violation
> > has occurred, which is not itself a recommended behavior.
> >
> How about if we move section 5.1 into the introduction?

That helps, and I'll go clear my discuss after I send this note.  That
said, I'd still prefer to also apply Alissa's suggestion to Section 1.1

OLD:
   Per [RFC8200], except for Hop by Hop options, extension headers are
   not examined or processed by intermediate nodes.  Many intermediate
   nodes, however, do examine extension headers for various purposes.

NEW:
   Per [RFC8200], except for Hop by Hop options, extension headers are
   not examined or processed by intermediate nodes.  However, in deployed
   networks many intermediate nodes do examine extension headers for
   various purposes.


> > In light of the updated usage described in Section 2.2, should we direct
> > IANA to make the "Reference" column for the "unrecognized Next Header
> > type encountered" parameter problem list both this document and RFC
> > 4443?  (It currently does not list any reference, as is the case for
> > many entries on the containing page...)
> > This codepoint is not currently mentioned in the IANA Considerations at
> > all
> 
> Will make a new code for this in the next revision.

That works too!

> >
> >
> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> >
> > I agree with Alissa's COMMENT.
> >
> > The [IANA-ICMPV6] reference is used as reference for both the "parameter
> > problem" and the "destination unreachable" codes, but the link points
> > specifically to the "destination unreachable" table at present.  (My
> > understanding is that IANA likes the URL fragment to be stripped from
> > the final RFC, in which case the distinction becomes moot.)
> >
> Okay, will fix.
> 
> > Can you help me walk through the various places where we discuss the
> > cardinality of new registered values, to ensure that we're internally
> > consistent?
> > The Abstract just says "several new ICMPv6 errors", nice and vague;
> > clearly fine.
> > Section 1 says "five of the errors are specific to processing of
> > extension headers; another error is used when [...]".  Are the "five"
> > the five "parameter problem" codepoints that we list, or the four new
> > "parameter problem" codepoints plus the one new "destination
> > unreachable" codepoint?  If the former, we are calling "unrecognized
> > next header type" a "new ICMPv6 code point", which is not exactly true.
> > Section 1.1 says "four parameter problem codes and extends the
> > applicability of an existing code".  It seems pretty clear that the four
> > are the new ones and the one is "unrecognized next header", so this is
> > fine.
> > Section 1.2 talks about the one "destination unreachable" code; which
> > seems okay on its own.  Maybe we want to check for parallelism in how,
> > e.g., Section 1's high-level overview discusses it vs. the other
> > allocations.
> 
> Okay, with update there will be six parameter problems having to do
> with extension header limits, and one destination unreachable for
> aggregate limits. Hopefully that is clear.
> 
> > There doesn't seem to be an introduction subsection that mentions the
> > extended object class and subtype used for aggregate header limits,
> > which might be useful to have.
> 
> Okay, will add text.
> 
> >
> > Section 1
> >
> >    limits. New ICMPv6 code points are defined as an update to [RFC4443].
> >
> > I suggest phrasing this as "to supplement those defined in [RFC4443]"
> > since we're just allocating from a registry and are not marking this
> > document with an "Updates: 4443" header.
> 
> Okay, will fix.
> 
> >
> > Section 1.3
> >
> > Please use the BCP 14 boilerplate from RFC 8174.
> 
> Okay.
> 
> >
> > Section 2.1
> >
> >          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.
> >
> > I understand that we're just reusing the RFC 4443 format for "parameter
> > problem" messages, but I'd suggest adding a note that recipients
> > should/MUST sanity-check the pointer value [perhaps with specifics].
> >
> Okay.

Thanks!

> > Section 2.2
> >
> >    This code SHOULD be sent by an intermediate node that discards a
> >    packet because it encounters a Next Header type that is unknown in
> >    its examination. The ICMPv6 Pointer field is set to the offset of the
> >
> > To what extent is this diverging from the RFC 8200 dictum that extension
> > headers not be examined or processed by intermediate nodes?
> 
> I don't think this divulges from RFC8200, again we've already
> acknowledged that the draft does not advocate violation of RFC8200 or
> any other standards. On the other hand, given the recent discussion on
> 6man list and in Spring WG, there are some differences of opinion as
> to what exactly RFC8200 requires and when an intermediate node would
> violate it (for instance, "processed" could be open to a lot of
> interpretation). I'd prefer not to have ICMP limits be dragged into
> those conversations, which is why the theme of this draft is "If you
> drop a packet because of its content then please tell us why so we
> might be able to fix the problem (we won't judge you! :-) )"

I can endorse this position! :)

> >
> >    Note that when the original sender receives the ICMPv6 error it can
> >    differentiate between the message being sent by a destination host,
> >    per [RFC4443], and an error sent by an intermediate host based on
> >    matching the source address of the ICMPv6 packet and the destination
> >    address of the packet in the ICMPv6 data.
> >
> > This is, of course, true only in the face of honest participants; there
> > is no cryptographic protection on ICMP packets so spoofing is trivial,
> > potentially even from off path(?).  (As RFC 4443 notes, IPsec AH helps.)
> >
> A new code will be added for intermediate nodes to send unrecognized
> next header. Generally, spoofing is trivial for all of ICMP and I
> don't believe there's anything in this draft that particularly
> increases risk compared to other ICMP errors. In the security we do
> mention that mitigations in RFC4443, which would include AH, are
> applicable.
> 
> 
> > Section 2.4
> >
> >    There are two different limits that might be applied: a limit on the
> >    total size in octets of the header chain, and a limit on the number
> >    of extension headers in the chain. This error code is used in both
> >    cases. In the case that the size limit is exceeded, the ICMPv6
> >    Pointer is set to first octet beyond the limit. In the case that the
> >    number of extension headers is exceeded, the ICMPv6 Pointer is set to
> >    the offset of first octet of the first extension header that is
> >    beyond the limit.
> >
> > Do we want to mention the potential ambiguity when the size limit
> > happens to align with the first octet of an extension header?
> >
> Will create separate codes for these two cases.
> 
> > Section 2.6
> >
> >       * The length of a PADN options in destination options or hop-by-
> >         hop options is limited seven octets [RFC8504].
> >
> > nit: some singular/plural mismatch here, and probably "limited to".
> >
> Okay.
> 
> > Section 4.1
> >
> > I appreciate the thought that went into the priority of reporting list;
> > thanks for it!
> >
> > Section 6
> >
> > It's probably worth reiterating that applications that change their
> > behavior in response to unauthenticated ICMP messages without proper
> > message validation are susceptible to an attacker in the network forcing
> > them into a certain configuration.  When such a configuration is a
> > vulnerable one, that is an easy way for the attacker to escalate an
> > attack.
> >
> > What does it mean for something to "conceptually be exploited for denial
> > of service attack"?
> >
> It's probably no different then DOS attack exposure for other ICMP
> errors in general. I'll remove that, and only discuss the capabilities
> exposure.
> 
> > I see that RFC 4443 recommends that upper layers receiving ICMP messages
> > "perform some form of validation" upon them, but a brief read does not
> > seem to find it suggesting that, say, application-level traffic and ACKs
> > continuing to flow are an indication that ICMP messages indicating
> > failure to deliver packets are inaccurate; is that worth revisiting?
> >
> I'm not sure we need to revisit that in this draft. The problem
> existed long before these messages.

Understood.

Thanks for the updates!

-Ben