buglwIP - A Lightweight TCP/IP stack - Bugs: bug #27390, Source IP check in ip_input()...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #27390: Source IP check in ip_input() causes it to drop valid DHCP packets

Submitted by:  Mandeep Sandhu <mandeepsandhu>
Submitted on:  Mon 07 Sep 2009 07:42:10 AM UTC  
 
Category: IPv4Severity: 3 - Normal
Item Group: Faulty BehaviourStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: 1.3.0

Fri 16 Oct 2009 12:39:48 PM UTC, comment #3:

Fixed by allowing 0.0.0.0 as source address when LWIP_DHCP is 1.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 16 Oct 2009 06:03:47 AM UTC, comment #2:

The part that resets check_ip_src does so only if we haven't found a suitable interface. This would happen when we're running as a DHCP client and haven't configured the interface yet. But, in my case I was running a DHCP server and already had the interface configured. So, the netif on which the packet was received is selected. This causes the reset (of check_ip_src) part to be skipped. I think we should do a check for DHCP server port (68) as well. Something like:

if(netif == NULL) {
// Check for DHCP client port
} else {
// Check for DHCP server port
}

What say?

Mandeep Sandhu <mandeepsandhu>
Thu 15 Oct 2009 04:21:24 PM UTC, comment #1:

I also think it should be ok to allow 0.0.0.0 as src IP. But maybe it is better to include the necessary check in the part that resets check_ip_src.

Any objections to that?

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Mon 07 Sep 2009 07:42:10 AM UTC, original submission:

I'm running my ECOS application with lwIP 1.3.1. The app is a small DHCP server (with limited functionality).

DHCP discover packets coming from clients have src addr as 0.0.0.0 and dst IP as 255.255.255.255.

On reception of DHCP discover packets I see that the ip_input function drops it:

...
...
lwip_recvfrom(0, 0x02001c64, 548, 0x0, ..)
lwip_recvfrom: top while sock->lastdata=0x00000000
ip_input: iphdr->dest 0xffffffff netif->ip_addr 0x101010b (0xffffff,
0x1010b, 0xff000000)
ip_input: packet accepted on interface et
ip_input: packet source is not valid.
...
...

On looking at the code, I see that we are intentionally dropping
packets with src IP as either broadcast or 0.0.0.0 (referred to as 'old skool' bcast in code).

The variable "check_ip_src" also does not help as the only place where it is reset is when we do a check on the UDP destination port (to see if it's a DHCP client port or not). This does not work however, since we have found a 'netif' by then. When searching for a 'netif' with a broadcast address, we will always return with the first netif struct and hence never do exception processing for DHCP packets (where we reset the "check_ip_src" flag).

Moreover, the comment the place where we do src IP check says:

/* broadcast or multicast packet source address? Compliant with RFC 1122: 3.2.1.3 */

But a 0.0.0.0 address IS compliant with the RFC!
From RFC 1122, section 3.2.1.3:

(a) { 0, 0 }

This host on this network. MUST NOT be sent, except as
a source address as part of an initialization procedure
by which the host learns its own IP address.

I guess they are talking about DHCP here. I think it should be ok to allow 0.0.0.0 as src IP.

Regards,
-mandeep

Mandeep Sandhu <mandeepsandhu>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by kieranm (Updated the item)
  • -unavailable- added by mandeepsandhu (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 16 Oct 2009 12:39:48 PM UTCgoldsimonStatusIn Progress=>Fixed
      Open/ClosedOpen=>Closed
    Thu 15 Oct 2009 04:21:24 PM UTCgoldsimonStatusNone=>In Progress
      Assigned toNone=>goldsimon
    Thu 15 Oct 2009 03:35:45 PM UTCkieranmPlanned Release=>1.3.2

    Back to the top


    Powered by Savane 3.1-cleanup1