Merge in a commit from upstream..

- Fix validation failure in case upstream forwarder (ISC BIND) does
  not have the same trust anchors and decides to insert unsigned NS
  record in authority section.

ok sthen@

3 files changed, 101 insertions, 24 deletions

diff --git a/usr.sbin/unbound/validator/val_utils.c b/usr.sbin/unbound/validator/val_utils.c
index b0475d8031c..475b0c9054e 100644
--- a/usr.sbin/unbound/validator/val_utils.c
+++ b/usr.sbin/unbound/validator/val_utils.c
@@ -21,16 +21,16 @@
  * specific prior written permission.
  * 
  * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
- * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE
- * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED
+ * TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 /**
@@ -54,7 +54,6 @@
 #include "util/net_help.h"
 #include "util/module.h"
 #include "util/regional.h"
-#include "util/config_file.h"
 
 enum val_classification 
 val_classify_response(uint16_t query_flags, struct query_info* origqinf,
@@ -487,7 +486,7 @@ val_verify_DNSKEY_with_DS(struct module_env* env, struct val_env* ve,
 		/* Once we see a single DS with a known digestID and 
 		 * algorithm, we cannot return INSECURE (with a 
 		 * "null" KeyEntry). */
-		has_useful_ds = true;
+		has_useful_ds = 1;
 
 		sec = verify_dnskeys_with_ds_rr(env, ve, dnskey_rrset, 
 			ds_rrset, i, reason);
