Hey Simon,

I recently noticed that the "--rev-server" option is limited to 8, 16,
24, and 32 bit prefixes for IPv4 addresses. Since I needed support for
22 bit subnets in the network I'm currently setting up, I decided to
extend dnsmasq to support arbitrary IPv4 prefix-lengths. This mail has
three patches attached that contain my changes. I hope this is helpful
and can be merged into upstream dnsmasq.

Patch 1 implements support for arbitrary IPv4 prefixes by largely
rewriting option.c:add_rev4() and option.c:add_rev6()

Patch 2 adds error checking for IPv6 prefix-lengths as the current
algorithm only makes sense in 4 bit steps. As 4 bit steps in 128 bit
wide addresses seems sufficient to me, I didn't change the nice and
short algorithm used here.

Patch 3 adds logging for the inner details of what rev-server is doing.
It is an entirely optional patch. However, I would still recommend
adding it as it facilitates understanding of what the rev-server option
actually does (and if it does what the user is thinking it should be doing).

Exemplary log outputs:

./dnsmasq --rev-server=10.0.3.10/22,10.0.0.1
>>> dnsmasq[29637]: rev-server 10.0.3.10/22: address-to-name queries for
10.0.0.0 to 10.0.3.255 are sent to 10.0.0.1

./dnsmasq --rev-server=10.0.3.10/17,10.0.0.1#5353
>>> dnsmasq[29662]: rev-server 10.0.3.10/17: address-to-name queries for
10.0.0.0 to 10.0.127.255 are sent to 10.0.0.1#5353

./dnsmasq --rev-server=fe80::3aea:a711:fea1:2101/64,10.0.0.1
>>> dnsmasq[29667]: rev-server fe80::3aea:a711:fea1:2101/64:
address-to-name queries for fe80::3a00:0:0:0 to
fe80::3aff:ffff:ffff:ffff are sent to 10.0.0.1

(the displayed IPv6 addresses try to be as short as possible)

Best regards,
Dominik

From d038771987084c09df60af9c9c117e3fe14cdaa8 Mon Sep 17 00:00:00 2001
From: Dominik Derigs <dl...@dl6er.de>
Date: Tue, 14 Apr 2020 19:40:21 +0200
Subject: [PATCH 3/3] Add explanatory log message to rev-server to facilitate
 understanding of the --rev-server option.
---
 src/option.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 68 insertions(+), 7 deletions(-)

diff --git a/src/option.c b/src/option.c
index 809739e..1cb3caf 100644
--- a/src/option.c
+++ b/src/option.c
@@ -914,7 +914,7 @@ static int add_rev_serv(char* domain, char *comma, const int serv_flags, char *e
   return 1;
 }
 
