On Mon, May 29, 2023 at 3:39 AM Adam Goldman <ad...@pobox.com> wrote: > This patch makes udhcpd respond correctly to queries from BOOTP clients. > > It contains the following changes: > > The end field, or DHCP_END option, is required in DHCP requests but > optional in BOOTP requests. Only complain about missing end fields if > there are DHCP options in the packet. However, we still send an end > field in all replies, because some BOOTP clients expect one in replies > even if they didn't send one in the request. > > Requests without a DHCP_MESSAGE_TYPE are recognized as BOOTP requests > and handled appropriately, instead of being discarded. We still require > an RFC 1048 options field, but we allow it to be empty. > > Since a BOOTP client will keep using the assigned IP forever, we only > send a BOOTP reply if a static lease exists for that client. > > BOOTP replies shouldn't contain DHCP_* options, so we omit them if there > was no DHCP_MESSAGE_TYPE in the request. Options other than DHCP_* > options are still sent. > > The options field of a BOOTP reply must be exactly 64 bytes. If we > construct a reply with more than 64 bytes of options, we give up and log > an error instead of sending it. udhcp_send_raw_packet already pads the > options field to 64 bytes if it is too short. > > This implementation has been tested against an HP PA-RISC client. > > > --- networking/udhcp/common.c.orig 2023-05-22 18:41:39.000000000 -0700 > +++ networking/udhcp/common.c 2023-05-21 19:18:15.000000000 -0700 > @@ -234,6 +234,7 @@ > void FAST_FUNC init_scan_state(struct dhcp_packet *packet, struct > dhcp_scan_state *scan_state) > { > scan_state->overload = 0; > + scan_state->is_dhcp = 0; > scan_state->rem = sizeof(packet->options); > scan_state->optionptr = packet->options; > } > @@ -251,12 +252,17 @@ > > /* option bytes: [code][len][data1][data2]..[dataLEN] */ > while (1) { > + if (scan_state->rem == 0 && !scan_state->is_dhcp) > + break; /* BOOTP packet without end field */ > if (scan_state->rem <= 0) { > complain: > bb_simple_error_msg("bad packet, malformed option > field"); > return NULL; > } > > + if (scan_state->optionptr[OPT_CODE] >= DHCP_REQUESTED_IP && > scan_state->optionptr[OPT_CODE] <= DHCP_CLIENT_ID) > + scan_state->is_dhcp = 1; > +
scan_state->is_dhcp is not particularly useful. I'm dropping it. > +++ networking/udhcp/packet.c 2023-05-23 00:22:45.000000000 -0700 > @@ -18,6 +18,7 @@ > memset(packet, 0, sizeof(*packet)); > packet->op = BOOTREQUEST; /* if client to a server */ > switch (type) { > + case 0: A symbolic name is better here. > - init_packet(&packet, oldpacket, DHCPOFFER); > + if (!udhcp_get_option(oldpacket, DHCP_MESSAGE_TYPE)) > + message_type = 0; > + init_packet(&packet, oldpacket, message_type); > > /* If it is a static lease, use its IP */ > packet.yiaddr = static_lease_nip; > @@ -785,8 +789,13 @@ > } > > lease_time_sec = select_lease_time(oldpacket); > - udhcp_add_simple_option(&packet, DHCP_LEASE_TIME, > htonl(lease_time_sec)); > + if (udhcp_get_option(oldpacket, DHCP_MESSAGE_TYPE)) > + udhcp_add_simple_option(&packet, DHCP_LEASE_TIME, > htonl(lease_time_sec)); > add_server_options(&packet); > + if (!udhcp_get_option(oldpacket, DHCP_MESSAGE_TYPE) && > udhcp_end_option(packet.options) > 63) { > + bb_simple_error_msg("BOOTP BOOTREPLY would be too large, not > sending"); > + return; > + } Let's just pass the known result of udhcp_get_option(DHCP_MESSAGE_TYPE) down to here, rather than calling it three times. Applied with some changes. Thank you. _______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox