[Dnsmasq-discuss] rev-server=fe80::/10, 192.168.178.1 no longer accepted (version 2.86)

2021-09-10 Thread joerg

Hello

After upgrading dnsmasq from 2.85 to 2.86 (debian sid) the rev-server 
lines regarding IPv6 are no longer accepted


error:

bad IPv6 prefix at line 26 of /etc/dnsmasq.d/05-local.conf

config line:

rev-server=fe80::/10,192.168.178.1

Best Regards

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


Re: [Dnsmasq-discuss] rev-server=fe80::/10, 192.168.178.1 no longer accepted (version 2.86)

2021-09-10 Thread Dominik DL6ER
Hey Joerg,

On Fri, 2021-09-10 at 10:13 +0200, jo...@schuetter.org wrote:
> rev-server=fe80::/10,192.168.178.1

dnsmasq always only accepted IPv6 prefixes that are a multiple of
4. There was just no enforcing of this before v2.86.

Looking at the v2.85 code, dnsmasq will have added
> fe80::/8
for you in version v2.85 without any warnings.

I submitted patches for supporting arbitrary prefix lengths (IPv4
is even more restrictive (only /8, /16, /24 and /32 are allowed)
last year. I already rebased my patches on the current master and
will submit them in the next few days. This means support for /10
might come in the next release (if the patches are accepted).

Best,
Dominik


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


Re: [Dnsmasq-discuss] rev-server=fe80::/10, 192.168.178.1 no longer accepted (version 2.86)

2021-09-10 Thread joerg

Hello Dominik

On 2021-09-10 12:27, Dominik DL6ER wrote:


Hey Joerg,

On Fri, 2021-09-10 at 10:13 +0200, jo...@schuetter.org wrote:


rev-server=fe80::/10,192.168.178.1


dnsmasq always only accepted IPv6 prefixes that are a multiple of
4. There was just no enforcing of this before v2.86.


From a DNS perspective the block size of 4 (or 8 for IPv4) absolutely 
makes
sense, but having the option to provide the CIDR with any figure (as 
comon

on routing) would be great. Thanks for pointing out the issue.

I have added the address with four /12 networks:
rev-server=fe80::/12,192.168.178.1
rev-server=fe90::/12,192.168.178.1
rev-server=fea0::/12,192.168.178.1
rev-server=feb0::/12,192.168.178.1

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


Re: [Dnsmasq-discuss] rev-server=fe80::/10, 192.168.178.1 no longer accepted (version 2.86)

2021-09-10 Thread Simon Kelley
On 10/09/2021 11:27, Dominik DL6ER wrote:
> Hey Joerg,
> 
> On Fri, 2021-09-10 at 10:13 +0200, jo...@schuetter.org wrote:
>> rev-server=fe80::/10,192.168.178.1
> 
> dnsmasq always only accepted IPv6 prefixes that are a multiple of
> 4. There was just no enforcing of this before v2.86.
> 
> Looking at the v2.85 code, dnsmasq will have added
>> fe80::/8
> for you in version v2.85 without any warnings.
> 
> I submitted patches for supporting arbitrary prefix lengths (IPv4
> is even more restrictive (only /8, /16, /24 and /32 are allowed)
> last year. I already rebased my patches on the current master and
> will submit them in the next few days. This means support for /10
> might come in the next release (if the patches are accepted).
> 


For IPv6, this seems like an obviously good thing to do, since one
--rev-server can create as most eight .ip6.arpa, but for IPv4, that
goes up to 128 since for instance 192.168.1.0/25 expands to

0.1.168.192.in-addr.arpa
1.1.168.192.in-addr.arpa
.
.
127.1.168.192.in-addr.arpa


I guess that now we have efficient searches over domains, that's more
likely to be OK, but it needs some consideration.


Cheers,

Simon.

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


Re: [Dnsmasq-discuss] rev-server=fe80::/10, 192.168.178.1 no longer accepted (version 2.86)

2021-09-11 Thread Dominik DL6ER
Hey Simon,

On Fri, 2021-09-10 at 23:48 +0100, Simon Kelley wrote:
> For IPv6, this seems like an obviously good thing to do, since
> one
> --rev-server can create as most eight .ip6.arpa, but for
> IPv4, that
> goes up to 128 since for instance 192.168.1.0/25 expands to
> 
> 0.1.168.192.in-addr.arpa
> 1.1.168.192.in-addr.arpa
> .
> .
> 127.1.168.192.in-addr.arpa
> 
> 
> I guess that now we have efficient searches over domains, that's
> more
> likely to be OK, but it needs some consideration.

I see, however, this is surely the worst case and not something
that'd happen very often. The typical use case I've seen being
requested for Pi-hole is something like /22 which would result in
4 generated entries.

Patch attached, it allows arbitrary subnets and explains what it
is doing to syslog (similar to how we do it for DHCP). It applies
cleanly to the current master.

I guess it's pretty self-explanatory, more details can be found
in my last submission:

https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2020q2/013985.html
https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2020q2/013997.html

Everything is combined in one patch now.

Best,
Dominik
From bb26695b7e6db84736c9725c50d9a95aec9eb936 Mon Sep 17 00:00:00 2001
From: Dominik DL6ER 
Date: Mon, 6 Sep 2021 22:27:00 +0200
Subject: [PATCH] --rev-server: Add support for arbitrary IPv4/6
 prefix-lengths. So far, the prefix was limited to [8,16,24,32] for IPv4 and
 to multiples of 4 for IPv6. We log infos about what is happening behind the
 scenes to the log facilty (INFO). This patch also makes the prefix-length
 optional.

Signed-off-by: DL6ER 
---
 man/dnsmasq.8 |   3 +-
 src/option.c  | 238 ++
 2 files changed, 186 insertions(+), 55 deletions(-)

diff --git a/man/dnsmasq.8 b/man/dnsmasq.8
index 7ffccad..1448ba2 100644
--- a/man/dnsmasq.8
+++ b/man/dnsmasq.8
@@ -505,13 +505,14 @@ source address specified but the port may be specified directly as
 part of the source address. Forcing queries to an interface is not
 implemented on all platforms supported by dnsmasq.
 .TP
-.B --rev-server=/[,][#][@][@[#]]
+.B --rev-server=[/][,][#][@][@[#]]
 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
 is exactly equivalent to 
 .B --server=/3.2.1.in-addr.arpa/192.168.0.1
+Allowed prefix lengths are 1-32 (IPv4) and 1-128 (IPv6). If the prefix length is omitted, dnsmasq substitutes either 32 (IPv4) or 128 (IPv6).
 .TP
 .B \-A, --address=/[/...]/[]
 Specify an IP address to return for any host in the given domains.
diff --git a/src/option.c b/src/option.c
index ffce9fc..7c625b9 100644
--- a/src/option.c
+++ b/src/option.c
@@ -935,52 +935,172 @@ char *parse_server(char *arg, union mysockaddr *addr, union mysockaddr *source_a
   return NULL;
 }
 
-static int domain_rev4(char *domain, struct in_addr addr, int msize)
+static int gen_rev4(struct in_addr *addr, int msize, char ***domains, int explain, char *comma, char *errstr)
 {
-  in_addr_t a = ntohl(addr.s_addr);
- 
-  *domain = 0;
-  
-  switch (msize)
+  unsigned int i, mina, maxa, octnum, N;
+  struct in_addr bitmask, a_min, a_max;
+
+  if(msize < 1 || msize > 32)
+ret_err("bad IPv4 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;
+  /* Print derived subnet address range to ease configuration (if requested) */
+  if(explain)
 {
-case 32:
-  domain += sprintf(domain, "%u.", a & 0xff);
-  /* fall through */
-case 24:
-  domain += sprintf(domain, "%d.", (a >> 8) & 0xff);
-  /* fall through */
-case 16:
-  domain += sprintf(domain, "%d.", (a >> 16) & 0xff);
-  /* fall through */
-case 8:
-  domain += sprintf(domain, "%d.", (a >> 24) & 0xff);
-  break;
-default:
-  return 0;
+  const char *target = (comma != NULL && strlen(comma) > 0) ? comma : "local server";
+  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, target);
+	}
+  else
+	{
+	  my_syslog(LOG_INFO, "rev-server %s[/%i]: address-to-name queries for %s are sent to %s",
+		original_address, msize, min_address, target);
+	}
 }
-  
-  domain += sprintf(domain, "in-addr.arpa");
-  
-  return 1;
+
+ 

Re: [Dnsmasq-discuss] rev-server=fe80::/10, 192.168.178.1 no longer accepted (version 2.86)

2021-09-11 Thread Simon Kelley
On 11/09/2021 16:23, Dominik DL6ER wrote:
> Hey Simon,
> 
> On Fri, 2021-09-10 at 23:48 +0100, Simon Kelley wrote:
>> For IPv6, this seems like an obviously good thing to do, since
>> one
>> --rev-server can create as most eight .ip6.arpa, but for
>> IPv4, that
>> goes up to 128 since for instance 192.168.1.0/25 expands to
>>
>> 0.1.168.192.in-addr.arpa
>> 1.1.168.192.in-addr.arpa
>> .
>> .
>> 127.1.168.192.in-addr.arpa
>>
>>
>> I guess that now we have efficient searches over domains, that's
>> more
>> likely to be OK, but it needs some consideration.
> 
> I see, however, this is surely the worst case and not something
> that'd happen very often. The typical use case I've seen being
> requested for Pi-hole is something like /22 which would result in
> 4 generated entries.
> 
> Patch attached, it allows arbitrary subnets and explains what it
> is doing to syslog (similar to how we do it for DHCP). It applies
> cleanly to the current master.
> 
> I guess it's pretty self-explanatory, more details can be found
> in my last submission:
> 
> https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2020q2/013985.html
> https://lists.thekelleys.org.uk/pipermail/dnsmasq-discuss/2020q2/013997.html
> 
> Everything is combined in one patch now.
> 
> Best,
> Dominik
> 


That looks good, but I hit a couple of problems.

1) gen_rev6 fails to allocate the *domains array, and crashes.

2) The explanation stuff falls foul of the problem that it's generally
running during whilst reading options, and before the logging system has
been initialised. To make that work sensibly, you need to store the
information and do the logging after around line 860 of dnsmasq.c
I'd be inclined just to omit this. The final results of the process get
logged, after all.

I've run out of time to look further now, and I'm away next week and
unlikely to get more time until the end of the week. Then, I'll do more
testing.


cheers,

Simon.


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


Re: [Dnsmasq-discuss] rev-server=fe80::/10, 192.168.178.1 no longer accepted (version 2.86)

2021-09-11 Thread Dominik DL6ER
Hey Simon,

On Sat, 2021-09-11 at 23:42 +0100, Simon Kelley wrote:
> 1) gen_rev6 fails to allocate the *domains array, and crashes.

what did you try? It didn't crash for me. But I see that adding
malloc failsafe checks is something I've forgotten. I'll resubmit
the patch, also with removed explanation (I don't think we have
had the server explanation last year when I wrote this patch).

Best,
Dominik


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


Re: [Dnsmasq-discuss] rev-server=fe80::/10, 192.168.178.1 no longer accepted (version 2.86)

2021-09-14 Thread Dominik DL6ER
Hey Simon,


I'm resubmitting the patch with these changes:

0. Submit the correct patch with the correct memory allocation

1. Rebased onto Petr's change to option.c

2. Removed the explain feature

3. Added failsafe checks to the mallocs

4. Added a hint in network.c when there are more servers to be
shown that we log (this is limited by SERVERS_LOGGED = 30)


Best,
Dominik
From 58343618ffee18c24a9c81b7a668cb5dcc7e847d Mon Sep 17 00:00:00 2001
From: Dominik DL6ER 
Date: Mon, 6 Sep 2021 22:27:00 +0200
Subject: [PATCH] --rev-server: Add support for arbitrary IPv4/6
 prefix-lengths. So far, the prefix was limited to [8,16,24,32] for IPv4 and
 to multiples of 4 for IPv6. This patch also makes the prefix-length optional.

Signed-off-by: DL6ER 
---
 man/dnsmasq.8 |   3 +-
 src/network.c |   3 +
 src/option.c  | 210 +-
 3 files changed, 162 insertions(+), 54 deletions(-)

diff --git a/man/dnsmasq.8 b/man/dnsmasq.8
index 7ffccad..1448ba2 100644
--- a/man/dnsmasq.8
+++ b/man/dnsmasq.8
@@ -505,13 +505,14 @@ source address specified but the port may be specified directly as
 part of the source address. Forcing queries to an interface is not
 implemented on all platforms supported by dnsmasq.
 .TP
-.B --rev-server=/[,][#][@][@[#]]
+.B --rev-server=[/][,][#][@][@[#]]
 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
 is exactly equivalent to 
 .B --server=/3.2.1.in-addr.arpa/192.168.0.1
+Allowed prefix lengths are 1-32 (IPv4) and 1-128 (IPv6). If the prefix length is omitted, dnsmasq substitutes either 32 (IPv4) or 128 (IPv6).
 .TP
 .B \-A, --address=/[/...]/[]
 Specify an IP address to return for any host in the given domains.
diff --git a/src/network.c b/src/network.c
index 3fc179d..f92d06d 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1586,6 +1586,9 @@ void check_servers(int no_loop_check)
   if (serv->sfd)
 	serv->sfd->used = 1;
   
+  if (count == SERVERS_LOGGED)
+	my_syslog(LOG_INFO, _("more servers are defined but not logged"));
+
   if (++count > SERVERS_LOGGED)
 	continue;
   
diff --git a/src/option.c b/src/option.c
index 11655fd..778b90a 100644
--- a/src/option.c
+++ b/src/option.c
@@ -935,52 +935,146 @@ char *parse_server(char *arg, union mysockaddr *addr, union mysockaddr *source_a
   return NULL;
 }
 
-static int domain_rev4(char *domain, struct in_addr addr, int msize)
+static int gen_rev4(struct in_addr *addr, int msize, char ***domains, char *errstr)
 {
-  in_addr_t a = ntohl(addr.s_addr);
- 
-  *domain = 0;
-  
-  switch (msize)
+  unsigned int i, mina, maxa, octnum, N;
+  struct in_addr bitmask, a_min, a_max;
+
+  if(msize < 1 || msize > 32)
+ret_err("bad IPv4 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 */
+  in_addr_t a = ntohl((*addr).s_addr);
+
+  /* Derive number of octets to be printed during string-assembly */
+  octnum = ((msize-1)/8);
+
+  /* Extract min and max address range for string-assembly loop */
+  mina = (ntohl(a_min.s_addr) >> 8*(3-octnum)) & 0xFF;
+  maxa = (ntohl(a_max.s_addr) >> 8*(3-octnum)) & 0xFF;
+
+  /* Get number of domains we are going to generate here */
+  N = maxa - mina + 1;
+
+  /* Allocate space for domain array */
+  *domains = opt_malloc(N * sizeof(char*));
+  if(!domains)
+return 0;
+
+  /* Generate the individual domains */
+  for(i = 0; i < N; i++)
 {
-case 32:
-  domain += sprintf(domain, "%u.", a & 0xff);
-  /* fall through */
-case 24:
-  domain += sprintf(domain, "%d.", (a >> 8) & 0xff);
-  /* fall through */
-case 16:
-  domain += sprintf(domain, "%d.", (a >> 16) & 0xff);
-  /* fall through */
-case 8:
-  domain += sprintf(domain, "%d.", (a >> 24) & 0xff);
-  break;
-default:
-  return 0;
+  char *domain = opt_malloc(29); /* strlen("xxx.yyy.zzz.ttt.in-addr.arpa")+1 */
+  if(!domain)
+	return 0;
+  unsigned char j = i + mina;
+
+  /* Build in-addr.arpa address */
+  if(octnum == 3)
+	sprintf(domain, "%u.%u.%u.%u.in-addr.arpa", j, (a >> 8) & 0xff, (a >> 16) & 0xff, (a >> 24) & 0xff);
+  else if(octnum == 2)
+	sprintf(domain, "%u.%u.%u.in-addr.arpa", j, (a >> 16) & 0xff, (a >> 24) & 0xff);
+  else if(octnum == 1)
+	sprintf(domain, "%u.%u.in-addr.arpa", j, (a >> 24) & 0xff);
+  else
+	sprintf(domain, "%u.in-addr.arpa", j);
+
+  /* Append domain to array */
+  (*domains)[i] = domain;
 }
-  
-  sprintf(domain, "in-addr.arpa");
-  
-  return 1;
+
+  /* Return number of generated rev-domains (> 0 if successful) */
+  ret

Re: [Dnsmasq-discuss] rev-server=fe80::/10, 192.168.178.1 no longer accepted (version 2.86)

2021-09-19 Thread Simon Kelley
On 14/09/2021 13:43, Dominik DL6ER wrote:
> Hey Simon,
> 
> 
> I'm resubmitting the patch with these changes:
> 
> 0. Submit the correct patch with the correct memory allocation
> 
> 1. Rebased onto Petr's change to option.c
> 
> 2. Removed the explain feature
> 
> 3. Added failsafe checks to the mallocs
> 
> 4. Added a hint in network.c when there are more servers to be
> shown that we log (this is limited by SERVERS_LOGGED = 30)
> 
> 
> Best,
> Dominik
> 

Thanks for this.

I ended up re-writing it rather than merging, mainly to extend the
arbitrary prefix length support to --domain=example.com/192.168.1.0/24,local

In the git repo now.


Cheers,

Simon.


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