Allow servers that emit broken NSEC records to be identified

'server <prefix> { broken-nsec yes; };' can now be used to stop
NSEC records from negative responses from servers in the given
prefix being cached and hence available to synth-from-dnssec.

CHANGES

@@ -1,7 +1,13 @@
 5774.	[func]		Restore NSEC Aggressive Cache ("synth-from-dnssec")
 			as active by default. It is limited to NSEC only
 			and by default ignores NSEC records with next name
-			in form \000.domain. [GL #1265]
+			in form \000.domain.
+
+			Added 'server <prefix> { broken-nsec yes; };' to
+			identify servers from which NSEC records in negative
+			responses will not be cached.  These records will
+			then not be available for synth-from-dnssec to use.
+			[GL #1265]
 
 5773.	[func]		Change the message when accepting TCP connection has
 			failed to say "Accepting TCP connection failed" and

bin/named/named.conf.rst

@@ -513,6 +513,7 @@ SERVER
 
   server netprefix {
   	bogus boolean;
+  	broken-nsec boolean;
   	edns boolean;
   	edns-udp-size integer;
   	edns-version integer;
@@ -822,6 +823,7 @@ VIEW
   	serial-update-method ( date | increment | unixtime );
   	server netprefix {
   		bogus boolean;
+  		broken-nsec boolean;
   		edns boolean;
   		edns-udp-size integer;
   		edns-version integer;

bin/named/server.c

@@ -1437,6 +1437,12 @@ configure_peer(const cfg_obj_t *cpeer, isc_mem_t *mctx, dns_peer_t **peerp) {
 		CHECK(dns_peer_setbogus(peer, cfg_obj_asboolean(obj)));
 	}
 
+	obj = NULL;
+	(void)cfg_map_get(cpeer, "broken-nsec", &obj);
+	if (obj != NULL) {
+		CHECK(dns_peer_setbrokennsec(peer, cfg_obj_asboolean(obj)));
+	}
+
 	obj = NULL;
 	(void)cfg_map_get(cpeer, "provide-ixfr", &obj);
 	if (obj != NULL) {

doc/arm/reference.rst

@@ -2239,8 +2239,13 @@ Boolean Options
    is started.
 
 ``synth-from-dnssec``
-   This option synthesizes answers from cached NSEC, NSEC3, and other RRsets that have been
-   proved to be correct using DNSSEC. The default is ``yes``.
+   This option synthesizes answers from cached NSEC, NSEC3, and
+   other RRsets that have been proved to be correct using DNSSEC.
+   The default is ``yes``.
+
+   ``server <prefix> { broken-nsec yes; };`` can be used to stop
+   named caching broken NSEC records from negative responses from servers
+   that emit broken NSEC records with missing types that actually exist.
 
    .. note:: DNSSEC validation must be enabled for this option to be effective.
       This initial implementation only covers synthesis of answers from
@@ -4532,6 +4537,12 @@ If a remote server is giving out bad data, marking it
 as bogus prevents further queries to it. The default value of
 ``bogus`` is ``no``.
 
+If a remote server is giving out broken NSEC records with type maps
+that are missing types that actually exist, ``broken-nsec`` can be
+used to stop NSEC records from negative responses from the given
+servers being cached and thus available to ``synth-from-dnssec``.
+The default value is ``no``.
+
 The ``provide-ixfr`` clause determines whether the local server, acting
 as primary, responds with an incremental zone transfer when the given
 remote server, a secondary, requests it. If set to ``yes``, incremental

doc/man/named.conf.5in

@@ -596,6 +596,7 @@ primaries string [ port integer ] [ dscp
 .ft C
 server netprefix {
       bogus boolean;
+      broken\-nsec boolean;
       edns boolean;
       edns\-udp\-size integer;
       edns\-version integer;
@@ -925,6 +926,7 @@ view string [ class ] {
       serial\-update\-method ( date | increment | unixtime );
       server netprefix {
               bogus boolean;
+              broken\-nsec boolean;
               edns boolean;
               edns\-udp\-size integer;
               edns\-version integer;

doc/misc/options

@@ -419,6 +419,7 @@ primaries <string> [ port <integer> ] [ dscp
 
 server <netprefix> {
         bogus <boolean>;
+        broken-nsec <boolean>;
         edns <boolean>;
         edns-udp-size <integer>;
         edns-version <integer>;
@@ -702,6 +703,7 @@ view <string> [ <class> ] {
         serial-update-method ( date | increment | unixtime );
         server <netprefix> {
                 bogus <boolean>;
+                broken-nsec <boolean>;
                 edns <boolean>;
                 edns-udp-size <integer>;
                 edns-version <integer>;

doc/misc/options.active

@@ -416,6 +416,7 @@ primaries <string> [ port <integer> ] [ dscp
 
 server <netprefix> {
         bogus <boolean>;
+        broken-nsec <boolean>;
         edns <boolean>;
         edns-udp-size <integer>;
         edns-version <integer>;
@@ -698,6 +699,7 @@ view <string> [ <class> ] {
         serial-update-method ( date | increment | unixtime );
         server <netprefix> {
                 bogus <boolean>;
+                broken-nsec <boolean>;
                 edns <boolean>;
                 edns-udp-size <integer>;
                 edns-version <integer>;

doc/misc/server.grammar.rst

@@ -2,6 +2,7 @@
 
   server <netprefix> {
   	bogus <boolean>;
+  	broken-nsec <boolean>;
   	edns <boolean>;
   	edns-udp-size <integer>;
   	edns-version <integer>;

doc/notes/notes-current.rst

@@ -58,11 +58,17 @@ Feature Changes
   events: ``socket is not connected``, ``quota reached``, and ``soft
   quota reached``. :gl:`#2700`
 
-- Restore NSEC Aggressive Cache (``synth-from-dnssec``) as active by default.
+- Restore NSEC Aggressive Cache (``synth-from-dnssec``) as active by default
+  following reworking of the code to find the potentially covering NSEC record.
   The implementation was optimized for better efficiency, and also tuned
   to ignore certain types of broken NSEC records.  This feature currently
   supports answer synthtesis only for zones using NSEC.  :gl:`#1265`
 
+  The new server clause ``broken-nsec`` was added to identify servers
+  that emit bad NSEC records in negative responses so they will not be
+  cached.  This can be used to work around cases where
+  ``synth-from-dnssec`` hides data that exists. :gl:`#1265`
+
 Bug Fixes
 ~~~~~~~~~
 

lib/dns/include/dns/peer.h

@@ -86,6 +86,12 @@ dns_peer_attach(dns_peer_t *source, dns_peer_t **target);
 void
 dns_peer_detach(dns_peer_t **list);
 
+isc_result_t
+dns_peer_setbrokennsec(dns_peer_t *peer, bool newval);
+
+isc_result_t
+dns_peer_getbrokennsec(dns_peer_t *peer, bool *retval);
+
 isc_result_t
 dns_peer_setbogus(dns_peer_t *peer, bool newval);
 

lib/dns/peer.c

@@ -58,6 +58,7 @@ struct dns_peer {
 	bool force_tcp;
 	bool tcp_keepalive;
 	bool check_axfr_id;
+	bool broken_nsec;
 	dns_name_t *key;
 	isc_sockaddr_t *transfer_source;
 	isc_dscp_t transfer_dscp;
@@ -96,6 +97,7 @@ struct dns_peer {
 #define FORCE_TCP_BIT		   15
 #define SERVER_PADDING_BIT	   16
 #define REQUEST_TCP_KEEPALIVE_BIT  17
+#define BROKEN_NSEC		   18
 
 static void
 peerlist_delete(dns_peerlist_t **list);
@@ -590,6 +592,33 @@ dns_peer_gettcpkeepalive(dns_peer_t *peer, bool *retval) {
 	}
 }
 
+isc_result_t
+dns_peer_setbrokennsec(dns_peer_t *peer, bool newval) {
+	bool existed;
+
+	REQUIRE(DNS_PEER_VALID(peer));
+
+	existed = DNS_BIT_CHECK(BROKEN_NSEC, &peer->bitflags);
+
+	peer->broken_nsec = newval;
+	DNS_BIT_SET(BROKEN_NSEC, &peer->bitflags);
+
+	return (existed ? ISC_R_EXISTS : ISC_R_SUCCESS);
+}
+
+isc_result_t
+dns_peer_getbrokennsec(dns_peer_t *peer, bool *retval) {
+	REQUIRE(DNS_PEER_VALID(peer));
+	REQUIRE(retval != NULL);
+
+	if (DNS_BIT_CHECK(BROKEN_NSEC, &peer->bitflags)) {
+		*retval = peer->broken_nsec;
+		return (ISC_R_SUCCESS);
+	} else {
+		return (ISC_R_NOTFOUND);
+	}
+}
+
 isc_result_t
 dns_peer_settransfers(dns_peer_t *peer, uint32_t newval) {
 	bool existed;

lib/dns/resolver.c

@@ -5228,6 +5228,7 @@ validated(isc_task_t *task, isc_event_t *event) {
 	dns_valarg_t *valarg;
 	dns_validatorevent_t *vevent;
 	fetchctx_t *fctx = NULL;
+	bool broken_nsec = false;
 	bool chaining;
 	bool negative;
 	bool sentresponse;
@@ -5240,6 +5241,8 @@ validated(isc_task_t *task, isc_event_t *event) {
 	dns_fixedname_t fwild;
 	dns_name_t *wild = NULL;
 	dns_message_t *message = NULL;
+	dns_peer_t *peer = NULL;
+	isc_netaddr_t ipaddr;
 
 	UNUSED(task); /* for now */
 
@@ -5567,6 +5570,12 @@ validated(isc_task_t *task, isc_event_t *event) {
 	}
 
 answer_response:
+
+	isc_netaddr_fromsockaddr(&ipaddr, &addrinfo->sockaddr);
+	(void)dns_peerlist_peerbyaddr(fctx->res->view->peers, &ipaddr, &peer);
+	if (peer != NULL) {
+		(void)dns_peer_getbrokennsec(peer, &broken_nsec);
+	}
 	/*
 	 * Cache any SOA/NS/NSEC records that happened to be validated.
 	 */
@@ -5600,6 +5609,15 @@ answer_response:
 				continue;
 			}
 
+			/*
+			 * If this peer has been marked as emitting broken
+			 * NSEC records do not cache it.
+			 */
+			if (rdataset->type == dns_rdatatype_nsec && broken_nsec)
+			{
+				continue;
+			}
+
 			/*
 			 * Don't cache NSEC if missing NSEC or RRSIG types.
 			 */

lib/isccfg/namedconf.c

@@ -2510,6 +2510,7 @@ static cfg_type_t cfg_type_key = { "key",	  cfg_parse_named_map,
  */
 static cfg_clausedef_t server_clauses[] = {
 	{ "bogus", &cfg_type_boolean, 0 },
+	{ "broken-nsec", &cfg_type_boolean, 0 },
 	{ "edns", &cfg_type_boolean, 0 },
 	{ "edns-udp-size", &cfg_type_uint32, 0 },
 	{ "edns-version", &cfg_type_uint32, 0 },