All unicast packets allowed on inactive endpoint?

Libraries
1

Hi everyone!
I am working on a project that uses FreeRTOS+TCP, network configuration is DHCP.

I am experiencing an issue when obtaining IP through DHCP, here is what happens:

  • the MCU is on the network doing its thing, then it gets reset;
  • the MCU issues a discover;
  • the DHCP already has a lease for the MCU’s MAC address, it tries to ping the lease before offering it;
  • a ICMP ping request is issued with destination MAC of the MCU, ARP request is skipped as the host already has a fresh ARP entry for that MAC address;
  • the MCU replies to that ICMP ping request!
  • the DHCP server deems that lease occupied, so it abandons it and offers the next available IP.

I looked at the code to understand why this happens, I landed in “FreeRTOS_IPv4.c”, function “prvAllowIPPacketIPv4”, line 320.
The condition to DROP the packet is that ALL of the followings are true:

  • all endpoints are up (I only have one)
  • the packet dst IP is not multicast
  • the packet dst IP is not broadcast
  • no endpoint matches ANY the following criteria:
    • packet dst IP == 0 (in which case any interface is fine)
    • endpoint IP == 0 (so if the interface is down)
    • packet dst IP == endpoint IP (the packet is for this interface)

In other words, KEEP the packet if ANY of the following:

  • at least one endpoint is down
  • the packet dst IP is multicast
  • the packet dst IP is broadcast
  • there is an endpoint that either:
    • is down
    • has an IP that matches packet’s dst IP

This logic allows all unicast packets through my inactive endpoint. This allows my MCU to generate the ICMP ping reply even if it doesn’t have the requested IP (the ICMP ping reply handler simply swaps the IPs around to generate a reply).
I have made two little modifications to fix the issue I described at the beginning. I have not tested it thoroughly, I don’t know if this breaks other use-cases, but I did not find any issue yet, neither in code nor in practice.

What are your thoughts?
Was that the intended behaviour?
Thanks!

--- ./FreeRTOS/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/source/FreeRTOS_IPv4.c
+++ ./FreeRTOS/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/source/FreeRTOS_IPv4.c.patch
@@ -321,9 +321,7 @@
             ( FreeRTOS_FindEndPointOnIP_IPv4( ulDestinationIPAddress, 4 ) == NULL ) &&
             /* Is it an IPv4 broadcast address x.x.x.255 ? */
             ( ( FreeRTOS_ntohl( ulDestinationIPAddress ) & 0xffU ) != 0xffU ) &&
-            ( xIsIPv4Multicast( ulDestinationIPAddress ) == pdFALSE ) &&
-            /* Or (during DHCP negotiation) we have no IP-address yet? */
-            ( FreeRTOS_IsNetworkUp() != pdFALSE ) )
+            ( xIsIPv4Multicast( ulDestinationIPAddress ) == pdFALSE ) )
         {
             /* Packet is not for this node, release it */
             eReturn = eReleaseBuffer;

--- ./FreeRTOS/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/source/FreeRTOS_Routing.c
+++ ./FreeRTOS/FreeRTOS-Plus/Source/FreeRTOS-Plus-TCP/source/FreeRTOS_Routing.c.patch
@@ -410,7 +410,6 @@
                 #endif
                 {
                     if( ( ulIPAddress == 0U ) ||
-                        ( pxEndPoint->ipv4_settings.ulIPAddress == 0U ) ||
                         ( pxEndPoint->ipv4_settings.ulIPAddress == ulIPAddress ) )
                     {
                         break;
2

Hi @marcosegato,

As the comment mentioned, it’s used to accept DHCP packets. Can you help check if you can pass DHCP flow with this change?

I can’t understand what would this change help. Could you help explain?

My personal suggestion is to limit this in ICMP.c file only. Checking destination IP address before sending PING response might help you solve this scenario. But, why does DHCP server check the lease? In my experience, DHCP server simply assigns the same IP address to the same MAC address as lease renewal/rebinding. It might be worth checking.

Thank you.

3

Hello @ActoryOu,
Thank you for your answer!

Yes, DHCP packets are still allowed through as they are broadcast packets. The MCU is able to complete the DHCP transaction.

Both this changes have affected the logic to now NOT let through unicast packets on inactive endpoints.
The function “prvAllowIPPacketIPv4” used to allow through any packet if at least one endpoint is down, so I removed that condition.
The function “FreeRTOS_FindEndPointOnIP_IPv4” used to return endpoints that are down too, so I removed that condition in order to affect “prvAllowIPPacketIPv4”. From what I understand, this change should not affect the other places where this function is called.
So now the packet is KEPT if ANY of the following:

  • the packet dst IP is multicast
  • the packet dst IP is broadcast
  • there is an endpoint that either:
    • has an IP that matches packet’s dst IP
    • if packet dst IP == 0 (in which case any interface is fine)

That could also fix my specific issue, but other protocols would still be allowed.

As per RFC 2131:

As a consistency check, the allocating server SHOULD probe the reused address before allocating the address

Not all DHCP server implementation may do this, but this is an expected behavior.
I am not allowed to link it :frowning:, it is in section 2.2.

1 Like
4

Hi @marcosegato,
Thank you for the clear explanation. It has helped me understand the changes better. I appreciate the effort you’ve put into clarifying the details. I’m comfortable with the proposed changes. If you don’t mind, could you please raise a pull request for these changes? That would be greatly appreciated.

Thank you once again.

5

Hello @ActoryOu,
I will!
Thanks

6

The pull request just got accepted.

1 Like