udhcpc: paranoia when using kernel UDP mode for sending renew: server ID may be bogus
authorDenys Vlasenko <vda.linux@googlemail.com>
Fri, 29 Sep 2017 13:55:24 +0000 (15:55 +0200)
committerDenys Vlasenko <vda.linux@googlemail.com>
Fri, 29 Sep 2017 14:02:11 +0000 (16:02 +0200)
With new code, we request that target IP (server ID) must be directly reachable.
If it's not, this happens:

udhcpc: waiting 2000 seconds
udhcpc: entering listen mode: kernel
udhcpc: opening listen socket on *:68 wlan0
udhcpc: entering renew state
udhcpc: sending renew to 1.1.1.1
udhcpc: send: Network is unreachable
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 1.1.1.1 needs routing, this is fishy!
udhcpc: entering rebinding state
udhcpc: entering listen mode: raw
udhcpc: created raw socket
udhcpc: sending renew to 0.0.0.0
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ going to use broadcast

which is the desired behavior. Before the patch, packet to 1.1.1.1 was routed
over eth0 (!) and maybe even into Internet (!!!).

function                                             old     new   delta
udhcpc_main                                         2752    2763     +11
udhcp_send_kernel_packet                             295     301      +6
send_renew                                            82      84      +2
send_packet                                          166     168      +2
bcast_or_ucast                                        23      25      +2
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 5/0 up/down: 23/0)               Total: 23 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
networking/udhcp/common.h
networking/udhcp/d6_dhcpc.c
networking/udhcp/dhcpc.c
networking/udhcp/dhcpd.c
networking/udhcp/packet.c

index a9c23a1869192c6b1a208e83ae57a2d4ff448a52..04939e70773ef06b79df0cc2f07d50207df43a9c 100644 (file)
@@ -308,7 +308,9 @@ int udhcp_send_raw_packet(struct dhcp_packet *dhcp_pkt,
 
 int udhcp_send_kernel_packet(struct dhcp_packet *dhcp_pkt,
                uint32_t source_nip, int source_port,
-               uint32_t dest_nip, int dest_port) FAST_FUNC;
+               uint32_t dest_nip, int dest_port,
+               int send_flags
+) FAST_FUNC;
 
 void udhcp_sp_setup(void) FAST_FUNC;
 void udhcp_sp_fd_set(struct pollfd *pfds, int extra_fd) FAST_FUNC;
index d4bb3507b70828a6034da513f7f04d628b5e0fe3..c13f23505c558bc70176565637527e61a0536966 100644 (file)
@@ -702,13 +702,15 @@ static NOINLINE int send_d6_renew(uint32_t xid, struct in6_addr *server_ipv6, st
        opt_ptr = add_d6_client_options(opt_ptr);
 
        bb_error_msg("sending %s", "renew");
-       if (server_ipv6)
+       if (server_ipv6) {
                return d6_send_kernel_packet(
                        &packet, (opt_ptr - (uint8_t*) &packet),
                        our_cur_ipv6, CLIENT_PORT6,
                        server_ipv6, SERVER_PORT6,
                        client_config.ifindex
+                       /* TODO? send_flags: MSG_DONTROUTE (see IPv4 code for reason why) */
                );
+       }
        return d6_mcast_from_client_config_ifindex(&packet, opt_ptr);
 }
 
index 6c74996ef65a06a9196f28eb5f539569670fef2f..2ae8bcc4de6337a52489e690308948815229cde8 100644 (file)
@@ -694,10 +694,16 @@ static int raw_bcast_from_client_config_ifindex(struct dhcp_packet *packet, uint
 
 static int bcast_or_ucast(struct dhcp_packet *packet, uint32_t ciaddr, uint32_t server)
 {
-       if (server)
+       if (server) {
+               /* Without MSG_DONTROUTE, the packet was seen routed over
+                * _other interface_ if server ID is bogus (example: 1.1.1.1).
+                */
                return udhcp_send_kernel_packet(packet,
                        ciaddr, CLIENT_PORT,
-                       server, SERVER_PORT);
+                       server, SERVER_PORT,
+                       /*send_flags: "to hosts only on directly connected networks" */ MSG_DONTROUTE
+               );
+       }
        return raw_bcast_from_client_config_ifindex(packet, ciaddr);
 }
 
