[Dnsmasq-discuss] [PATCH] Use IP[V6]_UNICAST_IF socket option instead of SO_BINDTODEVICE for dns
Simon Kelley
simon at thekelleys.org.uk
Fri Oct 13 17:58:47 BST 2017
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 <marcan at marcan.st>
> Signed-off-by: Beniamino Galvani <bgalvani at 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);
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/attachments/20171013/efa7c9b6/attachment.sig>
More information about the Dnsmasq-discuss
mailing list