Handle DNSSEC-unaware upstream servers better.

An oversight meant that non-existance checking was being done
anyway.

(Should probably alter the default for this.)

CHANGELOG

@@ -2,6 +2,20 @@ version 2.80
 	Add support for RFC 4039 DHCP rapid commit. Thanks to Ashram Method
 	for the initial patch and motivation.
 
+	Alter the default for dnssec-check-unsigned. Versions of
+	dnsmasq prior to 2.80 defaulted to not checking unsigned
+	replies, and used --dnssec-check-unsigned to switch
+        this on. Such configurations will continue to work as before,
+        but those which used the default of no checking will need to be
+        altered to explicitly select no checking. The new default is
+        because switching off checking for unsigned replies is
+	inherently dangerous. Not only does it open the possiblity of forged
+        replies, but it allows everything to appear to be working even
+        when the upstream namesevers do not support DNSSEC, and in this
+        case no DNSSEC validation at all is occuring.
+
+
+
 
 version 2.79
 	Fix parsing of CNAME arguments, which are confused by extra spaces.

man/dnsmasq.8

@@ -712,10 +712,7 @@ permitted to reduce the cache size below the default when DNSSEC is
 enabled. The nameservers upstream of dnsmasq must be DNSSEC-capable,
 ie capable of returning DNSSEC records with data. If they are not,
 then dnsmasq will not be able to determine the trusted status of
-answers. In the default mode, this means that all replies will be
-marked as untrusted. If 
-.B --dnssec-check-unsigned
-is set and the upstream servers don't support DNSSEC, then DNS service will be entirely broken.
+answers and this means that DNS service will be entirely broken.
 .TP
 .B --trust-anchor=[<class>],<domain>,<key-tag>,<algorithm>,<digest-type>,<digest>
 Provide DS records to act a trust anchors for DNSSEC
@@ -724,17 +721,19 @@ key(s) (KSK) of the root zone,
 but trust anchors for limited domains are also possible. The current
 root-zone trust anchors may be downloaded from https://data.iana.org/root-anchors/root-anchors.xml 
 .TP
-.B --dnssec-check-unsigned
-As a default, dnsmasq does not check that unsigned DNS replies are
-legitimate: they are assumed to be valid and passed on (without the
+.B --dnssec-check-unsigned[=no]
+As a default, dnsmasq checks that unsigned DNS replies are
+legitimate: this entails possible extra queries even for the majority of DNS
+zones which are not, at the moment, signed. If
+.B --dnssec-check-unsigned=no
+appears in the configuration, then such replies they are assumed to be valid and passed on (without the
 "authentic data" bit set, of course). This does not protect against an
 attacker forging unsigned replies for signed DNS zones, but it is
-fast. If this flag is set, dnsmasq will check the zones of unsigned
-replies, to ensure that unsigned replies are allowed in those
-zones. The cost of this is more upstream queries and slower
-performance. See also the warning about upstream servers in the
-section on 
-.B --dnssec
+fast.
+
+Versions of dnsmasq prior to 2.80 defaulted to not checking unsigned replies, and used 
+.B --dnssec-check-unsigned
+to switch this on. Such configurations will continue to work as before, but those which used the default of no checking will need to be altered to explicitly select no checking. The new default is because switching off checking for unsigned replies is inherently dangerous. Not only does it open the possiblity of forged replies, but it allows everything to appear to be working even when the upstream namesevers do not support DNSSEC, and in this case no DNSSEC validation at all is occuring.
 .TP
 .B --dnssec-no-timecheck
 DNSSEC signatures are only valid for specified time windows, and should be rejected outside those windows. This generates an

src/dnsmasq.c

@@ -768,7 +768,10 @@ int main (int argc, char **argv)
 	  _exit(0);
 	}
       
-      my_syslog(LOG_INFO, _("DNSSEC validation enabled"));
+      if (option_bool(OPT_DNSSEC_IGN_NS))
+	my_syslog(LOG_INFO, _("DNSSEC validation enabled but all unsigned answers are trusted"));
+      else
+	my_syslog(LOG_INFO, _("DNSSEC validation enabled"));
       
       daemon->dnssec_no_time_check = option_bool(OPT_DNSSEC_TIME);
       if (option_bool(OPT_DNSSEC_TIME) && !daemon->back_to_the_future)

