[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH] udhcpd: Serve BOOTP clients
From:       Denys Vlasenko <vda.linux () googlemail ! com>
Date:       2023-06-12 11:23:59
Message-ID: CAK1hOcOTiaTmk5MzgW=3yKcNK1HZ0ZtDWKHEkUdxP1+cw4_-Mg () mail ! gmail ! com
[Download RAW message or body]

On Mon, May 29, 2023 at 3:39 AM Adam Goldman <adamg@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


[prev in list] [next in list] [prev in thread] [next in thread]