On 07/09/14 15:29, Wissam Shoukair wrote:
Checking for valid XID is not enough, because if two PXE clients
sends the DHCP Discover with the same XID then both of them will get
both offers while one of them with the wrong client HW address.
The XID should not be duplicated across two nodes; that in itself would
be an error. Is this actually happening in practice?
Signed-off-by: Wissam Shoukair <[email protected]>
---
src/net/udp/dhcp.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/src/net/udp/dhcp.c b/src/net/udp/dhcp.c
index e6d3edd..d89728f 100644
--- a/src/net/udp/dhcp.c
+++ b/src/net/udp/dhcp.c
@@ -1157,10 +1157,12 @@ static int dhcp_deliver ( struct dhcp_session *dhcp,
size_t data_len;
struct dhcp_packet *dhcppkt;
struct dhcphdr *dhcphdr;
+ uint8_t local_mac[MAX_LL_ADDR_LEN];
Should probably be
uint8_t chaddr[ sizeof ( dhcphdr->chaddr ) ];
since the buffer is to hold a DHCP chaddr rather than a MAC address.
uint8_t msgtype = 0;
struct in_addr server_id = { 0 };
int rc = 0;
+
Extra blank line probably not needed.
/* Sanity checks */
if ( ! meta->src ) {
DBGC ( dhcp, "DHCP %p received packet without source port\n",
@@ -1193,6 +1195,24 @@ static int dhcp_deliver ( struct dhcp_session *dhcp,
dhcppkt_fetch ( dhcppkt, DHCP_SERVER_IDENTIFIER,
&server_id, sizeof ( server_id ) );
+ /* Validate L2 address */
+#define MAC_FMT "%02x:%02x:%02x:%02x:%02x:%02x"
+#define MAC_PTR(x) ((uint8_t*)(x))[0], ((uint8_t*)(x))[1],
((uint8_t*)(x))[2], \
+ ((uint8_t*)(x))[3], ((uint8_t*)(x))[4],
((uint8_t*)(x))[5]
The chaddr is not necessarily an Ethernet MAC. Probably best to define
a function dhcp_chaddr_ntoa() (modelled on eth_ntoa() and similar
functions) which can handle a variable-length chaddr.
+ dhcp_chaddr ( dhcp->netdev, local_mac, NULL );
+ DBGC ( dhcp, "DHCP %p %s from %s:%d is for MAC ["MAC_FMT"]."
+ "Local MAC is ["MAC_FMT"]. ", dhcp, dhcp_msgtype_name (
msgtype ),
+ inet_ntoa ( peer->sin_addr ), ntohs ( peer->sin_port ),
+ MAC_PTR( dhcppkt->dhcphdr->chaddr ), MAC_PTR( local_mac ) );
+ if ( memcmp ( dhcppkt->dhcphdr->chaddr, local_mac,
dhcppkt->dhcphdr->hlen ) ) {
+ DBGC ( dhcp, "Packet discarded.\n" );
+ rc = -EINVAL;
+ goto err_mac;
+ } else {
+ DBGC ( dhcp, "Handling packet.\n" );
+ }
Please match the style of other debugging messages in the same
file/function. For example, the pattern of the other debug messages in
dhcp_deliver() is to produce output only on discovering a reason to
reject the packet, and the general format is
DBGC ( dhcp, "DHCP %p %s ....", dhcp, dhcp_msgtype_name ( msgtype ),
... );
Thanks,
Michael
_______________________________________________
ipxe-devel mailing list
[email protected]
https://lists.ipxe.org/mailman/listinfo.cgi/ipxe-devel
