From 12767d2202e620e32aef3fbdd2c4ad30c4e5ac22 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Mon, 19 Sep 2011 17:30:39 +0100 Subject: [PATCH] [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 Signed-off-by: Michael Brown --- src/include/ipxe/dhcp.h | 9 +++++--- src/net/fakedhcp.c | 9 +++++--- src/net/udp/dhcp.c | 47 +++++++++++++++++++++-------------------- 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/src/include/ipxe/dhcp.h b/src/include/ipxe/dhcp.h index dbca8e633..06fb33bc0 100644 --- 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 b43e9b124..b182ab6f2 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 a0b74dfe5..682191f06 100644 --- 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,