Re: vmd(8): Improve RFC 2132 compliance (DHCP)

[Reyk Floeter]

Hi,

thank you for the patch and the detailed explanation.

I knew that Android is having similar problems under vmd, maybe that's
also because of busybox' udhcpc.

I have to clarify that vmd does not implement "DHCP" but "BOOTP".
I picked BOOTP because it was simpler to implement and totally sufficient
for vmd's use case: we don't need lease times, stateful configuration, or
any of the fancy DHCP options. Less code and complexity.

My assumption was that DHCP is a superset of BOOTP; most DHCP
clients support BOOTP responses (with vendor extensions). udhcpc is
the first DHCP-only client that I've seen.

But now I stumbled over RFC 1534 where it says:

"3. DHCP clients and BOOTP servers

   A DHCP client MAY use a reply from a BOOTP server if the
   configuration returned from the BOOTP server is acceptable to the
   DHCP client.  A DHCP client MUST assume that an IP address returned
   in a message from a BOOTP server has an infinite lease.  A DHCP
   client SHOULD choose to use a reply from a DHCP server in preference
   to a reply from a BOOTP server."

So udhcpc is stupid but it is actually not wrong.

So I'm wondering if your diff is the right approach: should we add the
minimal DHCP-in-BOOTP fields as a workaround for udhcpc or should
we rather change it to be some kind of minimal RFC-compliant DHCP?

Reyk

