Hey Petr,
while Simon is still away, we can discuss this a little further.
On Wed, 2021-09-29 at 22:48 +0200, Petr Menšík wrote:
> If no --bind-interface is given, iface->name pointing to eth0
> rather
> than eth0:0 is correct. Alias is useful only for reading of the
> address
> from the interface name. Otherwise it works as the interface
> itself.
> Thas was reason behind warn_bound_listeners creation. When
> incoming
> packet is checked for acceptance, it is compared to primary
> interface
> identified by ifindex. I think we may even remove name from
> struct irec
> and get the name on few places it needs to be printed. It makes
> debugging more comfortable, but is not required anyway.
I checked once again why I created the patch initially and found
the following bug/misbehavior (whatever you wanna call it):
Real interface is eth0. an alias is created as eth0:0
1. Config --interface=eth0
Queries on eth0 and eth0:0 are accepted because dnsmasq only
compares the physical interface name string.
2. Config --interface=eth0:0
Queries on eth0 and eth0:0 are rejected (at first!) because of
the physical interface's name mismatch. But there is another
check "label_expection()" that does said iteration and would lead
to accepting the eth0:0 query. The eth0 query is correctly
rejected.
In an ideal world, we should reject also the eth0:0 query when
configured with "--interface=eth0". It can rather easily be done
when comparing the configured interface's IP addresses instead of
the name strings (or ifindex).
When doing this, the warn_wild_listeners() can be dropped
altogether as the strange behavior we used to warn about is
fixed.
I addressed this in the attached patch and would appreciate if
you could take a look (I don't want to break any other features).
The patch isn't highly optimized but prepared for readability.
Best,
Dominik
From 763f46948844eab25859e7ab72816733be3e533c Mon Sep 17 00:00:00 2001
From: Dominik Derigs <dl...@dl6er.de>
Date: Fri, 19 Nov 2021 10:59:25 +0100
Subject: [PATCH] Don't accept queries ariving on alias interface if configured
not to do this.
Signed-off-by: DL6ER <dl...@dl6er.de>
---
src/dnsmasq.c | 2 --
src/dnsmasq.h | 3 +--
src/forward.c | 16 +++++++++++-----
src/network.c | 13 ++-----------
src/tftp.c | 11 ++++++-----
5 files changed, 20 insertions(+), 25 deletions(-)
diff --git a/src/dnsmasq.c b/src/dnsmasq.c
index 2fe9808..b3c8d54 100644
--- a/src/dnsmasq.c
+++ b/src/dnsmasq.c
@@ -945,8 +945,6 @@ int main (int argc, char **argv)
if (option_bool(OPT_NOWILD))
warn_bound_listeners();
- else if (!option_bool(OPT_CLEVERBIND))
- warn_wild_labels();
warn_int_names();
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index bf7685d..4aff343 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1449,12 +1449,11 @@ int enumerate_interfaces(int reset);
void create_wildcard_listeners(void);
void create_bound_listeners(int dienow);
void warn_bound_listeners(void);
-void warn_wild_labels(void);
void warn_int_names(void);
int is_dad_listeners(void);
int iface_check(int family, union all_addr *addr, char *name, int *auth);
int loopback_exception(int fd, int family, union all_addr *addr, char *name);
-int label_exception(int index, int family, union all_addr *addr);
+int label_match(int index, int family, union all_addr *addr);
int fix_fd(int fd);
int tcp_interface(int fd, int af);
int set_ipv6pktinfo(int fd);
diff --git a/src/forward.c b/src/forward.c
index 04635b3..b1d03e6 100644
--- a/src/forward.c
+++ b/src/forward.c
@@ -1484,12 +1484,18 @@ void receive_query(struct listener *listen, time_t now)
if (!indextoname(listen->fd, if_index, ifr.ifr_name))
return;
- if (!iface_check(family, &dst_addr, ifr.ifr_name, &auth_dns))
+ if (!option_bool(OPT_CLEVERBIND))
+ enumerate_interfaces(0);
+ /* interface=eth0 and query over eth0 -> ifchk = 1, label = 1 - ACCEPTED */
+ /* interface=eth0 and query over eth0:0 -> ifchk = 1, label = 0 - REJECTED */
+ /* interface=eth0:0 and query over eth0:0 -> ifchk = 0, label = 1 - ACCEPTED */
+ /* interface=eth0:0 and query over eth0:0 -> ifchk = 0, label = 0 - REJECTED */
+ /* If the interace is not IPv4, label_match return 2 and we use iface_check */
+ const int label = label_match(if_index, family, &dst_addr);
+ const int ifchk = iface_check(family, &dst_addr, ifr.ifr_name, &auth_dns);
+ if (label == 0 || (label == 2 && !ifchk))
{
- if (!option_bool(OPT_CLEVERBIND))
- enumerate_interfaces(0);
- if (!loopback_exception(listen->fd, family, &dst_addr, ifr.ifr_name) &&
- !label_exception(if_index, family, &dst_addr))
+ if (!loopback_exception(listen->fd, family, &dst_addr, ifr.ifr_name))
return;
}
diff --git a/src/network.c b/src/network.c
index 3c1c176..b930553 100644
--- a/src/network.c
+++ b/src/network.c
@@ -207,13 +207,13 @@ int loopback_exception(int fd, int family, union all_addr *addr, char *name)
on the relevant address, but the name of the arrival interface, derived from the
index won't match the config. Check that we found an interface address for the arrival
interface: daemon->interfaces must be up-to-date. */
-int label_exception(int index, int family, union all_addr *addr)
+int label_match(int index, int family, union all_addr *addr)
{
struct irec *iface;
/* labels only supported on IPv4 addresses. */
if (family != AF_INET)
- return 0;
+ return 2;
for (iface = daemon->interfaces; iface; iface = iface->next)
if (iface->index == index && iface->addr.sa.sa_family == AF_INET &&
@@ -1215,15 +1215,6 @@ void warn_bound_listeners(void)
my_syslog(LOG_WARNING, _("LOUD WARNING: use --bind-dynamic rather than --bind-interfaces to avoid DNS amplification attacks via these interface(s)"));
}
-void warn_wild_labels(void)
-{
- struct irec *iface;
-
- for (iface = daemon->interfaces; iface; iface = iface->next)
- if (iface->found && iface->name && iface->label)
- my_syslog(LOG_WARNING, _("warning: using interface %s instead"), iface->name);
-}
-
void warn_int_names(void)
{
struct interface_name *intname;
diff --git a/src/tftp.c b/src/tftp.c
index 3d87523..c14b011 100644
--- a/src/tftp.c
+++ b/src/tftp.c
@@ -211,13 +211,14 @@ void tftp_request(struct listener *listen, time_t now)
}
else
{
+ if (!option_bool(OPT_CLEVERBIND))
+ enumerate_interfaces(0);
/* Do the same as DHCP */
- if (!iface_check(family, &addra, name, NULL))
+ const int label = label_match(if_index, family, &addra);
+ const int ifchk = iface_check(family, &addra, name, NULL);
+ if (!label || (label == 2 && ifchk))
{
- if (!option_bool(OPT_CLEVERBIND))
- enumerate_interfaces(0);
- if (!loopback_exception(listen->tftpfd, family, &addra, name) &&
- !label_exception(if_index, family, &addra))
+ if (!loopback_exception(listen->tftpfd, family, &addra, name))
return;
}
--
2.25.1
_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
