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

Submitter:  Mandeep Sandhu <mandeepsandhu>
Submitted:  Mon 07 Sep 2009 07:42:10 AM UTC
   
 
Category:  IPv4 Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned 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>
Group administrator
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>
Group administrator
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>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2009-10-16 goldsimon StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2009-10-15 goldsimon StatusNone In Progress
        Assigned toNone goldsimon
    2009-10-15 kieranm Planned Release 1.3.2

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code