From ee9c6200f9dcdfe6c777f48c0a27d9aa918683e0 Mon Sep 17 00:00:00 2001 From: florian Date: Tue, 13 Feb 2024 12:53:05 +0000 Subject: [PATCH] Only generate a new xid at the start of getting a new lease. "RFC 2131 4.1 Constructing and sending DHCP messages" has this: | 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. We used to change xid for each request / response cycle but this ran into problems with slow dhcp servers where we would change the xid too frequently and would ignore late coming replies from the server. Andre S points out that table 5 in "4.4.1 Initialization and allocation of network address" says for the xid field in "DHCPREQUEST" messages: | 'xid' from server DHCPOFFER message This seems to suggest that we need to use the same xid for the whole DHCPDISCOVER / DHCPOFFER / DHCPREQUEST / DHCPACK exchange of messages. Nothing else in the RFC is saying this though. But since there are DHCP servers out there that depend on this, we only generate a new xid when entering the INIT, REBOOTING and RENEWING state. I do wonder if we should just go with a static value of 0x04, which was chosen by a fair dice roll, so guaranteed to be random. Issue reported, initial diff and fix tested by Andre S deraadt likes this version OK tb --- sbin/dhcpleased/engine.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sbin/dhcpleased/engine.c b/sbin/dhcpleased/engine.c index 6d371a5a112..84d02cc12ee 100644 --- a/sbin/dhcpleased/engine.c +++ b/sbin/dhcpleased/engine.c @@ -1,4 +1,4 @@ -/* $OpenBSD: engine.c,v 1.42 2024/01/26 21:14:08 jan Exp $ */ +/* $OpenBSD: engine.c,v 1.43 2024/02/13 12:53:05 florian Exp $ */ /* * Copyright (c) 2017, 2021 Florian Obser @@ -1385,8 +1385,6 @@ state_transition(struct dhcpleased_iface *iface, enum if_state new_state) char ifnamebuf[IF_NAMESIZE], *if_name; iface->state = new_state; - if (new_state != old_state) - iface->xid = arc4random(); switch (new_state) { case IF_DOWN: @@ -1426,6 +1424,7 @@ state_transition(struct dhcpleased_iface *iface, enum if_state new_state) case IF_DOWN: case IF_IPV6_ONLY: iface->timo.tv_sec = START_EXP_BACKOFF; + iface->xid = arc4random(); break; case IF_BOUND: fatal("invalid transition Bound -> Init"); @@ -1436,8 +1435,10 @@ state_transition(struct dhcpleased_iface *iface, enum if_state new_state) case IF_REBOOTING: if (old_state == IF_REBOOTING) iface->timo.tv_sec *= 2; - else + else { iface->timo.tv_sec = START_EXP_BACKOFF; + iface->xid = arc4random(); + } request_dhcp_request(iface); break; case IF_REQUESTING: @@ -1458,6 +1459,7 @@ state_transition(struct dhcpleased_iface *iface, enum if_state new_state) if (old_state == IF_BOUND) { iface->timo.tv_sec = (iface->rebinding_time - iface->renewal_time) / 2; /* RFC 2131 4.4.5 */ + iface->xid = arc4random(); } else iface->timo.tv_sec /= 2; -- 2.20.1