Hey Simon, I found the following bug/misbehavior (whatever you wanna call it):
Real interface is eth0. an alias is created as eth0:0 Config --interface=eth0 Queries on eth0 and eth0:0 are accepted because dnsmasq only compares the physical interface name string. 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. The patch addresses this. The second patch ensures we compare label instead of interface name against dhcp_except and tftp interfaces to extend their scope to interface aliases. The man page does not mention that they are limited to "real" interfaces and stop working once an alias interface is specified (even if valid). Best, Dominik [resubmission of rebased patches, original submission in https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2021q4/015938.html]
From cc07a92ba26c3d9b3142a97e1c750fdb1a09e6e5 Mon Sep 17 00:00:00 2001 From: Dominik Derigs <dl...@dl6er.de> Date: Fri, 19 Nov 2021 10:59:25 +0100 Subject: [PATCH 1/2] 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 0de8d18..15c9620 100644 --- a/src/dnsmasq.c +++ b/src/dnsmasq.c @@ -949,8 +949,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 1b00298..7fb4e3b 100644 --- a/src/dnsmasq.h +++ b/src/dnsmasq.h @@ -1463,12 +1463,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 f22c080..a35d897 100644 --- a/src/forward.c +++ b/src/forward.c @@ -1497,12 +1497,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 e8474d9..e59b996 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
From 14a9cf5382aaa411971254f231c8349665069bba Mon Sep 17 00:00:00 2001 From: Dominik Derigs <dl...@dl6er.de> Date: Sun, 26 Dec 2021 10:35:00 +0100 Subject: [PATCH 2/2] Compare label instead of interface name against dhcp_except and tftp interfaces to extend their scope to interface aliases. The man page does not mention that they are limited to "real" interfaces and stop working once an alias interface is specified (even if valid). Signed-off-by: DL6ER <dl...@dl6er.de> --- src/network.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/network.c b/src/network.c index b930553..0ac25c0 100644 --- a/src/network.c +++ b/src/network.c @@ -506,7 +506,7 @@ static int iface_allowed(struct iface_param *param, int if_index, char *label, } else for (tmp = daemon->dhcp_except; tmp; tmp = tmp->next) - if (tmp->name && wildcard_match(tmp->name, ifr.ifr_name)) + if (tmp->name && wildcard_match(tmp->name, label)) { tftp_ok = 0; dhcp_ok = 0; @@ -520,7 +520,7 @@ static int iface_allowed(struct iface_param *param, int if_index, char *label, /* dedicated tftp interface list */ tftp_ok = 0; for (tmp = daemon->tftp_interfaces; tmp; tmp = tmp->next) - if (tmp->name && wildcard_match(tmp->name, ifr.ifr_name)) + if (tmp->name && wildcard_match(tmp->name, label)) tftp_ok = 1; } #endif -- 2.25.1
_______________________________________________ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss