Investigate unusual MPLS example about trippy

[ytti]

This byte string looks ok to me, its location in the packet, I'm not sure, it seems as if there is bunch of zero padding, and I didn't bother figuring out if this is fine or not.

`20 00 78 56 00 08 01 01 65 9f 01 01``

The ICMP extension header 20 00 78 56

• 2 version, 4bits
• 000 reserved, 12bits
• 7856, 32bits checksum

The Extension Object Header 00 08 01 01

• 00 08 length, 32bits, including object header (excluding icmp extension header)
• 01 class-value, 8bits
• 01 c-type, 8bits

The Extension Payload 65 9f 01 01

• 659f0, 20bits mpls label
• 1, 4bits (exp:000 BOS:1)
• 01, 8bits ttl

[ytti]

https://datatracker.ietf.org/doc/html/rfc4884#section-3

   When the ICMP Extension Structure is appended to an ICMP message
    and that ICMP message contains an "original datagram" field, the
    "original datagram" field MUST contain at least 128 octets

I think this explains the padding. The original embedded packet is exactly 128B, even though it's actually shorter, so to get it to 128B, zeroes were padded, before the ICMP EO.

The ICMP EO is after the padded embedded packet.

That is, before ICMP EO, embedded packets were 64B, with ICMP EO, embedded packets are 128B at least. If the original packet is less, padding is added to get to the 128B.

[fujiapple852]

@ytti yes it is right that senders will do this padding, the question is how the receiver will parse the message, specifically a compliant vs non-compliant implementation processing a compliant message (i.e. one where the sender has set the "length" attribute).

I think I know what is going on.

One thing that jumped out at me today when I re-read this thread (with fresher eyes!) is that the TE payload is 140 bytes and the MLPS object is 12 bytes and 140 - 12 = 128 which is the magic number.

From section 5.3:

When a non-compliant application receives a message that contains
compliant ICMP extensions, it will parse those extensions correctly
only if the "original datagram" field contains exactly 128 octets.
This is because non-compliant applications are insensitive to the
length attribute that is associated with the "original datagram"
field. (They assume its value to be 128.)

Therefore I believe tcpdump can be classified as a "non-compliant" implementation as it is "insensitive to the length attribute" whereas both Trippy and Wireshark are compliant implementations and respect the "length" attribute (17 * 4 = 68 in this example) and therefore fail to parse the extension. Looking at the code I don't see tcpdump handling this at all other than this case.

I did a quick test in Trippy and by ignoring the "length" attribute of the compliant TE packet, the alternative logic for a compliant application to process a (not actually) non-compliant message kicks in (i.e. assume its at offset 128) and we are able to parse the extension.

rfc4884 goes on to say:

When a compliant application receives an ICMP message, it examines
the length attribute that is associated with the "original datagram"
field. If the length attribute is zero, the compliant application
MUST determine that the message contains no extensions. In this
case, that determination is technically correct, but not backwards
compatible with the non-compliant implementation that originated the
ICMP message.

So, to ease transition yet encourage compliant implementation,
compliant TRACEROUTE implementations MUST include a non-default
operation mode to also interpret non-compliant responses.
Specifically, when a TRACEROUTE application operating in non-
compliant mode receives a sufficiently long ICMP message that does
not specify a length attribute, it will parse for a valid extension
header at a fixed location, assuming a 128-octet "original datagram"
field. If the application detects a valid version and checksum, it
will treat the octets that follow as an extension structure.

I notice Wireshark has a setting related to this, I tried it and it didn't seem to change anything.

The question is, what is the right behaviour for Trippy? Can we handle both and avoid making the user choose?

The quote you mentioned is intriguing:

When the ICMP Extension Structure is appended to an ICMP message
and that ICMP message contains an "original datagram" field, the
"original datagram" field MUST contain at least 128 octets.

Therefore, if a compliant implementation like Trippy receives a seemingly compliant message (i.e. one with a non-zero length) but that length is less than 128 then we can conclude that the message is, in fact, non-compliant and parse it accordingly. That would work in the case we have been examining here.

[ytti]

I think what you are saying, is that the DO node which generated the TTL exceeded message, can be assumed to be compliant, because it added padding and added ICMP EO. However, you are also saying, that it should have set the TTL exceeded message length type to a higher value than it did, that it incorrectly didn't include the padding bytes in the length.

So you are saying, this particular implementation is only partially correct. And tcpdump happens to work, because it doesn't rely on the length field for logic.

Personally I'm not sure why there would be benefit relying in the length field. But if I understood you correctly, you could approach DO and ask them about this message, if they believe it to be correct.

[fujiapple852]

the DO node...

Nit: the node which generates this (questionable) EO is 62.115.112.244 (AS1299 nyk-bb1-link.ip.twelve99.net), the recipient is a DO node I run.

...which generated the TTL exceeded message, can be assumed to be compliant, because it added padding and added ICMP EO

Not quite, the way i've explained it in the code is:

// We have a 'compliant' ICMP extension as defined by rfc4884.
//
// Specifically, it specifies a non-zero `length` attribute of at
// least 128 octets.
//
// From rfc4884 (section 3) entitled "Summary of Changes to ICMP":
//
// "When the ICMP Extension Structure is appended to an ICMP message
// and that ICMP message contains an "original datagram" field, the
// "original datagram" field MUST contain at least 128 octets."
//
// Therefore, we assume that any non-zero `length` attribute less
// than 128 octets is a non-compliant message.

However, you are also saying, that it should have set the TTL exceeded message length type to a higher value than it did, that it incorrectly didn't include the padding bytes in the length.

Yes that's right, based on my interpretation of the section quoted.

And tcpdump happens to work, because it doesn't rely on the length field for logic.

Seemingly so yes, but I'll have to dig more into the tcpdump code to be sure.

Personally I'm not sure why there would be benefit relying in the length field

It allows for an EO to be safely added to a TE message with an original datagram > 128 octets. As rfc4884 says:

Unfortunately, the "original datagram" field lacks a length
attribute. Application software infers the length of this field from
the total length of the ICMP message. If an extension structure were
appended to the message without adding a length attribute for the
"original datagram" field, the message would become unparsable.
Specifically, application software would not be able to determine
where the "original datagram" field ends and where the extension
structure begins.

[ytti]

It does feel ambiguous.

1. include padding in length

   • we cannot tell for packets under 128B what part, if any, is padding and what is message

2. do not include padding in length

   • if we see length is below 128B, we know it'll also, in addition, contain padding to go to 128B, because RFC says so.
   • we know what part is message, what part is padding and what part is EO
   • so we implicitly read 128-length after the end of the embedded packet, when it's below 128

I can see how people can arrive to either conclusion, and I guess proof is in the the captures.

[fujiapple852]

@ytti The is the logic I've ended up with* which passes the tests I've created covering the various packets observed in the wild including the one discussed here. Would appreciate a review. Thanks again!

    pub fn split(rfc4884_length: u8, icmp_payload: &[u8]) -> (&[u8], Option<&[u8]>) {
        let length = usize::from(rfc4884_length * 4);
        if length > icmp_payload.len() {
            return (&[], None);
        }
        if icmp_payload.len() > ICMP_ORIG_DATAGRAM_MIN_LENGTH {
            if length > ICMP_ORIG_DATAGRAM_MIN_LENGTH {
                // a 'compliant' ICMP extension as defined by rfc4884.
                do_split(length, icmp_payload)
            } else if length > 0 {
                // a 'compliant' ICMP extension padded to at least 128 octets.
                match do_split(ICMP_ORIG_DATAGRAM_MIN_LENGTH, icmp_payload) {
                    (&[], ext) => (&[], ext),
                    (payload, extension) => (&payload[..length], extension),
                }
            } else {
                // a 'non-compliant' ICMP extension padded to 128 octets.
                do_split(ICMP_ORIG_DATAGRAM_MIN_LENGTH, icmp_payload)
            }
        } else {
            // no extension present
            (icmp_payload, None)
        }
    }

    /// Split the ICMP payload into payload and extension parts.
    ///
    /// If the extension is not empty and is at least as long as the minimum
    /// extension header then Some(extension) is returned.
    ///
    /// If the extension is empty then None is returned.
    ///
    /// If the extension is non-empty but not as long as the minimum extension
    /// header then the payload is invalid and so we return an empty payload
    /// and extension.
    fn do_split(index: usize, icmp_payload: &[u8]) -> (&[u8], Option<&[u8]>) {
        match icmp_payload.split_at(index) {
            (payload, extension) if extension.len() >= MIN_HEADER => (payload, Some(extension)),
            (payload, extension) if extension.is_empty() => (payload, None),
            _ => (&[], None),
        }
    }

*Apart from some IPv6 adjustment needed