buglwIP - A Lightweight TCP/IP stack - Bugs: bug #29072, DHCPDISCOVER doesn't have ciaddr...

 
 

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

bug #29072: DHCPDISCOVER doesn't have ciaddr == 0

Submitter:  Bill Auerbach <billauerbach>
Submitted:  Thu 04 Mar 2010 07:10:08 PM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  CVS Head

Jump to the original submission

Mon 08 Mar 2010 09:23:11 PM UTC, comment #15: 

I want to follow up on DHCP:

RFC2131 4.4.2 and 4.4.3 do mention MUST include your IP address in DHCPDISCOVER (if one is assigned) and that 'the client sends a DHCPINFORM message'.  There is no MUST on this statement.  If the user has to call dhcp_inform, we're leaving RFC-specific rules as a user's responsibility.  Is this right?

And if we have an LLA assigned address and DHCP is continuing to operate then do we need to ensure that the subsequent DHCPDISCOVER's have the LLA?  RFC 2131 doesn't stipulate a rule on the type of address if the device has one already but it makes sense to not send a routable address to it.

Bill Auerbach <billauerbach>
Sat 06 Mar 2010 09:13:23 AM UTC, comment #14: 

Discussion about continuing LLA-connections should be continued in task #10241.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 05 Mar 2010 07:06:52 PM UTC, comment #13: 

lol :-) I know I digressed into Link-Local Addressing (LLA) but this RFC for LLA does add constraints on DHCP.  Since we support LLA (compliant or not) don't we now fall under RFC3927's DHCP provisions when LLA is enabled?

I believe because the netif is used to build the packet header with current netif parameters that if an IP address is changed on the netif it will break open connections.

Perhaps the pcb should also be passed to lower levels to get its time-of-connection IP address and only use the netif for the MAC address.  Or put a hwaddr pointer in the PCB and pass a pcb instead of a netif (this hwaddr cannot change and can be taken from the netif at creation.  (I think)

Bill Auerbach <billauerbach>
Fri 05 Mar 2010 06:11:15 PM UTC, comment #12: 

I only wanted to point out that this is a different issue (worth of its own bug entry or task), not that I didn't want to implement it. And I certainly didn't want to hurt let alone seriously injure anybody! :-)

Simon Goldschmidt <goldsimon>
Group administrator
Fri 05 Mar 2010 05:36:01 PM UTC, comment #11: 

OK.  It's the "host" side with the MUSTs.

However, consider a CNC controller running lwIP that's using a link-local address and a DHCP server comes on online.  If we change IP addresses and kill the connection, we destroy an expensive piece being milled at best or potentially injure the operator at worst.

Bill Auerbach <billauerbach>
Fri 05 Mar 2010 04:50:34 PM UTC, comment #10: 

OK, but now you are talking about Link-Local Addressing, not DHCP. And as I see it, the RFC does not say "must not disturb" but "SHOULD" not, so we do respect the standard here.

However, I didn't test what happens to existing connections when switching from a link-local address to a dhcp- or static-address. Since ip_pcbs already hold the local address, we might implement this to work, but that would be another task (and would have to be checked, first).

Simon Goldschmidt <goldsimon>
Group administrator
Fri 05 Mar 2010 04:32:31 PM UTC, comment #9: 

Mixing static is in fact valid using Link-Local Addressing (AutoIP).  You will have a link local address and later DHCP provides an IP address.  The RFC says you must not disturb connections on the link local address, you may use it thereafter and must use the DHCP provided address going forward.  See RFC3927 - 1.9.

Bill Auerbach <billauerbach>
Fri 05 Mar 2010 04:20:12 PM UTC, comment #8: 

Well, until now I didn't think that anyone would mix setting a static IP address (netif_set_addr) with dhcp. And it looks like this combination leads to a wrong behaviour, so maybe dhcp_start should set the netif's address to zero.

I wouldn't let netif_set_addr influence dhcp: an application or port should call dhcp_stop before switching to a static IP address (also, netif_set_addr is used by dhcp for setting the received address).

As to 3.: How would we not break TCP connections when the local address changes? Are you sure the RFC is talking about TCP there? Also, I didn't find that part in the RFC, could you point me to the chapter?

Simon Goldschmidt <goldsimon>
Group administrator
Fri 05 Mar 2010 03:43:10 PM UTC, comment #7: 