@@ -597,7 +596,7 @@ val_verify_DNSKEY_with_TA(struct module_env* env, struct val_env* ve,
 		/* Once we see a single DS with a known digestID and 
 		 * algorithm, we cannot return INSECURE (with a 
 		 * "null" KeyEntry). */
-		has_useful_ta = true;
+		has_useful_ta = 1;
 
 		sec = verify_dnskeys_with_ds_rr(env, ve, dnskey_rrset, 
 			ta_ds, i, reason);
@@ -623,7 +622,7 @@ val_verify_DNSKEY_with_TA(struct module_env* env, struct val_env* ve,
 			continue;
 
 		/* we saw a useful TA */
-		has_useful_ta = true;
+		has_useful_ta = 1;
 
 		sec = dnskey_verify_rrset(env, ve, dnskey_rrset,
 			ta_dnskey, i, reason);
@@ -774,6 +773,8 @@ rrset_has_signer(struct ub_packed_rrset_key* rrset, uint8_t* name, size_t len)
 	for(i = d->count; i< d->count+d->rrsig_count; i++) {
 		if(d->rr_len[i] > 2+18+len) {
 			/* at least rdatalen + signature + signame (+1 sig)*/
+			if(!dname_valid(d->rr_data[i]+2+18, d->rr_len[i]-2-18))
+				continue;
 			if(query_dname_compare(name, d->rr_data[i]+2+18) == 0)
 			{
 				return 1;
@@ -845,6 +846,18 @@ val_fill_reply(struct reply_info* chase, struct reply_info* orig,
 		chase->ar_numrrsets;
 }
 
+void val_reply_remove_auth(struct reply_info* rep, size_t index)
+{
+	log_assert(index < rep->rrset_count);
+	log_assert(index >= rep->an_numrrsets);
+	log_assert(index < rep->an_numrrsets+rep->ns_numrrsets);
+	memmove(rep->rrsets+index, rep->rrsets+index+1,
+		sizeof(struct ub_packed_rrset_key*)*
+		(rep->rrset_count - index - 1));
+	rep->ns_numrrsets--;
+	rep->rrset_count--;
+}
+
 void
 val_check_nonsecure(struct val_env* ve, struct reply_info* rep) 
 {
diff --git a/usr.sbin/unbound/validator/val_utils.h b/usr.sbin/unbound/validator/val_utils.h
index f0afc3756e9..cdb87697e1c 100644
--- a/usr.sbin/unbound/validator/val_utils.h
+++ b/usr.sbin/unbound/validator/val_utils.h
@@ -21,16 +21,16 @@
  * specific prior written permission.
  * 
  * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
- * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE
- * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
- * POSSIBILITY OF SUCH DAMAGE.
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED
+ * TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 /**
@@ -295,6 +295,13 @@ void val_fill_reply(struct reply_info* chase, struct reply_info* orig,
 	size_t cname_skip, uint8_t* name, size_t len, uint8_t* signer);
 
 /**
+ * Remove rrset with index from reply, from the authority section.
+ * @param rep: reply to remove it from.
+ * @param index: rrset to remove, must be in the authority section.
+ */
+void val_reply_remove_auth(struct reply_info* rep, size_t index);
+
+/**
  * Remove all unsigned or non-secure status rrsets from NS and AR sections.
  * So that unsigned data does not get let through to clients, when we have
  * found the data to be secure.
diff --git a/usr.sbin/unbound/validator/validator.c b/usr.sbin/unbound/validator/validator.c
index 9d5d5c39025..cc07cc2b152 100644
--- a/usr.sbin/unbound/validator/validator.c
+++ b/usr.sbin/unbound/validator/validator.c
@@ -574,6 +574,61 @@ detect_wrongly_truncated(struct reply_info* rep)
 	return 1;
 }
 
+/**
+ * For messages that are not referrals, if the chase reply contains an
+ * unsigned NS record in the authority section it could have been
+ * inserted by a (BIND) forwarder that thinks the zone is insecure, and
+ * that has an NS record without signatures in cache.  Remove the NS
+ * record since the reply does not hinge on that record (in the authority
+ * section), but do not remove it if it removes the last record from the
+ * answer+authority sections.
+ * @param chase_reply: the chased reply, we have a key for this contents,
+ * 	so we should have signatures for these rrsets and not having
+ * 	signatures means it will be bogus.
+ * @param orig_reply: original reply, remove NS from there as well because
+ * 	we cannot mark the NS record as DNSSEC valid because it is not
+ * 	validated by signatures.
+ */
+static void
+remove_spurious_authority(struct reply_info* chase_reply,
+	struct reply_info* orig_reply)
+{
+	size_t i, found = 0;
+	int remove = 0;
+	/* if no answer and only 1 auth RRset, do not remove that one */
+	if(chase_reply->an_numrrsets == 0 && chase_reply->ns_numrrsets == 1)
+		return;
+	/* search authority section for unsigned NS records */
+	for(i = chase_reply->an_numrrsets;
+		i < chase_reply->an_numrrsets+chase_reply->ns_numrrsets; i++) {
+		struct packed_rrset_data* d = (struct packed_rrset_data*)
+			chase_reply->rrsets[i]->entry.data;
+		if(ntohs(chase_reply->rrsets[i]->rk.type) == LDNS_RR_TYPE_NS
+			&& d->rrsig_count == 0) {
+			found = i;
+			remove = 1;
+			break;
+		}
+	}
+	/* see if we found the entry */
+	if(!remove) return;
+	log_rrset_key(VERB_ALGO, "Removing spurious unsigned NS record "
+		"(likely inserted by forwarder)", chase_reply->rrsets[found]);
+
+	/* find rrset in orig_reply */
+	for(i = orig_reply->an_numrrsets;
+		i < orig_reply->an_numrrsets+orig_reply->ns_numrrsets; i++) {
+		if(ntohs(orig_reply->rrsets[i]->rk.type) == LDNS_RR_TYPE_NS
+			&& query_dname_compare(orig_reply->rrsets[i]->rk.dname,
+				chase_reply->rrsets[found]->rk.dname) == 0) {
+			/* remove from orig_msg */
+			val_reply_remove_auth(orig_reply, i);
+			break;
+		}
+	}
+	/* remove rrset from chase_reply */
+	val_reply_remove_auth(chase_reply, found);
+}
 
 /**
  * Given a "positive" response -- a response that contains an answer to the
@@ -1642,6 +1697,8 @@ processValidate(struct module_qstate* qstate, struct val_qstate* vq,
 	}
 	subtype = val_classify_response(qstate->query_flags, &qstate->qinfo,
 		&vq->qchase, vq->orig_msg->rep, vq->rrset_skip);
+	if(subtype != VAL_CLASS_REFERRAL)
+		remove_spurious_authority(vq->chase_reply, vq->orig_msg->rep);
 
 	/* check signatures in the message; 
 	 * answer and authority must be valid, additional is only checked. */