Subject: [Int-area] Feedback on draft-bonica-internet-icmp-08

From: Fernando Gont <[email-masked]>
Date: Sun, 24 Sep 2006 23:09:21 -0300

Hello all,

I'm sending my feedback on draft-bonica-internet-icmp-08.

My main concern with this document is that of backwards 
compatibility. However, there are is also a misreading of the IP spec 
(wrt IPv4 minimum MTU) which makes it impossible for an 
implementation to comply with a requirement stated in this doc as a "MUST".

Here's my feedback:

Page 4, Section 2: Protocol
"   designers can make an ICMP message carry additional information by
    encoding that information in an extension."

Maybe s/extension/extension object/ ?

Page 4, Section 2:
"   This document also addresses a fundamental problem in ICMP
    extensibility.  Many of the ICMP messages addressed by this memo
    include an "original datagram" field.  The "original datagram" field
    contains the initial octets of the datagram to which the ICMP message
    is a response.  Although the "original datagram" field is of variable
    length, the ICMP message does not include a field that specifies its
    length."

I'm not sure I see the "fundamental problem". When it comes to 
"extensibility", you can always define a new ICMP type/code.

Page 7, section 4:
"   The length attribute represents the length of the "original datagram"
    field, measured in 32-bit words.  When the length attribute is
    specified, the "original datagram" field MUST be zero padded to the
    nearest 32-bit boundary.  Space for the length attribute is claimed
    from reserved octets, whose value was previously required to be zero."

In the case of ICMPv6, this new length field will not accommodate 
those cases in which as much information from the original payload is 
being including without exceeding IPv6's minimum MTU (i.e., a 
one-byte length field is not enough).

Page 10, Section 4.6:
"   The ICMP Extension Structure MAY NOT be appended to any of the other
    ICMP messages mentioned in Section 4."

Did you mean "MUST NOT"?

Page 12, Section 5.1:
"   When a classic application receives an ICMP message that includes
    extensions, it will incorrectly interpret those extensions as being
    part of the "original datagram" field.  Fortunately, the extensions
    are guaranteed to begin at least 128 octets beyond the beginning of
    the "original datagram" field.  So, only those ICMP applications that
    process the 129th octet of the "original datagram" field will be
    adversely effected.  To date, no such applications have been
    identified."

I think this assumption is wrong. Some implementations may be 
checking the TCP checksum included in the TCP header of the original 
datagram. In those cases, these extensions will likely lead to an 
error, with the ICMP message being discarded.

Also, if some kind of tunnelling mechanism was in place when the 
error was elicited, important information such as TCP's port numbers 
may be at (or past) the 129th octet. This may lead to the ICMP error 
messagebeing demultiplexed, for example, to the wrong TCP endpoint. 
Thus, guaranteeing that the extensions are "at least 128 octets 
beyond the original datagram" may not be enough.

Page 13, section 5.3:
    Provided that the entire ICMP messages does not exceed the minimum
    MTU (576 octets for ICMPv4 or 1280 octets for ICMPv6), there is no
    upper limit upon the length of the "original datagram" field.

This is incorrect. IPv4's minimum MTU is 68 octects, not 576 (576 is 
the minimum reassembly buffer size). In the case of ICMPv4 The limit 
is imposed by the reassembly buffer size, not by the MTU.

Page 13, Section 5.3:
"   However, each vendor will decide how many octets to include.  Those
    wishing to be backward compatible with non-compliant TRACEROUTE
    implementations will include exactly 128 octets.  Those not requiring
    compatibility with non-compliant TRACEROUTE applications may include
    more octets."

This is not compliant with RFC 1122 itself. I think this modifies a 
"SHOULD" in RFC 1812. (RFC 1812 states that as much information from 
the original datagram SHOULD be included, without exceeding....?)

Page 14, Section 6:
"   As stated above, the total length of the ICMP message, including
    extensions, MUST NOT exceed the minimum protocol MTU.  Figure 6
    depicts the ICMP Extension Header."

As explained above, this requirement cannot be met: IPv4's minimum 
MTU is 68 bytes. You may change this as explained above (s/minimum 
MTU/minimum reassembly buffer size/), though.

Page 15, Section 6:
    Version: 4 bits

       ICMP extension version number.  This is version 2.

Version 1 is that used by the non-compliant implementations?

Page 15, Section 6:
"   Checksum: 16 bits

       The one's complement of the one's complement sum of the data
       structure, with the checksum field replaced by zero for the
       purpose of computing the checksum.  An all-zero value means that
       no checksum was transmitted."

You should probably clarify the reason of this checksum, as there 
already is a checksum field in ICMP messages.

Kindest regards,

--
Fernando Gont
e-mail: [email-masked] || [email-masked]
PGP Fingerprint: 7809 84F5 322E 45C7 F1C9 3945 96EE A9EF D076 FFF1

