[Dnsmasq-discuss] [RFC] dns: add option to ban domains

[Matteo Croce]

I propose adding an option to allow banning some domains.

add `--ban-hosts' which accepts a file name which contains a list of
domains to block, one per line.
Domains are blocked by simply returning NXDOMAIN.

TODO: allow blocking also subdomains via wildcard: eg. *.example.com
---
dnsmasq.conf.example | 3 ++
src/dnsmasq.c | 2 ++
src/dnsmasq.h | 9 ++++++
src/option.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++-
src/rfc1035.c | 16 +++++++++++
5 files changed, 108 insertions(+), 1 deletion(-)

diff --git a/dnsmasq.conf.example b/dnsmasq.conf.example
index 790eaf5..5d824c7 100644
--- a/dnsmasq.conf.example
+++ b/dnsmasq.conf.example
@@ -134,6 +134,9 @@
# automatically added to simple names in a hosts-file.
#expand-hosts

+# List of hostnames to ban, one per line. Queries will simply return NXDOMAIN
+#ban-hosts=/etc/banned_hosts
+
# Set the domain for dnsmasq. this is optional, but if it is set, it
# does the following things.
# 1) Allows DHCP hosts to have fully qualified domain names, as long
diff --git a/src/dnsmasq.c b/src/dnsmasq.c
index 771bec1..56dea1b 100644
--- a/src/dnsmasq.c
+++ b/src/dnsmasq.c
@@ -885,6 +885,8 @@ int main (int argc, char **argv)
/* Using inotify, have to select a resolv file at startup */
poll_resolv(1, 0, now);
#endif
+
+ my_syslog(LOG_INFO, "banned %d domains", banned_host.num);

while (1)
{
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index 24dda08..efc84a8 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1076,6 +1076,15 @@ extern struct daemon {
} *daemon;

/* cache.c */
+
+struct banned_host_t {
+ int num;
+ char **domains;
+ char *data;
+};
+
+extern struct banned_host_t banned_host;
+
void cache_init(void);
void log_query(unsigned int flags, char *name, struct all_addr *addr, char *arg);
char *record_source(unsigned int index);
diff --git a/src/option.c b/src/option.c
index 6a14c4d..e23fbe4 100644
--- a/src/option.c
+++ b/src/option.c
@@ -160,6 +160,7 @@ struct myoption {
#define LOPT_DHCPTTL 348
#define LOPT_TFTP_MTU 349
#define LOPT_REPLY_DELAY 350
+#define LOPT_BAN_HOSTS 351

#ifdef HAVE_GETOPT_LONG
static const struct option opts[] =
@@ -325,6 +326,7 @@ static const struct myoption opts[] =
{ "script-arp", 0, 0, LOPT_SCRIPT_ARP },
{ "dhcp-ttl", 1, 0 , LOPT_DHCPTTL },
{ "dhcp-reply-delay", 1, 0, LOPT_REPLY_DELAY },
+ { "ban-hosts", 1, 0, LOPT_BAN_HOSTS },
{ NULL, 0, 0, 0 }
};

@@ -497,6 +499,7 @@ static struct {
{ 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 },
{ LOPT_REPLY_DELAY, ARG_ONE, "<integer>", gettext_noop("Delay DHCP replies for at least number of seconds."), NULL },
+ { LOPT_BAN_HOSTS, ARG_ONE, "<path>", gettext_noop("File containing domains to block."), NULL },
{ 0, 0, NULL, NULL, NULL }
};

@@ -683,6 +686,15 @@ static void add_txt(char *name, char *txt, int stat)
}
#endif

+static int cmpstringp(const void *p1, const void *p2)
+{
+ /* The actual arguments to this function are "pointers to
+ pointers to char", but strcmp(3) arguments are "pointers
+ to char", hence the following cast plus dereference */
+
+ return strcmp(* (char * const *) p1, * (char * const *) p2);
+}
+
static void do_usage(void)
{
char buff[100];
@@ -4172,7 +4184,72 @@ err:
break;
}
#endif
-
+
+ case LOPT_BAN_HOSTS:
+ {
+ int i, j;
+ char *last;
+ struct stat statbuf;
+ FILE *ban_file = fopen(arg, "r");
+
+ /* Open banned hosts file and read its size */
+ if (!ban_file)
+ ret_err(_("can't open banhost file"));
+ stat(arg, &statbuf);
+
+ /* Allocate a buffer and read the whole file in it */
+ banned_host.num = 0;
+ banned_host.data = opt_malloc(statbuf.st_size + 1);
+ if (fread(banned_host.data, 1, statbuf.st_size, ban_file) != (size_t)statbuf.st_size)
+ {
+ free(banned_host.data);
+ goto out_close;
+ }
+ banned_host.data[statbuf.st_size] = 0;
+
+ /* Count the domains to allocate the pointers array */
+ for (i = 0; i < statbuf.st_size; i++)
+ {
+ /* Translates \n to NULL */
+ if (banned_host.data[i] == '\n')
+ {
+ banned_host.data[i] = 0;
+ banned_host.num++;
+ }
+ }
+
+ /* Allocate the domain pointers array and fill it */
+ banned_host.domains = opt_malloc(banned_host.num * sizeof(char*));
+ last = banned_host.data;
+ j = 0;
+ for (i = 0; i < statbuf.st_size; i++)
+ {
+ if (!banned_host.data[i])
+ {
+ /* Skip empty lines */
+ if (i)
+ banned_host.domains[j++] = last;
+ for (i++; i < statbuf.st_size; i++)
+ {
+ if (banned_host.data[i])
+ {
+ last = banned_host.data + i;
+ break;
+ }
+ }
+ }
+ }
+ /* Adjust the host number */
+ banned_host.num = j;
+
+ qsort(banned_host.domains, banned_host.num, sizeof(char*), cmpstringp);
+
+ out_close:
+ fclose(ban_file);
+
+ break;
+ }
+
default:
ret_err(_("unsupported option (check that dnsmasq was compiled with DHCP/TFTP/DNSSEC/DBus support)"));

diff --git a/src/rfc1035.c b/src/rfc1035.c
index 26f5301..82e471c 100644
--- a/src/rfc1035.c
+++ b/src/rfc1035.c
@@ -16,6 +16,15 @@

#include "dnsmasq.h"

+struct banned_host_t banned_host;
+
+static int bsearch_strcmp(const void *s1, const void *s2)
+{
+ const char *key = s1;
+ const char * const *arg = s2;
+ return strcmp(key, *arg);
+}
+
int extract_name(struct dns_header *header, size_t plen, unsigned char **pp,
char *name, int isExtract, int extrabytes)
{
@@ -1223,6 +1232,13 @@ size_t answer_request(struct dns_header *header, char *limit, size_t qlen,
struct mx_srv_record *rec;
size_t len;

+ if (bsearch(name, banned_host.domains, banned_host.num, sizeof(char*), bsearch_strcmp))
+ {
+ header->hb3 = 0x81;
+ header->hb4 = 0x83;
+ return qlen;
+ }
+
/* Clear buffer beyond request to avoid risk of
information disclosure. */
memset(((char *)header) + qlen, 0,

[w***@gmail.com]

Post by Matteo Croce
I propose adding an option to allow banning some domains.
add `--ban-hosts' which accepts a file name which contains a list of
domains to block, one per line.
Domains are blocked by simply returning NXDOMAIN.

