On Wed, Nov 24, 2021 at 4:46 AM Florian Obser <flor...@openbsd.org> wrote:
> Thanks, I had indeed missed this. I went through the RFC and found that
> we MUST NOT send the server identifier in rebooting state. While here I
> also made it explicit that we are not sending server identifier in
> rebinding state. This was already the case since we only transition from
> renewing to rebinding, but this is clearer.
The behavior for this option looks consistent with prior behavior.
However, the RFC confuses me wrt this option and when it should/should
not be set so I'm only relying on the fact that broadcast and unicast
requests are working fine in testing.
> @@ -1486,9 +1488,17 @@ request_dhcp_request(struct dhcpleased_iface *iface)
> iface->xid = arc4random();
> imsg_req_request.if_index = iface->if_index;
> imsg_req_request.xid = iface->xid;
> + if (iface->state == IF_RENEWING || iface->state == IF_REBINDING)
> + imsg_req_request.ciaddr.s_addr = iface->requested_ip.s_addr;
> + else
> + imsg_req_request.ciaddr.s_addr = 0;
> imsg_req_request.server_identifier.s_addr =
> iface->server_identifier.s_addr;
> - imsg_req_request.requested_ip.s_addr = iface->requested_ip.s_addr;
> + if (iface->state == IF_RENEWING || iface->state == IF_REBINDING)
> + imsg_req_request.requested_ip.s_addr = 0;
> + else
> + imsg_req_request.requested_ip.s_addr =
> + iface->requested_ip.s_addr;
These two if/else statements could be merged.
> ssize_t
> build_packet(uint8_t message_type, char *if_name, uint32_t xid,
> - struct ether_addr *hw_address, struct in_addr *requested_ip,
> - struct in_addr *server_identifier)
> + struct ether_addr *hw_address, struct in_addr *ciaddr, struct in_addr
> + *requested_ip, struct in_addr *server_identifier)
> {
> static uint8_t dhcp_cookie[] = DHCP_COOKIE;
> static uint8_t dhcp_message_type[] = {DHO_DHCP_MESSAGE_TYPE, 1,
> @@ -926,6 +929,7 @@ build_packet(uint8_t message_type, char *if_name,
> uint32_t xid,
> hdr->hops = 0;
> hdr->xid = xid;
> hdr->secs = 0;
> + hdr->ciaddr.s_addr = ciaddr->s_addr;
> memcpy(hdr->chaddr, hw_address, sizeof(*hw_address));
> p += sizeof(struct dhcp_hdr);
> memcpy(p, dhcp_cookie, sizeof(dhcp_cookie));
The problem with build_packet() now is that it will unconditionally
insert opt 50 (requested IP address) even if ciaddr is being set. The
request packet ends up with ciaddr as the correct client IP and opt 50
as "0.0.0.0".
.joel