src/dnsmasq.h

@@ -241,7 +241,7 @@ struct event_desc {
 #define OPT_DNSSEC_VALID   45
 #define OPT_DNSSEC_TIME    46
 #define OPT_DNSSEC_DEBUG   47
-#define OPT_DNSSEC_NO_SIGN 48 
+#define OPT_DNSSEC_IGN_NS  48 
 #define OPT_LOCAL_SERVICE  49
 #define OPT_LOOP_DETECT    50
 #define OPT_EXTRALOG       51

src/dnssec.c

@@ -875,8 +875,16 @@ int dnssec_validate_ds(time_t now, struct dns_header *header, size_t plen, char
     rc = dnssec_validate_reply(now, header, plen, name, keyname, NULL, 0, &neganswer, &nons);
   
   if (rc == STAT_INSECURE)
-    rc = STAT_BOGUS;
- 
+    {
+      static int reported = 0;
+      if (!reported)
+	{
+	  reported = 1;
+	  my_syslog(LOG_WARNING, _("Insecure DS reply received, do upstream DNS servers support DNSSEC?"));
+	}
+      rc = STAT_BOGUS;
+    }
+  
   p = (unsigned char *)(header+1);
   extract_name(header, plen, &p, name, 1, 4);
   p += 4; /* qtype, qclass */
@@ -1906,7 +1914,6 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch
 	      
 	      if (rc == STAT_BOGUS || rc == STAT_NEED_KEY || rc == STAT_NEED_DS)
 		{
-		  /* Zone is insecure, don't need to validate RRset */
 		  if (class)
 		    *class = class1; /* Class for NEED_DS or NEED_KEY */
 		  return rc;
@@ -1966,35 +1973,36 @@ int dnssec_validate_reply(time_t now, struct dns_header *header, size_t plen, ch
     }
 
   /* OK, all the RRsets validate, now see if we have a missing answer or CNAME target. */
-  for (j = 0; j <targetidx; j++)
-    if ((p2 = targets[j]))
-      {
-	if (neganswer)
-	  *neganswer = 1;
-
-	if (!extract_name(header, plen, &p2, name, 1, 10))
-	  return STAT_BOGUS; /* bad packet */
-	    
-	/* NXDOMAIN or NODATA reply, unanswered question is (name, qclass, qtype) */
-
-	/* For anything other than a DS record, this situation is OK if either
-	   the answer is in an unsigned zone, or there's a NSEC records. */
-	if (!prove_non_existence(header, plen, keyname, name, qtype, qclass, NULL, nons))
-	  {
-	    /* Empty DS without NSECS */
-	    if (qtype == T_DS)
-	      return STAT_BOGUS;
-	    
-	    if ((rc = zone_status(name, qclass, keyname, now)) != STAT_SECURE)
-	      {
-		if (class)
-		  *class = qclass; /* Class for NEED_DS or NEED_KEY */
-		return rc;
-	      } 
-	    
-	    return STAT_BOGUS; /* signed zone, no NSECs */
-	  }
-      }
+  if (secure == STAT_SECURE)
+    for (j = 0; j <targetidx; j++)
+      if ((p2 = targets[j]))
+	{
+	  if (neganswer)
+	    *neganswer = 1;
+	  
+	  if (!extract_name(header, plen, &p2, name, 1, 10))
+	    return STAT_BOGUS; /* bad packet */
+	  
+	  /* NXDOMAIN or NODATA reply, unanswered question is (name, qclass, qtype) */
+	  
+	  /* For anything other than a DS record, this situation is OK if either
+	     the answer is in an unsigned zone, or there's a NSEC records. */
+	  if (!prove_non_existence(header, plen, keyname, name, qtype, qclass, NULL, nons))
+	    {
+	      /* Empty DS without NSECS */
+	      if (qtype == T_DS)
+		return STAT_BOGUS;
+	      
+	      if ((rc = zone_status(name, qclass, keyname, now)) != STAT_SECURE)
+		{
+		  if (class)
+		    *class = qclass; /* Class for NEED_DS or NEED_KEY */
+		  return rc;
+		} 
+	      
+	      return STAT_BOGUS; /* signed zone, no NSECs */
+	    }
+	}
   
   return secure;
 }

src/forward.c

@@ -919,7 +919,7 @@ void reply_query(int fd, int family, time_t now)
 		    status = dnssec_validate_ds(now, header, n, daemon->namebuff, daemon->keyname, forward->class);
 		  else
 		    status = dnssec_validate_reply(now, header, n, daemon->namebuff, daemon->keyname, &forward->class, 
-						   option_bool(OPT_DNSSEC_NO_SIGN) && (server->flags & SERV_DO_DNSSEC),
+						   !option_bool(OPT_DNSSEC_IGN_NS) && (server->flags & SERV_DO_DNSSEC),
 						   NULL, NULL);
 		}
 	      
@@ -1504,7 +1504,7 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si
 	new_status = dnssec_validate_ds(now, header, n, name, keyname, class);
       else 
 	new_status = dnssec_validate_reply(now, header, n, name, keyname, &class,
-					   option_bool(OPT_DNSSEC_NO_SIGN) && (server->flags & SERV_DO_DNSSEC),
+					   !option_bool(OPT_DNSSEC_IGN_NS) && (server->flags & SERV_DO_DNSSEC),
 					   NULL, NULL);
       
       if (new_status != STAT_NEED_DS && new_status != STAT_NEED_KEY)

src/option.c

@@ -311,7 +311,7 @@ static const struct myoption opts[] =
     { "dnssec", 0, 0, LOPT_SEC_VALID },
     { "trust-anchor", 1, 0, LOPT_TRUST_ANCHOR },
     { "dnssec-debug", 0, 0, LOPT_DNSSEC_DEBUG },
-    { "dnssec-check-unsigned", 0, 0, LOPT_DNSSEC_CHECK },
+    { "dnssec-check-unsigned", 2, 0, LOPT_DNSSEC_CHECK },
     { "dnssec-no-timecheck", 0, 0, LOPT_DNSSEC_TIME },
     { "dnssec-timestamp", 1, 0, LOPT_DNSSEC_STAMP },
 #ifdef OPTION6_PREFIX_CLASS 
@@ -484,7 +484,7 @@ static struct {
   { LOPT_SEC_VALID, OPT_DNSSEC_VALID, NULL, gettext_noop("Activate DNSSEC validation"), NULL },
   { LOPT_TRUST_ANCHOR, ARG_DUP, "<domain>,[<class>],...", gettext_noop("Specify trust anchor key digest."), NULL },
   { LOPT_DNSSEC_DEBUG, OPT_DNSSEC_DEBUG, NULL, gettext_noop("Disable upstream checking for DNSSEC debugging."), NULL },
-  { LOPT_DNSSEC_CHECK, OPT_DNSSEC_NO_SIGN, NULL, gettext_noop("Ensure answers without DNSSEC are in unsigned zones."), NULL },
+  { LOPT_DNSSEC_CHECK, ARG_DUP, NULL, gettext_noop("Ensure answers without DNSSEC are in unsigned zones."), NULL },
   { LOPT_DNSSEC_TIME, OPT_DNSSEC_TIME, NULL, gettext_noop("Don't check DNSSEC signature timestamps until first cache-reload"), NULL },
   { LOPT_DNSSEC_STAMP, ARG_ONE, "<path>", gettext_noop("Timestamp file to verify system clock for DNSSEC"), NULL },
 #ifdef OPTION6_PREFIX_CLASS 
@@ -4139,6 +4139,16 @@ err:
       daemon->timestamp_file = opt_string_alloc(arg); 
       break;
 
+    case LOPT_DNSSEC_CHECK:
+      if (arg)
+	{
+	  if (strcmp(arg, "no") == 0)
+	    set_option_bool(OPT_DNSSEC_IGN_NS);
+	  else
+	    ret_err(_("bad value for dnssec-check-unsigned"));
+	}
+      break;
+      
     case LOPT_TRUST_ANCHOR:
       {
 	struct ds_config *new = opt_malloc(sizeof(struct ds_config));