is the following in dnsmasq.conf broken???


# block these domains with NXDOMAIN
server=/example.com/
server=/facebook.com/
server=/fbcdn.net/
server=/fbcdn.com/
server=/facebook.net/

[Matteo Croce]

Post by w***@gmail.com

Post by Matteo Croce
I propose adding an option to allow banning some domains.
add `--ban-hosts' which accepts a file name which contains a list of
domains to block, one per line.
Domains are blocked by simply returning NXDOMAIN.

is the following in dnsmasq.conf broken???
# block these domains with NXDOMAIN
server=/example.com/
server=/facebook.com/
server=/fbcdn.net/
server=/fbcdn.com/
server=/facebook.net/

Nope, but it's unpractical when the ban list is huge

# wc -l /etc/banhosts
13090 /etc/banhosts

also, having it in a separate file will allow updating it without
messing with the configuration file

[w***@gmail.com]

Post by Matteo Croce

Post by w***@gmail.com

Post by Matteo Croce
I propose adding an option to allow banning some domains.
add `--ban-hosts' which accepts a file name which contains a list of
domains to block, one per line.
Domains are blocked by simply returning NXDOMAIN.

is the following in dnsmasq.conf broken???
# block these domains with NXDOMAIN
server=/example.com/
server=/facebook.com/
server=/fbcdn.net/
server=/fbcdn.com/
server=/facebook.net/

