[dhcp] Use a random DHCP transaction identifier (xid)

iPXE currently uses the last four bytes of the MAC address as the DHCP
transaction identifier.  Reduce the probability of collisions by
generating a random transaction identifier.

Originally-implemented-by: Amos Kong <akong@redhat.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/include/ipxe/dhcp.h b/src/include/ipxe/dhcp.h
index dbca8e633cc3fdfcfac3e88d0e9988d3e12d336b..06fb33bc0cfc8c7cd2eddda7087cca098a78ce36 100644 (file)
--- a/src/include/ipxe/dhcp.h
+++ b/src/include/ipxe/dhcp.h
@@ -660,15 +660,18 @@ struct dhcphdr {
 /** Setting block name used for BootServerDHCP responses */
 #define PXEBS_SETTINGS_NAME "pxebs"
 
+extern uint32_t dhcp_last_xid;
 extern unsigned int dhcp_chaddr ( struct net_device *netdev, void *chaddr,
                                  uint16_t *flags );
 extern int dhcp_create_packet ( struct dhcp_packet *dhcppkt,
                                struct net_device *netdev, uint8_t msgtype,
-                               const void *options, size_t options_len,
-                               void *data, size_t max_len );
+                               uint32_t xid, const void *options,
+                               size_t options_len, void *data,
+                               size_t max_len );
 extern int dhcp_create_request ( struct dhcp_packet *dhcppkt,
                                 struct net_device *netdev,
-                                unsigned int msgtype, struct in_addr ciaddr,
+                                unsigned int msgtype, uint32_t xid,
+                                struct in_addr ciaddr,
                                 void *data, size_t max_len );
 extern int start_dhcp ( struct interface *job, struct net_device *netdev );
 extern int start_pxebs ( struct interface *job, struct net_device *netdev,

diff --git a/src/net/fakedhcp.c b/src/net/fakedhcp.c
index b43e9b12486d0144227a5720a8a1787f75e2cdca..b182ab6f2fc65379317efbcc0cc7d60c7e171fd5 100644 (file)
--- a/src/net/fakedhcp.c
+++ b/src/net/fakedhcp.c
@@ -114,7 +114,8 @@ int create_fakedhcpdiscover ( struct net_device *netdev,
        int rc;
 
        if ( ( rc = dhcp_create_request ( &dhcppkt, netdev, DHCPDISCOVER,
-                                         ciaddr, data, max_len ) ) != 0 ) {
+                                         dhcp_last_xid, ciaddr, data,
+                                         max_len ) ) != 0 ) {
                DBG ( "Could not create DHCPDISCOVER: %s\n",
                      strerror ( rc ) );
                return rc;
@@ -139,7 +140,8 @@ int create_fakedhcpack ( struct net_device *netdev,
        int rc;
 
        /* Create base DHCPACK packet */
-       if ( ( rc = dhcp_create_packet ( &dhcppkt, netdev, DHCPACK, NULL, 0,
+       if ( ( rc = dhcp_create_packet ( &dhcppkt, netdev, DHCPACK,
+                                        dhcp_last_xid, NULL, 0,
                                         data, max_len ) ) != 0 ) {
                DBG ( "Could not create DHCPACK: %s\n", strerror ( rc ) );
                return rc;
@@ -190,7 +192,8 @@ int create_fakepxebsack ( struct net_device *netdev,
        }
 
        /* Create base DHCPACK packet */
-       if ( ( rc = dhcp_create_packet ( &dhcppkt, netdev, DHCPACK, NULL, 0,
+       if ( ( rc = dhcp_create_packet ( &dhcppkt, netdev, DHCPACK,
+                                        dhcp_last_xid, NULL, 0,
                                         data, max_len ) ) != 0 ) {
                DBG ( "Could not create PXE BS ACK: %s\n",
                      strerror ( rc ) );

diff --git a/src/net/udp/dhcp.c b/src/net/udp/dhcp.c
index a0b74dfe5647804cd495cd709ad767f12d1136b9..682191f0692085e42f202d30b5eb18a549266b12 100644 (file)
--- a/src/net/udp/dhcp.c
+++ b/src/net/udp/dhcp.c
@@ -116,6 +116,14 @@ struct setting use_cached_setting __setting ( SETTING_MISC ) = {
        .type = &setting_type_uint8,
 };
 
+/**
+ * Most recent DHCP transaction ID
+ *
+ * This is exposed for use by the fakedhcp code when reconstructing
+ * DHCP packets for PXE NBPs.
+ */
+uint32_t dhcp_last_xid;
+
 /**
  * Name a DHCP packet type
  *
@@ -137,23 +145,6 @@ static inline const char * dhcp_msgtype_name ( unsigned int msgtype ) {
        }
 }
 
-/**
- * Calculate DHCP transaction ID for a network device
- *
- * @v netdev           Network device
- * @ret xid            DHCP XID
- *
- * Extract the least significant bits of the hardware address for use
- * as the transaction ID.
- */
-static uint32_t dhcp_xid ( struct net_device *netdev ) {
-       uint32_t xid;
-
-       memcpy ( &xid, ( netdev->ll_addr + netdev->ll_protocol->ll_addr_len
-                        - sizeof ( xid ) ), sizeof ( xid ) );
-       return xid;
-}
-
 /****************************************************************************
  *
  * DHCP session
@@ -219,6 +210,8 @@ struct dhcp_session {
        struct sockaddr_in local;
        /** State of the session */
        struct dhcp_session_state *state;
+       /** Transaction ID (in network-endian order) */
+       uint32_t xid;
 
        /** Offered IP address */
        struct in_addr offer;
@@ -916,6 +909,7 @@ unsigned int dhcp_chaddr ( struct net_device *netdev, void *chaddr,
  * @v dhcppkt          DHCP packet structure to fill in
  * @v netdev           Network device
  * @v msgtype          DHCP message type
+ * @v xid              Transaction ID (in network-endian order)
  * @v options          Initial options to include (or NULL)
  * @v options_len      Length of initial options
  * @v data             Buffer for DHCP packet
@@ -927,7 +921,7 @@ unsigned int dhcp_chaddr ( struct net_device *netdev, void *chaddr,
  */
 int dhcp_create_packet ( struct dhcp_packet *dhcppkt,
                         struct net_device *netdev, uint8_t msgtype,
-                        const void *options, size_t options_len,
+                        uint32_t xid, const void *options, size_t options_len,
                         void *data, size_t max_len ) {
        struct dhcphdr *dhcphdr = data;
        int rc;
@@ -938,7 +932,7 @@ int dhcp_create_packet ( struct dhcp_packet *dhcppkt,
 
        /* Initialise DHCP packet content */
        memset ( dhcphdr, 0, max_len );
-       dhcphdr->xid = dhcp_xid ( netdev );
+       dhcphdr->xid = xid;
        dhcphdr->magic = htonl ( DHCP_MAGIC_COOKIE );
        dhcphdr->htype = ntohs ( netdev->ll_protocol->ll_proto );
        dhcphdr->op = dhcp_op[msgtype];
@@ -964,6 +958,7 @@ int dhcp_create_packet ( struct dhcp_packet *dhcppkt,
  * @v dhcppkt          DHCP packet structure to fill in
  * @v netdev           Network device
  * @v msgtype          DHCP message type
+ * @v xid              Transaction ID (in network-endian order)
  * @v ciaddr           Client IP address
  * @v data             Buffer for DHCP packet
  * @v max_len          Size of DHCP packet buffer
@@ -974,7 +969,8 @@ int dhcp_create_packet ( struct dhcp_packet *dhcppkt,
  */
 int dhcp_create_request ( struct dhcp_packet *dhcppkt,
                          struct net_device *netdev, unsigned int msgtype,
-                         struct in_addr ciaddr, void *data, size_t max_len ) {
+                         uint32_t xid, struct in_addr ciaddr,
+                         void *data, size_t max_len ) {
        struct dhcp_netdev_desc dhcp_desc;
        struct dhcp_client_id client_id;
        struct dhcp_client_uuid client_uuid;
@@ -985,7 +981,7 @@ int dhcp_create_request ( struct dhcp_packet *dhcppkt,
        int rc;
 
        /* Create DHCP packet */
-       if ( ( rc = dhcp_create_packet ( dhcppkt, netdev, msgtype,
+       if ( ( rc = dhcp_create_packet ( dhcppkt, netdev, msgtype, xid,
                                         dhcp_request_options_data,
                                         sizeof ( dhcp_request_options_data ),
                                         data, max_len ) ) != 0 ) {
@@ -1099,7 +1095,8 @@ static int dhcp_tx ( struct dhcp_session *dhcp ) {
 
        /* Create basic DHCP packet in temporary buffer */
        if ( ( rc = dhcp_create_request ( &dhcppkt, dhcp->netdev, msgtype,
-                                         dhcp->local.sin_addr, iobuf->data,
+                                         dhcp->xid, dhcp->local.sin_addr,
+                                         iobuf->data,
                                          iob_tailroom ( iobuf ) ) ) != 0 ) {
                DBGC ( dhcp, "DHCP %p could not construct DHCP request: %s\n",
                       dhcp, strerror ( rc ) );
@@ -1187,7 +1184,7 @@ static int dhcp_deliver ( struct dhcp_session *dhcp,
                        &server_id, sizeof ( server_id ) );
 
        /* Check for matching transaction ID */
-       if ( dhcphdr->xid != dhcp_xid ( dhcp->netdev ) ) {
+       if ( dhcphdr->xid != dhcp->xid ) {
                DBGC ( dhcp, "DHCP %p %s from %s:%d has bad transaction "
                       "ID\n", dhcp, dhcp_msgtype_name ( msgtype ),
                       inet_ntoa ( peer->sin_addr ),
@@ -1311,6 +1308,10 @@ int start_dhcp ( struct interface *job, struct net_device *netdev ) {
        dhcp->netdev = netdev_get ( netdev );
        dhcp->local.sin_family = AF_INET;
        dhcp->local.sin_port = htons ( BOOTPC_PORT );
+       dhcp->xid = random();
+
+       /* Store DHCP transaction ID for fakedhcp code */
+       dhcp_last_xid = dhcp->xid;
 
        /* Instantiate child objects and attach to our interfaces */
        if ( ( rc = xfer_open_socket ( &dhcp->xfer, SOCK_DGRAM, &dhcp_peer,