[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>

2 files changed, 30 insertions, 26 deletions

diff --git a/src/net/fakedhcp.c b/src/net/fakedhcp.c
index b43e9b12..b182ab6f 100644
--- 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 a0b74dfe..682191f0 100644
--- a/src/net/udp/dhcp.c
+++ b/src/net/udp/dhcp.c
@@ -117,6 +117,14 @@ struct setting use_cached_setting __setting ( SETTING_MISC ) = {
 };
 
 /**
+ * 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
  *
  * @v msgtype		DHCP message 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,