- Fix#516 dnssec lameness detection for answers that are improper.

git-svn-id: https://unbound.nlnetlabs.nl/svn/trunk@2933 be551aaa-1e26-0410-a405-d3ace91eadb9
RakhithJK · Aug 8, 2013 · ef7be2a · ef7be2a
1 parent fe14e57
commit ef7be2a
Show file tree

Hide file tree

Showing 9 changed files with 140 additions and 48 deletions.
diff --git a/doc/Changelog b/doc/Changelog
@@ -1,3 +1,6 @@
+8 Aug 2013: Wouter
+	- Fix#516 dnssec lameness detection for answers that are improper.
+
 30 Jun 2013: Wouter
 	- tag 1.4.21rc1
 

diff --git a/iterator/iter_delegpt.c b/iterator/iter_delegpt.c
@@ -71,7 +71,7 @@ struct delegpt* delegpt_copy(struct delegpt* dp, struct regional* region)
 	copy->bogus = dp->bogus;
 	copy->has_parent_side_NS = dp->has_parent_side_NS;
 	for(ns = dp->nslist; ns; ns = ns->next) {
-		if(!delegpt_add_ns(copy, region, ns->name, (int)ns->lame))
+		if(!delegpt_add_ns(copy, region, ns->name, ns->lame))
 			return NULL;
 		copy->nslist->resolved = ns->resolved;
 		copy->nslist->got4 = ns->got4;
@@ -98,7 +98,7 @@ delegpt_set_name(struct delegpt* dp, struct regional* region, uint8_t* name)
 
 int 
 delegpt_add_ns(struct delegpt* dp, struct regional* region, uint8_t* name,
-	int lame)
+	uint8_t lame)
 {
 	struct delegpt_ns* ns;
 	size_t len;
@@ -119,7 +119,7 @@ delegpt_add_ns(struct delegpt* dp, struct regional* region, uint8_t* name,
 	ns->resolved = 0;
 	ns->got4 = 0;
 	ns->got6 = 0;
-	ns->lame = (uint8_t)lame;
+	ns->lame = lame;
 	ns->done_pside4 = 0;
 	ns->done_pside6 = 0;
 	return ns->name != 0;
@@ -156,7 +156,7 @@ delegpt_find_addr(struct delegpt* dp, struct sockaddr_storage* addr,
 int 
 delegpt_add_target(struct delegpt* dp, struct regional* region, 
 	uint8_t* name, size_t namelen, struct sockaddr_storage* addr, 
-	socklen_t addrlen, int bogus, int lame)
+	socklen_t addrlen, uint8_t bogus, uint8_t lame)
 {
 	struct delegpt_ns* ns = delegpt_find_ns(dp, name, namelen);
 	log_assert(!dp->dp_type_mlc);
@@ -176,8 +176,8 @@ delegpt_add_target(struct delegpt* dp, struct regional* region,
 
 int 
 delegpt_add_addr(struct delegpt* dp, struct regional* region, 
-	struct sockaddr_storage* addr, socklen_t addrlen, int bogus, 
-	int lame)
+	struct sockaddr_storage* addr, socklen_t addrlen, uint8_t bogus, 
+	uint8_t lame)
 {
 	struct delegpt_addr* a;
 	log_assert(!dp->dp_type_mlc);
@@ -204,6 +204,7 @@ delegpt_add_addr(struct delegpt* dp, struct regional* region,
 	a->attempts = 0;
 	a->bogus = bogus;
 	a->lame = lame;
+	a->dnsseclame = 0;
 	return 1;
 }
 
@@ -376,7 +377,7 @@ delegpt_from_message(struct dns_msg* msg, struct regional* region)
 
 int 
 delegpt_rrset_add_ns(struct delegpt* dp, struct regional* region,
-        struct ub_packed_rrset_key* ns_rrset, int lame)
+        struct ub_packed_rrset_key* ns_rrset, uint8_t lame)
 {
 	struct packed_rrset_data* nsdata = (struct packed_rrset_data*)
 		ns_rrset->entry.data;
@@ -398,7 +399,7 @@ delegpt_rrset_add_ns(struct delegpt* dp, struct regional* region,
 
 int 
 delegpt_add_rrset_A(struct delegpt* dp, struct regional* region,
-	struct ub_packed_rrset_key* ak, int lame)
+	struct ub_packed_rrset_key* ak, uint8_t lame)
 {
         struct packed_rrset_data* d=(struct packed_rrset_data*)ak->entry.data;
         size_t i;
@@ -422,7 +423,7 @@ delegpt_add_rrset_A(struct delegpt* dp, struct regional* region,
 
 int 
 delegpt_add_rrset_AAAA(struct delegpt* dp, struct regional* region,
-	struct ub_packed_rrset_key* ak, int lame)
+	struct ub_packed_rrset_key* ak, uint8_t lame)
 {
         struct packed_rrset_data* d=(struct packed_rrset_data*)ak->entry.data;
         size_t i;
@@ -446,7 +447,7 @@ delegpt_add_rrset_AAAA(struct delegpt* dp, struct regional* region,
 
 int 
 delegpt_add_rrset(struct delegpt* dp, struct regional* region,
-        struct ub_packed_rrset_key* rrset, int lame)
+        struct ub_packed_rrset_key* rrset, uint8_t lame)
 {
 	if(!rrset)
 		return 1;
@@ -548,7 +549,7 @@ int delegpt_set_name_mlc(struct delegpt* dp, uint8_t* name)
 	return (dp->name != NULL);
 }
 
-int delegpt_add_ns_mlc(struct delegpt* dp, uint8_t* name, int lame)
+int delegpt_add_ns_mlc(struct delegpt* dp, uint8_t* name, uint8_t lame)
 {
 	struct delegpt_ns* ns;
 	size_t len;
@@ -579,7 +580,7 @@ int delegpt_add_ns_mlc(struct delegpt* dp, uint8_t* name, int lame)
 }
 
 int delegpt_add_addr_mlc(struct delegpt* dp, struct sockaddr_storage* addr,
-	socklen_t addrlen, int bogus, int lame)
+	socklen_t addrlen, uint8_t bogus, uint8_t lame)
 {
 	struct delegpt_addr* a;
 	log_assert(dp->dp_type_mlc);
@@ -605,11 +606,13 @@ int delegpt_add_addr_mlc(struct delegpt* dp, struct sockaddr_storage* addr,
 	a->attempts = 0;
 	a->bogus = bogus;
 	a->lame = lame;
+	a->dnsseclame = 0;
 	return 1;
 }
 
 int delegpt_add_target_mlc(struct delegpt* dp, uint8_t* name, size_t namelen,
-	struct sockaddr_storage* addr, socklen_t addrlen, int bogus, int lame)
+	struct sockaddr_storage* addr, socklen_t addrlen, uint8_t bogus,
+	uint8_t lame)
 {
 	struct delegpt_ns* ns = delegpt_find_ns(dp, name, namelen);
 	log_assert(dp->dp_type_mlc);

diff --git a/iterator/iter_delegpt.h b/iterator/iter_delegpt.h
@@ -140,9 +140,13 @@ struct delegpt_addr {
 	int sel_rtt;
 	/** if true, the A or AAAA RR was bogus, so this address is bad.
 	 * Also check the dp->bogus to see if everything is bogus. */
-	int bogus;
+	uint8_t bogus;
 	/** if true, this address is dispreferred: it is a lame IP address */
-	int lame;
+	uint8_t lame;
+	/** if the address is dnsseclame, but this cannot be cached, this
+	 * option is useful to mark the address dnsseclame.
+	 * This value is not copied in addr-copy and dp-copy. */
+	uint8_t dnsseclame;
 };
 
 /**
@@ -179,7 +183,7 @@ int delegpt_set_name(struct delegpt* dp, struct regional* regional,
  * @return false on error.
  */
 int delegpt_add_ns(struct delegpt* dp, struct regional* regional, 
-	uint8_t* name, int lame);
+	uint8_t* name, uint8_t lame);
 
 /**
  * Add NS rrset; calls add_ns repeatedly.
@@ -190,7 +194,7 @@ int delegpt_add_ns(struct delegpt* dp, struct regional* regional,
  * @return 0 on alloc error.
  */
 int delegpt_rrset_add_ns(struct delegpt* dp, struct regional* regional,
-	struct ub_packed_rrset_key* ns_rrset, int lame);
+	struct ub_packed_rrset_key* ns_rrset, uint8_t lame);
 
 /**
  * Add target address to the delegation point.
@@ -207,7 +211,7 @@ int delegpt_rrset_add_ns(struct delegpt* dp, struct regional* regional,
  */
 int delegpt_add_target(struct delegpt* dp, struct regional* regional, 
 	uint8_t* name, size_t namelen, struct sockaddr_storage* addr, 
-	socklen_t addrlen, int bogus, int lame);
+	socklen_t addrlen, uint8_t bogus, uint8_t lame);
 
 /**
  * Add A RRset to delegpt.
@@ -218,7 +222,7 @@ int delegpt_add_target(struct delegpt* dp, struct regional* regional,
  * @return 0 on alloc error.
  */
 int delegpt_add_rrset_A(struct delegpt* dp, struct regional* regional, 
-	struct ub_packed_rrset_key* rrset, int lame);
+	struct ub_packed_rrset_key* rrset, uint8_t lame);
 
 /**
  * Add AAAA RRset to delegpt.
@@ -229,7 +233,7 @@ int delegpt_add_rrset_A(struct delegpt* dp, struct regional* regional,
  * @return 0 on alloc error.
  */
 int delegpt_add_rrset_AAAA(struct delegpt* dp, struct regional* regional, 
-	struct ub_packed_rrset_key* rrset, int lame);
+	struct ub_packed_rrset_key* rrset, uint8_t lame);
 
 /**
  * Add any RRset to delegpt.
@@ -241,7 +245,7 @@ int delegpt_add_rrset_AAAA(struct delegpt* dp, struct regional* regional,
  * @return 0 on alloc error.
  */
 int delegpt_add_rrset(struct delegpt* dp, struct regional* regional, 
-	struct ub_packed_rrset_key* rrset, int lame);
+	struct ub_packed_rrset_key* rrset, uint8_t lame);
 
 /**
  * Add address to the delegation point. No servername is associated or checked.
@@ -254,7 +258,8 @@ int delegpt_add_rrset(struct delegpt* dp, struct regional* regional,
  * @return false on error.
  */
 int delegpt_add_addr(struct delegpt* dp, struct regional* regional, 
-	struct sockaddr_storage* addr, socklen_t addrlen, int bogus, int lame);
+	struct sockaddr_storage* addr, socklen_t addrlen,
+	uint8_t bogus, uint8_t lame);
 
 /** 
  * Find NS record in name list of delegation point.
@@ -376,7 +381,7 @@ int delegpt_set_name_mlc(struct delegpt* dp, uint8_t* name);
  * @param lame: the name is lame, disprefer.
  * @return false on error.
  */
