Hi Simon,

We have chosen to listen by default just on localhost interface, like
default configuration of BIND9 and Unbound has. However I have received
multiple bug reports since that. People do not reconfigure defaults,
because that was not required before. They stumble often on the default.

It would be nice, if default configuration could be ignored the same way
for local-service. But local-service default conflicts with
systemd-resolved for example. Therefore I made a change and propose
listening on just localhost. But behaving similar way to local-service.

The second patch adds simpler way to skip reading default configuration.
Just handle --conf-file= as skipping config file reading. It were just
ignored until now.

What would you think of it?

Cheers,
Petr

-- 
Petr Menšík
Software Engineer
Red Hat, http://www.redhat.com/
email: pemen...@redhat.com
PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB
From 9ce66ed5d42ca66d09ed645ea37cda41161c147b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Tue, 5 Oct 2021 13:46:51 +0200
Subject: [PATCH 1/2] Introduce new locahost-service option
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Similar to local-service, but more strict. Listen only on localhost
unless other interface is specified. Has no effect when interface is
done explicitly. I had multiple bugs fillen on Fedora, because I have
changed default configuration to:

interface=lo
bind-interfaces

People just adding configuration parts to /etc/dnsmasq.d or appending to
existing configuration often fail to see some defaults are already there.
Give them auto-ignored configuration as smart default.

Signed-off-by: Petr Menšík <pemen...@redhat.com>
---
 man/dnsmasq.8 |  4 ++++
 src/dnsmasq.c |  2 ++
 src/dnsmasq.h |  3 ++-
 src/option.c  | 32 ++++++++++++++++++++++++--------
 4 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/man/dnsmasq.8 b/man/dnsmasq.8
index 457667a..6987439 100644
--- a/man/dnsmasq.8
+++ b/man/dnsmasq.8
@@ -257,6 +257,10 @@ only has effect if there are no \fB--interface\fP, \fB--except-interface\fP,
 \fB--listen-address\fP or \fB--auth-server\fP options. It is intended to be set as
 a default on installation, to allow unconfigured installations to be
 useful but also safe from being used for DNS amplification attacks.
+.TP
+.B --localhost-service
+Listen to DNS queries only on localhost interface. Has no effect in similar cases to
+\fB--local-service\fP.
 .TP 
 .B \-2, --no-dhcp-interface=<interface name>
 Do not provide DHCP or TFTP on the specified interface, but do provide DNS service.
diff --git a/src/dnsmasq.c b/src/dnsmasq.c
index cfbbbf1..46d8b74 100644
--- a/src/dnsmasq.c
+++ b/src/dnsmasq.c
@@ -872,6 +872,8 @@ int main (int argc, char **argv)
 
       if (option_bool(OPT_LOCAL_SERVICE))
 	my_syslog(LOG_INFO, _("DNS service limited to local subnets"));
