[Dnsmasq-discuss] [PATCH] Fix DNSSEC handling of CNAME records.

[chris at yourdreamnet.co.uk]

From: Chris Staite <chris at yourdreamnet.co.uk>

Fixes the case where a CNAME is valid and unsigned and the target of the
CNAME is returned with the CNAME but with no RRSIG (due to the CNAME not
being in a signed zone).

Since the CNAME is unsigned, there is no additional security risk returning
an insecure response for the A/AAAA records that should have an RRSIG.

There's still the outstanding question of whether dnsmasq handles the case
where a secure CNAME points to a secure A/AAAA in a different zone.  I'm not
sure whether it ought to perform another query to to target SOA to get the
RRSIG for that in addition.
---
 src/dnsmasq.c |  1 +
 src/dnsmasq.h |  1 +
 src/dnssec.c  | 66 +++++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/src/dnsmasq.c b/src/dnsmasq.c
index 7cfb493..57e2e24 100644
--- a/src/dnsmasq.c
+++ b/src/dnsmasq.c
@@ -136,6 +136,7 @@ int main (int argc, char **argv)
       daemon->namebuff = safe_malloc(MAXDNAME * 2);
       daemon->keyname = safe_malloc(MAXDNAME * 2);
       daemon->workspacename = safe_malloc(MAXDNAME * 2);
+      daemon->workspacename2 = safe_malloc(MAXDNAME * 2);
       /* one char flag per possible RR in answer section (may get extended). */
       daemon->rr_status_sz = 64;
       daemon->rr_status = safe_malloc(sizeof(*daemon->rr_status) * daemon->rr_status_sz);
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index bfc0fd4..9c10df8 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1195,6 +1195,7 @@ extern struct daemon {
   char *workspacename;
 #endif
 #ifdef HAVE_DNSSEC
+  char *workspacename2;
   char *keyname; /* MAXDNAME size buffer */
   unsigned long *rr_status; /* ceiling in TTL from DNSSEC or zero for insecure */
   int rr_status_sz;
diff --git a/src/dnssec.c b/src/dnssec.c
index 9965eea..5000ed2 100644
--- a/src/dnssec.c
+++ b/src/dnssec.c
@@ -1841,6 +1841,51 @@ static int zone_status(char *name, int class, char *keyname, time_t now)
 
   return STAT_SECURE;
 }
+
+/* Find a matching CNAME and check if it is insecure
+   Return code:
+   STAT_SECURE   if the CNAME for the given name is secure
+   STAT_INSECURE if the CNAME for the given name is insecure
+   STAT_BOGUS    something went wrong
+   STAT_NEED_KEY need DNSKEY to complete validation (name is returned in keyname, class in *class)
+   STAT_NEED_DS  need DS to complete validation (name is returned in keyname)
+*/
+int cname_insecure(struct dns_header *header, size_t plen, char* name, char* keyname, int* oclass, time_t now)
+{
+  int i, type, class, length;
+  int rc = STAT_BOGUS;
+  unsigned char* p1 = (unsigned char *)(header+1);
+
+  if (!(p1 = skip_name(p1, header, plen, 10)))
+    return STAT_BOGUS; /* bad packet */
+
+  p1 += 4;  /* skip type and class */
+
+  for (i = 0; i < ntohs(header->ancount); ++i)
+  {
+    if (!extract_name(header, plen, &p1, daemon->workspacename2, 1, 0))
+      return STAT_BOGUS;
+
+    GETSHORT(type, p1);
+    GETSHORT(class, p1);
+    p1 += 4;  /* TTL */
+    GETSHORT(length, p1);
+
+    if (!extract_name(header, plen, &p1, keyname, 1, 0))
+      return STAT_BOGUS;
+
+    if (type == T_CNAME && hostname_isequal(name, keyname))
+    {
+        strcpy(name, keyname);
+	    rc = zone_status(daemon->workspacename2, class, keyname, now);
+	    if ((STAT_ISEQUAL(rc, STAT_NEED_DS) || STAT_ISEQUAL(rc, STAT_NEED_KEY)) && oclass)
+	        *oclass = class;
+	    return rc;
+    }
+  }
+
+  return rc;
+}
        
 /* Validate all the RRsets in the answer and authority sections of the reply (4035:3.2.3) 
    Return code:
@@ -2001,13 +2046,24 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch
 		  if (check_unsigned && i < ntohs(header->ancount))
 		    {
 		      rc = zone_status(name, class1, keyname, now);
+		      if (STAT_ISEQUAL(rc, STAT_SECURE) && (type1 == T_A || type1 == T_AAAA))
+		      {
+		        /* There can't be an RRSIG over a CNAME and A record if the
+		           CNAME is insecure, so simply allow in that case */
+		        my_syslog(LOG_INFO, "Check name for %s", name);
+		        rc = cname_insecure(header, plen, name, keyname, class, now);
+		      }
+
 		      if (STAT_ISEQUAL(rc, STAT_SECURE))
-			rc = STAT_BOGUS | DNSSEC_FAIL_NOSIG;
-		      
-		      if (class)
-			*class = class1; /* Class for NEED_DS or NEED_KEY */
+		      {
+		        rc = STAT_BOGUS | DNSSEC_FAIL_NOSIG;
+		      }
+		      else if (type1 != T_A && type1 != T_AAAA && class)
+		      {
+		        *class = class1; /* Class for NEED_DS or NEED_KEY */
+		      }
 		    }
-		  else 
+		  else
 		    rc = STAT_INSECURE; 
 		  
 		  if (!STAT_ISEQUAL(rc, STAT_INSECURE))
-- 
2.31.1