Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve frame filtering #1100

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Improve frame filtering #1100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bda977e
add checks for frame types
HTRamsey Feb 10, 2024
857638b
address review comments
HTRamsey Feb 10, 2024
a205317
Fix multicast filtering and finding endpoint
HTRamsey Feb 15, 2024
9718f55
Remove ipCONSIDER_FRAME_FOR_PROCESSING macro
HTRamsey Feb 15, 2024
046ec05
add ipv4 multicast frame check
HTRamsey Feb 16, 2024
c1db70f
make tests finish compiling
HTRamsey Feb 16, 2024
3136cbd
fix tests
HTRamsey Feb 16, 2024
6cac71e
use multicast macros
HTRamsey Feb 16, 2024
f3d960f
improve test coverage
HTRamsey Feb 16, 2024
b5a21d1
Uncrustify: triggered by comment.
actions-user Mar 13, 2024
28ae29b
Merge branch 'main' into dev-frame-filter
HTRamsey Apr 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
104 changes: 42 additions & 62 deletions source/FreeRTOS_DNS.c
View file
Open in desktop
Expand Up @@ -57,6 +57,48 @@
#include "FreeRTOS_DNS_Callback.h"


/** @brief The MAC address used for LLMNR. */
const MACAddress_t xLLMNR_MacAddress = { { 0x01, 0x00, 0x5e, 0x00, 0x00, 0xfc } };

/** @brief The IPv6 link-scope multicast MAC address */
const MACAddress_t xLLMNR_MacAddressIPv6 = { { 0x33, 0x33, 0x00, 0x01, 0x00, 0x03 } };

/** @brief The IPv6 link-scope multicast address */
const IPv6_Address_t ipLLMNR_IP_ADDR_IPv6 =
{
{ /* ff02::1:3 */
0xff, 0x02,
0x00, 0x00,
0x00, 0x00,
0x00, 0x00,
0x00, 0x00,
0x00, 0x00,
0x00, 0x01,
0x00, 0x03,
}
};

/** @brief The MAC address used for MDNS. */
const MACAddress_t xMDNS_MacAddress = { { 0x01, 0x00, 0x5e, 0x00, 0x00, 0xfb } };

/** @brief The IPv6 multicast DNS MAC address. */
const MACAddress_t xMDNS_MACAddressIPv6 = { { 0x33, 0x33, 0x00, 0x00, 0x00, 0xFB } };

/** @brief multicast DNS IPv6 address */
const IPv6_Address_t ipMDNS_IP_ADDR_IPv6 =
{
{ /* ff02::fb */
0xff, 0x02,
0x00, 0x00,
0x00, 0x00,
0x00, 0x00,
0x00, 0x00,
0x00, 0x00,
0x00, 0x00,
0x00, 0xfb,
}
};

/* Exclude the entire file if DNS is not enabled. */
#if ( ipconfigUSE_DNS != 0 )

Expand Down Expand Up @@ -95,69 +137,7 @@
struct freertos_addrinfo ** ppxAddressInfo,
BaseType_t xFamily );

#if ( ipconfigUSE_LLMNR == 1 )
/** @brief The MAC address used for LLMNR. */
const MACAddress_t xLLMNR_MacAddress = { { 0x01, 0x00, 0x5e, 0x00, 0x00, 0xfc } };
#endif /* ipconfigUSE_LLMNR == 1 */

/*-----------------------------------------------------------*/
#if ( ipconfigUSE_LLMNR == 1 ) && ( ipconfigUSE_IPv6 != 0 )

/**
* @brief The IPv6 link-scope multicast address
*/
const IPv6_Address_t ipLLMNR_IP_ADDR_IPv6 =
{
{ /* ff02::1:3 */
0xff, 0x02,
0x00, 0x00,
0x00, 0x00,
0x00, 0x00,
0x00, 0x00,
0x00, 0x00,
0x00, 0x01,
0x00, 0x03,
}
};

/**
* @brief The IPv6 link-scope multicast MAC address
*/
const MACAddress_t xLLMNR_MacAddressIPv6 = { { 0x33, 0x33, 0x00, 0x01, 0x00, 0x03 } };
#endif /* ipconfigUSE_LLMNR && ipconfigUSE_IPv6 */