Ok, checked my code.  I do dhcp_start and if I don't get an IP address in 5 seconds, I do netif_set_addr to a static fallback (but didn't do a dhcp_stop).

What you're seeing does look incorrect from what I see in the RFC.  RFC2131 covers a lot of points for both DHCP client and server.

This brings up 3 points:

1.  Should netif_set_addr call dhcp_stop?  If netif_set_addr is called without stopping, DHCP can come along and change it.  If netif_set_addr is used internally calling dhcp_stop would be a problem.

2.  Should netif_set_addr call dhcp_inform?

3.  This is the part I mentioned where lwIP might not be RFC compliant: The RFC states that if an interface has an IP address and later if DHCP offers an IP address, the address must be taken, but the original connections must not be broken or changed. I believe the bug is that we do change the netif IP address and lwIP will break open connections on the pre-DHCP-assigned address.

Bill Auerbach <billauerbach>
Fri 05 Mar 2010 02:55:13 PM UTC, comment #6: 

Hm, when I set an IP-address before calling dhcp_start, I see that the IP-header source-address of the DHCP packets is that address, which I think is a bit odd, but still works.

And the ARP-check looks really strange, too: "who has <DHCP-addr>? Tell <temp-static-address>". I wonder if that is what it's supposed to be like?

However, even going back to the old code, I don't see ciaddr set when setting netif->ip_addr before calling dhcp_start.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 05 Mar 2010 01:58:15 PM UTC, comment #5: 


>Bill, how did you get a ciaddr != 0 in a DHCPDISCOVER message?


I don't know - I saw it in Wireshark and wondered why I was seeing the default IP address in the discovery.  So I checked the if condition and thought it shouldn't be there, but it was in WireShark.  I set an IP address on the netif and then use dhcp_start.

Bill Auerbach <billauerbach>
Fri 05 Mar 2010 01:37:42 PM UTC, comment #4: 

Should be fixed, thanks for reporting.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 05 Mar 2010 01:14:42 PM UTC, comment #3: 

Bill, how did you get a ciaddr != 0 in a DHCPDISCOVER message? With the current code, ciaddr should only be set in states RENEWING or REBINDING, while DHCPDISCOVER is only sent in SELECTING.

However, the decision loginc when to set ciaddr is still not correct, I only want to clarify this.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 05 Mar 2010 01:07:42 PM UTC, comment #2: 

On second thought, I guess DHCPREQUEST should include ciaddr != 0 in BOUND/RENEW/REBIND, so the state check is still needed in addition to the message-type check. Sorry for the confusion.

I guess the original code relied on netif->addr being 0 until dhcp succeeds.

BTW: We don't send REQUESTs in state BIND, because we change the state before sending, so that part of the if-condition could be removed.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 05 Mar 2010 12:54:50 PM UTC, comment #1: 

I'd fix this by passing the message-type to dhcp_create_request and basing the choice (whether to set ciaddr to zero or use netif->ip_addr) on the message-type instead of the current state.

BTW: RFC2131 4.4.1 Table 5 doesn't mention the INIT-REBOOT or REBOOT state regarding ciaddr/DHCPREQUEST, but I guess it's OK to set the client's address there, too...?

> Also, is there some "give up" timeout for DHCP? I'm seeing DHCP
> Discover packets in WireShark forever since there is no server
> present.


None that I know of. Currently, you have to call dhcp_stop() on your own.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 04 Mar 2010 07:10:08 PM UTC, original submission:  

RFC2131 (4.4.1) calls for DHCPDISCOVER to have a ciaddr of 0.

Line 1667 (CVS Head) is:

  if (dhcp->state==DHCP_BOUND || dhcp->state==DHCP_RENEWING || dhcp->state==DHCP_REBINDING) {
    ip_addr_copy(dhcp->msg_out->ciaddr, netif->ip_addr);
  }

This should skip setting ciaddr to non-0 because it's in state DHCP_REQUESTING, but it must become non-0 elsewhere because in WireShark it's set to the static fallback address I have set until DHCP does comes online.

Also, is there some "give up" timeout for DHCP?  I'm seeing DHCP Discover packets in WireShark forever since there is no server present.

Bill Auerbach <billauerbach>

 

(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 billauerbach (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 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-03-05 goldsimon StatusNone Fixed
        Open/ClosedOpen Closed
    2010-03-05 goldsimon Assigned toNone goldsimon

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code