-static int add_rev4(struct in_addr addr, int msize, char *comma, const int serv_flags, char *errstr)
+static int add_rev4(struct in_addr addr, int msize, char *comma, const int serv_flags, int explain, char *errstr)
 {
   int i;
   unsigned char a[3];
@@ -930,6 +930,25 @@ static int add_rev4(struct in_addr addr, int msize, char *comma, const int serv_
   a_min.s_addr = addr.s_addr & bitmask.s_addr;
   a_max.s_addr = a_min.s_addr + ~bitmask.s_addr;
 
+  /* Print derived subnet address range to ease configuration (if requested) */
+  if(explain)
+    {
+      char original_address[INET_ADDRSTRLEN], min_address[INET_ADDRSTRLEN], max_address[INET_ADDRSTRLEN];
+      inet_ntop(AF_INET, &addr, original_address, sizeof(original_address));
+      inet_ntop(AF_INET, &a_min, min_address, sizeof(min_address));
+      if(a_min.s_addr != a_max.s_addr)
+	{
+	  inet_ntop(AF_INET, &a_max, max_address, sizeof(max_address));
+	  my_syslog(LOG_INFO, "rev-server %s/%i: address-to-name queries for %s to %s are sent to %s",
+		    original_address, msize, min_address, max_address, comma);
+	}
+      else
+	{
+	  my_syslog(LOG_INFO, "rev-server %s/%i: address-to-name queries for %s are sent to %s",
+		    original_address, msize, min_address, comma);
+	}
+    }
+
   /* Extract IP address octets in host-format for in-addr.arpa string-assembly */
   for(i = 0; i < 3; i++)
     a[i] = (ntohl(a_min.s_addr) >> 8*(3-i)) & 0xFF;
@@ -961,7 +980,7 @@ static int add_rev4(struct in_addr addr, int msize, char *comma, const int serv_
   return 1;
 }
 
-static int add_rev6(struct in6_addr *addr, int msize, char *comma, const int serv_flags, char *errstr)
+static int add_rev6(struct in6_addr *addr, int msize, char *comma, const int serv_flags, int explain, char *errstr)
 {
   int i;
   char *p, *domain;
@@ -970,7 +989,49 @@ static int add_rev6(struct in6_addr *addr, int msize, char *comma, const int ser
     ret_err("bad prefix");
 
   p = domain = opt_malloc(73); /* strlen("32*<n.>ip6.arpa")+1 */
-  
+
+  /* Print derived subnet address range to ease configuration (if requested) */
+  if(explain)
+    {
+      struct in6_addr bitmask, a_min, a_max;
+      memset(&bitmask, 0, sizeof(bitmask));
+      for(i = 0; i < 16; i++)
+	{
+	  if(msize >= 8*i)
+	    bitmask.s6_addr[i] = 0xFF;
+	  else if(msize > 8*(i) && msize < 8*(i+1))
+	    bitmask.s6_addr[i] = ~(((uint16_t)1 << (8 - (msize%8))) - 1) & 0xFF;
+	  else
+	    break;
+	}
+
+      /* Apply bitmask to address to compute subnet address range */
+      int addresses_differ = 0;
+      for(i = 0; i < 16; i++)
+	{
+	  a_min.s6_addr[i] = (*addr).s6_addr[i] & bitmask.s6_addr[i];
+	  a_max.s6_addr[i] = a_min.s6_addr[i] + ~bitmask.s6_addr[i];
+	  if(a_min.s6_addr[i] != a_max.s6_addr[i])
+	    addresses_differ = 1;
+	}
+
+      /* Print derived subnet address range */
+      char original_address[INET6_ADDRSTRLEN], min_address[INET6_ADDRSTRLEN], max_address[INET6_ADDRSTRLEN];
+      inet_ntop(AF_INET6, addr, original_address, sizeof(original_address));
+      inet_ntop(AF_INET6, &a_min, min_address, sizeof(min_address));
+      if(addresses_differ)
+	{
+	  inet_ntop(AF_INET6, &a_max, max_address, sizeof(max_address));
+	  my_syslog(LOG_INFO, "rev-server %s/%i: address-to-name queries for %s to %s are sent to %s",
+		original_address, msize, min_address, max_address, comma);
+	}
+      else
+	{
+	  my_syslog(LOG_INFO, "rev-server %s/%i: address-to-name queries for %s are sent to %s",
+		original_address, msize, min_address, comma);
+	}
+    }
+
   for (i = msize-1; i >= 0; i -= 4)
     { 
       int dig = ((unsigned char *)addr)[i>>3];
@@ -2309,7 +2370,7 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
 				{
 				   /* generate the equivalent of
 				      local=/xxx.yyy.zzz.in-addr.arpa/ */
-				  if (!add_rev4(new->start, msize, NULL, SERV_NO_ADDR, errstr))
+				  if (!add_rev4(new->start, msize, NULL, SERV_NO_ADDR, 0, errstr))
 				    {
 				      free(new);
 				      return 0;
@@ -2355,7 +2416,7 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
 				{
 				  /* generate the equivalent of
 				     local=/xxx.yyy.zzz.ip6.arpa/ */
-				  if (!add_rev6(&new->start6, msize, NULL, SERV_NO_ADDR, errstr))
+				  if (!add_rev6(&new->start6, msize, NULL, SERV_NO_ADDR, 0, errstr))
 				    {
 				      free(new);
 				      return 0;
@@ -2697,12 +2758,12 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
 
 	if (inet_pton(AF_INET, arg, &addr4))
 	  {
-	    if (!add_rev4(addr4, size, comma, serv_flags, errstr))
+	    if (!add_rev4(addr4, size, comma, serv_flags, 1, errstr))
 	      return 0;
 	  }
 	else if (inet_pton(AF_INET6, arg, &addr6))
 	  {
-	    if(!add_rev6(&addr6, size, comma, serv_flags, errstr))
+	    if(!add_rev6(&addr6, size, comma, serv_flags, 1, errstr))
 	      return 0;
 	  }
 	else
-- 
2.17.1

From 119c9d4ac033a64b450e8fd5edb6be28d5878a81 Mon Sep 17 00:00:00 2001
From: Dominik Derigs <dl...@dl6er.de>
Date: Tue, 14 Apr 2020 19:24:12 +0200
Subject: [PATCH 2/3] Check for valid prefix-length in IPv6 rev-server
 settings. The current algorithm does only make sense for prefix-lengths which
 are not a multiple of 4. This should still be sufficient for IPv6. However,
 we should complain in case an unspported prefix is provided instead of
 silently accepting but not using it.
---
 src/option.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/option.c b/src/option.c
index 0ad2fe1..809739e 100644
--- a/src/option.c
+++ b/src/option.c
@@ -966,6 +966,9 @@ static int add_rev6(struct in6_addr *addr, int msize, char *comma, const int ser
   int i;
   char *p, *domain;
 
+  if(msize < 1 || msize > 128 || msize%4)
+    ret_err("bad prefix");
+
   p = domain = opt_malloc(73); /* strlen("32*<n.>ip6.arpa")+1 */
   
   for (i = msize-1; i >= 0; i -= 4)
-- 
2.17.1

From 87eb3f5c3786ed141a348a9ca5db744948f9605c Mon Sep 17 00:00:00 2001
From: Dominik Derigs <dl...@dl6er.de>
Date: Tue, 14 Apr 2020 19:20:52 +0200
Subject: [PATCH 1/3] Add support for arbitrary IPv4 prefix-lengths in
 --rev-server.
---
 man/dnsmasq.8 |   9 ++-
 src/option.c  | 162 ++++++++++++++++++++++++++++++--------------------
 2 files changed, 105 insertions(+), 66 deletions(-)

diff --git a/man/dnsmasq.8 b/man/dnsmasq.8
index a2a60d5..c836ae5 100644
--- a/man/dnsmasq.8
+++ b/man/dnsmasq.8
@@ -493,9 +493,14 @@ implemented on all platforms supported by dnsmasq.
 This is functionally the same as 
 .B --server, 
 but provides some syntactic sugar to make specifying address-to-name queries easier. For example
-.B --rev-server=1.2.3.0/24,192.168.0.1
+.B --rev-server=192.168.3.0/24,192.168.0.1
 is exactly equivalent to 
-.B --server=/3.2.1.in-addr.arpa/192.168.0.1
+.B --server=/3.168.192.in-addr.arpa/192.168.0.1
+while
+.B --rev-server=192.168.3.10/22,192.168.0.1
+is equivalent to 
+.B --server=/0.168.192.in-addr.arpa/192.168.0.1 --server=/1.168.192.in-addr.arpa/192.168.0.1 --server=/2.168.192.in-addr.arpa/192.168.0.1 --server=/3.168.192.in-addr.arpa/192.168.0.1 
+While any valid prefix-length may be used with IPv4 addresses (/1 - /32), dnsmasq only accepts IPv6 prefix-lengths in steps of 4 (/4, /8, /12, ..., /128).
 .TP
 .B \-A, --address=/<domain>[/<domain>...]/[<ipaddr>]
 Specify an IP address to return for any host in the given domains.
diff --git a/src/option.c b/src/option.c
index 1f698da..0ad2fe1 100644
--- a/src/option.c
+++ b/src/option.c
@@ -886,53 +886,87 @@ char *parse_server(char *arg, union mysockaddr *addr, union mysockaddr *source_a
   return NULL;
 }
 
-static struct server *add_rev4(struct in_addr addr, int msize)
+static int add_rev_serv(char* domain, char *comma, const int serv_flags, char *errstr)
 {
+  char *string;
   struct server *serv = opt_malloc(sizeof(struct server));
-  in_addr_t  a = ntohl(addr.s_addr);
-  char *p;
-
   memset(serv, 0, sizeof(struct server));
-  p = serv->domain = opt_malloc(29); /* strlen("xxx.yyy.zzz.ttt.in-addr.arpa")+1 */
 
-  switch (msize)
-    {
-    case 32:
-      p += sprintf(p, "%u.", a & 0xff);
-      /* fall through */
-    case 24:
-      p += sprintf(p, "%d.", (a >> 8) & 0xff);
-      /* fall through */
-    case 16:
-      p += sprintf(p, "%d.", (a >> 16) & 0xff);
-      /* fall through */
-    case 8:
-      p += sprintf(p, "%d.", (a >> 24) & 0xff);
-      break;
-    default:
-      free(serv->domain);
-      free(serv);
-      return NULL;
-    }
-
-  p += sprintf(p, "in-addr.arpa");
-  
+  /* Append server to linked list */
+  serv->domain = domain;
   serv->flags = SERV_HAS_DOMAIN;
   serv->next = daemon->servers;
   daemon->servers = serv;
 
-  return serv;
+  /* Parse target if applicable */
+  if(comma != NULL)
+    {
+      string = parse_server(comma, &serv->addr, &serv->source_addr, serv->interface, &serv->flags);
+      if (string)
+	ret_err(string);
+    }
+
+  /* Append additional server flags if applicable */
+  if (serv_flags)
+    serv->flags |= serv_flags;
 
+  /* No error */
+  return 1;
 }
 
-static struct server *add_rev6(struct in6_addr *addr, int msize)
+static int add_rev4(struct in_addr addr, int msize, char *comma, const int serv_flags, char *errstr)
 {
-  struct server *serv = opt_malloc(sizeof(struct server));
-  char *p;
   int i;
-				  
-  memset(serv, 0, sizeof(struct server));
-  p = serv->domain = opt_malloc(73); /* strlen("32*<n.>ip6.arpa")+1 */
+  unsigned char a[3];
+  struct in_addr bitmask, a_min, a_max;
+
+  if(msize < 1 || msize > 32)
+    ret_err("bad prefix");
+
+  /* Construct binary mask from specified prefix-length */
+  bitmask.s_addr = ~htonl((1 << (32 - msize)) - 1);
+
+  /* Apply bitmask to address to compute subnet address range */
+  a_min.s_addr = addr.s_addr & bitmask.s_addr;
+  a_max.s_addr = a_min.s_addr + ~bitmask.s_addr;
+
+  /* Extract IP address octets in host-format for in-addr.arpa string-assembly */
+  for(i = 0; i < 3; i++)
+    a[i] = (ntohl(a_min.s_addr) >> 8*(3-i)) & 0xFF;
+
+  /* Derive number of octets to be printed during string-assembly */
+  unsigned int octnum = ((msize-1)/8);
+
+  /* Extract min and max address range for string-assembly loop */
+  int min_cnt = (ntohl(a_min.s_addr) >> 8*(3-octnum)) & 0xFF;
+  int max_cnt = (ntohl(a_max.s_addr) >> 8*(3-octnum)) & 0xFF;
+  for(i = min_cnt; i <= max_cnt; i++)
+    {
+      char *domain = opt_malloc(29); /* strlen("xxx.yyy.zzz.ttt.in-addr.arpa")+1 */
+      if(octnum == 3)
+	sprintf(domain, "%u.%u.%u.%u.in-addr.arpa", i, a[2], a[1], a[0]);
+      else if(octnum == 2)
+	sprintf(domain, "%u.%u.%u.in-addr.arpa", i, a[1], a[0]);
+      else if(octnum == 1)
+	sprintf(domain, "%u.%u.in-addr.arpa", i, a[0]);
+      else
+	sprintf(domain, "%u.in-addr.arpa", i);
+
+      /* Append server information, return 0 in case of an error */
+      if(!add_rev_serv(domain, comma, serv_flags, errstr))
+	return 0;
+    }
+
+  /* No error */
+  return 1;
+}
+
+static int add_rev6(struct in6_addr *addr, int msize, char *comma, const int serv_flags, char *errstr)
+{
+  int i;
+  char *p, *domain;
+
+  p = domain = opt_malloc(73); /* strlen("32*<n.>ip6.arpa")+1 */
   
   for (i = msize-1; i >= 0; i -= 4)
     { 
@@ -940,12 +974,13 @@ static struct server *add_rev6(struct in6_addr *addr, int msize)
       p += sprintf(p, "%.1x.", (i>>2) & 1 ? dig & 15 : dig >> 4);
     }
   p += sprintf(p, "ip6.arpa");
-  
-  serv->flags = SERV_HAS_DOMAIN;
-  serv->next = daemon->servers;
-  daemon->servers = serv;
-  
-  return serv;
+
+  /* Append server information, return 0 in case of an error */
+  if(!add_rev_serv(domain, comma, serv_flags, errstr))
+    return 0;
+
+  /* No error */
+  return 1;
 }
 
 #ifdef HAVE_DHCP
@@ -2271,14 +2306,14 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
 				{
 				   /* generate the equivalent of
 				      local=/xxx.yyy.zzz.in-addr.arpa/ */
-				  struct server *serv = add_rev4(new->start, msize);
-				  if (!serv)
-				    ret_err_free(_("bad prefix"), new);
-
-				  serv->flags |= SERV_NO_ADDR;
+				  if (!add_rev4(new->start, msize, NULL, SERV_NO_ADDR, errstr))
+				    {
+				      free(new);
+				      return 0;
+				    }
 
 				  /* local=/<domain>/ */
-				  serv = opt_malloc(sizeof(struct server));
+				  struct server *serv = opt_malloc(sizeof(struct server));
 				  memset(serv, 0, sizeof(struct server));
 				  serv->domain = d;
 				  serv->flags = SERV_HAS_DOMAIN | SERV_NO_ADDR;
@@ -2317,11 +2352,14 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
 				{
 				  /* generate the equivalent of
 				     local=/xxx.yyy.zzz.ip6.arpa/ */
-				  struct server *serv = add_rev6(&new->start6, msize);
-				  serv->flags |= SERV_NO_ADDR;
-				  
+				  if (!add_rev6(&new->start6, msize, NULL, SERV_NO_ADDR, errstr))
+				    {
+				      free(new);
+				      return 0;
+				    }
+
 				  /* local=/<domain>/ */
-				  serv = opt_malloc(sizeof(struct server));
+				  struct server *serv = opt_malloc(sizeof(struct server));
 				  memset(serv, 0, sizeof(struct server));
 				  serv->domain = d;
 				  serv->flags = SERV_HAS_DOMAIN | SERV_NO_ADDR;
@@ -2638,8 +2676,7 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
     case LOPT_REV_SERV: /* --rev-server */
       {
 	char *string;
-	int size;
-	struct server *serv;
+	int size, serv_flags = 0;
 	struct in_addr addr4;
 	struct in6_addr addr6;
  
@@ -2652,24 +2689,21 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
 	if (!(string = split_chr(arg, '/')) || !atoi_check(string, &size))
 	  ret_err(gen_err);
 	
+	if (servers_only)
+	  serv_flags = SERV_FROM_FILE;
+
 	if (inet_pton(AF_INET, arg, &addr4))
 	  {
-	    serv = add_rev4(addr4, size);
-	    if (!serv)
-	      ret_err(_("bad prefix"));
+	    if (!add_rev4(addr4, size, comma, serv_flags, errstr))
+	      return 0;
 	  }
 	else if (inet_pton(AF_INET6, arg, &addr6))
-	  serv = add_rev6(&addr6, size);
+	  {
+	    if(!add_rev6(&addr6, size, comma, serv_flags, errstr))
+	      return 0;
+	  }
 	else
 	  ret_err(gen_err);
- 
-	string = parse_server(comma, &serv->addr, &serv->source_addr, serv->interface, &serv->flags);
-	
-	if (string)
-	  ret_err(string);
-	
-	if (servers_only)
-	  serv->flags |= SERV_FROM_FILE;
 	
 	break;
       }
-- 
2.17.1

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

Reply via email to