Hello Simon and others,
we have discovered issues with TCP DNS query on dnsmasq, when running in
bind-dynamic or bind-interfaces mode. dnsmasq scans automatically new
interfaces or do that on new query in second case. However, because used
speedup comparing only IP adresses in iface_allowed function, it never
gets updated index of an interface.
In case where named interface is destroyed and created again, that drops
TCP queries on that interface. They are checked for incoming interface
number. If such number is not found in interfaces list, query is denied.
Luckily, there was a bug in checking, hiding this problem from usual
configuration. If IPv6 address is enabled on the new device, new iface
entry would be created, because scope_id of sockaddr_in6 does not match
previous. That makes even IPv4 queries succeed.
Bug on bugzilla [1] is partly private.
I propose three changes. First is just helper to log what happens with
listeners on bind-dynamic configuration.
Second is the most important. Create new interface every time index
changes. Also test address family of incoming TCP query when checking
allowed clients.
Third is cleanup of unused interfaces. On some virtual machines hosts,
interfaces may often be created and destroyed. It might have negative
effect on walking trough interfaces list. I think listeners should be
garbage collected also on bind-interfaces configuration. But for now,
release memory for unused interfaces at least for bind-dynamic.
1. https://bugzilla.redhat.com/show_bug.cgi?id=1721668
--
Petr Menšík
Software Engineer
Red Hat, http://www.redhat.com/
email: pemen...@redhat.com PGP: 65C6C973
From c9cc7aa2fb5463626bf6795531390ca3f2d2752b Mon Sep 17 00:00:00 2001
From: Petr Mensik <pemen...@redhat.com>
Date: Tue, 9 Jul 2019 14:05:59 +0200
Subject: [PATCH 3/3] Cleanup interfaces no longer available
Clean addresses and interfaces not found after enumerate. Free unused
records to speed up checking active interfaces and reduce used memory.
---
src/network.c | 31 +++++++++++++++++++++++++++++--
1 file changed, 29 insertions(+), 2 deletions(-)
diff --git a/src/network.c b/src/network.c
index f487617..5a37f61 100644
--- a/src/network.c
+++ b/src/network.c
@@ -533,7 +533,29 @@ static int iface_allowed_v4(struct in_addr local, int if_index, char *label,
return iface_allowed((struct iface_param *)vparam, if_index, label, &addr, netmask, prefix, 0);
}
-
+
+/*
+ * Clean old interfaces no longer found.
+ */
+static void clean_interfaces()
+{
+ struct irec *iface;
+ struct irec **up = &daemon->interfaces;
+
+ for (iface = *up; iface; iface = *up)
+ {
+ if (!iface->found && !iface->done) {
+ *up = iface->next;
+ my_syslog(LOG_DEBUG, _("dropping interface %s(#%d) family %d"),
+ iface->name, iface->index, iface->addr.sa.sa_family);
+ free(iface->name);
+ free(iface);
+ } else {
+ up = &iface->next;
+ }
+ }
+}
+
int enumerate_interfaces(int reset)
{
static struct addrlist *spare = NULL;
@@ -631,6 +653,7 @@ int enumerate_interfaces(int reset)
in OPT_CLEVERBIND mode, that at listener will just disappear after
a call to enumerate_interfaces, this is checked OK on all calls. */
struct listener *l, *tmp, **up;
+ int freed = 0;
for (up = &daemon->listeners, l = daemon->listeners; l; l = tmp)
{
@@ -660,10 +683,14 @@ int enumerate_interfaces(int reset)
close(l->tftpfd);
free(l);
+ freed = 1;
}
}
+
+ if (freed)
+ clean_interfaces();
}
-
+
errno = errsave;
spare = param.spare;
--
2.20.1
From e3a22a2ad93c518507cd1b41fabbf0c1160daef1 Mon Sep 17 00:00:00 2001
From: Petr Mensik <pemen...@redhat.com>
Date: Wed, 3 Jul 2019 17:02:16 +0200
Subject: [PATCH 2/3] Compare address and interface index for allowed interface
If interface is recreated with the same address but different index, it
would not change any other parameter.
Test also address family on incoming TCP queries.
---
src/dnsmasq.c | 3 ++-
src/network.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/dnsmasq.c b/src/dnsmasq.c
index 704475f..939e69e 100644
--- a/src/dnsmasq.c
+++ b/src/dnsmasq.c
@@ -1792,7 +1792,8 @@ static void check_dns_listeners(time_t now)
addr.addr4 = tcp_addr.in.sin_addr;
for (iface = daemon->interfaces; iface; iface = iface->next)
- if (iface->index == if_index)
+ if (iface->index == if_index &&
+ iface->addr.sa.sa_family == tcp_addr.sa.sa_family)
break;
if (!iface && !loopback_exception(listener->tcpfd, tcp_addr.sa.sa_family, &addr, intr_name))
diff --git a/src/network.c b/src/network.c
index 1fe25f7..f487617 100644
--- a/src/network.c
+++ b/src/network.c
@@ -388,10 +388,11 @@ static int iface_allowed(struct iface_param *param, int if_index, char *label,
/* check whether the interface IP has been added already
we call this routine multiple times. */
for (iface = daemon->interfaces; iface; iface = iface->next)
- if (sockaddr_isequal(&iface->addr, addr))
+ if (sockaddr_isequal(&iface->addr, addr) && iface->index == if_index)
{
iface->dad = !!(iface_flags & IFACE_TENTATIVE);
iface->found = 1; /* for garbage collection */
+ iface->netmask = netmask;
return 1;
}
--
2.20.1
From 2d7212c79ea76a297c3e9107c5e2e5791c3d3a1d Mon Sep 17 00:00:00 2001
From: Petr Mensik <pemen...@redhat.com>
Date: Thu, 4 Jul 2019 20:28:08 +0200
Subject: [PATCH 1/3] Log listening on new interfaces
Log in debug mode listening on interfaces. They can be dynamically
found, include interface number, since it is checked on TCP connections.
Print also addresses found on them.
---
src/network.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/src/network.c b/src/network.c
index c1077b4..1fe25f7 100644
--- a/src/network.c
+++ b/src/network.c
@@ -640,6 +640,13 @@ int enumerate_interfaces(int reset)
else
{
*up = l->next;
+ if (l->iface->done)
+ {
+ iface = l->iface;
+ (void)prettyprint_addr(&iface->addr, daemon->addrbuff);
+ my_syslog(LOG_DEBUG, _("stopped listening on %s(#%d): %s"),
+ iface->name, iface->index, daemon->addrbuff);
+ }
/* In case it ever returns */
l->iface->done = 0;
@@ -949,6 +956,9 @@ void create_bound_listeners(int dienow)
new->next = daemon->listeners;
daemon->listeners = new;
iface->done = 1;
+ (void)prettyprint_addr(&iface->addr, daemon->addrbuff);
+ my_syslog(LOG_DEBUG, _("listening on %s(#%d): %s"),
+ iface->name, iface->index, daemon->addrbuff);
}
/* Check for --listen-address options that haven't been used because there's
@@ -968,6 +978,8 @@ void create_bound_listeners(int dienow)
{
new->next = daemon->listeners;
daemon->listeners = new;
+ (void)prettyprint_addr(&if_tmp->addr, daemon->addrbuff);
+ my_syslog(LOG_DEBUG, _("listening on %s"), daemon->addrbuff);
}
}
--
2.20.1
_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
