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