- 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

diff --git a/doc/Changelog b/doc/Changelog
index 3060b97df9c643cde384bbfade87ad35116f6264..e7fe28fe7d59bd0ad78c892cb0d93f52c5b6e17a 100644 (file)
--- 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.

diff --git a/iterator/iter_resptype.c b/iterator/iter_resptype.c
index 40d71358b5ef3e7f401ae1abfd5c76b2f2c566b0..9f7726c603baaf88fade2f287f2e6eb68a3965d5 100644 (file)
--- 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 

diff --git a/iterator/iter_scrub.c b/iterator/iter_scrub.c
index 7ec032a7216fe69891a3ddc281d1e3110a61fbe1..f9a88f2b2fd4db7268e100e60392f75cab6fe886 100644 (file)
--- 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)) {

diff --git a/testdata/iter_scrub_ns.rpl b/testdata/iter_scrub_ns.rpl
new file mode 100644 (file)
index 0000000..365f0b5
--- /dev/null
+++ 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

diff --git a/testdata/iter_scrub_ns_fwd.rpl b/testdata/iter_scrub_ns_fwd.rpl
new file mode 100644 (file)
index 0000000..239dc37
--- /dev/null
+++ 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