- Fix resolve of names that use a mix of public and private addresses.

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

diff --git a/doc/Changelog b/doc/Changelog
index 3f90165757f6d5881689c329482f0b4718e9f186..6830765c9dd3c8caa38c5ebb104dbcf0bb3a21e7 100644 (file)
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -1,3 +1,6 @@
+22 March 2013: Wouter
+       - Fix resolve of names that use a mix of public and private addresses.
+
 21 March 2013: Wouter
        - release 1.4.20
        - trunk has 1.4.21

diff --git a/iterator/iter_priv.c b/iterator/iter_priv.c
index db7dbe5faa65e02fe63abbca6e07271606ff4ac3..8f60489c3030aadd8e63324be8d2162e1b4f18b9 100644 (file)
--- a/iterator/iter_priv.c
+++ b/iterator/iter_priv.c
@@ -208,6 +208,27 @@ size_t priv_get_mem(struct iter_priv* priv)
        return sizeof(*priv) + regional_get_mem(priv->region);
 }
 
+/** remove RR from msgparse RRset, return true if rrset is entirely bad */
+static int
+remove_rr(const char* str, ldns_buffer* pkt, struct rrset_parse* rrset,
+       struct rr_parse* prev, struct rr_parse** rr, struct sockaddr_storage* addr, socklen_t addrlen)
+{
+       if(verbosity >= VERB_QUERY && rrset->dname_len <= LDNS_MAX_DOMAINLEN) {
+               uint8_t buf[LDNS_MAX_DOMAINLEN+1];
+               dname_pkt_copy(pkt, buf, rrset->dname);
+               log_name_addr(VERB_QUERY, str, buf, addr, addrlen);
+       }
+       if(prev)
+               prev->next = (*rr)->next;
+       else    rrset->rr_first = (*rr)->next;
+       if(rrset->rr_last == *rr)
+               rrset->rr_last = prev;
+       rrset->rr_count --;
+       rrset->size -= (*rr)->size;
+       (*rr) = (*rr)->next;
+       return rrset->rr_count == 0;
+}
+
 int priv_rrset_bad(struct iter_priv* priv, ldns_buffer* pkt,
        struct rrset_parse* rrset)
 {
@@ -221,7 +242,7 @@ int priv_rrset_bad(struct iter_priv* priv, ldns_buffer* pkt,
        } else {
                /* so its a public name, check the address */
                socklen_t len;
-               struct rr_parse* rr;
+               struct rr_parse* rr, *prev = NULL;
                if(rrset->type == LDNS_RR_TYPE_A) {
                        struct sockaddr_storage addr;
                        struct sockaddr_in sa;
@@ -232,13 +253,19 @@ int priv_rrset_bad(struct iter_priv* priv, ldns_buffer* pkt,
                        sa.sin_port = (in_port_t)htons(UNBOUND_DNS_PORT);
                        for(rr = rrset->rr_first; rr; rr = rr->next) {
                                if(ldns_read_uint16(rr->ttl_data+4) 
-                                       != INET_SIZE)
+                                       != INET_SIZE) {
+                                       prev = rr;
                                        continue;
+                               }
                                memmove(&sa.sin_addr, rr->ttl_data+4+2, 
                                        INET_SIZE);
                                memmove(&addr, &sa, len);
-                               if(priv_lookup_addr(priv, &addr, len))
-                                       return 1;
+                               if(priv_lookup_addr(priv, &addr, len)) {
+                                       if(remove_rr("sanitize: removing public name with private address", pkt, rrset, prev, &rr, &addr, len))
+                                               return 1;
+                                       continue;
+                               }
+                               prev = rr;
                        }
                } else if(rrset->type == LDNS_RR_TYPE_AAAA) {
                        struct sockaddr_storage addr;
@@ -249,13 +276,19 @@ int priv_rrset_bad(struct iter_priv* priv, ldns_buffer* pkt,
                        sa.sin6_port = (in_port_t)htons(UNBOUND_DNS_PORT);
                        for(rr = rrset->rr_first; rr; rr = rr->next) {
                                if(ldns_read_uint16(rr->ttl_data+4) 
-                                       != INET6_SIZE)
+                                       != INET6_SIZE) {
+                                       prev = rr;
                                        continue;
+                               }
                                memmove(&sa.sin6_addr, rr->ttl_data+4+2, 
                                        INET6_SIZE);
                                memmove(&addr, &sa, len);
-                               if(priv_lookup_addr(priv, &addr, len)) 
-                                       return 1;
+                               if(priv_lookup_addr(priv, &addr, len)) {
+                                       if(remove_rr("sanitize: removing public name with private address", pkt, rrset, prev, &rr, &addr, len))
+                                               return 1;
+                                       continue;
+                               }
+                               prev = rr;
                        }
                } 
        }

diff --git a/iterator/iter_priv.h b/iterator/iter_priv.h
index f6264f8d0c9bcb7c053ed9a89bfc20287909ef84..e6e51c159b09b9419c15c65fe4845c46f4b34e25 100644 (file)
--- a/iterator/iter_priv.h
+++ b/iterator/iter_priv.h
@@ -92,6 +92,8 @@ int priv_apply_cfg(struct iter_priv* priv, struct config_file* cfg);
 
 /**
  * See if rrset is bad.
+ * Will remove individual RRs that are bad (if possible) to
+ * sanitize the RRset without removing it completely.
  * @param priv: structure for private address storage.
  * @param pkt: packet to decompress rrset name in.
  * @param rrset: the rrset to examine, A or AAAA.

diff --git a/iterator/iter_scrub.c b/iterator/iter_scrub.c
index 6147c96a9e37eb9e0e7e18fa48d90ed299b01d88..999f878027fde642a578c480adaf20e2c235b576 100644 (file)
--- a/iterator/iter_scrub.c
+++ b/iterator/iter_scrub.c
@@ -62,7 +62,7 @@ static void
 remove_rrset(const char* str, ldns_buffer* pkt, struct msg_parse* msg, 
        struct rrset_parse* prev, struct rrset_parse** rrset)
 {
-       if(verbosity >= VERB_QUERY 
+       if(verbosity >= VERB_QUERY && str
                && (*rrset)->dname_len <= LDNS_MAX_DOMAINLEN) {
                uint8_t buf[LDNS_MAX_DOMAINLEN+1];
                dname_pkt_copy(pkt, buf, (*rrset)->dname);
@@ -646,14 +646,16 @@ scrub_sanitize(ldns_buffer* pkt, struct msg_parse* msg,
 
                /* remove private addresses */
                if( (rrset->type == LDNS_RR_TYPE_A || 
-                       rrset->type == LDNS_RR_TYPE_AAAA) &&
-                       priv_rrset_bad(ie->priv, pkt, rrset)) {
+                       rrset->type == LDNS_RR_TYPE_AAAA)) {
 
                        /* do not set servfail since this leads to too
                         * many drops of other people using rfc1918 space */
-                       remove_rrset("sanitize: removing public name with "
-                               "private address", pkt, msg, prev, &rrset);
-                       continue;
+                       /* also do not remove entire rrset, unless all records
+                        * in it are bad */
+                       if(priv_rrset_bad(ie->priv, pkt, rrset)) {
+                               remove_rrset(NULL, pkt, msg, prev, &rrset);
+                               continue;
+                       }
                }
                
                /* skip DNAME records -- they will always be followed by a 

diff --git a/testdata/iter_privaddr.rpl b/testdata/iter_privaddr.rpl
index 63fa5331bb2082dc0700e4f37dfeedbcbeda6b1e..4b1780874fa3ddd117a6d79eaa0b6c4ad4ac0f30 100644 (file)
--- a/testdata/iter_privaddr.rpl
+++ b/testdata/iter_privaddr.rpl
@@ -163,6 +163,21 @@ example.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
+toss.example.com. IN A
+SECTION ANSWER
+toss.example.com. IN A 10.20.30.40
+toss.example.com. IN A 1.2.3.4
+SECTION AUTHORITY
+example.com.   IN NS   ns.example.com.
+SECTION ADDITIONAL
+ns.example.com.                IN      A       1.2.3.4
+ENTRY_END
 RANGE_END
 
 ; public address is not scrubbed
@@ -243,4 +258,27 @@ SECTION ANSWER
 mail.example.net. IN A 10.20.30.40
 ENTRY_END
 
+; rest of RRset intact, only 10/8 tossed away.
+STEP 60 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+toss.example.com. IN A
+ENTRY_END
+
+STEP 70 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD RA NOERROR
+SECTION QUESTION
+toss.example.com. IN A
+SECTION ANSWER
+; toss.example.com. IN A       10.20.30.40
+toss.example.com. IN A 1.2.3.4
+SECTION AUTHORITY
+example.com.   IN NS   ns.example.com.
+SECTION ADDITIONAL
+ns.example.com.                IN      A       1.2.3.4
+ENTRY_END
+
 SCENARIO_END