[PATCH] udhcpd: Serve BOOTP clients

2023-05-28 Thread Adam Goldman
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.0 -0700
+++ networking/udhcp/common.c   2023-05-21 19:18:15.0 -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;
+
/* DHCP_PADDING and DHCP_END have no [len] byte */
if (scan_state->optionptr[OPT_CODE] == DHCP_PADDING) {
scan_state->rem--;
--- networking/udhcp/common.h.orig  2023-01-03 06:17:01.0 -0800
+++ networking/udhcp/common.h   2023-05-21 19:01:24.0 -0700
@@ -123,6 +123,7 @@
 
 struct dhcp_scan_state {
int overload;
+   int is_dhcp;
int rem;
uint8_t *optionptr;
 };
--- networking/udhcp/packet.c.orig  2023-01-03 06:17:01.0 -0800
+++ networking/udhcp/packet.c   2023-05-23 00:22:45.0 -0700
@@ -18,6 +18,7 @@
memset(packet, 0, sizeof(*packet));
packet->op = BOOTREQUEST; /* if client to a server */
switch (type) {
+   case 0:
case DHCPOFFER:
case DHCPACK:
case DHCPNAK:
@@ -28,7 +29,8 @@
packet->cookie = htonl(DHCP_MAGIC);
if (DHCP_END != 0)
packet->options[0] = DHCP_END;
-   udhcp_add_simple_option(packet, DHCP_MESSAGE_TYPE, type);
+   if (type)
+   udhcp_add_simple_option(packet, DHCP_MESSAGE_TYPE, type);
 }
 #endif
 
--- networking/udhcp/dhcpd.c.orig   2023-01-03 06:17:01.0 -0800
+++ networking/udhcp/dhcpd.c2023-05-28 17:08:51.0 -0700
@@ -649,7 +649,8 @@
packet->flags = oldpacket->flags;
packet->gateway_nip = oldpacket->gateway_nip;
packet->ciaddr = oldpacket->ciaddr;
-   udhcp_add_simple_option(packet, DHCP_SERVER_ID, server_data.server_nip);
+   if(type)
+   udhcp_add_simple_option(packet, DHCP_SERVER_ID, 
server_data.server_nip);
 }
 
 /* Fill options field, siaddr_nip, and sname and boot_file fields.
@@ -733,8 +734,11 @@
 {
struct dhcp_packet packet;
uint32_t lease_time_sec;
+   char message_type = DHCPOFFER;
 
-   init_packet(, oldpacket, DHCPOFFER);
+   if (!udhcp_get_option(oldpacket, DHCP_MESSAGE_TYPE))
+   message_type = 0;
+   init_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(, DHCP_LEASE_TIME, 
htonl(lease_time_sec));
+   if (udhcp_get_option(oldpacket, DHCP_MESSAGE_TYPE))
+   udhcp_add_simple_option(, DHCP_LEASE_TIME, 
htonl(lease_time_sec));
add_server_options();
+   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");
+   

Re: [PATCH] libbb: in xmalloc_fgetline, use getline if enabled

2023-05-28 Thread tito
On Sun, 28 May 2023 19:01:52 +0200
Elvira Khabirova  wrote:

> When xmalloc_fgetline was introduced, getline(3) was a GNU extension
> and was not necessarily implemented in libcs. Since then,
> it's become a part of POSIX.1-2008 and is now implemented in
> glibc, uClibc-ng, and musl.
> 
> Previously, xmalloc_fgetline directly called bb_get_chunk_from_file.
> The issue with that approach is that bb_get_chunk_from_file stops
> both at \n and at \0. This introduces unexpected behavior when tools
> are presented with inputs containing \0. For example, in a comparison
> of GNU core utils cut with busybox cut:
> 
> % echo -ne '\x00\x01\x00\x03\x04\x05\x06' | cut -b1-3 | hexdump -C
>   00 01 00 0a   ||
> 0004
> % echo -ne '\x00\x01\x00\x03\x04\x05\x06' | ./busybox cut -b1-3 | hexdump -C
>   0a 01 0a 03 04 05 0a  |...|
> 0007
> 
> Here, due to bb_get_chunk_from_file splitting lines by \n and \0,
> busybox cut interprets the input as three lines instead of one.
> 
> Also, since xmalloc_fgetline never returned strings with embedded \0,
> cut_file expects strlen of the returned string to match the string's
> total length.
> 
> To fix the behavior of the cut utility, introduce xmalloc_fgetline_n,
> that fully matches the behavior of xmalloc_fgetline,
> but also returns the amount of bytes read.
> 
> Add a configuration option, FEATURE_USE_GETLINE, and enable it
> by default. Use getline in xmalloc_fgetline_n if the feature is enabled.
> 
> Change the behavior of cut_file to use the values returned
> by the new function instead of calling strlen.
> 
> Call xmalloc_fgetline_n from xmalloc_fgetline.
> 
> Add a test for the previously mentioned case.
> 
> With FEATURE_USE_GETLINE:
> 
> function old new   delta
> xmalloc_fgetline_n - 173+173
> xmalloc_fgetline  85  58 -27
> cut_main14061367 -39
> --
> (add/remove: 1/0 grow/shrink: 0/2 up/down: 173/-66)   Total: 107 bytes
> 
> Without FEATURE_USE_GETLINE:
> 
> function old new   delta
> xmalloc_fgetline_n -  41 +41
> xmalloc_fgetline  85  58 -27
> cut_main14061367 -39
> --
> (add/remove: 1/0 grow/shrink: 0/2 up/down: 41/-66)Total: -25 bytes
> 
> Fixes: https://bugs.busybox.net/show_bug.cgi?id=15276
> Signed-off-by: Elvira Khabirova 
> ---
>  coreutils/cut.c|  4 ++--
>  include/libbb.h|  2 ++
>  libbb/Config.src   | 11 +++
>  libbb/get_line_from_file.c | 37 -
>  testsuite/cut.tests|  2 ++
>  5 files changed, 49 insertions(+), 7 deletions(-)
> 
> diff --git a/coreutils/cut.c b/coreutils/cut.c
> index 55bdd9386..7c87467ca 100644
> --- a/coreutils/cut.c
> +++ b/coreutils/cut.c
> @@ -89,6 +89,7 @@ static void cut_file(FILE *file, const char *delim, const 
> char *odelim,
>   const struct cut_list *cut_lists, unsigned nlists)
>  {
>   char *line;
> + size_t linelen = 0;
>   unsigned linenum = 0;   /* keep these zero-based to be consistent */
>   regex_t reg;
>   int spos, shoe = option_mask32 & CUT_OPT_REGEX_FLGS;
> @@ -96,10 +97,9 @@ static void cut_file(FILE *file, const char *delim, const 
> char *odelim,
>   if (shoe) xregcomp(, delim, REG_EXTENDED);
>  
>   /* go through every line in the file */
> - while ((line = xmalloc_fgetline(file)) != NULL) {
> + while ((line = xmalloc_fgetline_n(file, )) != NULL) {
>  
>   /* set up a list so we can keep track of what's been printed */
> - int linelen = strlen(line);
>   char *printed = xzalloc(linelen + 1);
>   char *orig_line = line;
>   unsigned cl_pos = 0;
> diff --git a/include/libbb.h b/include/libbb.h
> index 6191debb1..73d16647a 100644
> --- a/include/libbb.h
> +++ b/include/libbb.h
> @@ -1048,6 +1048,8 @@ extern char *xmalloc_fgetline_str(FILE *file, const 
> char *terminating_string) FA
>  extern char *xmalloc_fgets(FILE *file) FAST_FUNC RETURNS_MALLOC;
>  /* Chops off '\n' from the end, unlike fgets: */
>  extern char *xmalloc_fgetline(FILE *file) FAST_FUNC RETURNS_MALLOC;
> +/* Same as xmalloc_fgetline but returns number of bytes read: */
> +extern char* xmalloc_fgetline_n(FILE *file, size_t *n) FAST_FUNC 
> RETURNS_MALLOC;
>  /* Same, but doesn't try to conserve space (may have some slack after the 
> end) */
>  /* extern char *xmalloc_fgetline_fast(FILE *file) FAST_FUNC RETURNS_MALLOC; 
> */
>  
> 

[PATCH] libbb: in xmalloc_fgetline, use getline if enabled

2023-05-28 Thread Elvira Khabirova
When xmalloc_fgetline was introduced, getline(3) was a GNU extension
and was not necessarily implemented in libcs. Since then,
it's become a part of POSIX.1-2008 and is now implemented in
glibc, uClibc-ng, and musl.

Previously, xmalloc_fgetline directly called bb_get_chunk_from_file.
The issue with that approach is that bb_get_chunk_from_file stops
both at \n and at \0. This introduces unexpected behavior when tools
are presented with inputs containing \0. For example, in a comparison
of GNU core utils cut with busybox cut:

% echo -ne '\x00\x01\x00\x03\x04\x05\x06' | cut -b1-3 | hexdump -C
  00 01 00 0a   ||
0004
% echo -ne '\x00\x01\x00\x03\x04\x05\x06' | ./busybox cut -b1-3 | hexdump -C
  0a 01 0a 03 04 05 0a  |...|
0007

Here, due to bb_get_chunk_from_file splitting lines by \n and \0,
busybox cut interprets the input as three lines instead of one.

Also, since xmalloc_fgetline never returned strings with embedded \0,
cut_file expects strlen of the returned string to match the string's
total length.

To fix the behavior of the cut utility, introduce xmalloc_fgetline_n,
that fully matches the behavior of xmalloc_fgetline,
but also returns the amount of bytes read.

Add a configuration option, FEATURE_USE_GETLINE, and enable it
by default. Use getline in xmalloc_fgetline_n if the feature is enabled.

Change the behavior of cut_file to use the values returned
by the new function instead of calling strlen.

Call xmalloc_fgetline_n from xmalloc_fgetline.

Add a test for the previously mentioned case.

With FEATURE_USE_GETLINE:

function old new   delta
xmalloc_fgetline_n - 173+173
xmalloc_fgetline  85  58 -27
cut_main14061367 -39
--
(add/remove: 1/0 grow/shrink: 0/2 up/down: 173/-66)   Total: 107 bytes

Without FEATURE_USE_GETLINE:

function old new   delta
xmalloc_fgetline_n -  41 +41
xmalloc_fgetline  85  58 -27
cut_main14061367 -39
--
(add/remove: 1/0 grow/shrink: 0/2 up/down: 41/-66)Total: -25 bytes

Fixes: https://bugs.busybox.net/show_bug.cgi?id=15276
Signed-off-by: Elvira Khabirova 
---
 coreutils/cut.c|  4 ++--
 include/libbb.h|  2 ++
 libbb/Config.src   | 11 +++
 libbb/get_line_from_file.c | 37 -
 testsuite/cut.tests|  2 ++
 5 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/coreutils/cut.c b/coreutils/cut.c
index 55bdd9386..7c87467ca 100644
--- a/coreutils/cut.c
+++ b/coreutils/cut.c
@@ -89,6 +89,7 @@ static void cut_file(FILE *file, const char *delim, const 
char *odelim,
const struct cut_list *cut_lists, unsigned nlists)
 {
char *line;
+   size_t linelen = 0;
unsigned linenum = 0;   /* keep these zero-based to be consistent */
regex_t reg;
int spos, shoe = option_mask32 & CUT_OPT_REGEX_FLGS;
@@ -96,10 +97,9 @@ static void cut_file(FILE *file, const char *delim, const 
char *odelim,
if (shoe) xregcomp(, delim, REG_EXTENDED);
 
/* go through every line in the file */
-   while ((line = xmalloc_fgetline(file)) != NULL) {
+   while ((line = xmalloc_fgetline_n(file, )) != NULL) {
 
/* set up a list so we can keep track of what's been printed */
-   int linelen = strlen(line);
char *printed = xzalloc(linelen + 1);
char *orig_line = line;
unsigned cl_pos = 0;
diff --git a/include/libbb.h b/include/libbb.h
index 6191debb1..73d16647a 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -1048,6 +1048,8 @@ extern char *xmalloc_fgetline_str(FILE *file, const char 
*terminating_string) FA
 extern char *xmalloc_fgets(FILE *file) FAST_FUNC RETURNS_MALLOC;
 /* Chops off '\n' from the end, unlike fgets: */
 extern char *xmalloc_fgetline(FILE *file) FAST_FUNC RETURNS_MALLOC;
+/* Same as xmalloc_fgetline but returns number of bytes read: */
+extern char* xmalloc_fgetline_n(FILE *file, size_t *n) FAST_FUNC 
RETURNS_MALLOC;
 /* Same, but doesn't try to conserve space (may have some slack after the end) 
*/
 /* extern char *xmalloc_fgetline_fast(FILE *file) FAST_FUNC RETURNS_MALLOC; */
 
diff --git a/libbb/Config.src b/libbb/Config.src
index b980f19a9..7a37929d6 100644
--- a/libbb/Config.src
+++ b/libbb/Config.src
@@ -147,6 +147,17 @@ config MONOTONIC_SYSCALL
will be used instead (which gives wrong results if date/time
is