On Thursday 21 January 2021 14:52:22 Matthias May wrote: > On 21/01/2021 14:20, Pali Rohár wrote: > > Hello! > > > > On Thursday 21 January 2021 14:00:10 Matthias May wrote: > >> On Linux when IPv6 is disabled, one would not expect Dnsmasq to bind > >> the IPv6 wildcard. > > > > And it is a problem? I would expect that if IPv6 is disabled then > > applications (including dnsmasq) cannot bind to the IPv6 address for > > listening... Or can applications bind to IPv6 address also when IPv6 is > > disabled? I just have not caught what is the problem (from description). > > > > Well not a problem per se, but i was quite surprised to see dnsmasq binding > :: with IPv6 disabled. > Yes it seems to be possible to bind :: while IPv6 is disabled.
Hello! Now I see where is the issue. And seems that you misunderstood meaning of the all/disable_ipv6 sysctl value. Value of /proc/sys/net/ipv6/conf/all/disable_ipv6 can be 1 also in the case when you have assigned IPv6 addresses and you have IPv6 sockets in use. You try it yourself. Call following commands: # echo 1 > /proc/sys/net/ipv6/conf/all/disable_ipv6 # echo 0 > /proc/sys/net/ipv6/conf/eth0/disable_ipv6 First command disable IPv6 on all interfaces and then second command enable IPv6 just for eth0. Also in this state you can verify that all/disable_ipv6 is still 1, but explicitly enabled eth0 is 0: # cat /proc/sys/net/ipv6/conf/all/disable_ipv6 1 # cat /proc/sys/net/ipv6/conf/eth0/disable_ipv6 0 And IPv6 is working fine in this state on eth0. So checking for /proc/sys/net/ipv6/conf/all/disable_ipv6 whether IPv6 is enabled or disabled is incorrect. Correct check should be really trying to opening AF_INET6 socket and checking if bind() success on wildcard :: IPv6 address. So your proposed patch breaks IPv6 support for machines where IPv6 is disabled by default and enabled only for explicitly whitelisted interfaces. You should look at other sysfs files about IPv6 state. Probably some of them are: /sys/module/ipv6/parameters/disable /sys/module/ipv6/parameters/disable_ipv6 > See the previous discussion on this list at: > https://www.mail-archive.com/dnsmasq-discuss@lists.thekelleys.org.uk/msg14032.html > > >> This patch adds a condition to the wildcard bind function, which checks > >> on Linux if IPv6 is disabled. > >> > >> Signed-off-by: Matthias May <matthias....@westermo.com> > >> Signed-off-by: Zefir Kurtisi <zefir.kurt...@westermo.com> > >> --- > >> src/dnsmasq.h | 1 + > >> src/network.c | 24 +++++++++++++----------- > >> src/util.c | 22 ++++++++++++++++++++++ > >> 3 files changed, 36 insertions(+), 11 deletions(-) > >> > >> diff --git a/src/dnsmasq.h b/src/dnsmasq.h > >> index 4220798..2cfb1cb 100644 > >> --- a/src/dnsmasq.h > >> +++ b/src/dnsmasq.h > >> @@ -1292,6 +1292,7 @@ int read_write(int fd, unsigned char *packet, int > >> size, int rw); > >> void close_fds(long max_fd, int spare1, int spare2, int spare3); > >> int wildcard_match(const char* wildcard, const char* match); > >> int wildcard_matchn(const char* wildcard, const char* match, int num); > >> +int is_ipv6_disabled(void); > >> #ifdef HAVE_LINUX_NETWORK > >> int kernel_version(void); > >> #endif > >> diff --git a/src/network.c b/src/network.c > >> index c7d002b..0d35fb7 100644 > >> --- a/src/network.c > >> +++ b/src/network.c > >> @@ -990,19 +990,21 @@ void create_wildcard_listeners(void) > >> > >> l = create_listeners(&addr, !!option_bool(OPT_TFTP), 1); > >> > >> - memset(&addr, 0, sizeof(addr)); > >> + if (!is_ipv6_disabled()) { > >> + memset(&addr, 0, sizeof(addr)); > >> #ifdef HAVE_SOCKADDR_SA_LEN > >> - addr.in6.sin6_len = sizeof(addr.in6); > >> + addr.in6.sin6_len = sizeof(addr.in6); > >> #endif > >> - addr.in6.sin6_family = AF_INET6; > >> - addr.in6.sin6_addr = in6addr_any; > >> - addr.in6.sin6_port = htons(daemon->port); > >> - > >> - l6 = create_listeners(&addr, !!option_bool(OPT_TFTP), 1); > >> - if (l) > >> - l->next = l6; > >> - else > >> - l = l6; > >> + addr.in6.sin6_family = AF_INET6; > >> + addr.in6.sin6_addr = in6addr_any; > >> + addr.in6.sin6_port = htons(daemon->port); > >> + > >> + l6 = create_listeners(&addr, !!option_bool(OPT_TFTP), 1); > >> + if (l) > >> + l->next = l6; > >> + else > >> + l = l6; > >> + } > >> > >> daemon->listeners = l; > >> } > >> diff --git a/src/util.c b/src/util.c > >> index 5f13027..5cd461f 100644 > >> --- a/src/util.c > >> +++ b/src/util.c > >> @@ -787,6 +787,28 @@ int wildcard_matchn(const char* wildcard, const char* > >> match, int num) > >> return (!num) || (*wildcard == *match); > >> } > >> > >> +#ifndef HAVE_LINUX_NETWORK > >> +/* implement for other platforms */ > >> +int is_ipv6_disabled(void) > >> +{ > >> + return 0; > >> +} > >> +#else /* HAVE_LINUX_NETWORK */ > >> +int is_ipv6_disabled(void) > >> +{ > >> + FILE *f; > >> + char *fname = "/proc/sys/net/ipv6/conf/all/disable_ipv6"; > >> + char buf[4]; > >> + int ipv6_disabled = 0; > >> + if ((f = fopen(fname, "r"))) { > >> + if (fgets(buf, 4, f)) > >> + ipv6_disabled = atoi(buf) == 1; > >> + fclose(f); > >> + } > >> + return ipv6_disabled; > > > > This check is incorrect. If IPv6 support is disabled at kernel compile > > time then fopen() fails and this function returns 0, meaning IPv6 is > > enabled. > > True. > Although when IPv6 support is not even compiled in, i would guess that the > attempt to bind :: will also fail. > Functionally i don't see a difference. > But you are right, the function isn't doing what its name says. > Will send a v2. > > > > >> +} > >> +#endif /* HAVE_LINUX_NETWORK */ > >> + > >> #ifdef HAVE_LINUX_NETWORK > >> int kernel_version(void) > >> { > >> -- > >> 2.27.0 > >> > >> _______________________________________________ > >> Dnsmasq-discuss mailing list > >> Dnsmasq-discuss@lists.thekelleys.org.uk > >> https://urldefense.com/v3/__http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss__;!!I9LPvj3b!Tm5sQkbTBCVA0WatEAaSi5Dn2ao9xNGjrzD85qAFtb8F6NfsJiCwHQogwebRlh5_cp8$ > >> > > > > BR > Matthias -- Pali Rohár pali.ro...@gmail.com _______________________________________________ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
