udhcpd: don't fail ARP check if returned MAC matches client's one

Also, do not unicast replies to yiaddr.

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

1 files changed, 31 insertions, 20 deletions

diff --git a/networking/udhcp/serverpacket.c b/networking/udhcp/serverpacket.c
index 8b0f1856b..157d157ba 100644
--- a/networking/udhcp/serverpacket.c
+++ b/networking/udhcp/serverpacket.c
@@ -31,7 +31,8 @@ static int send_packet_to_relay(struct dhcpMessage *payload)
 {
         DEBUG("Forwarding packet to relay");
 
-        return udhcp_send_kernel_packet(payload, server_config.server, SERVER_PORT,
+        return udhcp_send_kernel_packet(payload,
+                        server_config.server, SERVER_PORT,
                         payload->giaddr, SERVER_PORT);
 }
 
@@ -42,23 +43,31 @@ static int send_packet_to_client(struct dhcpMessage *payload, int force_broadcas
         const uint8_t *chaddr;
         uint32_t ciaddr;
 
-        if (force_broadcast) {
-                DEBUG("broadcasting packet to client (NAK)");
+        // Was:
+        //if (force_broadcast) { /* broadcast */ }
+        //else if (payload->ciaddr) { /* unicast to payload->ciaddr */ }
+        //else if (payload->flags & htons(BROADCAST_FLAG)) { /* broadcast */ }
+        //else { /* unicast to payload->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 payload->yiaddr!
+        // payload->ciaddr, OTOH, comes from client's request packet,
+        // and can be used.
+
+        if (force_broadcast
+         || (payload->flags & htons(BROADCAST_FLAG))
+         || !payload->ciaddr
+        ) {
+                DEBUG("broadcasting packet to client");
                 ciaddr = INADDR_BROADCAST;
                 chaddr = MAC_BCAST_ADDR;
-        } else if (payload->ciaddr) {
+        } else {
                 DEBUG("unicasting packet to client ciaddr");
                 ciaddr = payload->ciaddr;
                 chaddr = payload->chaddr;
-        } else if (payload->flags & htons(BROADCAST_FLAG)) {
-                DEBUG("broadcasting packet to client (requested)");
-                ciaddr = INADDR_BROADCAST;
-                chaddr = MAC_BCAST_ADDR;
-        } else {
-                DEBUG("unicasting packet to client yiaddr");
-                ciaddr = payload->yiaddr;
-                chaddr = payload->chaddr;
         }
+
         return udhcp_send_raw_packet(payload,
                 /*src*/ server_config.server, SERVER_PORT,
                 /*dst*/ ciaddr, CLIENT_PORT, chaddr,
@@ -118,17 +127,18 @@ int FAST_FUNC send_offer(struct dhcpMessage *oldpacket)
                 struct dhcpOfferedAddr *lease;
 
                 lease = find_lease_by_chaddr(oldpacket->chaddr);
-                /* the client is in our lease/offered table */
+                /* The client is in our lease/offered table */
                 if (lease) {
                         signed_leasetime_t tmp = lease->expires - time(NULL);
                         if (tmp >= 0)
                                 lease_time_aligned = tmp;
                         packet.yiaddr = lease->yiaddr;
-                /* Or the client has requested an ip */
-                } else if ((req = get_option(oldpacket, DHCP_REQUESTED_IP)) != NULL
-                 /* Don't look here (ugly hackish thing to do) */
+                }
+                /* Or the client has requested an IP */
+                else if ((req = get_option(oldpacket, DHCP_REQUESTED_IP)) != NULL
+                 /* (read IP) */
                  && (move_from_unaligned32(req_align, req), 1)
-                 /* and the ip is in the lease range */
+                 /* and the IP is in the lease range */
                  && ntohl(req_align) >= server_config.start_ip
                  && ntohl(req_align) <= server_config.end_ip
                  /* and is not already taken/offered */
@@ -137,9 +147,10 @@ int FAST_FUNC send_offer(struct dhcpMessage *oldpacket)
                         || lease_expired(lease))
                 ) {
                         packet.yiaddr = req_align;
-                /* otherwise, find a free IP */
-                } else {
-                        packet.yiaddr = find_free_or_expired_address();
+                }
+                /* Otherwise, find a free IP */
+                else {
+                        packet.yiaddr = find_free_or_expired_address(oldpacket->chaddr);
                 }
 
                 if (!packet.yiaddr) {