Patch applied, thanks.

This has the added advantage that it doesn't need elevated privileges to
set IP_UNICAST_IF, unlike SO_BINDTODEVICE. Therefore binding upstream
nameservers to an interface works even when dnsmasq is started as a
non-root user, eg for testing.

Cheers,

Simon.



On 27/09/17 17:14, Beniamino Galvani wrote:
> dnsmasq allows to specify a interface for each name server passed with
> the -S option or pushed through D-Bus; when an interface is set,
> queries to the server will be forced via that interface.
> 
> Currently dnsmasq uses SO_BINDTODEVICE to enforce that traffic goes
> through the given interface; SO_BINDTODEVICE also guarantees that any
> response coming from other interfaces is ignored.
> 
> This can cause problems in some scenarios: consider the case where
> eth0 and eth1 are in the same subnet and eth0 has a name server ns0
> associated.  There is no guarantee that the response to a query sent
> via eth0 to ns0 will be received on eth0 because the local router may
> have in the ARP table the MAC address of eth1 for the IP of eth0. This
> can happen because Linux sends ARP responses for all the IPs of the
> machine through all interfaces. The response packet on the wrong
> interface will be dropped because of SO_BINDTODEVICE and the
> resolution will fail.
> 
> To avoid this situation, dnsmasq should only restrict queries, but not
> responses, to the given interface. A way to do this on Linux is with
> the IP_UNICAST_IF and IPV6_UNICAST_IF socket options which were added
> in kernel 3.4 and, respectively, glibc versions 2.16 and 2.26.
> 
> Reported-by: Hector Martin <mar...@marcan.st>
> Signed-off-by: Beniamino Galvani <bgalv...@redhat.com>
> ---
>  src/dnsmasq.h |  2 +-
>  src/forward.c |  4 ++--
>  src/network.c | 24 +++++++++++++++++++++---
>  3 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/src/dnsmasq.h b/src/dnsmasq.h
> index 24dda08..c983470 100644
> --- a/src/dnsmasq.h
> +++ b/src/dnsmasq.h
> @@ -1248,7 +1248,7 @@ void free_rfd(struct randfd *rfd);
>  
>  /* network.c */
>  int indextoname(int fd, int index, char *name);
> -int local_bind(int fd, union mysockaddr *addr, char *intname, int is_tcp);
> +int local_bind(int fd, union mysockaddr *addr, char *intname, unsigned int 
> ifindex, int is_tcp);
>  int random_sock(int family);
>  void pre_allocate_sfds(void);
>  int reload_servers(char *fname);
> diff --git a/src/forward.c b/src/forward.c
> index 942b02d..c2c50ec 100644
> --- a/src/forward.c
> +++ b/src/forward.c
> @@ -1551,7 +1551,7 @@ static int tcp_key_recurse(time_t now, int status, 
> struct dns_header *header, si
>                 setsockopt(server->tcpfd, SOL_SOCKET, SO_MARK, &mark, 
> sizeof(unsigned int));
>  #endif       
>               
> -             if (!local_bind(server->tcpfd,  &server->source_addr, 
> server->interface, 1) ||
> +             if (!local_bind(server->tcpfd,  &server->source_addr, 
> server->interface, 0, 1) ||
>                   connect(server->tcpfd, &server->addr.sa, 
> sa_len(&server->addr)) == -1)
>                 {
>                   close(server->tcpfd);
> @@ -1847,7 +1847,7 @@ unsigned char *tcp_request(int confd, time_t now,
>                           setsockopt(last_server->tcpfd, SOL_SOCKET, SO_MARK, 
> &mark, sizeof(unsigned int));
>  #endif       
>                     
> -                       if ((!local_bind(last_server->tcpfd,  
> &last_server->source_addr, last_server->interface, 1) ||
> +                       if ((!local_bind(last_server->tcpfd,  
> &last_server->source_addr, last_server->interface, 0, 1) ||
>                              connect(last_server->tcpfd, 
> &last_server->addr.sa, sa_len(&last_server->addr)) == -1))
>                           {
>                             close(last_server->tcpfd);
> diff --git a/src/network.c b/src/network.c
> index c87b879..a47e3ee 100644
> --- a/src/network.c
> +++ b/src/network.c
> @@ -1187,7 +1187,7 @@ int random_sock(int family)
>  }
>    
>  
> -int local_bind(int fd, union mysockaddr *addr, char *intname, int is_tcp)
> +int local_bind(int fd, union mysockaddr *addr, char *intname, unsigned int 
> ifindex, int is_tcp)
>  {
>    union mysockaddr addr_copy = *addr;
>  
> @@ -1204,7 +1204,25 @@ int local_bind(int fd, union mysockaddr *addr, char 
> *intname, int is_tcp)
>    
>    if (bind(fd, (struct sockaddr *)&addr_copy, sa_len(&addr_copy)) == -1)
>      return 0;
> -    
> +
> +  if (!is_tcp && ifindex > 0)
> +    {
> +#if defined(IP_UNICAST_IF)
> +      if (addr_copy.sa.sa_family == AF_INET)
> +        {
> +          uint32_t ifindex_opt = htonl(ifindex);
> +          return setsockopt(fd, IPPROTO_IP, IP_UNICAST_IF, &ifindex_opt, 
> sizeof(ifindex_opt)) == 0;
> +        }
> +#endif
> +#if defined (IPV6_UNICAST_IF)
> +      if (addr_copy.sa.sa_family == AF_INET6)
> +        {
> +          uint32_t ifindex_opt = htonl(ifindex);
> +          return setsockopt(fd, IPPROTO_IPV6, IPV6_UNICAST_IF, &ifindex_opt, 
> sizeof(ifindex_opt)) == 0;
> +        }
> +#endif
> +    }
> +
>  #if defined(SO_BINDTODEVICE)
>    if (intname[0] != 0 &&
>        setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, intname, IF_NAMESIZE) == 
> -1)
> @@ -1260,7 +1278,7 @@ static struct serverfd *allocate_sfd(union mysockaddr 
> *addr, char *intname)
>        return NULL;
>      }
>    
> -  if (!local_bind(sfd->fd, addr, intname, 0) || !fix_fd(sfd->fd))
> +  if (!local_bind(sfd->fd, addr, intname, ifindex, 0) || !fix_fd(sfd->fd))
>      { 
>        errsave = errno; /* save error from bind. */
>        close(sfd->fd);
> 


Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss

Reply via email to