-int delegpt_add_ns_mlc(struct delegpt* dp, uint8_t* name, int lame);
+int delegpt_add_ns_mlc(struct delegpt* dp, uint8_t* name, uint8_t lame);
 
 /**
  * add an address to a malloced delegation point.
@@ -388,7 +393,7 @@ int delegpt_add_ns_mlc(struct delegpt* dp, uint8_t* name, int lame);
  * @return false on error.
  */
 int delegpt_add_addr_mlc(struct delegpt* dp, struct sockaddr_storage* addr,
-	socklen_t addrlen, int bogus, int lame);
+	socklen_t addrlen, uint8_t bogus, uint8_t lame);
 
 /**
  * Add target address to the delegation point.
@@ -403,7 +408,8 @@ int delegpt_add_addr_mlc(struct delegpt* dp, struct sockaddr_storage* addr,
  * @return false on error.
  */
 int delegpt_add_target_mlc(struct delegpt* dp, uint8_t* name, size_t namelen,
-	struct sockaddr_storage* addr, socklen_t addrlen, int bogus, int lame);
+	struct sockaddr_storage* addr, socklen_t addrlen, uint8_t bogus,
+	uint8_t lame);
 
 /** get memory in use by dp */
 size_t delegpt_get_mem(struct delegpt* dp);