+      else if (option_bool(OPT_LOCALHOST_SERVICE))
+	my_syslog(LOG_INFO, _("DNS service limited to localhost"));
     }
   
   my_syslog(LOG_INFO, _("compile time options: %s"), compile_opts);
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index b2f0b57..f3af87b 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -275,7 +275,8 @@ struct event_desc {
 #define OPT_UMBRELLA_DEVID 64
 #define OPT_CMARK_ALST_EN  65
 #define OPT_QUIET_TFTP     66
-#define OPT_LAST           67
+#define OPT_LOCALHOST_SERVICE  67
+#define OPT_LAST               68
 
 #define OPTION_BITS (sizeof(unsigned int)*8)
 #define OPTION_SIZE ( (OPT_LAST/OPTION_BITS)+((OPT_LAST%OPTION_BITS)!=0) )
diff --git a/src/option.c b/src/option.c
index 485f367..8ad5338 100644
--- a/src/option.c
+++ b/src/option.c
@@ -175,6 +175,7 @@ struct myoption {
 #define LOPT_CMARK_ALST    366
 #define LOPT_QUIET_TFTP    367
 #define LOPT_NFTSET        368
+#define LOPT_LOCALHOST_SVC 369
  
 #ifdef HAVE_GETOPT_LONG
 static const struct option opts[] =  
@@ -207,6 +208,7 @@ static const struct myoption opts[] =
     { "interface", 1, 0, 'i' },
     { "listen-address", 1, 0, 'a' },
     { "local-service", 0, 0, LOPT_LOCAL_SERVICE },
+    { "localhost-service", 0, 0, LOPT_LOCALHOST_SVC },
     { "bogus-priv", 0, 0, 'b' },
     { "bogus-nxdomain", 1, 0, 'B' },
     { "ignore-address", 1, 0, LOPT_IGNORE_ADDR },
@@ -532,6 +534,7 @@ static struct {
   { LOPT_QUIET_RA, OPT_QUIET_RA, NULL, gettext_noop("Do not log RA."), NULL },
   { LOPT_LOG_DEBUG, OPT_LOG_DEBUG, NULL, gettext_noop("Log debugging information."), NULL }, 
   { LOPT_LOCAL_SERVICE, OPT_LOCAL_SERVICE, NULL, gettext_noop("Accept queries only from directly-connected networks."), NULL },
+  { LOPT_LOCALHOST_SVC, OPT_LOCALHOST_SERVICE, NULL, gettext_noop("Listen for queries on localhost interface only."), NULL },
   { LOPT_LOOP_DETECT, OPT_LOOP_DETECT, NULL, gettext_noop("Detect and remove DNS forwarding loops."), NULL },
   { LOPT_IGNORE_ADDR, ARG_DUP, "<ipaddr>", gettext_noop("Ignore DNS responses containing ipaddr."), NULL }, 
   { LOPT_DHCPTTL, ARG_ONE, "<ttl>", gettext_noop("Set TTL in DNS responses with DHCP-derived addresses."), NULL }, 
@@ -1194,6 +1197,16 @@ static void dhcp_opt_free(struct dhcp_opt *opt)
   free(opt);
 }
 
+static void if_names_add(const char *ifname)
+{
+  struct iname *new = opt_malloc(sizeof(struct iname));
+  new->next = daemon->if_names;
+  daemon->if_names = new;
+  /* new->name may be NULL if someone does
+     "interface=" to disable all interfaces except loop. */
+  new->name = opt_string_alloc(ifname);
+  new->used = 0;
+}
 
 /* This is too insanely large to keep in-line in the switch */
 static int parse_dhcp_opt(char *errstr, char *arg, int flags)
@@ -2585,14 +2598,8 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
       
     case 'i':  /* --interface */
       do {
-	struct iname *new = opt_malloc(sizeof(struct iname));
 	comma = split(arg);
-	new->next = daemon->if_names;
-	daemon->if_names = new;
-	/* new->name may be NULL if someone does
-	   "interface=" to disable all interfaces except loop. */
-	new->name = opt_string_alloc(arg);
-	new->used = 0;
+	if_names_add(arg);
 	arg = comma;
       } while (arg);
       break;
@@ -5695,7 +5702,16 @@ void read_opts(int argc, char **argv, char *compile_opts)
   /* If there's access-control config, then ignore --local-service, it's intended
      as a system default to keep otherwise unconfigured installations safe. */
   if (daemon->if_names || daemon->if_except || daemon->if_addrs || daemon->authserver)
-    reset_option_bool(OPT_LOCAL_SERVICE); 
+    {
+      reset_option_bool(OPT_LOCAL_SERVICE);
+      reset_option_bool(OPT_LOCALHOST_SERVICE);
+    }
+  else if (option_bool(OPT_LOCALHOST_SERVICE) && !option_bool(OPT_LOCAL_SERVICE))
+    {
+      /* listen only on localhost */
+      if_names_add(NULL);
+      set_option_bool(OPT_NOWILD);
+    }
 
   if (testmode)
     {
-- 
2.31.1

From 58ef1d8042fbdc8a7e32668ef02a192edd61aa17 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Tue, 5 Oct 2021 14:46:56 +0200
Subject: [PATCH 2/2] Simplify skipping default conf-file
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Support --conf-file= as an alternative to --conf-file=/dev/null. It is
shorter and cleaner, also a bit more obvious.

Signed-off-by: Petr Menšík <pemen...@redhat.com>
---
 man/dnsmasq.8 |  1 +
 src/option.c  | 13 ++++++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/man/dnsmasq.8 b/man/dnsmasq.8
index 6987439..6f42bbd 100644
--- a/man/dnsmasq.8
+++ b/man/dnsmasq.8
@@ -2095,6 +2095,7 @@ Specify a configuration file. The presence of this option stops dnsmasq from rea
 file (normally /etc/dnsmasq.conf). Multiple files may be specified by repeating the option
 either on the command line or in configuration files. A
 filename of "-" causes dnsmasq to read configuration from stdin.
+Empty file just skips reading default configuration file.
 .TP
 .B \-7, --conf-dir=<directory>[,<file-extension>......],
 Read all the files in the given directory as configuration
diff --git a/src/option.c b/src/option.c
index 8ad5338..409be27 100644
--- a/src/option.c
+++ b/src/option.c
@@ -5493,11 +5493,17 @@ void read_opts(int argc, char **argv, char *compile_opts)
       else if (option == 'C')
 	{
           if (!conffile)
-	    conffile = opt_string_alloc(arg);
+	    {
+	      if (!arg[0])
+		conffile = strdup(arg);
+	      else
+		conffile = opt_string_alloc(arg);
+	    }
 	  else
 	    {
 	      char *extra = opt_string_alloc(arg);
-	      one_file(extra, 0);
+	      if (extra)
+		one_file(extra, 0);
 	      free(extra);
 	    }
 	}
@@ -5516,7 +5522,8 @@ void read_opts(int argc, char **argv, char *compile_opts)
 
   if (conffile)
     {
-      one_file(conffile, 0);
+      if (conffile[0])
+	one_file(conffile, 0);
       free(conffile);
     }
   else
-- 
2.31.1

_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss

Reply via email to