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

diff --git a/sbin/dhcpleased/engine.c b/sbin/dhcpleased/engine.c
index 6d371a5..84d02cc 100644 (file)
--- 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 <florian@openbsd.org>
@@ -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;