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>

diff --git a/networking/udhcp/arpping.c b/networking/udhcp/arpping.c
index b10bff6514e24599403f4e8fb671b41ee8d37dcc..fa0989d0fc8b112fc0b2a05a17f2af20e59d2dc3 100644 (file)
--- a/networking/udhcp/arpping.c
+++ b/networking/udhcp/arpping.c
@@ -41,7 +41,11 @@ enum {
 
 /* Returns 1 if no reply received */
 
-int FAST_FUNC arpping(uint32_t test_ip, uint32_t from_ip, uint8_t *from_mac, const char *interface)
+int FAST_FUNC arpping(uint32_t test_ip,
+               const uint8_t *safe_mac,
+               uint32_t from_ip,
+               uint8_t *from_mac,
+               const char *interface)
 {
        int timeout_ms;
        struct pollfd pfd[1];
@@ -73,7 +77,7 @@ int FAST_FUNC arpping(uint32_t test_ip, uint32_t from_ip, uint8_t *from_mac, con
        arp.operation = htons(ARPOP_REQUEST);           /* ARP op code */
        memcpy(arp.sHaddr, from_mac, 6);                /* source hardware address */
        memcpy(arp.sInaddr, &from_ip, sizeof(from_ip)); /* source IP address */
-       /* tHaddr is zero-fiiled */                     /* target hardware address */
+       /* tHaddr is zero-filled */                     /* target hardware address */
        memcpy(arp.tInaddr, &test_ip, sizeof(test_ip)); /* target IP address */
 
        memset(&addr, 0, sizeof(addr));
@@ -98,13 +102,24 @@ int FAST_FUNC arpping(uint32_t test_ip, uint32_t from_ip, uint8_t *from_mac, con
                        r = read(s, &arp, sizeof(arp));
                        if (r < 0)
                                break;
+
+                       //bb_error_msg("sHaddr %02x:%02x:%02x:%02x:%02x:%02x",
+                       //      arp.sHaddr[0], arp.sHaddr[1], arp.sHaddr[2],
+                       //      arp.sHaddr[3], arp.sHaddr[4], arp.sHaddr[5]);
+
                        if (r >= ARP_MSG_SIZE
                         && arp.operation == htons(ARPOP_REPLY)
                         /* don't check it: Linux doesn't return proper tHaddr (fixed in 2.6.24?) */
                         /* && memcmp(arp.tHaddr, from_mac, 6) == 0 */
                         && *((uint32_t *) arp.sInaddr) == test_ip
                        ) {
-                               rv = 0;
+                                /* if ARP source MAC matches safe_mac
+                                 * (which is client's MAC), then it's not a conflict
+                                 * (client simply already has this IP and replies to ARPs!)
+                                 */
+                               if (!safe_mac || memcmp(safe_mac, arp.sHaddr, 6) != 0)
+                                       rv = 0;
+                               //else bb_error_msg("sHaddr == safe_mac");
                                break;
                        }
                }

diff --git a/networking/udhcp/common.h b/networking/udhcp/common.h
index 5a258c064d767621f9a283b6f46220ca34d886c4..ca96847a7154278439dc65fdb7677cfade3829b6 100644 (file)
--- a/networking/udhcp/common.h
+++ b/networking/udhcp/common.h
@@ -92,7 +92,11 @@ int udhcp_read_interface(const char *interface, int *ifindex, uint32_t *addr, ui
 int udhcp_raw_socket(int ifindex) FAST_FUNC;
 int udhcp_listen_socket(/*uint32_t ip,*/ int port, const char *inf) FAST_FUNC;
 /* Returns 1 if no reply received */
-int arpping(uint32_t test_ip, uint32_t from_ip, uint8_t *from_mac, const char *interface) FAST_FUNC;
+int arpping(uint32_t test_ip,
+               const uint8_t *safe_mac,
+               uint32_t from_ip,
+               uint8_t *from_mac,
+               const char *interface) FAST_FUNC;
 
 #if ENABLE_UDHCP_DEBUG
 # define DEBUG(str, args...) bb_info_msg("### " str, ## args)

diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c
index 2dd3cd077a233957024c9df3cc002fe6417bf415..ab34b0472cd638a65fafb3b87d99ceccf1fbf5e1 100644 (file)
--- a/networking/udhcp/dhcpc.c
+++ b/networking/udhcp/dhcpc.c
@@ -553,9 +553,10 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
  * the client MUST send a DHCPDECLINE message to the server and restarts
  * the configuration process..." */
                                                if (!arpping(packet.yiaddr,
-                                                           (uint32_t) 0,
-                                                           client_config.arp,
-                                                           client_config.interface)
+                                                               NULL,
+                                                               (uint32_t) 0,
+                                                               client_config.arp,
+                                                               client_config.interface)
                                                ) {
                                                        bb_info_msg("offered address is in use "
                                                                "(got ARP reply), declining");

diff --git a/networking/udhcp/dhcpd.h b/networking/udhcp/dhcpd.h
index 9667c61e816390c6b677116ab452461d583681da..4b5fcc00f45d96389243e2c746c03a9b9e963aa2 100644 (file)
--- a/networking/udhcp/dhcpd.h
+++ b/networking/udhcp/dhcpd.h
@@ -99,7 +99,7 @@ struct dhcpOfferedAddr *add_lease(
 int lease_expired(struct dhcpOfferedAddr *lease) FAST_FUNC;
 struct dhcpOfferedAddr *find_lease_by_chaddr(const uint8_t *chaddr) FAST_FUNC;
 struct dhcpOfferedAddr *find_lease_by_yiaddr(uint32_t yiaddr) FAST_FUNC;
-uint32_t find_free_or_expired_address(void) FAST_FUNC;
+uint32_t find_free_or_expired_address(const uint8_t *chaddr) FAST_FUNC;
 
 
 /*** static_leases.h ***/

diff --git a/networking/udhcp/leases.c b/networking/udhcp/leases.c
index e17fb9e3f91e005b094023f637ba5897f0ab1548..b2cdd19423416482dc30025afb92b77817ce6455 100644 (file)
--- a/networking/udhcp/leases.c
+++ b/networking/udhcp/leases.c
@@ -118,7 +118,7 @@ struct dhcpOfferedAddr* FAST_FUNC find_lease_by_yiaddr(uint32_t yiaddr)
 
 
 /* check is an IP is taken, if it is, add it to the lease table */
-static int nobody_responds_to_arp(uint32_t addr)
+static int nobody_responds_to_arp(uint32_t addr, const uint8_t *safe_mac)
 {
        /* 16 zero bytes */
        static const uint8_t blank_chaddr[16] = { 0 };
@@ -127,7 +127,9 @@ static int nobody_responds_to_arp(uint32_t addr)
        struct in_addr temp;
        int r;
 
-       r = arpping(addr, server_config.server, server_config.arp, server_config.interface);
+       r = arpping(addr, safe_mac,
+                       server_config.server, server_config.arp,
+                       server_config.interface);
        if (r)
                return r;
 
@@ -140,7 +142,7 @@ static int nobody_responds_to_arp(uint32_t addr)
 
 
 /* Find a new usable (we think) address. */
-uint32_t FAST_FUNC find_free_or_expired_address(void)
+uint32_t FAST_FUNC find_free_or_expired_address(const uint8_t *chaddr)
 {
        uint32_t addr;
        struct dhcpOfferedAddr *oldest_lease = NULL;
@@ -163,7 +165,7 @@ uint32_t FAST_FUNC find_free_or_expired_address(void)
 
                lease = find_lease_by_yiaddr(net_addr);
                if (!lease) {
-                       if (nobody_responds_to_arp(net_addr))
+                       if (nobody_responds_to_arp(net_addr, chaddr))
                                return net_addr;
                } else {
                        if (!oldest_lease || lease->expires < oldest_lease->expires)
@@ -172,7 +174,7 @@ uint32_t FAST_FUNC find_free_or_expired_address(void)
        }
 
        if (oldest_lease && lease_expired(oldest_lease)
-        && nobody_responds_to_arp(oldest_lease->yiaddr)
+        && nobody_responds_to_arp(oldest_lease->yiaddr, chaddr)
        ) {
                return oldest_lease->yiaddr;
        }

diff --git a/networking/udhcp/serverpacket.c b/networking/udhcp/serverpacket.c
index 8b0f1856bf05258cf095f212075df2b39fbdc44c..157d157ba53caf47625b7de151a1e9c401975f14 100644 (file)
--- 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) {