#if ( ipconfigUSE_MDNS == 1 ) && ( ipconfigUSE_IPv6 != 0 )

/**
* @brief multicast DNS IPv6 address
*/
const IPv6_Address_t ipMDNS_IP_ADDR_IPv6 =
{
{ /* ff02::fb */
0xff, 0x02,
0x00, 0x00,
0x00, 0x00,
0x00, 0x00,
0x00, 0x00,
0x00, 0x00,
0x00, 0x00,
0x00, 0xfb,
}
};

/**
* @brief The IPv6 multicast DNS MAC address.
* The MAC-addresses are provided here in case a network
* interface needs it.
*/
const MACAddress_t xMDNS_MACAddressIPv6 = { { 0x33, 0x33, 0x00, 0x00, 0x00, 0xFB } };
#endif /* ( ipconfigUSE_MDNS == 1 ) && ( ipconfigUSE_IPv6 != 0 ) */


#if ( ipconfigUSE_MDNS == 1 )
/** @brief The MAC address used for MDNS. */
const MACAddress_t xMDNS_MacAddress = { { 0x01, 0x00, 0x5e, 0x00, 0x00, 0xfb } };
#endif /* ipconfigUSE_MDNS == 1 */

/** @brief This global variable is being used to indicate to the driver which IP type
* is preferred for name service lookup, either IPv6 or IPv4. */
Expand Down
211 changes: 137 additions & 74 deletions source/FreeRTOS_IP.c
View file
Open in desktop
Expand Up @@ -92,20 +92,6 @@
#endif
#endif

/** @brief If ipconfigETHERNET_DRIVER_FILTERS_FRAME_TYPES is set to 1, then the Ethernet
* driver will filter incoming packets and only pass the stack those packets it
* considers need processing. In this case ipCONSIDER_FRAME_FOR_PROCESSING() can
* be #-defined away. If ipconfigETHERNET_DRIVER_FILTERS_FRAME_TYPES is set to 0
* then the Ethernet driver will pass all received packets to the stack, and the
* stack must do the filtering itself. In this case ipCONSIDER_FRAME_FOR_PROCESSING
* needs to call eConsiderFrameForProcessing.
*/
#if ipconfigETHERNET_DRIVER_FILTERS_FRAME_TYPES == 0
#define ipCONSIDER_FRAME_FOR_PROCESSING( pucEthernetBuffer ) eConsiderFrameForProcessing( ( pucEthernetBuffer ) )
#else
#define ipCONSIDER_FRAME_FOR_PROCESSING( pucEthernetBuffer ) eProcessBuffer
#endif

static void prvCallDHCP_RA_Handler( NetworkEndPoint_t * pxEndPoint );

