- Fix resolve of names that use a mix of public and private addresses.

git-svn-id: file:///svn/unbound/trunk@2868 be551aaa-1e26-0410-a405-d3ace91eadb9

doc/Changelog

@@ -1,3 +1,6 @@
+22 March 2013: Wouter
+	- Fix resolve of names that use a mix of public and private addresses.
+
 21 March 2013: Wouter
 	- release 1.4.20
 	- trunk has 1.4.21

iterator/iter_priv.c

@@ -208,6 +208,27 @@ size_t priv_get_mem(struct iter_priv* priv)
 	return sizeof(*priv) + regional_get_mem(priv->region);
 }
 
+/** remove RR from msgparse RRset, return true if rrset is entirely bad */
+static int
+remove_rr(const char* str, ldns_buffer* pkt, struct rrset_parse* rrset,
+	struct rr_parse* prev, struct rr_parse** rr, struct sockaddr_storage* addr, socklen_t addrlen)
+{
+	if(verbosity >= VERB_QUERY && rrset->dname_len <= LDNS_MAX_DOMAINLEN) {
+		uint8_t buf[LDNS_MAX_DOMAINLEN+1];
+		dname_pkt_copy(pkt, buf, rrset->dname);
+		log_name_addr(VERB_QUERY, str, buf, addr, addrlen);
+	}
+	if(prev)
+		prev->next = (*rr)->next;
+	else	rrset->rr_first = (*rr)->next;
+	if(rrset->rr_last == *rr)
+		rrset->rr_last = prev;
+	rrset->rr_count --;
+	rrset->size -= (*rr)->size;
+	(*rr) = (*rr)->next;
+	return rrset->rr_count == 0;
+}
+
 int priv_rrset_bad(struct iter_priv* priv, ldns_buffer* pkt,
 	struct rrset_parse* rrset)
 {
@@ -221,7 +242,7 @@ int priv_rrset_bad(struct iter_priv* priv, ldns_buffer* pkt,
 	} else {
 		/* so its a public name, check the address */
 		socklen_t len;
-		struct rr_parse* rr;
+		struct rr_parse* rr, *prev = NULL;
 		if(rrset->type == LDNS_RR_TYPE_A) {
 			struct sockaddr_storage addr;
 			struct sockaddr_in sa;
@@ -232,13 +253,19 @@ int priv_rrset_bad(struct iter_priv* priv, ldns_buffer* pkt,
 			sa.sin_port = (in_port_t)htons(UNBOUND_DNS_PORT);
 			for(rr = rrset->rr_first; rr; rr = rr->next) {
 				if(ldns_read_uint16(rr->ttl_data+4) 
-					!= INET_SIZE)
+					!= INET_SIZE) {
+					prev = rr;
 					continue;
+				}
 				memmove(&sa.sin_addr, rr->ttl_data+4+2, 
 					INET_SIZE);
 				memmove(&addr, &sa, len);
-				if(priv_lookup_addr(priv, &addr, len))
+				if(priv_lookup_addr(priv, &addr, len)) {
+					if(remove_rr("sanitize: removing public name with private address", pkt, rrset, prev, &rr, &addr, len))
 						return 1;
+					continue;
+				}
+				prev = rr;
 			}
 		} else if(rrset->type == LDNS_RR_TYPE_AAAA) {
 			struct sockaddr_storage addr;
@@ -249,13 +276,19 @@ int priv_rrset_bad(struct iter_priv* priv, ldns_buffer* pkt,
 			sa.sin6_port = (in_port_t)htons(UNBOUND_DNS_PORT);
 			for(rr = rrset->rr_first; rr; rr = rr->next) {
 				if(ldns_read_uint16(rr->ttl_data+4) 
-					!= INET6_SIZE)
+					!= INET6_SIZE) {
+					prev = rr;
 					continue;
+				}
 				memmove(&sa.sin6_addr, rr->ttl_data+4+2, 
 					INET6_SIZE);
 				memmove(&addr, &sa, len);
-				if(priv_lookup_addr(priv, &addr, len)) 
+				if(priv_lookup_addr(priv, &addr, len)) {
+					if(remove_rr("sanitize: removing public name with private address", pkt, rrset, prev, &rr, &addr, len))
 						return 1;
+					continue;
+				}
+				prev = rr;
 			}
 		} 
 	}

iterator/iter_priv.h

@@ -92,6 +92,8 @@ int priv_apply_cfg(struct iter_priv* priv, struct config_file* cfg);
 
 /**
  * See if rrset is bad.
+ * Will remove individual RRs that are bad (if possible) to
+ * sanitize the RRset without removing it completely.
  * @param priv: structure for private address storage.
  * @param pkt: packet to decompress rrset name in.
  * @param rrset: the rrset to examine, A or AAAA.

iterator/iter_scrub.c

@@ -62,7 +62,7 @@ static void
 remove_rrset(const char* str, ldns_buffer* pkt, struct msg_parse* msg, 
 	struct rrset_parse* prev, struct rrset_parse** rrset)
 {
-	if(verbosity >= VERB_QUERY 
+	if(verbosity >= VERB_QUERY && str
 		&& (*rrset)->dname_len <= LDNS_MAX_DOMAINLEN) {
 		uint8_t buf[LDNS_MAX_DOMAINLEN+1];
 		dname_pkt_copy(pkt, buf, (*rrset)->dname);
@@ -646,15 +646,17 @@ scrub_sanitize(ldns_buffer* pkt, struct msg_parse* msg,
 
 		/* remove private addresses */
 		if( (rrset->type == LDNS_RR_TYPE_A || 
-			rrset->type == LDNS_RR_TYPE_AAAA) &&
+			rrset->type == LDNS_RR_TYPE_AAAA)) {
-			priv_rrset_bad(ie->priv, pkt, rrset)) {
 
 			/* do not set servfail since this leads to too
 			 * many drops of other people using rfc1918 space */
-			remove_rrset("sanitize: removing public name with "
+			/* also do not remove entire rrset, unless all records
-				"private address", pkt, msg, prev, &rrset);
+			 * in it are bad */
+			if(priv_rrset_bad(ie->priv, pkt, rrset)) {
+				remove_rrset(NULL, pkt, msg, prev, &rrset);
 				continue;
 			}
+		}
 		
 		/* skip DNAME records -- they will always be followed by a 
 		 * synthesized CNAME, which will be relevant.

testdata/iter_privaddr.rpl

@@ -163,6 +163,21 @@ example.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
+toss.example.com. IN A
+SECTION ANSWER
+toss.example.com. IN A	10.20.30.40
+toss.example.com. IN A	1.2.3.4
+SECTION AUTHORITY
+example.com.	IN NS	ns.example.com.
+SECTION ADDITIONAL
+ns.example.com.		IN 	A	1.2.3.4
+ENTRY_END
 RANGE_END
 
 ; public address is not scrubbed
@@ -243,4 +258,27 @@ SECTION ANSWER
 mail.example.net. IN A 10.20.30.40
 ENTRY_END
 
+; rest of RRset intact, only 10/8 tossed away.
+STEP 60 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+toss.example.com. IN A
+ENTRY_END
+
+STEP 70 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+toss.example.com. IN A
+SECTION ANSWER
+; toss.example.com. IN A	10.20.30.40
+toss.example.com. IN A	1.2.3.4
+SECTION AUTHORITY
+example.com.	IN NS	ns.example.com.
+SECTION ADDITIONAL
+ns.example.com.		IN 	A	1.2.3.4
+ENTRY_END
+
 SCENARIO_END