diff --git a/iterator/iter_utils.c b/iterator/iter_utils.c
@@ -217,14 +217,16 @@ iter_filter_unsuitable(struct iter_env* iter_env, struct module_env* env,
 		/* select remainder from worst to best */
 		else if(reclame)
 			return rtt+USEFUL_SERVER_TOP_TIMEOUT*3; /* nonpref */
-		else if(dnsseclame )
+		else if(dnsseclame || a->dnsseclame)
 			return rtt+USEFUL_SERVER_TOP_TIMEOUT*2; /* nonpref */
 		else if(a->lame)
 			return rtt+USEFUL_SERVER_TOP_TIMEOUT+1; /* nonpref */
 		else	return rtt;
 	}
 	/* no server information present */
-	if(a->lame)
+	if(a->dnsseclame)
+		return rtt+USEFUL_SERVER_TOP_TIMEOUT*2; /* nonpref */
+	else if(a->lame)
 		return USEFUL_SERVER_TOP_TIMEOUT+1+UNKNOWN_SERVER_NICENESS; /* nonpref */
 	return UNKNOWN_SERVER_NICENESS;
 }

diff --git a/iterator/iterator.c b/iterator/iterator.c
@@ -1473,7 +1473,7 @@ processLastResort(struct module_qstate* qstate, struct iter_qstate* iq,
 			iq->chase_flags &= ~BIT_RD; /* go to authorities */
 			for(ns = p->nslist; ns; ns=ns->next) {
 				(void)delegpt_add_ns(iq->dp, qstate->region,
-					ns->name, (int)ns->lame);
+					ns->name, ns->lame);
 			}
 			for(a = p->target_list; a; a=a->next_target) {
 				(void)delegpt_add_addr(iq->dp, qstate->region,
@@ -1914,12 +1914,23 @@ processQueryResponse(struct module_qstate* qstate, struct iter_qstate* iq,
 		&& type != RESPONSE_TYPE_UNTYPED) {
 		/* a possible answer, see if it is missing DNSSEC */
 		/* but not when forwarding, so we dont mark fwder lame */
-		/* also make sure the answer is from the zone we expected,
-		 * otherwise, (due to parent,child on same server), we
-		 * might mark the server,zone lame inappropriately */
-		if(!iter_msg_has_dnssec(iq->response) &&
-			iter_msg_from_zone(iq->response, iq->dp, type,
-				iq->qchase.qclass)) {
+		if(!iter_msg_has_dnssec(iq->response)) {
+			/* Mark this address as dnsseclame in this dp,
+			 * because that will make serverselection disprefer
+			 * it, but also, once it is the only final option,
+			 * use dnssec-lame-bypass if it needs to query there.*/
+			if(qstate->reply) {
+				struct delegpt_addr* a = delegpt_find_addr(
+					iq->dp, &qstate->reply->addr,
+					qstate->reply->addrlen);
+				if(a) a->dnsseclame = 1;
+			}
+			/* test the answer is from the zone we expected,
+		 	 * otherwise, (due to parent,child on same server), we
+		 	 * might mark the server,zone lame inappropriately */
+			if(!iter_msg_from_zone(iq->response, iq->dp, type,
+				iq->qchase.qclass))
+				qstate->reply = NULL;
 			type = RESPONSE_TYPE_LAME;
 			dnsseclame = 1;
 		}
@@ -2151,8 +2162,7 @@ processQueryResponse(struct module_qstate* qstate, struct iter_qstate* iq,
 				*qstate->env->now, dnsseclame, 0,
 				iq->qchase.qtype))
 				log_err("mark host lame: out of memory");
-		} else log_err("%slame response from cache",
-			dnsseclame?"DNSSEC ":"");
+		}
 	} else if(type == RESPONSE_TYPE_REC_LAME) {
 		/* Cache the LAMEness. */
 		verbose(VERB_DETAIL, "query response REC_LAME: "
@@ -2360,12 +2370,12 @@ processTargetResponse(struct module_qstate* qstate, int id,
 			rrset->rk.dname_len)) {
 			/* if dpns->lame then set newcname ns lame too */
 			if(!delegpt_add_ns(foriq->dp, forq->region, 
-				rrset->rk.dname, (int)dpns->lame))
+				rrset->rk.dname, dpns->lame))
 				log_err("out of memory adding cnamed-ns");
 		}
 		/* if dpns->lame then set the address(es) lame too */
 		if(!delegpt_add_rrset(foriq->dp, forq->region, rrset, 
-			(int)dpns->lame))
+			dpns->lame))
 			log_err("out of memory adding targets");
 		verbose(VERB_ALGO, "added target response");
 		delegpt_log(VERB_ALGO, foriq->dp);

