From: Wouter Wijngaards Date: Wed, 24 Feb 2010 13:27:47 +0000 (+0000) Subject: - Fix scrubber bug that potentially let NS records through. Reported X-Git-Tag: release-1.4.2~18 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5dcbb54e632cb727630ba8c3c357d644f7ae5913;p=thirdparty%2Funbound.git - 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 3060b97df..e7fe28fe7 100644 --- 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 40d71358b..9f7726c60 100644 --- 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 7ec032a72..f9a88f2b2 100644 --- 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 index 000000000..365f0b54e --- /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 index 000000000..239dc37f9 --- /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