- 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.

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

diff --git a/doc/Changelog b/doc/Changelog
index 72a83a5b233c5b942cabbe15ccb612dc6f402c6e..93e4e5fb85839788373e64ba7d08cfaec6032970 100644 (file)
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -1,3 +1,8 @@
+9 February 2015: Wouter
+       - 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.
+
 2 February 2015: Wouter
        - infra-cache-min-rtt patch from Florian Riehm, for expected long
          uplink roundtrip times.

diff --git a/testdata/val_spurious_ns.rpl b/testdata/val_spurious_ns.rpl
new file mode 100644 (file)
index 0000000..741fd1a
--- /dev/null
+++ b/testdata/val_spurious_ns.rpl
@@ -0,0 +1,151 @@
+; config options
+; The island of trust is at example.com
+server:
+       trust-anchor: "example.com.    3600    IN      DS      2854 3 1 46e4ffc6e9a4793b488954bd3f0cc6af0dfb201b"
+       val-override-date: "20070916134226"
+       target-fetch-policy: "0 0 0 0 0"
+
+stub-zone:
+       name: "."
+       stub-addr: 193.0.14.129         # K.ROOT-SERVERS.NET.
+CONFIG_END
+
+SCENARIO_BEGIN Test validator with spurious unsigned NS in auth section 
+
+; 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
+
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION AUTHORITY
+com.   IN NS   a.gtld-servers.net.
+SECTION ADDITIONAL
+a.gtld-servers.net.    IN      A       192.5.6.30
+ENTRY_END
+RANGE_END
+
+; a.gtld-servers.net.
+RANGE_BEGIN 0 100
+       ADDRESS 192.5.6.30
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR NOERROR
+SECTION QUESTION
+com. IN NS
+SECTION ANSWER
+com.    IN NS   a.gtld-servers.net.
+SECTION ADDITIONAL
+a.gtld-servers.net.     IN      A       192.5.6.30
+ENTRY_END
+
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION AUTHORITY
+example.com.   IN NS   ns.example.com.
+SECTION ADDITIONAL
+ns.example.com.                IN      A       1.2.3.4
+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
+example.com. IN NS
+SECTION ANSWER
+example.com.    IN NS   ns.example.com.
+example.com.    3600    IN      RRSIG   NS 3 2 3600 20070926134150 20070829134150 2854 example.com. MC0CFQCN+qHdJxoI/2tNKwsb08pra/G7aAIUAWA5sDdJTbrXA1/3OaesGBAO3sI= ;{id = 2854}
+SECTION ADDITIONAL
+ns.example.com.         IN      A       1.2.3.4
+ns.example.com. 3600    IN      RRSIG   A 3 3 3600 20070926135752 20070829135752 2854 example.com. MC0CFQCMSWxVehgOQLoYclB9PIAbNP229AIUeH0vNNGJhjnZiqgIOKvs1EhzqAo= ;{id = 2854}
+ENTRY_END
+
+; response to DNSKEY priming query
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR NOERROR
+SECTION QUESTION
+example.com. IN DNSKEY
+SECTION ANSWER
+example.com.    3600    IN      DNSKEY  256 3 3 ALXLUsWqUrY3JYER3T4TBJII s70j+sDS/UT2QRp61SE7S3E EXopNXoFE73JLRmvpi/UrOO/Vz4Se 6wXv/CYCKjGw06U4WRgR YXcpEhJROyNapmdIKSx hOzfLVE1gqA0PweZR8d tY3aNQSRn3sPpwJr6Mi /PqQKAMMrZ9ckJpf1+b QMOOvxgzz2U1GS18b3y ZKcgTMEaJzd/GZYzi/B N2DzQ0MsrSwYXfsNLFO Bbs8PJMW4LYIxeeOe6rUgkWOF 7CC9Dh/dduQ1QrsJhmZAEFfd6ByYV+ ;{id = 2854 (zsk), size = 1688b}
+example.com.    3600    IN      RRSIG   DNSKEY 3 2 3600 20070926134802 20070829134802 2854 example.com. MCwCFG1yhRNtTEa3Eno2zhVVuy2EJX3wAhQeLyUp6+UXcpC5qGNu9tkrTEgPUg== ;{id = 2854}
+SECTION AUTHORITY
+example.com.   IN NS   ns.example.com.
+example.com.    3600    IN      RRSIG   NS 3 2 3600 20070926134150 20070829134150 2854 example.com. MC0CFQCN+qHdJxoI/2tNKwsb08pra/G7aAIUAWA5sDdJTbrXA1/3OaesGBAO3sI= ;{id = 2854}
+SECTION ADDITIONAL
+ns.example.com.                IN      A       1.2.3.4
+ns.example.com. 3600    IN      RRSIG   A 3 3 3600 20070926135752 20070829135752 2854 example.com. MC0CFQCMSWxVehgOQLoYclB9PIAbNP229AIUeH0vNNGJhjnZiqgIOKvs1EhzqAo= ;{id = 2854}
+ENTRY_END
+
+; response to query of interest
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. IN A  10.20.30.40
+ns.example.com. 3600    IN      RRSIG   A 3 3 3600 20070926134150 20070829134150 2854 example.com. MC0CFQCQMyTjn7WWwpwAR1LlVeLpRgZGuQIUCcJDEkwAuzytTDRlYK7nIMwH1CM= ;{id = 2854}
+SECTION AUTHORITY
+example.com.   IN NS   ns.example.com.
+;example.com.    3600    IN      RRSIG   NS 3 2 3600 20070926134150 20070829134150 2854 example.com. MC0CFQCN+qHdJxoI/2tNKwsb08pra/G7aAIUAWA5sDdJTbrXA1/3OaesGBAO3sI= ;{id = 2854}
+SECTION ADDITIONAL
+ns.example.com.                IN      A       1.2.3.4
+www.example.com.        3600    IN      RRSIG   A 3 3 3600 20070926134150 20070829134150 2854 example.com. MC0CFC99iE9K5y2WNgI0gFvBWaTi9wm6AhUAoUqOpDtG5Zct+Qr9F3mSdnbc6V4= ;{id = 2854}
+ENTRY_END
+RANGE_END
+
+STEP 1 QUERY
+ENTRY_BEGIN
+REPLY RD DO
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+
+; recursion happens here.
+STEP 10 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA AD DO NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. IN A  10.20.30.40
+www.example.com.        3600    IN      RRSIG   A 3 3 3600 20070926134150 20070829134150 2854 example.com. MC0CFC99iE9K5y2WNgI0gFvBWaTi9wm6AhUAoUqOpDtG5Zct+Qr9F3mSdnbc6V4= ;{id = 2854}
+SECTION AUTHORITY
+; removed by spurious NS record removal code
+;;example.com. IN NS   ns.example.com.
+;;example.com.    3600    IN      RRSIG   NS 3 2 3600 20070926134150 20070829134150 2854 example.com. MC0CFQCN+qHdJxoI/2tNKwsb08pra/G7aAIUAWA5sDdJTbrXA1/3OaesGBAO3sI= ;{id = 2854}
+SECTION ADDITIONAL
+ns.example.com.                IN      A       1.2.3.4
+ns.example.com. 3600    IN      RRSIG   A 3 3 3600 20070926134150 20070829134150 2854 example.com. MC0CFQCQMyTjn7WWwpwAR1LlVeLpRgZGuQIUCcJDEkwAuzytTDRlYK7nIMwH1CM= ;{id = 2854}
+ENTRY_END
+
+SCENARIO_END

diff --git a/validator/val_utils.c b/validator/val_utils.c
index ecf2dfaf05c89bd34c829277818598fc56fd8226..475b0c9054e04fe0dc90fb8810693fde0dc58089 100644 (file)
--- a/validator/val_utils.c
+++ b/validator/val_utils.c
@@ -846,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/validator/val_utils.h b/validator/val_utils.h
index b0344eff7de9f113a1d0c41ec7e707bd0854c839..cdb87697e1c2845ee26fdd1a7674e3f96d2dd938 100644 (file)
--- a/validator/val_utils.h
+++ b/validator/val_utils.h
@@ -294,6 +294,13 @@ int val_chase_cname(struct query_info* qchase, struct reply_info* rep,
 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

diff --git a/validator/validator.c b/validator/validator.c
index 9d5d5c39025473a56ee1b549f0f4e8be6bf0220f..cc07cc2b15255f59b099d58f52404bdc089af722 100644 (file)
--- a/validator/validator.c
+++ b/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. */