Check minimum size of ICMPv6 packets

[htibosch]

Description

This PR will correct the minimum length check for ICMP/ping packets.

The current code assumes that an ICMPv6 packet always has a length of 24 or more: ipSIZE_OF_ICMPv6_HEADER. In fact, echo packets have an ICMP header of 8 bytes only. The header may be followed by zero or more user data.

Test Steps

I sent pings of increasing size from 0 up to to 1452 bytes, where my MTU was 1500.

    ping -c 1 -s 0 fe80::8008
    ping -c 1 -s 1 fe80::8008
    ...
    ping -c 1 -s 1452 fe80::8008  // Results in a packet of 1500 + 14 bytes.

Checklist:

• I have tested my changes. No regression in existing tests.
• I have modified and/or added unit-tests to cover the code changes in this Pull Request.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[shubnil]

Hi Hein, I think it's best to take care of this in Network-interface file of the port. Mac also provides an option to zero pad the input packet if the size is smaller, can we explore that option as well.

[shubnil]

Hi Hein, I think it's best to take care of this in Network-interface file of the port. Mac also provides an option to zero pad the input packet if the size is smaller, can we explore that option as well.

[htibosch]

This is the proposed change to FreeRTOS_IPv6.c:

-    uxMinimumLength = ipSIZE_OF_ETH_HEADER + ipSIZE_OF_IPv6_HEADER + uxExtHeaderLength + ipSIZE_OF_ICMPv6_HEADER;
+    uint8_t ucTypeOfMessage;
+
+    uxMinimumLength = ipSIZE_OF_ETH_HEADER + ipSIZE_OF_IPv6_HEADER + uxExtHeaderLength;
+
+    ucTypeOfMessage = pucEthernetBuffer[ uxMinimumLength ];
+
+    if( ( ucTypeOfMessage == ipICMP_PING_REQUEST_IPv6 ) ||
+        ( ucTypeOfMessage == ipICMP_PING_REPLY_IPv6 ) )
+    {
+        uxMinimumLength += sizeof( ICMPEcho_IPv6_t );
+    }
+    else if( ucTypeOfMessage == ipICMP_ROUTER_SOLICITATION_IPv6 )
+    {
+        uxMinimumLength += sizeof( ICMPRouterSolicitation_IPv6_t );
+    }
+    else if( ucTypeOfMessage == ipICMP_ROUTER_ADVERTISEMENT_IPv6 )
+    {
+        uxMinimumLength += sizeof( ICMPRouterAdvertisement_IPv6_t );
+    }
+    else
+    {
+        uxMinimumLength += ipSIZE_OF_ICMPv6_HEADER;
+    }

Before this PR, only ICMPv6 packets with a header size of 24 bytes or longer were accepted by the stack.

An IPv6 echo packet containing 1 user byte has an ICMP header of 8+1 bytes long. Only when 16 or more user bytes are sent, the packet is accepted.

The same for small RS packets ( 8 bytes header ), they would get dropped.

And finally an RA packet would be dropped when sent with very few options.

After applying this PR, the minimum will still be 24 bytes, with the exception of a few ICMPv6 header types that are smaller.

@shubnil wrote:

Hi Hein, I think it's best to take care of this in Network-interface file of the port.

As I have tried to show here above, the ICMPv6 length check is part of the IPv6 code.

zero pad the input packet

We already extend the number of bytes sent to a minimum of 60+4 in case Ethernet is being used. And we also clear the extra bytes sent. See ipconfigETHERNET_MINIMUM_PACKET_BYTES.

But why would you want to "zero pad" input packets?

[htibosch]

/bot run formatting

[htibosch]

Hi Hein, I think it's best to take care of this in Network-interface file of the port. Mac also provides an option to zero pad the input packet if the size is smaller, can we explore that option as well.

It looks like there is some confusion here. All length fields are checked in the IP-task. Also the length of an ICMP packet is checked in the IP-task function xCheckIPv6SizeFields().

I don't see the relation between the ICMP length and zero padding?

[htibosch]

/bot run formatting