> On 08.09.2017, at 06:42, Anthony Coulter  wrote:
> 
> The DHCP client available in the Alpine Linux installer (udhcpc, part
> of BusyBox) does not accept responses that do not include the DHCP
> message type option. Worse, it expects the message type to be
> DHCPOFFER in some circumstances and DHCPREQUEST in others. The DHCP
> server in vmd omits this option entirely, which makes it impossible
> to install Alpine Linux in a virtual machine configured with "-L".
> 
> The simplest fix would be to use "resp.options[6] == DHCPOFFER" instead
> of is_discover (see the patch below) because in practice the DHCP
> message type will be the first option present after the magic cookie.
> This was the first thing I tried, and it worked. But it's incorrect.
> 
> RFC 1534 says that requests with no message type can be treated as
> BOOTP and not DHCP messages. It also says that we can send DHCP options
> to BOOTP messages if we so desire, so it doesn't really matter whether
> we initialize is_discover to zero or one.
> 
> Note that udhcpc also complains about two more options (server ID and
> lease time) that are missing from the response message. I didn't do
> anything about this because udhcpc uses sensible defaults.
> 
> I've tested this change with udhcpc (in a virtual Alpine Linux system)
> and dhclient (in a virtual OpenBSD system) and it works for both. I
> have not tried anything else.
> 
> Regards,
> Anthony Coulter
> 
> Index: dhcp.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/dhcp.c,v
> retrieving revision 1.3
> diff -u -p -u -p -r1.3 dhcp.c
> --- dhcp.c24 Apr 2017 07:14:27 -  1.3
> +++ dhcp.c8 Sep 2017 04:12:10 -
> @@ -44,6 +44,7 @@ dhcp_request(struct vionet_dev *dev, cha
>   struct dhcp_packet   req, resp;
>   struct in_addr   in, mask;
>   size_t   resplen, o;
> + int  is_discover = 1;
> 
>   if (buflen < (ssize_t)(BOOTP_MIN_LEN + sizeof(struct ether_header)))
>   return (-1);
> @@ -76,6 +77,15 @@ dhcp_request(struct vionet_dev *dev, cha
>   if (req.ciaddr.s_addr != 0 || req.file[0] != '\0' || req.hops != 0)
>   return (-1);
> 
> + for (o = DHCP_OPTIONS_COOKIE_LEN;
> + o + offsetof(struct dhcp_packet, options) < buflen &&
> + req.options[o] != DHO_END;
> + o += req.options[o+1] + 2)
> + if (req.options[o] == DHO_DHCP_MESSAGE_TYPE) {
> + is_discover = (req.options[o+2] == DHCPDISCOVER);
> + break;
> + }
> +
>   memset(, 0, sizeof(resp));
>   resp.op = BOOTREPLY;
>   resp.htype = req.htype;
> @@ -123,6 +133,10 @@ dhcp_request(struct vionet_dev *dev, cha
>   memcpy(,
>   DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN);
>   o+= DHCP_OPTIONS_COOKIE_LEN;
> +
> + resp.options[o++] = DHO_DHCP_MESSAGE_TYPE;
> + resp.options[o++] = 1;
> + resp.options[o++] = is_discover ? DHCPOFFER : DHCPACK;
> 
>   resp.options[o++] = DHO_SUBNET_MASK;
>   resp.options[o++] = sizeof(mask);
> 

vmd(8): Improve RFC 2132 compliance (DHCP)

[Anthony Coulter]

The DHCP client available in the Alpine Linux installer (udhcpc, part
of BusyBox) does not accept responses that do not include the DHCP
message type option. Worse, it expects the message type to be
DHCPOFFER in some circumstances and DHCPREQUEST in others. The DHCP
server in vmd omits this option entirely, which makes it impossible
to install Alpine Linux in a virtual machine configured with "-L".

The simplest fix would be to use "resp.options[6] == DHCPOFFER" instead
of is_discover (see the patch below) because in practice the DHCP
message type will be the first option present after the magic cookie.
This was the first thing I tried, and it worked. But it's incorrect.

RFC 1534 says that requests with no message type can be treated as
BOOTP and not DHCP messages. It also says that we can send DHCP options
to BOOTP messages if we so desire, so it doesn't really matter whether
we initialize is_discover to zero or one.

Note that udhcpc also complains about two more options (server ID and
lease time) that are missing from the response message. I didn't do
anything about this because udhcpc uses sensible defaults.

I've tested this change with udhcpc (in a virtual Alpine Linux system)
and dhclient (in a virtual OpenBSD system) and it works for both. I
have not tried anything else.

Regards,
Anthony Coulter

Index: dhcp.c
===
RCS file: /cvs/src/usr.sbin/vmd/dhcp.c,v
retrieving revision 1.3
diff -u -p -u -p -r1.3 dhcp.c
--- dhcp.c  24 Apr 2017 07:14:27 -  1.3
+++ dhcp.c  8 Sep 2017 04:12:10 -
@@ -44,6 +44,7 @@ dhcp_request(struct vionet_dev *dev, cha
struct dhcp_packet   req, resp;
struct in_addr   in, mask;
size_t   resplen, o;
+   int  is_discover = 1;
 
if (buflen < (ssize_t)(BOOTP_MIN_LEN + sizeof(struct ether_header)))
return (-1);
@@ -76,6 +77,15 @@ dhcp_request(struct vionet_dev *dev, cha
if (req.ciaddr.s_addr != 0 || req.file[0] != '\0' || req.hops != 0)
return (-1);
 
+   for (o = DHCP_OPTIONS_COOKIE_LEN;
+   o + offsetof(struct dhcp_packet, options) < buflen &&
+   req.options[o] != DHO_END;
+   o += req.options[o+1] + 2)
+   if (req.options[o] == DHO_DHCP_MESSAGE_TYPE) {
+   is_discover = (req.options[o+2] == DHCPDISCOVER);
+   break;
+   }
+
memset(, 0, sizeof(resp));
resp.op = BOOTREPLY;
resp.htype = req.htype;
@@ -123,6 +133,10 @@ dhcp_request(struct vionet_dev *dev, cha
memcpy(,
DHCP_OPTIONS_COOKIE, DHCP_OPTIONS_COOKIE_LEN);
o+= DHCP_OPTIONS_COOKIE_LEN;
+
+   resp.options[o++] = DHO_DHCP_MESSAGE_TYPE;
+   resp.options[o++] = 1;
+   resp.options[o++] = is_discover ? DHCPOFFER : DHCPACK;
 
resp.options[o++] = DHO_SUBNET_MASK;
resp.options[o++] = sizeof(mask);