Hi!

I have played with those tests a bit. I have realized dnsmasq should be
able to do all authoritative data too, so I made a modification to run
with dnsmasq directly [1].

I found out ANY queries are not handled by authoritative server. Fixed
basic support for it, it now gives any.ftl results with both addresses.

A minor issue is missing NS record, when it was provided to dnsmasq.
Fixed that also.

It does not pass pihole-FTL tests completely, but tests both
authoritative and forwarding service. A good step forward I think.

It should disentangle pihole parts from test suite, if its parts should
be used to test just dnsmasq.

What do you think? Should we try to reuse some parts for both projects
or just copy interesting stuff out and mostly forget its original source?

[1] https://github.com/pemensik/FTL/tree/tests/test/dnsmasq

On 10/12/21 05:22, Dominik Derigs wrote:
> Hey Petr,
>
> On Tue, 2021-10-12 at 04:40 +0200, Petr Menšík wrote:
>> Hi Dominik,
>>
>> those tests look great. Something like that is exactly what I had
>> on
>> mind for dnsmasq itself. Would you mind if I borrow few things
>> and try
>> to make some dnsmasq-only parts, not dependent on pihole?
> Go ahead and take what is useful! Just pointing out that you'll
> want to look at out development branch (not yet merged to master)
> because tests were recently improved to run a locally running
> powerDNS authoritative server + recursor. This is set up here:
>
> https://github.com/pi-hole/FTL/blob/development/test/pdns/setup.sh
>
> DNS records are tested to resolve as expected, e.g., here:
>
> https://github.com/pi-hole/FTL/blob/24af7889588f2567705ba100a4cfd9bee62b6ef2/test/test_suite.bats#L268-L357
>
> On Tue, 2021-10-12 at 04:40 +0200, Petr Menšík wrote:
>> Just curious. Why do you support linking only to static
>> libraries? Is
>> pihole project opposed to be eventually packages as a normal
>> distribution package?
> Yes, we do not want to get packaged into distro packets because
> this would maybe pin users to extremely old versions and would
> drastically increase workload on our side when we'd have to
> backport security fixes instead just shipping the most recent
> version all the time to users. All libraries are compiled
> statically so you can just take the binary and run it on every
> compatible processor. May it be Raspbian, Armbian or whatever
> else on an SoC. We even have seen users running this on embedded
> Linux smart plugs (yes, WiFi plugs). Furthermore, the same x86_64
> binary will work on all distributions so we don't have to
> provide/compile multiple versions for Fedora, Debian/Ubuntu and
> all their versions with different library versions. With our
> approach, only the processor architecture needs to match. Then
> one and the same binary will run everywhere.
> The x86_64-musl binary statically bundles musl-libc instead of
> gnu-libc (at the expense of being even larger) to work even when
> glibc is not available (like on alpine systems often used inside
> docker).
>
> This allows us to do things like quick fixes on dedicated branch
> and just having users to run
>
> pihole checkout ftl some_branch
>
> to get the fitting binary generated by the CI. No local compiling
> necessary (it would take ages to compile SQLite3 from source on a
> Raspberry Pi, at least one hour I'd say).
>
> On Tue, 2021-10-12 at 04:40 +0200, Petr Menšík wrote:
>> I had to modify CMakeLists.txt, when I had all
>> devel libraries needed. It would not even compile.
> That's interesting. When following
> https://docs.pi-hole.net/ftldns/compile/, it should work. But
> I'll admit that I stopped using Fedora some time ago because it
> was repeatedly showing annoying bugs on my hardware, so the steps
> over there may be outdated.
>
> On Tue, 2021-10-12 at 04:40 +0200, Petr Menšík wrote:
>> Though it seems
>> amazing to find my own commits in a project I never contributed
>> directly.
> We are preserving authorship on the commits imported for the
> embedded dnsmasq. 
>
> On Tue, 2021-10-12 at 04:40 +0200, Petr Menšík wrote:
>> Do you use some kind of container or dedicated VM to run these
>> tests?
> Yes, they run in a docker container providing everything that is
> needed (incl. precompiled static binaries to speed up the
> process). This containers also contain all the auxiliary stuff
> needed to run the tests (such as bats or said powerDNS).
>
> The docker container scripts for the various architectures are
> here:
>
> https://github.com/pi-hole/docker-base-images/tree/master/ftl-build
>
> On Tue, 2021-10-12 at 04:40 +0200, Petr Menšík wrote:
>> Do you rely just on github services to run those tests?
> No, running them on Github Actions is even a rather recent
> addition (master doesn't have it yet). We also run all the test
> on CircleCI. We started with Travis CI but the free plan was
> closed down pretty much so we'd be queued forever as we have
> independent jobs for architectures x86_64-musl, x86_64, x86_32,
> armv8a, armv7hf, armv6hf, armv5te, armv4t, aarch64. In they run
> in parallel, compiling + testing the generated binaries takes
> about 2 minutes. When they sequentially, this can take up to 20
> minutes and longer. CircleCI just started doing the same (only
> one job in parallel), hence we decided to test Github Actions as
> well now. Eventually, we will switch to using Github Actions only
> as it turns out to be very powerful yet free.
>
> On Tue, 2021-10-12 at 04:40 +0200, Petr Menšík wrote:
>> I admit those
>> checks run on every PR looks quite neat, I would love to run
>> something
>> similar also for dnsmasq. Once we had some tests to run, it might
>> be
>> possible to run them on all new commits just similar way.
> The first question is what infrastructure is used for tests. I'm
> not sure how this would fit together with the self-hosted Gitweb
> on Simon's server. But tests could be meant to run locally
> *before* committing only instead of running a second time on some
> CI that also spits out ready-to-use binaries.
>
> Don't hesitate to ask if any questions come up.
>
> Best,
> Dominik
>
-- 
Petr Menšík
Software Engineer
Red Hat, http://www.redhat.com/
email: pemen...@redhat.com
PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB
From feb1e1776cceb569f7f78dba95056168509a5fdd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Wed, 20 Oct 2021 18:44:35 +0200
Subject: [PATCH 2/2] Support ANY queries in auth-zone
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Return any queried type in auth zone data. Return all found record types
except SOA and NS queries.

Signed-off-by: Petr Menšík <pemen...@redhat.com>
---
 src/auth.c | 68 ++++++++++++++++++++++++++++++++----------------------
 1 file changed, 41 insertions(+), 27 deletions(-)

diff --git a/src/auth.c b/src/auth.c
index a70bb2e..7fcf4a0 100644
--- a/src/auth.c
+++ b/src/auth.c
@@ -17,6 +17,7 @@
 #include "dnsmasq.h"
 
 #ifdef HAVE_AUTH
+#define QTYPE_OR_ANY(qtype,t) (qtype == (t) || qtype == T_ANY)
 
 static struct addrlist *find_addrlist(struct addrlist *list, int flag, union all_addr *addr_u)
 {
@@ -296,7 +297,7 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
 	  {
 	    nxdomain = 0;
 	         
-	    if (rc == 2 && qtype == T_MX)
+	    if (rc == 2 && QTYPE_OR_ANY(qtype, T_MX))
 	      {
 		found = 1;
 		log_query(F_CONFIG | F_RRNAME, name, NULL, "<MX>", 0);
@@ -311,7 +312,7 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
 	  {
 	    nxdomain = 0;
 	    
-	    if (rc == 2 && qtype == T_SRV)
+	    if (rc == 2 && QTYPE_OR_ANY(qtype, T_SRV))
 	      {
 		found = 1;
 		log_query(F_CONFIG | F_RRNAME, name, NULL, "<SRV>", 0);
@@ -345,7 +346,7 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
 	if ((rc = hostname_issubdomain(name, txt->name)))
 	  {
 	    nxdomain = 0;
-	    if (rc == 2 && txt->class == qtype)
+	    if (rc == 2 && (txt->class == qtype || qtype == T_ANY))
 	      {
 		found = 1;
 		log_query(F_CONFIG | F_RRNAME, name, NULL, NULL, txt->class);
@@ -359,7 +360,7 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
 	if (txt->class == C_IN && (rc = hostname_issubdomain(name, txt->name)))
 	  {
 	    nxdomain = 0;
-	    if (rc == 2 && qtype == T_TXT)
+	    if (rc == 2 && QTYPE_OR_ANY(qtype, T_TXT))
 	      {
 		found = 1;
 		log_query(F_CONFIG | F_RRNAME, name, NULL, "<TXT>", 0);
@@ -373,7 +374,7 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
 	 if ((rc = hostname_issubdomain(name, na->name)))
 	   {
 	     nxdomain = 0;
-	     if (rc == 2 && qtype == T_NAPTR)
+	     if (rc == 2 && QTYPE_OR_ANY(qtype, T_NAPTR))
 	       {
 		 found = 1;
 		 log_query(F_CONFIG | F_RRNAME, name, NULL, "<NAPTR>", 0);
@@ -383,33 +384,38 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
 			  anscount++;
 	       }
 	   }
-    
-       if (qtype == T_A)
-	 flag = F_IPV4;
-       
-       if (qtype == T_AAAA)
-	 flag = F_IPV6;
+
+       if (!flag)
+	{
+	  if (qtype == T_A)
+	    flag = F_IPV4;
+	  else if (qtype == T_AAAA)
+	    flag = F_IPV6;
+	  else if (qtype == T_ANY)
+	    flag = F_IPV4 | F_IPV6;
+	}
        
        for (intr = daemon->int_names; intr; intr = intr->next)
 	 if ((rc = hostname_issubdomain(name, intr->name)))
 	   {
 	     struct addrlist *addrlist;
-	     
+	     int al6;
+
 	     nxdomain = 0;
-	     
+
 	     if (rc == 2 && flag)
-	       for (addrlist = intr->addr; addrlist; addrlist = addrlist->next)  
-		 if (((addrlist->flags & ADDRLIST_IPV6)  ? T_AAAA : T_A) == qtype &&
+	       for (addrlist = intr->addr; addrlist; addrlist = addrlist->next)
+		 if ((( (al6=(addrlist->flags & ADDRLIST_IPV6)) && (flag & F_IPV6)) ||
+		       (!al6 && (flag & F_IPV4))) &&
+		      !(addrlist->flags & ADDRLIST_REVONLY) &&
 		     (local_query || filter_zone(zone, flag, &addrlist->addr)))
 		   {
-		     if (addrlist->flags & ADDRLIST_REVONLY)
-		       continue;
-
 		     found = 1;
 		     log_query(F_FORWARD | F_CONFIG | flag, name, &addrlist->addr, NULL, 0);
 		     if (add_resource_record(header, limit, &trunc, nameoffset, &ansp, 
-					     daemon->auth_ttl, NULL, qtype, C_IN, 
-					     qtype == T_A ? "4" : "6", &addrlist->addr))
+					     daemon->auth_ttl, NULL,
+					     al6 ? T_AAAA : T_A, C_IN, al6 ? "6" : "4",
+					     &addrlist->addr))
 		       anscount++;
 		   }
 	     }
@@ -420,7 +426,9 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
 	   
 	   log_query(F_FORWARD | F_CONFIG | flag, name, &addr, NULL, 0);
 	   if (add_resource_record(header, limit, &trunc, nameoffset, &ansp, 
-				   daemon->auth_ttl, NULL, qtype, C_IN, qtype == T_A ? "4" : "6", &addr))
+				   daemon->auth_ttl, NULL,
+				   (flag == F_IPV6) ? T_AAAA : T_A, C_IN,
+				   (flag == F_IPV6) ? "6" : "4", &addr))
 	     anscount++;
 	 }
        
@@ -483,19 +491,22 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
 	  
 	  if (!strchr(name, '.') && (crecp = cache_find_by_name(NULL, name, now, F_IPV4 | F_IPV6)))
 	    {
+	      unsigned int addrf;
 	      if (crecp->flags & F_DHCP)
 		do
 		  { 
 		    nxdomain = 0;
-		    if ((crecp->flags & flag) && 
+		    addrf = (crecp->flags & flag);
+		    if (addrf &&
 			(local_query || filter_zone(zone, flag, &(crecp->addr))))
 		      {
 			*cut = '.'; /* restore domain part */
 			log_query(crecp->flags, name, &crecp->addr, record_source(crecp->uid), 0);
 			*cut  = 0; /* remove domain part */
 			if (add_resource_record(header, limit, &trunc, nameoffset, &ansp, 
-						daemon->auth_ttl, NULL, qtype, C_IN, 
-						qtype == T_A ? "4" : "6", &crecp->addr))
+						daemon->auth_ttl, NULL,
+						addrf == F_IPV6 ? T_AAAA : T_A, C_IN,
+						addrf == F_IPV6 ? "6" : "4", &crecp->addr))
 			  anscount++;
 		      }
 		  } while ((crecp = cache_find_by_name(crecp, name, now,  F_IPV4 | F_IPV6)));
@@ -506,16 +517,19 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
       
       if ((crecp = cache_find_by_name(NULL, name, now, F_IPV4 | F_IPV6)))
 	{
+	  unsigned int addrf;
 	  if ((crecp->flags & F_HOSTS) || (((crecp->flags & F_DHCP) && option_bool(OPT_DHCP_FQDN))))
 	    do
 	      { 
 		 nxdomain = 0;
-		 if ((crecp->flags & flag) && (local_query || filter_zone(zone, flag, &(crecp->addr))))
+		 addrf = (crecp->flags & flag);
+		 if (addrf && (local_query || filter_zone(zone, flag, &(crecp->addr))))
 		   {
 		     log_query(crecp->flags, name, &crecp->addr, record_source(crecp->uid), 0);
 		     if (add_resource_record(header, limit, &trunc, nameoffset, &ansp, 
-					     daemon->auth_ttl, NULL, qtype, C_IN, 
-					     qtype == T_A ? "4" : "6", &crecp->addr))
+					     daemon->auth_ttl, NULL,
+					     addrf == F_IPV6 ? T_AAAA : T_A, C_IN,
+					     addrf == F_IPV6 ? "6" : "4", &crecp->addr))
 		       anscount++;
 		   }
 	      } while ((crecp = cache_find_by_name(crecp, name, now, F_IPV4 | F_IPV6)));
-- 
2.31.1

From d4f39373f97f47187a045fb3f768c389cd6f271e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Wed, 20 Oct 2021 14:55:11 +0200
Subject: [PATCH 1/2] Provide NS record with --auth-server=test
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Current auth server does not provide NS record in auth zone unless
interface is used. It used to be mandatory, but is not anymore. Provide
nameserver if we know it.

Signed-off-by: Petr Menšík <pemen...@redhat.com>
---
 src/auth.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/auth.c b/src/auth.c
index 52b6ed1..a70bb2e 100644
--- a/src/auth.c
+++ b/src/auth.c
@@ -647,10 +647,10 @@ size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t n
 	  struct name_list *secondary;
 	  
 	  /* Only include the machine running dnsmasq if it's acting as an auth server */
-	  if (daemon->authinterface)
+	  if (daemon->authserver)
 	    {
 	      newoffset = ansp - (unsigned char *)header;
-	      if (add_resource_record(header, limit, &trunc, -offset, &ansp, 
+	      if (add_resource_record(header, limit, &trunc, -offset, &ansp,
 				      daemon->auth_ttl, NULL, T_NS, C_IN, "d", offset == 0 ? authname : NULL, daemon->authserver))
 		{
 		  if (offset == 0) 
-- 
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