Nope, but it's unpractical when the ban list is huge

impractical?

Post by Matteo Croce
# wc -l /etc/banhosts
13090 /etc/banhosts
also, having it in a separate file will allow updating it without
messing with the configuration file

well, you asked for comments so i did... as for separate files, can't it be done
in another file that is included in the main one? i can't remember if dnsmasq
allows one to include additional files or not...

eg: include bannedhosts.conf


maybe i'm just not seeing the overall point as compared to existing capabilities?

[Kevin Darbyshire-Bryant]

Post by w***@gmail.com

Post by Matteo Croce

Post by w***@gmail.com

Post by Matteo Croce
I propose adding an option to allow banning some domains.
add `--ban-hosts' which accepts a file name which contains a list of
domains to block, one per line.
Domains are blocked by simply returning NXDOMAIN.

is the following in dnsmasq.conf broken???
# block these domains with NXDOMAIN
server=/example.com/
server=/facebook.com/
server=/fbcdn.net/
server=/fbcdn.com/
server=/facebook.net/

Nope, but it's unpractical when the ban list is huge

impractical?

Post by Matteo Croce
# wc -l /etc/banhosts
13090 /etc/banhosts
also, having it in a separate file will allow updating it without
messing with the configuration file

well, you asked for comments so i did... as for separate files, can't it
be done in another file that is included in the main one? i can't
remember if dnsmasq allows one to include additional files or not...

LEDE/Openwrt does exactly that. The startup script conditionally
includes a config file with a list of RFC6761 related domains to never
forward "--conf-file=$RFC6761FILE" - The referenced file contains
"server=/exclude/" type references.

So the functionality is already there, though not quite with perfect
syntax in the sense that 'server=/ /' is repeated each line.

How is the 'ban-hosts' file updated? Does it need a SIGHUP to dnsmasq
(please not another thing hanging off SIGHUP) Does it need a complete
restart?

If 'ban-hosts' can be dynamically updated then I can see some value in
it, until then it looks like it's a syntax nicety. Perhaps there's some
other feature we're all missing... is it faster for example?

Kevin

[r***@gmail.com]

Post by Kevin Darbyshire-Bryant
How is the 'ban-hosts' file updated? Does it need a SIGHUP to dnsmasq
(please not another thing hanging off SIGHUP) Does it need a complete
restart?
If 'ban-hosts' can be dynamically updated then I can see some value in it,
until then it looks like it's a syntax nicety. Perhaps there's some other
feature we're all missing... is it faster for example?
Kevin

There is already --servers-file, which can be reloaded without restarting
dnsmasq (via SIGHUP).


And even if --ban-hosts is added as a syntax nicety, what's the rationale
for not just internally using the same well-tested data structure already
implemented for domains banned via --server? If that lookup isn't
efficient enough, better to improve it instead of adding a parallel one.