Hey Simon,

On Tue, 2021-07-06 at 23:05 +0100, Simon Kelley wrote:
> I wonder, is this situation already covered by EDE_NO_DNSKEY - EDE 9
> 
> From RFC-8914:
> 
>    Extended DNS Error Code 9 - DNSKEY Missing
> 
>    A DS record existed at a parent, but no supported matching DNSKEY
>    record could be found for the child.
> 
> 
> Note the word "matching" in the definition.
> 
> Good to catch this situation, but I'm not sure we need to invent a
> new
> error.

Sure, I didn't really check the existing error codes too closely
because my thoughts were in line of "if there would be an existing
error code for it, Simon would surely have added it".
Revised (= shorter) patch attached.

Best regards,
Dominik
From 2c38512eade5c14482dfb1f523d62a009b9ada09 Mon Sep 17 00:00:00 2001
From: Dominik Derigs <dl...@dl6er.de>
Date: Wed, 7 Jul 2021 06:05:58 +0200
Subject: [PATCH] Add extended DNS error message in case no matching DNSKEY was
 found (result is BOGUS)

Signed-off-by: Dominik Derigs <dl...@dl6er.de>
---
 src/cache.c        |  8 ++++----
 src/dns-protocol.h |  3 ++-
 src/dnssec.c       |  5 +++--
 src/forward.c      | 12 ++++++------
 4 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index 4a62560..00a7df7 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -1923,10 +1923,10 @@ void log_query(unsigned int flags, char *name, union all_addr *addr, char *arg)
 	  else
 	    sprintf(daemon->addrbuff, "%u", rcode);
 
-	  if (addr->log.ede != -1)
+	  if (addr->log.ede != EDE_UNSET)
 	    {
 	      extra = daemon->addrbuff;
-	      sprintf(extra, " (EDE:%s)", edestr(addr->log.ede));
+	      sprintf(extra, " (EDE: %s)", edestr(addr->log.ede));
 	    }
 	}
       else
@@ -1974,10 +1974,10 @@ void log_query(unsigned int flags, char *name, union all_addr *addr, char *arg)
     source = "reply";
   else if (flags & F_SECSTAT)
     {
-      if (addr && addr->log.ede != -1)
+      if (addr && addr->log.ede != EDE_UNSET)
 	{
 	  extra = daemon->addrbuff;
-	  sprintf(extra, " (EDE:%s)", edestr(addr->log.ede));
+	  sprintf(extra, " (EDE: %s)", edestr(addr->log.ede));
 	}
       source = "validation";
       dest = arg;
diff --git a/src/dns-protocol.h b/src/dns-protocol.h
index 01d5f8f..496a4bb 100644
--- a/src/dns-protocol.h
+++ b/src/dns-protocol.h
@@ -85,7 +85,8 @@
 #define EDNS0_OPTION_NOMCPEID       65074 /* Nominum temporary assignment */
 #define EDNS0_OPTION_UMBRELLA       20292 /* Cisco Umbrella temporary assignment */
 
-/* RFC-8914 extended errors */
+/* RFC-8914 extended errors, negative values are our definitions */
+#define EDE_UNSET          -1  /* No extended DNS error available */
 #define EDE_OTHER           0  /* Other */
 #define EDE_USUPDNSKEY      1  /* Unsupported DNSKEY algo */
 #define EDE_USUPDS          2  /* Unsupported DS Digest */
diff --git a/src/dnssec.c b/src/dnssec.c
index 3152d83..94ebb6f 100644
--- a/src/dnssec.c
+++ b/src/dnssec.c
@@ -744,7 +744,8 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in
 	}
     }
 
-  return STAT_BOGUS | failflags;
+  /* If we reach this point, no verifying key was found */
+  return STAT_BOGUS | failflags | DNSSEC_FAIL_NOKEY;
 }
  
 
@@ -2193,6 +2194,6 @@ int errflags_to_ede(int status)
   else if (status & DNSSEC_FAIL_NOSIG)
     return EDE_NO_RRSIG;
   else
-    return -1;
+    return EDE_UNSET;
 }
 #endif /* HAVE_DNSSEC */
diff --git a/src/forward.c b/src/forward.c
index 7545abc..f5bd19e 100644
--- a/src/forward.c
+++ b/src/forward.c
@@ -177,7 +177,7 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
   int subnet, cacheable, forwarded = 0;
   size_t edns0_len;
   unsigned char *pheader;
-  int ede = -1;
+  int ede = EDE_UNSET;
   (void)do_bit;
   
   if (header->hb4 & HB4_CD)
@@ -537,7 +537,7 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
 	{
 	  u16 swap = htons((u16)ede);
 
-	  if (ede != -1)
+	  if (ede != -EDE_UNSET)
 	    plen = add_pseudoheader(header, plen, (unsigned char *)limit, daemon->edns_pktsz, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 0);
 	  else
 	    plen = add_pseudoheader(header, plen, (unsigned char *)limit, daemon->edns_pktsz, 0, NULL, 0, do_bit, 0);
@@ -749,7 +749,7 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server
      if it was removed. */
   n = resize_packet(header, n, pheader, plen);
 
-  if (pheader && ede != -1)
+  if (pheader && ede != EDE_UNSET)
     {
       u16 swap = htons((u16)ede);
       n = add_pseudoheader(header, n, limit, daemon->edns_pktsz, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 1);
@@ -1094,7 +1094,7 @@ static void return_reply(time_t now, struct frec *forward, struct dns_header *he
 {
   int check_rebind = 0, no_cache_dnssec = 0, cache_secure = 0, bogusanswer = 0;
   size_t nn;
-  int ede = -1;
+  int ede = EDE_UNSET;
 
   (void)status;
 
@@ -1918,7 +1918,7 @@ unsigned char *tcp_request(int confd, time_t now,
 
   while (1)
     {
-      int ede = -1;
+      int ede = EDE_UNSET;
 
       if (query_count == TCP_MAX_QUERIES ||
 	  !packet ||
@@ -2149,7 +2149,7 @@ unsigned char *tcp_request(int confd, time_t now,
 	    {
 	      u16 swap = htons((u16)ede);
 
-	       if (ede != -1)
+	       if (ede != EDE_UNSET)
 		 m = add_pseudoheader(header, m, ((unsigned char *) header) + 65536, daemon->edns_pktsz, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 0);
 	       else
 		 m = add_pseudoheader(header, m, ((unsigned char *) header) + 65536, daemon->edns_pktsz, 0, NULL, 0, do_bit, 0);
-- 
2.25.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