udhcpd: send DHCPOFFERs as unicast (unless clients specifically asks for bcast)

RFC 2131 says we should do that.
Evidently, since for so many years no one complained, sending them broadcast
works too, but finally we've got someone who wants RFC-compliand behavior.

function                                             old     new   delta
send_packet                                          141     179     +38
.rodata                                           105680  105681      +1
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 2/0 up/down: 39/0)               Total: 39 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

1 files changed, 42 insertions, 16 deletions

diff --git a/networking/udhcp/dhcpd.c b/networking/udhcp/dhcpd.c
index 2904119e5..b9cbd6464 100644
--- a/networking/udhcp/dhcpd.c
+++ b/networking/udhcp/dhcpd.c
@@ -575,29 +575,51 @@ static void send_packet_to_client(struct dhcp_packet *dhcp_pkt, int force_broadc
         const uint8_t *chaddr;
         uint32_t ciaddr;
 
-        // Was:
+        // Logic:
         //if (force_broadcast) { /* broadcast */ }
         //else if (dhcp_pkt->ciaddr) { /* unicast to dhcp_pkt->ciaddr */ }
+        // ^^^ dhcp_pkt->ciaddr comes from client's request packet.
+        // We expect such clients to have an UDP socket listening on that IP.
         //else if (dhcp_pkt->flags & htons(BROADCAST_FLAG)) { /* broadcast */ }
         //else { /* unicast to dhcp_pkt->yiaddr */ }
-        // But this is wrong: yiaddr is _our_ idea what client's IP is
-        // (for example, from lease file). Client may not know that,
-        // and may not have UDP socket listening on that IP!
-        // We should never unicast to dhcp_pkt->yiaddr!
-        // dhcp_pkt->ciaddr, OTOH, comes from client's request packet,
-        // and can be used.
-
-        if (force_broadcast
-         || (dhcp_pkt->flags & htons(BROADCAST_FLAG))
-         || dhcp_pkt->ciaddr == 0
+        // ^^^ The last case is confusing, but *should* work.
+        // It's a case where client have sent a DISCOVER
+        // and does not have a kernel UDP socket listening on the IP
+        // we are offering in yiaddr (it does not know the IP yet)!
+        // This *should* work because client *should* listen on a raw socket
+        // instead at this time (IOW: it should examine ALL IPv4 packets
+        // "by hand", not relying on kernel's UDP stack.)
+
+        chaddr = dhcp_pkt->chaddr;
+
+        if (dhcp_pkt->ciaddr == 0
+         || force_broadcast /* sending DHCPNAK pkt? */
         ) {
-                log1s("broadcasting packet to client");
-                ciaddr = INADDR_BROADCAST;
-                chaddr = MAC_BCAST_ADDR;
+                if (dhcp_pkt->flags & htons(BROADCAST_FLAG)
+                 || force_broadcast /* sending DHCPNAK pkt? */
+                ) {
+// RFC 2131:
+// If 'giaddr' is zero and 'ciaddr' is zero, and the broadcast bit is
+// set, then the server broadcasts DHCPOFFER and DHCPACK messages to
+// 0xffffffff. ...
+// In all cases, when 'giaddr' is zero, the server broadcasts any DHCPNAK
+// messages to 0xffffffff.
+                        ciaddr = INADDR_BROADCAST;
+                        chaddr = MAC_BCAST_ADDR;
+                        log1s("broadcasting packet to client");
+                } else {
+// If the broadcast bit is not set and 'giaddr' is zero and
+// 'ciaddr' is zero, then the server unicasts DHCPOFFER and DHCPACK
+// messages to the client's hardware address and 'yiaddr' address.
+                        ciaddr = dhcp_pkt->yiaddr;
+                        log1("unicasting packet to client %ciaddr", 'y');
+                }
         } else {
-                log1s("unicasting packet to client ciaddr");
+// If the 'giaddr'
+// field is zero and the 'ciaddr' field is nonzero, then the server
+// unicasts DHCPOFFER and DHCPACK messages to the address in 'ciaddr'.
                 ciaddr = dhcp_pkt->ciaddr;
-                chaddr = dhcp_pkt->chaddr;
+                log1("unicasting packet to client %ciaddr", 'c');
         }
 
         udhcp_send_raw_packet(dhcp_pkt,
@@ -624,6 +646,10 @@ static void send_packet_to_relay(struct dhcp_packet *dhcp_pkt)
 static void send_packet(struct dhcp_packet *dhcp_pkt, int force_broadcast)
 {
         if (dhcp_pkt->gateway_nip)
+// RFC 2131:
+// If the 'giaddr' field in a DHCP message from a client is non-zero,
+// the server sends any return messages to the 'DHCP server' port on the
+// BOOTP relay agent whose address appears in 'giaddr'.
                 send_packet_to_relay(dhcp_pkt);
         else
                 send_packet_to_client(dhcp_pkt, force_broadcast);