diff --git a/services/cache/dns.c b/services/cache/dns.c
@@ -267,7 +267,7 @@ cache_fill_missing(struct module_env* env, uint16_t qclass,
 		akey = rrset_cache_lookup(env->rrset_cache, ns->name, 
 			ns->namelen, LDNS_RR_TYPE_A, qclass, 0, now, 0);
 		if(akey) {
-			if(!delegpt_add_rrset_A(dp, region, akey, (int)ns->lame)) {
+			if(!delegpt_add_rrset_A(dp, region, akey, ns->lame)) {
 				lock_rw_unlock(&akey->entry.lock);
 				return 0;
 			}
@@ -285,7 +285,7 @@ cache_fill_missing(struct module_env* env, uint16_t qclass,
 		akey = rrset_cache_lookup(env->rrset_cache, ns->name, 
 			ns->namelen, LDNS_RR_TYPE_AAAA, qclass, 0, now, 0);
 		if(akey) {
-			if(!delegpt_add_rrset_AAAA(dp, region, akey, (int)ns->lame)) {
+			if(!delegpt_add_rrset_AAAA(dp, region, akey, ns->lame)) {
 				lock_rw_unlock(&akey->entry.lock);
 				return 0;
 			}

diff --git a/testdata/autotrust_revtp_use.rpl b/testdata/autotrust_revtp_use.rpl
@@ -89,16 +89,24 @@ example.com.	10800	IN	RRSIG	DNSKEY 5 2 10800 20091124111500 20091018111500 16614
 ENTRY_END
 
 ENTRY_BEGIN
-MATCH opcode subdomain
+MATCH opcode qname qtype
 ADJUST copy_id copy_query
 REPLY QR
 SECTION QUESTION
 www.example.com. IN A
 SECTION ANSWER
 www.example.com. IN A 10.20.30.40
 ENTRY_END
-RANGE_END
 
+ENTRY_BEGIN
+MATCH opcode qname qtype
+ADJUST copy_id copy_query
+REPLY QR
+SECTION QUESTION
+ns.example.com. IN AAAA
+SECTION ANSWER
+; no AAAA
+ENTRY_END
 RANGE_END
 
 STEP 20 QUERY