buglwIP - A Lightweight TCP/IP stack - Bugs: bug #40303, DHCP xid renewed when sending a...

 
 

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

bug #40303: DHCP xid renewed when sending a DHCPREQUEST

Submitter:  Florent Matignon <fmatignon>
Submitted:  Fri 18 Oct 2013 02:48:54 PM UTC
   
 
Category:  DHCP Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  git head

Jump to the original submission

Thu 27 Feb 2014 10:02:00 PM UTC, comment #6: 

Yes, it is OK since it's only changed in dhcp_create_message and then stored in dhcp->xid;  There is no requirement that x_id's increment.

If using LWIP_DHCP_RAND it assumes rand() never returns 2 values the same.  Hopefully no one starts DHCP and then uses an srand()!

Bill Auerbach <billauerbach>
Thu 27 Feb 2014 09:44:40 PM UTC, comment #5: 

Fixed, thanks for reporting.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 27 Feb 2014 09:35:50 PM UTC, comment #4: 

Quotes from RFC 2131:
Chapter 4.1:
"Selecting a new 'xid' for each retransmission is an implementation decision. A client may choose to reuse the same 'xid' or select a new 'xid' for each retransmitted message."

Being like that, the only bug seems to be that we create a new xid for DHCPREQUEST instead of reusing the one from DHCPOFFER. However, I don't know how a retransmission of DHCPREQUEST fits into that... Probably we should just reuse the xid when sending DHCPREQUEST...

Also, why shouldn't DHCP work with multiple interfaces? The only strange behaviour would be that xid is not simply increased on each netif since it is used for both netifs. But since the current xid is stored in struct dhcp, this should still work.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 18 Oct 2013 04:53:49 PM UTC, comment #3: 

I see now.  You're right, as far as I read it.  It looks like xid should be changed only on a non-retransmission of a DHCPDISCOVER.  You're right too in that it is a session id.

DHCP also won't work with multiple interfaces because xid is a global (or function static).  xid should be stored in the netif waiting to be served an IP address. 2 netifs could be wanting a DHCP-served address on startup.

Bill Auerbach <billauerbach>
Fri 18 Oct 2013 04:02:59 PM UTC, comment #2: 

The quote is from the "DHCP client behavior" section.
From my understanding, xid should be reused by both the server and the client.

Here is an example message flow:
- DHCPDISCOVER sent by the client ; xid is randomly generated
- DHCPOFFER coming from server(s) ; xis is reused from previous DHCPDISCOVER message ("The server inserts the 'xid' field from the DHCPDISCOVER message into the 'xid' field of the DHCPOFFER message" - section 4.3.1 "DHCP server behavior" / "DHCPDISCOVER message")
- DHCPREQUEST from the client ; xid is reused from previous DHCPOFFER message ("The DHCPREQUEST message contains the same 'xid' as the DHCPOFFER message.")
- DHCPACK from the server ; xid is reused from previous DHCPREQUEST

Fields usage is summarized in the RFC in tables 3 (server messages) and 5 (client messages): for DHCPREQUEST message, 'xid' field should come from previous DHCPOFFER server message.

Florent Matignon <fmatignon>
Fri 18 Oct 2013 03:39:52 PM UTC, comment #1: 

Isn't this part of the RFC you quite making a statement to the requirements of the server?  If lwIP sends a DHCPREQUEST the server needs to keep xid the same in the DHCPOFFER.  lwIP doesn't send a DHCPOFFER.  Unless of course I'm misreading the code.

Bill Auerbach <billauerbach>
Fri 18 Oct 2013 02:48:54 PM UTC, original submission:  

lwIP always renews the 'xid' field of a DHCP packet when sending a new packet:

see dhcp_create_msg() (dhcp.c line 1675)

  if (dhcp->tries == 0) {
#if DHCP_CREATE_RAND_XID && defined(LWIP_RAND)
    xid = LWIP_RAND();
#else /* DHCP_CREATE_RAND_XID && defined(LWIP_RAND) */
    xid++;
#endif /* DHCP_CREATE_RAND_XID && defined(LWIP_RAND) */
  }
  dhcp->xid = xid;
  (...)
  dhcp->msg_out->xid = htonl(dhcp->xid);


This is not compliant with the RFC 2131:

"The DHCPREQUEST message contains the same 'xid' as the DHCPOFFER message." (RFC 2131, section 4.4.1 "Initialization and allocation of network address")

The RFC does not explicitely indicates if a new xid must be regenerated when a DHCPREQUEST is sent from BOUND, RENEWING or REBINDING states; my guess would be that a xid is specific to a given DHCP "session" and should not be generated in those cases.

I've actually never seen a DHCP server checking that the DHCPREQUEST xid matches the DHCPOFFER's one... But it's just to be compliant with the RFC.

Florent Matignon <fmatignon>

 

(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 (Posted a comment)
  • -email is unavailable- added by fmatignon (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
    2014-02-27 goldsimon StatusNone Fixed
        Assigned toNone goldsimon
        Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code