_______________________________________________
Int-area mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/int-area

---

From: Joe Touch <[email-masked]>
Date: Wed, 27 Sep 2006 13:31:56 -0700

Some notes below...

Fernando Gont wrote:
> My main concern with this document is that of backwards compatibility.
> However, there are is also a misreading of the IP spec (wrt IPv4 minimum
> MTU) which makes it impossible for an implementation to comply with a
> requirement stated in this doc as a "MUST".
> 
> Here's my feedback:
> 

There are two issues:

- supporting sending the MPLS header in addition to the IP packet (this
is the primary motivation)

- supporting sending other information with an IP packet, e.g., the
name/ID of the interface on which the error occurred in a router

(the latter was indraft-atlas-icmp-unnumbered-01.txt)

...
> I think this assumption is wrong. Some implementations may be checking
> the TCP checksum included in the TCP header of the original datagram. In
> those cases, these extensions will likely lead to an error, with the
> ICMP message being discarded.

Agreed, but the statement in the doc says that no such implementations
have been identified. Althought they could check the TCP cheksum, they
currently don't.

> Also, if some kind of tunnelling mechanism was in place when the error
> was elicited, important information such as TCP's port numbers may be at
> (or past) the 129th octet. This may lead to the ICMP error messagebeing
> demultiplexed, for example, to the wrong TCP endpoint. Thus,
> guaranteeing that the extensions are "at least 128 octets beyond the
> original datagram" may not be enough.

ICMPs in tunnels are meaningful per se only to the tunnel endpoint,
i.e., to the source of the tunnel. That endpoint MAY generate a
subsequent ICMP back to the source or might not (as per Sec 4 of
RFC2003), but full unrolling of all interior headers is not presumed to
be possible (see Sec 5 for a solution involving soft-state, albeit
cumbersome. If they were, then RFC1122 would require MUST rather than
SHOULD for as many payload bytes as possible.

> "SHOULD" in RFC 1812. (RFC 1812 states that as much information from the
> original datagram SHOULD be included, without exceeding....?)

It's consistent with a SHOULD to provide a specific case where the
SHOULD is not applicable, especially where the case is atypical.

Joe

---

From: Fernando Gont <[email-masked]>
Date: Wed, 27 Sep 2006 20:10:48 -0300

>(the latter was indraft-atlas-icmp-unnumbered-01.txt)

Good. This is what I said I was missing.

>have been identified. Althought they could check the TCP cheksum, they

Unless you have assessed every existing implementation (which is very 
unlikely), the statement "No such implementations have been 
identified" doesn't mean much.

Again, this is not at all a killer for the document. I just mean that 
some text should be included both on this and on the possibility of 
overwriting some fields that may lead to unexpected results.

>cumbersome.

There may be more than one instance of tunelling. In some scenarios 
it may be possible to unroll the interior headers - one per tunnel endpoint.

Also, by overwriting the 129th byte, you may overwrite the TCP ACK. 
And there are firewall implementations (OpenBSD's PF, at least) which 
check the ACK field when it's available.

>If they were, then RFC1122 would require MUST rather than

RFC1222 was written, as all other specs, by humans.
Also, I don't think tunnels were as common in 1989 as they are nowadays.

Last, but not least, enforcing a such a MUST for ICMP would not make 
much sense. If you really mean a MUST, you wouldn't be using 
non-reliable signalling, after all.

>SHOULD is not applicable, especially where the case is atypical.

Agreed.

Kindest regards,

---

From: Joe Touch <[email-masked]>
Date: Wed, 27 Sep 2006 16:57:09 -0700

Notes below...

Fernando Gont wrote:
> unlikely), the statement "No such implementations have been identified"
> doesn't mean much.
> 

I thought that was what the checksum was there for (maybe I'm
remembering it fuzzily; I didn't dig deep into it since the last IETF).
It might be useful to highlight that where this is mentioned, though.

> There may be more than one instance of tunelling. In some scenarios it
> may be possible to unroll the interior headers - one per tunnel endpoint.

Agreed; 2003 addresses that specifically.

> Also, by overwriting the 129th byte, you may overwrite the TCP ACK. And
> there are firewall implementations (OpenBSD's PF, at least) which check
> the ACK field when it's available.

Again, 2003 covers this. There are cases where the interior is
meaningless to parse.

> Also, I don't think tunnels were as common in 1989 as they are nowadays.

Agreed, but that's what 2003 specifically addresses anyway in Sections 4
and 5 (i.e., that 1122 didn't talk about tunnels, and that sending less
than the full packet affects relaying).

> non-reliable signalling, after all.

What's inside can be a MUST, even if the packet is lost ;-)

Joe