- Fix scrubber bug that potentially let NS records through. Reported

by Amanda Constant. - Also delete potential poison references from additional. - Fix: no classification of a forwarder as lame, throwaway instead. git-svn-id: file:///svn/unbound/trunk@1993 be551aaa-1e26-0410-a405-d3ace91eadb9
2025-12-20 23:00:56 -05:00 · 2010-02-24 13:27:47 +00:00 · 2010-02-24 13:27:47 +00:00 · 5dcbb54e63
commit 5dcbb54e63
parent 2bda842014
5 changed files with 250 additions and 1 deletions
--- a/doc/Changelog
+++ b/doc/Changelog
@ -1,3 +1,9 @@
+24 February 2010: Wouter
+	- Fix scrubber bug that potentially let NS records through.  Reported
+	  by Amanda Constant.
+	- Also delete potential poison references from additional.
+	- Fix: no classification of a forwarder as lame, throwaway instead.
+
 23 February 2010: Wouter
 	- libunbound ub_ctx_get_option() added.
 	- unbound-control set_option and get_option commands.
--- a/iterator/iter_resptype.c
+++ b/iterator/iter_resptype.c
@ -234,6 +234,8 @@ response_type_from_server(int rdset,
 			/* If we are getting a referral upwards (or to 
 			 * the same zone), then the server is 'lame'. */
 			if(dname_subdomain_c(origzone, s->rk.dname)) {
+				if(rdset) /* forward or reclame not LAME */
+					return RESPONSE_TYPE_THROWAWAY;
 				return RESPONSE_TYPE_LAME;
 			}
 			/* If the NS set is below the delegation point we 
--- a/iterator/iter_scrub.c
+++ b/iterator/iter_scrub.c
@ -303,6 +303,15 @@ pkt_sub(ldns_buffer* pkt, uint8_t* comprname, uint8_t* zone)
 	return dname_subdomain_c(buf, zone);
 }

+/** check subdomain with decompression, compressed is parent */
+static int
+sub_of_pkt(ldns_buffer* pkt, uint8_t* zone, uint8_t* comprname)
+{
+	uint8_t buf[LDNS_MAX_DOMAINLEN+1];
+	dname_pkt_copy(pkt, buf, comprname);
+	return dname_subdomain_c(zone, buf);
+}
+
 /**
 * This routine normalizes a response. This includes removing "irrelevant"
 * records from the answer and additional sections and (re)synthesizing
@ -518,6 +527,18 @@ store_rrset(ldns_buffer* pkt, struct msg_parse* msg, struct module_env* env,
 		env->alloc, now);
 }

+/** Check if there are SOA records in the authority section (negative) */
+static int
+soa_in_auth(struct msg_parse* msg)
+{
+	struct rrset_parse* rrset;
+	for(rrset = msg->rrset_first; rrset; rrset = rrset->rrset_all_next)
+		if(rrset->type == LDNS_RR_TYPE_SOA &&
+			rrset->section == LDNS_SECTION_AUTHORITY) 
+			return 1;
+	return 0;
+}
+ 
 /**
 * Check if right hand name in NSEC is within zone
 * @param rrset: the NSEC rrset
@ -566,6 +587,8 @@ scrub_sanitize(ldns_buffer* pkt, struct msg_parse* msg,
 	struct query_info* qinfo, uint8_t* zonename, struct module_env* env,
 	struct iter_env* ie)
 {
+	int del_addi = 0; /* if additional-holding rrsets are deleted, we
+		do not trust the normalized additional-A-AAAA any more */
 	struct rrset_parse* rrset, *prev;
 	prev = NULL;
 	rrset = msg->rrset_first;
@ -590,6 +613,7 @@ scrub_sanitize(ldns_buffer* pkt, struct msg_parse* msg,
 	 * Remainders of CNAME chains are cut off and resolved by iterator. */
 	while(rrset && rrset->section == LDNS_SECTION_ANSWER) {
 		if(dname_pkt_compare(pkt, qinfo->qname, rrset->dname) != 0) {
+			if(has_additional(rrset->type)) del_addi = 1;
 			remove_rrset("sanitize: removing extraneous answer "
 				"RRset:", pkt, msg, prev, &rrset);
 			continue;
@ -633,10 +657,15 @@ scrub_sanitize(ldns_buffer* pkt, struct msg_parse* msg,
 				rrset->type == LDNS_RR_TYPE_NS && 
 				rrset->section == LDNS_SECTION_AUTHORITY &&
 				FLAGS_GET_RCODE(msg->flags) == 
-				LDNS_RCODE_NOERROR) {
+				LDNS_RCODE_NOERROR && !soa_in_auth(msg) &&
+				sub_of_pkt(pkt, zonename, rrset->dname)) {
 				/* noerror, nodata and this NS rrset is above
 				 * the zone. This is LAME! 
 				 * Leave in the NS for lame classification. */
+				/* remove everything from the additional
+				 * (we dont want its glue that was approved
+				 * during the normalize action) */
+				del_addi = 1;
 			} else if(!env->cfg->harden_glue) {
 				/* store in cache! Since it is relevant
 				 * (from normalize) it will be picked up 
@ -646,11 +675,17 @@ scrub_sanitize(ldns_buffer* pkt, struct msg_parse* msg,
 				"poison RRset:", pkt, msg, prev, &rrset);
 				continue;
 			} else {
+				if(has_additional(rrset->type)) del_addi = 1;
 				remove_rrset("sanitize: removing potential "
 				"poison RRset:", pkt, msg, prev, &rrset);
 				continue;
 			}
 		}
+		if(del_addi && rrset->section == LDNS_SECTION_ADDITIONAL) {
+			remove_rrset("sanitize: removing potential "
+			"poison reference RRset:", pkt, msg, prev, &rrset);
+			continue;
+		}
 		/* check if right hand side of NSEC is within zone */
 		if(rrset->type == LDNS_RR_TYPE_NSEC &&
 			sanitize_nsec_is_overreach(rrset, zonename)) {
--- a/testdata/iter_scrub_ns.rpl
+++ b/testdata/iter_scrub_ns.rpl
@ -0,0 +1,103 @@
+; config options
+server:
+	target-fetch-policy: "0 0 0 0 0"
+
+stub-zone:
+	name: "."
+	stub-addr: 193.0.14.129 	# K.ROOT-SERVERS.NET.
+
+stub-zone:
+	name: "example.com"
+	stub-addr: 1.2.3.4
+CONFIG_END
+
+SCENARIO_BEGIN Test scrubber to scrub NS record for lamelike reply from stub
+
+; K.ROOT-SERVERS.NET.
+RANGE_BEGIN 0 100
+	ADDRESS 193.0.14.129 
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR NOERROR
+SECTION QUESTION
+. IN NS
+SECTION ANSWER
+. IN NS	K.ROOT-SERVERS.NET.
+SECTION ADDITIONAL
+K.ROOT-SERVERS.NET.	IN	A	193.0.14.129
+ENTRY_END
+RANGE_END
+
+; ns.example.com.
+RANGE_BEGIN 0 100
+	ADDRESS 1.2.3.4
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+; must be scrubbed
+www.burritolovers.com. IN A	10.20.30.40
+SECTION AUTHORITY
+example1234.com.	IN NS	ns.example.com.
+SECTION ADDITIONAL
+ns.example.com.		IN 	A	1.2.3.4
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR NOERROR
+SECTION QUESTION
+mail.example.com. IN A
+SECTION ANSWER
+SECTION AUTHORITY
+; LAME
+com.	IN NS	ns.example.com.
+SECTION ADDITIONAL
+ns.example.com.		IN 	A	1.2.3.4
+ENTRY_END
+
+RANGE_END
+
+STEP 1 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+; recursion happens here.
+STEP 10 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+SECTION AUTHORITY
+SECTION ADDITIONAL
+ENTRY_END
+
+STEP 20 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+mail.example.com. IN A
+ENTRY_END
+
+STEP 30 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA SERVFAIL
+SECTION QUESTION
+mail.example.com. IN A
+SECTION ANSWER
+SECTION AUTHORITY
+SECTION ADDITIONAL
+ENTRY_END
+
+SCENARIO_END
--- a/testdata/iter_scrub_ns_fwd.rpl
+++ b/testdata/iter_scrub_ns_fwd.rpl
@ -0,0 +1,103 @@
+; config options
+server:
+	target-fetch-policy: "0 0 0 0 0"
+
+stub-zone:
+	name: "."
+	stub-addr: 193.0.14.129 	# K.ROOT-SERVERS.NET.
+
+forward-zone:
+	name: "example.com"
+	forward-addr: 1.2.3.4
+CONFIG_END
+
+SCENARIO_BEGIN Test scrubber to scrub NS record for lamelike reply from fwd
+
+; K.ROOT-SERVERS.NET.
+RANGE_BEGIN 0 100
+	ADDRESS 193.0.14.129 
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR NOERROR
+SECTION QUESTION
+. IN NS
+SECTION ANSWER
+. IN NS	K.ROOT-SERVERS.NET.
+SECTION ADDITIONAL
+K.ROOT-SERVERS.NET.	IN	A	193.0.14.129
+ENTRY_END
+RANGE_END
+
+; ns.example.com.
+RANGE_BEGIN 0 100
+	ADDRESS 1.2.3.4
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY RD RA QR NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+; must be scrubbed
+www.burritolovers.com. IN A	10.20.30.40
+SECTION AUTHORITY
+example1234.com.	IN NS	ns.example.com.
+SECTION ADDITIONAL
+ns.example.com.		IN 	A	1.2.3.4
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY RD RA QR NOERROR
+SECTION QUESTION
+mail.example.com. IN A
+SECTION ANSWER
+SECTION AUTHORITY
+; LAME
+com.	IN NS	ns.example.com.
+SECTION ADDITIONAL
+ns.example.com.		IN 	A	1.2.3.4
+ENTRY_END
+
+RANGE_END
+
+STEP 1 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+; recursion happens here.
+STEP 10 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+SECTION AUTHORITY
+SECTION ADDITIONAL
+ENTRY_END
+
+STEP 20 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+mail.example.com. IN A
+ENTRY_END
+
+STEP 30 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA SERVFAIL
+SECTION QUESTION
+mail.example.com. IN A
+SECTION ANSWER
+SECTION AUTHORITY
+SECTION ADDITIONAL
+ENTRY_END
+
+SCENARIO_END