[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: [PATCH] udhcp: bind to device even for ucast packets
From:       Denys Vlasenko <vda.linux () googlemail ! com>
Date:       2020-12-15 20:56:11
Message-ID: CAK1hOcNCzBrvzxpEq_yZWAJQ+jR4QY8SxPT9B6egLYq7QeSvaw () mail ! gmail ! com
[Download RAW message or body]

applied, thanks

On Tue, Dec 15, 2020 at 10:54 AM Michal Kazior <kazikcz@gmail.com> wrote:
>
> From: Michal Kazior <michal@plume.com>
>
> There are cases where binding to source IP and
> destination IP is insufficient to guarantee sane
> xmit netdev.
>
> One case where this can fail is when
> route-matching netdev carrier is down (cable
> unplugged, wifi disconnected), or the netdev is
> admin down. Then all the IP based bindings (bind()
> + connect()) will seemingly succeed but the actual
> packet can go out through a default gw path.
>
> This isn't necessary fatal on its own.
> Functionally from, eg. udhcpc point of view it'll
> likely end up with no DHCP response which it
> wouldn't get from a non-running interface anyway.
>
> However depending on the network this happens on
> it can create issues or false alarms. It can
> also leak some subnet info across networks that
> shouldn't be routed.
>
> As such better be safe than sorry and bind to a
> netdev to be sure it's used for xmit.
>
> Signed-off-by: Michal Kazior <michal@plume.com>
> ---
>  networking/udhcp/common.h |  3 ++-
>  networking/udhcp/dhcpc.c  |  3 ++-
>  networking/udhcp/dhcpd.c  |  3 ++-
>  networking/udhcp/packet.c | 18 +++++++++++++++++-
>  4 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/networking/udhcp/common.h b/networking/udhcp/common.h
> index 3cbd2d3c8..cc0abd269 100644
> --- a/networking/udhcp/common.h
> +++ b/networking/udhcp/common.h
> @@ -343,7 +343,8 @@ 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,
> +               const char *ifname) FAST_FUNC;
>
>  void udhcp_sp_setup(void) FAST_FUNC;
>  void udhcp_sp_fd_set(struct pollfd *pfds, int extra_fd) FAST_FUNC;
> diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c
> index 66aa38c20..98720b45b 100644
> --- a/networking/udhcp/dhcpc.c
> +++ b/networking/udhcp/dhcpc.c
> @@ -702,7 +702,8 @@ static int bcast_or_ucast(struct dhcp_packet *packet, uint32_t ciaddr, uint32_t
>         if (server)
>                 return udhcp_send_kernel_packet(packet,
>                         ciaddr, CLIENT_PORT,
> -                       server, SERVER_PORT);
> +                       server, SERVER_PORT,
> +                       client_data.interface);
>         return raw_bcast_from_client_data_ifindex(packet, ciaddr);
>  }
>
> diff --git a/networking/udhcp/dhcpd.c b/networking/udhcp/dhcpd.c
> index de16cf955..9e950ca1f 100644
> --- a/networking/udhcp/dhcpd.c
> +++ b/networking/udhcp/dhcpd.c
> @@ -612,7 +612,8 @@ static void send_packet_to_relay(struct dhcp_packet *dhcp_pkt)
>
>         udhcp_send_kernel_packet(dhcp_pkt,
>                         server_data.server_nip, SERVER_PORT,
> -                       dhcp_pkt->gateway_nip, SERVER_PORT);
> +                       dhcp_pkt->gateway_nip, SERVER_PORT,
> +                       server_data.interface);
>  }
>
>  static void send_packet(struct dhcp_packet *dhcp_pkt, int force_broadcast)
> diff --git a/networking/udhcp/packet.c b/networking/udhcp/packet.c
> index 51374646d..504059290 100644
> --- a/networking/udhcp/packet.c
> +++ b/networking/udhcp/packet.c
> @@ -189,7 +189,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,
> +               const char *ifname)
>  {
>         struct sockaddr_in sa;
>         unsigned padding;
> @@ -204,6 +205,21 @@ int FAST_FUNC udhcp_send_kernel_packet(struct dhcp_packet *dhcp_pkt,
>         }
>         setsockopt_reuseaddr(fd);
>
> +       /* If interface carrier goes down then, unless we
> +        * bind socket to a particular netdev, the packet
> +        * can go out through another interface, eg. via
> +        * default route despite being bound to a specific
> +        * source IP. As such, bind to device hard and fail
> +        * otherwise. Sending renewal packets on foreign
> +        * interfaces makes no sense.
> +        */
> +       if (ifname) {
> +               if (setsockopt_bindtodevice(fd, ifname) < 0) {
> +                       msg = "bindtodevice";
> +                       goto ret_close;
> +               }
> +       }
> +
>         memset(&sa, 0, sizeof(sa));
>         sa.sin_family = AF_INET;
>         sa.sin_port = htons(source_port);
> --
> 2.27.0
>
> _______________________________________________
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]