static void prvIPTask_Initialise( void );
Expand Down Expand Up @@ -1451,85 +1437,164 @@ BaseType_t xSendEventStructToIPTask( const IPStackEvent_t * pxEvent,
*/
eFrameProcessingResult_t eConsiderFrameForProcessing( const uint8_t * const pucEthernetBuffer )
{
eFrameProcessingResult_t eReturn = eProcessBuffer;
const EthernetHeader_t * pxEthernetHeader = NULL;
const NetworkEndPoint_t * pxEndPoint = NULL;
eFrameProcessingResult_t eReturn = eReleaseBuffer;

if( pucEthernetBuffer == NULL )
{
eReturn = eReleaseBuffer;
}
else
do
{
/* Map the buffer onto Ethernet Header struct for easy access to fields. */
const EthernetHeader_t * pxEthernetHeader = NULL;
const NetworkEndPoint_t * pxEndPoint = NULL;
uint16_t usFrameType;

/* First, check the packet buffer is non-null. */
if( pucEthernetBuffer == NULL )
{
/* The packet buffer was null - release it. */
break;
}

/* Map the buffer onto Ethernet Header struct for easy access to fields. */
/* MISRA Ref 11.3.1 [Misaligned access] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* coverity[misra_c_2012_rule_11_3_violation] */
pxEthernetHeader = ( ( const EthernetHeader_t * ) pucEthernetBuffer );
usFrameType = pxEthernetHeader->usFrameType;

/* Second, filter based on ethernet frame type. */
if( FreeRTOS_ntohs( usFrameType ) <= 0x0600U )
{
/* The packet was not an Ethernet II frame */
#if ipconfigIS_ENABLED( ipconfigFILTER_OUT_NON_ETHERNET_II_FRAMES )
/* filtering is enabled - release it. */
break;
#else
/* filtering is disabled - continue filter checks. */
#endif
}
else if( usFrameType == ipARP_FRAME_TYPE )
{
/* The frame is an ARP type */
#if ipconfigIS_DISABLED( ipconfigUSE_IPv4 )
/* IPv4 is disabled - release it. */
break;
#else
/* IPv4 is enabled - Continue filter checks. */
#endif
}
else if( usFrameType == ipIPv4_FRAME_TYPE )
{
/* The frame is an IPv4 type */
#if ipconfigIS_DISABLED( ipconfigUSE_IPv4 )
/* IPv4 is disabled - release it. */
break;
#else
/* IPv4 is enabled - Continue filter checks. */
#endif
}
else if( usFrameType == ipIPv6_FRAME_TYPE )
{
/* The frame is an IPv6 type */
#if ipconfigIS_DISABLED( ipconfigUSE_IPv6 )
/* IPv6 is disabled - release it. */
break;
#else
/* IPv6 is enabled - Continue filter checks. */
#endif
}
else
{
/* The frame is an unsupported Ethernet II type */
#if ipconfigIS_DISABLED( ipconfigPROCESS_CUSTOM_ETHERNET_FRAMES )
/* Processing custom ethernet frames is disabled - release it. */
break;
#else
/* Processing custom ethernet frames is enabled - Continue filter checks. */
#endif
}

/* Examine the destination MAC from the Ethernet header to see if it matches
* that of an end point managed by FreeRTOS+TCP. */
/* Third, filter based on destination mac address. */
pxEndPoint = FreeRTOS_FindEndPointOnMAC( &( pxEthernetHeader->xDestinationAddress ), NULL );

if( pxEndPoint != NULL )
{
/* The packet was directed to this node - process it. */
eReturn = eProcessBuffer;
/* A destination endpoint was found - Continue filter checks. */
}
else if( memcmp( xBroadcastMACAddress.ucBytes, pxEthernetHeader->xDestinationAddress.ucBytes, sizeof( MACAddress_t ) ) == 0 )
{
/* The packet was a broadcast - process it. */
eReturn = eProcessBuffer;
/* The packet was a broadcast - Continue filter checks. */
}
else
#if ( ( ipconfigUSE_LLMNR == 1 ) && ( ipconfigUSE_DNS != 0 ) )
if( memcmp( xLLMNR_MacAddress.ucBytes, pxEthernetHeader->xDestinationAddress.ucBytes, sizeof( MACAddress_t ) ) == 0 )
{
/* The packet is a request for LLMNR - process it. */
eReturn = eProcessBuffer;
}
else
#endif /* ipconfigUSE_LLMNR */
#if ( ( ipconfigUSE_MDNS == 1 ) && ( ipconfigUSE_DNS != 0 ) )
if( memcmp( xMDNS_MacAddress.ucBytes, pxEthernetHeader->xDestinationAddress.ucBytes, sizeof( MACAddress_t ) ) == 0 )
{
/* The packet is a request for MDNS - process it. */
eReturn = eProcessBuffer;
}
else
#endif /* ipconfigUSE_MDNS */
if( ( pxEthernetHeader->xDestinationAddress.ucBytes[ 0 ] == ipMULTICAST_MAC_ADDRESS_IPv6_0 ) &&
( pxEthernetHeader->xDestinationAddress.ucBytes[ 1 ] == ipMULTICAST_MAC_ADDRESS_IPv6_1 ) )
HTRamsey marked this conversation as resolved.
Show resolved Hide resolved
else if( memcmp( xLLMNR_MacAddress.ucBytes, pxEthernetHeader->xDestinationAddress.ucBytes, sizeof( MACAddress_t ) ) == 0 )
{
/* The packet is a request for LLMNR - process it. */
eReturn = eProcessBuffer;
/* The packet is a request for LLMNR using IPv4 */
#if ( ipconfigIS_DISABLED( ipconfigUSE_DNS ) || ipconfigIS_DISABLED( ipconfigUSE_LLMNR ) || ipconfigIS_DISABLED( ipconfigUSE_IPv4 ) )
/* DNS, LLMNR, or IPv4 is disabled - release it. */
break;
#else
/* DNS, LLMNR, and IPv4 are enabled - Continue filter checks. */
#endif
}
else
else if( memcmp( xLLMNR_MacAddressIPv6.ucBytes, pxEthernetHeader->xDestinationAddress.ucBytes, sizeof( MACAddress_t ) ) == 0 )
{
/* The packet was not a broadcast, or for this node, just release
* the buffer without taking any other action. */
eReturn = eReleaseBuffer;
/* The packet is a request for LLMNR using IPv6 */
#if ( ipconfigIS_DISABLED( ipconfigUSE_DNS ) || ipconfigIS_DISABLED( ipconfigUSE_LLMNR ) || ipconfigIS_DISABLED( ipconfigUSE_IPv6 ) )
/* DNS, LLMNR, or IPv6 is disabled - release it. */
break;
#else
/* DNS, LLMNR, and IPv6 are enabled - Continue filter checks. */
#endif
}
}

#if ( ipconfigFILTER_OUT_NON_ETHERNET_II_FRAMES == 1 )
{
uint16_t usFrameType;

if( eReturn == eProcessBuffer )
else if( memcmp( xMDNS_MacAddress.ucBytes, pxEthernetHeader->xDestinationAddress.ucBytes, sizeof( MACAddress_t ) ) == 0 )
{
usFrameType = pxEthernetHeader->usFrameType;
usFrameType = FreeRTOS_ntohs( usFrameType );

if( usFrameType <= 0x600U )
{
/* Not an Ethernet II frame. */
eReturn = eReleaseBuffer;
}
/* The packet is a request for MDNS using IPv4 */
#if ( ipconfigIS_DISABLED( ipconfigUSE_DNS ) || ipconfigIS_DISABLED( ipconfigUSE_MDNS ) || ipconfigIS_DISABLED( ipconfigUSE_IPv4 ) )
/* DNS, MDNS, or IPv4 is disabled - release it. */
break;
#else
/* DNS, MDNS, and IPv4 are enabled - Continue filter checks. */
#endif
}
else if( memcmp( xMDNS_MACAddressIPv6.ucBytes, pxEthernetHeader->xDestinationAddress.ucBytes, sizeof( MACAddress_t ) ) == 0 )
{
/* The packet is a request for MDNS using IPv6 */
#if ( ipconfigIS_DISABLED( ipconfigUSE_DNS ) || ipconfigIS_DISABLED( ipconfigUSE_MDNS ) || ipconfigIS_DISABLED( ipconfigUSE_IPv6 ) )
/* DNS, MDNS, or IPv6 is disabled - release it. */
break;
#else
/* DNS, MDNS, and IPv6 are enabled - Continue filter checks. */
#endif
}
else if( ( pxEthernetHeader->xDestinationAddress.ucBytes[ 0 ] == ipMULTICAST_MAC_ADDRESS_IPv4_0 ) &&
( pxEthernetHeader->xDestinationAddress.ucBytes[ 1 ] == ipMULTICAST_MAC_ADDRESS_IPv4_1 ) &&
( pxEthernetHeader->xDestinationAddress.ucBytes[ 2 ] == ipMULTICAST_MAC_ADDRESS_IPv4_2 ) &&
( pxEthernetHeader->xDestinationAddress.ucBytes[ 3 ] <= 0x7fU ) )
{
/* The packet is an IPv4 Multicast */
#if ipconfigIS_DISABLED( ipconfigUSE_IPv4 )
/* IPv4 is disabled - release it. */
break;
#else
/* IPv4 is enabled - Continue filter checks. */
#endif
}
else if( ( pxEthernetHeader->xDestinationAddress.ucBytes[ 0 ] == ipMULTICAST_MAC_ADDRESS_IPv6_0 ) &&
( pxEthernetHeader->xDestinationAddress.ucBytes[ 1 ] == ipMULTICAST_MAC_ADDRESS_IPv6_1 ) )
{
/* The packet is an IPv6 Multicast */
#if ipconfigIS_DISABLED( ipconfigUSE_IPv6 )
/* IPv6 is disabled - release it. */
break;
#else
/* IPv6 is enabled - Continue filter checks. */
#endif
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MISRA 2012: else if constructs should be terminated with non-empty else case.

Copy link
Contributor Author

@HTRamsey HTRamsey Feb 16, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that else is reached, should we release the buffer? The destination address would be unsupported I guess.

Edit: Shouldn't we check for 0x01005E for IPv4 multicast, similar to how we do for IPv6?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If that else is reached, should we release the buffer? The destination address would be unsupported I guess.

Yes, I believe its good to release the buffer.

Shouldn't we check for 0x01005E for IPv4 multicast, similar to how we do for IPv6?

For multicast support, this check should be required. It would be good to use the new multicast MAC address macros in vSetMultiCastIPv4MacAddress as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small side note:
In #1019 I've changed this check to allow all multicast MAC addresses if ipconfigSUPPORT_IP_MULTICAST is enabled. I might change that in the future, but with multicasts enabled and especially if ipconfigPROCESS_CUSTOM_ETHERNET_FRAMES is enabled, it really is too early to drop any multicasts at this stage.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll change the multicast filtering part after that goes in, for now it just drops ipv6 multicasts if ipv6 is disabled and vice versa for ipv4

else
{
/* The packet was not a broadcast, or for this node - release it */
break;
}
}
#endif /* ipconfigFILTER_OUT_NON_ETHERNET_II_FRAMES == 1 */

/* All checks have been passed, process the packet. */
eReturn = eProcessBuffer;
} while( ipFALSE_BOOL );

return eReturn;
}
Expand Down Expand Up @@ -1575,8 +1640,6 @@ static void prvProcessEthernetPacket( NetworkBufferDescriptor_t * const pxNetwor
break;
}

eReturned = ipCONSIDER_FRAME_FOR_PROCESSING( pxNetworkBuffer->pucEthernetBuffer );

/* Map the buffer onto the Ethernet Header struct for easy access to the fields. */

/* MISRA Ref 11.3.1 [Misaligned access] */
Expand All @@ -1586,7 +1649,7 @@ static void prvProcessEthernetPacket( NetworkBufferDescriptor_t * const pxNetwor

/* The condition "eReturned == eProcessBuffer" must be true. */
#if ( ipconfigETHERNET_DRIVER_FILTERS_FRAME_TYPES == 0 )
if( eReturned == eProcessBuffer )
if( eConsiderFrameForProcessing( pxNetworkBuffer->pucEthernetBuffer ) == eProcessBuffer )
#endif
{
/* Interpret the received Ethernet packet. */
Expand Down
6 changes: 3 additions & 3 deletions source/FreeRTOS_IPv4_Utils.c
View file
Open in desktop
Expand Up @@ -59,9 +59,9 @@ void vSetMultiCastIPv4MacAddress( uint32_t ulIPAddress,
{
uint32_t ulIP = FreeRTOS_ntohl( ulIPAddress );

pxMACAddress->ucBytes[ 0 ] = ( uint8_t ) 0x01U;
pxMACAddress->ucBytes[ 1 ] = ( uint8_t ) 0x00U;
pxMACAddress->ucBytes[ 2 ] = ( uint8_t ) 0x5EU;
pxMACAddress->ucBytes[ 0 ] = ( uint8_t ) ipMULTICAST_MAC_ADDRESS_IPv4_0;
pxMACAddress->ucBytes[ 1 ] = ( uint8_t ) ipMULTICAST_MAC_ADDRESS_IPv4_1;
pxMACAddress->ucBytes[ 2 ] = ( uint8_t ) ipMULTICAST_MAC_ADDRESS_IPv4_2;
pxMACAddress->ucBytes[ 3 ] = ( uint8_t ) ( ( ulIP >> 16 ) & 0x7fU ); /* Use 7 bits. */
pxMACAddress->ucBytes[ 4 ] = ( uint8_t ) ( ( ulIP >> 8 ) & 0xffU ); /* Use 8 bits. */
pxMACAddress->ucBytes[ 5 ] = ( uint8_t ) ( ( ulIP ) & 0xffU ); /* Use 8 bits. */
Expand Down