@@ -735,7 +741,7 @@ static NOINLINE int send_discover(uint32_t xid, uint32_t requested)
 static NOINLINE int send_select(uint32_t xid, uint32_t server, uint32_t requested)
 {
        struct dhcp_packet packet;
-       struct in_addr addr;
+       struct in_addr temp_addr;
 
 /*
  * RFC 2131 4.3.2 DHCPREQUEST message
@@ -766,8 +772,8 @@ static NOINLINE int send_select(uint32_t xid, uint32_t server, uint32_t requeste
         */
        add_client_options(&packet);
 
-       addr.s_addr = requested;
-       bb_error_msg("sending select for %s", inet_ntoa(addr));
+       temp_addr.s_addr = requested;
+       bb_error_msg("sending select for %s", inet_ntoa(temp_addr));
        return raw_bcast_from_client_config_ifindex(&packet, INADDR_ANY);
 }
 
@@ -776,6 +782,7 @@ static NOINLINE int send_select(uint32_t xid, uint32_t server, uint32_t requeste
 static NOINLINE int send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr)
 {
        struct dhcp_packet packet;
+       struct in_addr temp_addr;
 
 /*
  * RFC 2131 4.3.2 DHCPREQUEST message
@@ -806,7 +813,8 @@ static NOINLINE int send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr)
         */
        add_client_options(&packet);
 
-       bb_error_msg("sending %s", "renew");
+       temp_addr.s_addr = server;
+       bb_error_msg("sending renew to %s", inet_ntoa(temp_addr));
        return bcast_or_ucast(&packet, ciaddr, server);
 }
 
@@ -1524,11 +1532,17 @@ int udhcpc_main(int argc UNUSED_PARAM, char **argv)
                         * Anyway, it does recover by eventually failing through
                         * into INIT_SELECTING state.
                         */
-                                       send_renew(xid, server_addr, requested_ip);
-                                       timeout >>= 1;
-                                       continue;
+                                       if (send_renew(xid, server_addr, requested_ip) >= 0) {
+                                               timeout >>= 1;
+                                               continue;
+                                       }
+                                       /* else: error sending.
+                                        * example: ENETUNREACH seen with server
+                                        * which gave us bogus server ID 1.1.1.1
+                                        * which wasn't reachable (and probably did not exist).
+                                        */
                                }
-                               /* Timed out, enter rebinding state */
+                               /* Timed out or error, enter rebinding state */
                                log1("entering rebinding state");
                                state = REBINDING;
                                /* fall right through */
index 05ddc864984764f7a24748acf6d3b367eab6dd58..57d8b36c5538fcf8f2799f3ddd3b7eaa5147959b 100644 (file)
@@ -588,7 +588,9 @@ static void send_packet_to_relay(struct dhcp_packet *dhcp_pkt)
 
        udhcp_send_kernel_packet(dhcp_pkt,
                        server_config.server_nip, SERVER_PORT,
-                       dhcp_pkt->gateway_nip, SERVER_PORT);
+                       dhcp_pkt->gateway_nip, SERVER_PORT,
+                       /*send_flags:*/ 0
+       );
 }
 
 static void send_packet(struct dhcp_packet *dhcp_pkt, int force_broadcast)
index 44d9ceec76743ed9a8849bc9f20a119a69b31c9c..ad0028bd04f6a399732732e5e1f7ab905bc3361c 100644 (file)
@@ -191,7 +191,8 @@ int FAST_FUNC udhcp_send_raw_packet(struct dhcp_packet *dhcp_pkt,
 /* Let the kernel do all the work for packet generation */
 int FAST_FUNC udhcp_send_kernel_packet(struct dhcp_packet *dhcp_pkt,
                uint32_t source_nip, int source_port,
-               uint32_t dest_nip, int dest_port)
+               uint32_t dest_nip, int dest_port,
+               int send_flags)
 {
        struct sockaddr_in sa;
        unsigned padding;
@@ -228,8 +229,8 @@ int FAST_FUNC udhcp_send_kernel_packet(struct dhcp_packet *dhcp_pkt,
        padding = DHCP_OPTIONS_BUFSIZE - 1 - udhcp_end_option(dhcp_pkt->options);
        if (padding > DHCP_SIZE - 300)
                padding = DHCP_SIZE - 300;
-       result = safe_write(fd, dhcp_pkt, DHCP_SIZE - padding);
-       msg = "write";
+       result = send(fd, dhcp_pkt, DHCP_SIZE - padding, send_flags);
+       msg = "send";
  ret_close:
        close(fd);
        if (result < 0) {