From 368386c01130e02b0705401f8d420b26bb9dc814 Mon Sep 17 00:00:00 2001 From: "W.C.A. Wijngaards" Date: Fri, 12 Jul 2019 14:34:35 +0200 Subject: [PATCH] - Fix #48: Unbound returns additional records on NODATA response, if minimal-responses is enabled, also the additional for negative responses is removed. --- doc/Changelog | 5 ++ testdata/fwd_minimal.rpl | 125 +++++++++++++++++++++++++++++++++++++++ util/data/msgencode.c | 66 ++++++++++++++------- 3 files changed, 175 insertions(+), 21 deletions(-) create mode 100644 testdata/fwd_minimal.rpl diff --git a/doc/Changelog b/doc/Changelog index 7b26c410a..3b9618cac 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,3 +1,8 @@ +12 July 2019: Wouter + - Fix #48: Unbound returns additional records on NODATA response, + if minimal-responses is enabled, also the additional for negative + responses is removed. + 9 July 2019: Ralph - Fix in respip addrtree selection. Absence of addr_tree_init_parents() call made it impossible to go up the tree when the matching netmask is diff --git a/testdata/fwd_minimal.rpl b/testdata/fwd_minimal.rpl new file mode 100644 index 000000000..e85d7124b --- /dev/null +++ b/testdata/fwd_minimal.rpl @@ -0,0 +1,125 @@ +; This is a comment. +; config options go here. +server: + ; the snoop is to elicit a referral and check the additional + ; is fine for that, not removed by minimal-responses. + access-control: 127.0.0.1 allow_snoop + minimal-responses: yes +forward-zone: name: "." forward-addr: 216.0.0.1 +CONFIG_END + +SCENARIO_BEGIN Test minimal-responses +RANGE_BEGIN 0 100 + ENTRY_BEGIN + MATCH opcode qtype qname + ADJUST copy_id + REPLY QR RD RA NOERROR + SECTION QUESTION +www.example.com. IN A + SECTION ANSWER +www.example.com. IN A 10.20.30.40 + SECTION AUTHORITY +example.com. IN NS ns.example.com. + SECTION ADDITIONAL +ns.example.com. IN A 10.20.30.50 +txt.example.com. IN TXT "foo" + ENTRY_END + + ENTRY_BEGIN + MATCH opcode qtype qname + ADJUST copy_id + REPLY QR RD RA NOERROR + SECTION QUESTION +a.example.com. IN A + SECTION ANSWER + SECTION AUTHORITY +example.com. IN SOA host.example.com. ns.example.com. 1 2 3 4 5 +example.com. IN NS ns.example.com. + SECTION ADDITIONAL +ns.example.com. IN A 10.20.30.50 +txt.example.com. IN TXT "foo" + ENTRY_END + + ENTRY_BEGIN + MATCH opcode qtype qname + ADJUST copy_id + REPLY QR RD RA NXDOMAIN + SECTION QUESTION +b.example.com. IN A + SECTION ANSWER + SECTION AUTHORITY +example.com. IN SOA host.example.com. ns.example.com. 1 2 3 4 5 + SECTION ADDITIONAL +ns.example.com. IN A 10.20.30.50 +txt.example.com. IN TXT "foo" + ENTRY_END +RANGE_END + +STEP 1 QUERY +ENTRY_BEGIN +REPLY RD +SECTION QUESTION +www.example.com. IN A +ENTRY_END +STEP 4 CHECK_ANSWER +ENTRY_BEGIN +MATCH opcode qname qtype all +REPLY QR RD RA +SECTION QUESTION +www.example.com. IN A +SECTION ANSWER +www.example.com. IN A 10.20.30.40 +ENTRY_END + +STEP 11 QUERY +ENTRY_BEGIN +REPLY RD +SECTION QUESTION +a.example.com. IN A +ENTRY_END +STEP 14 CHECK_ANSWER +ENTRY_BEGIN +MATCH opcode qname qtype all +REPLY QR RD RA +SECTION QUESTION +a.example.com. IN A +SECTION AUTHORITY +example.com. IN SOA host.example.com. ns.example.com. 1 2 3 4 5 +ENTRY_END + +STEP 21 QUERY +ENTRY_BEGIN +REPLY RD +SECTION QUESTION +b.example.com. IN A +ENTRY_END +STEP 24 CHECK_ANSWER +ENTRY_BEGIN +MATCH opcode qname qtype all +REPLY QR RD RA NXDOMAIN +SECTION QUESTION +b.example.com. IN A +SECTION AUTHORITY +example.com. IN SOA host.example.com. ns.example.com. 1 2 3 4 5 +ENTRY_END + +; get a referral, the additional is not removed. +STEP 31 QUERY +ENTRY_BEGIN +REPLY +SECTION QUESTION +c.example.com. IN A +ENTRY_END +STEP 34 CHECK_ANSWER +ENTRY_BEGIN +MATCH opcode qname qtype all +REPLY QR RA NOERROR +SECTION QUESTION +c.example.com. IN A +SECTION AUTHORITY +example.com. IN NS ns.example.com. + SECTION ADDITIONAL +ns.example.com. IN A 10.20.30.50 +ENTRY_END + +SCENARIO_END diff --git a/util/data/msgencode.c b/util/data/msgencode.c index 4c0a5550b..0be99c04f 100644 --- a/util/data/msgencode.c +++ b/util/data/msgencode.c @@ -639,15 +639,37 @@ positive_answer(struct reply_info* rep, uint16_t qtype) { return 0; } -int -reply_info_encode(struct query_info* qinfo, struct reply_info* rep, - uint16_t id, uint16_t flags, sldns_buffer* buffer, time_t timenow, +static int +negative_answer(struct reply_info* rep) { + size_t i; + int ns_seen = 0; + if(FLAGS_GET_RCODE(rep->flags) == LDNS_RCODE_NXDOMAIN) + return 1; + if(FLAGS_GET_RCODE(rep->flags) == LDNS_RCODE_NOERROR && + rep->an_numrrsets != 0) + return 0; /* positive */ + if(FLAGS_GET_RCODE(rep->flags) != LDNS_RCODE_NOERROR && + FLAGS_GET_RCODE(rep->flags) != LDNS_RCODE_NXDOMAIN) + return 0; + for(i=rep->an_numrrsets; ian_numrrsets+rep->ns_numrrsets; i++){ + if(ntohs(rep->rrsets[i]->rk.type) == LDNS_RR_TYPE_SOA) + return 1; + if(ntohs(rep->rrsets[i]->rk.type) == LDNS_RR_TYPE_NS) + ns_seen = 1; + } + if(ns_seen) return 0; /* could be referral, NS, but no SOA */ + return 1; +} + +int +reply_info_encode(struct query_info* qinfo, struct reply_info* rep, + uint16_t id, uint16_t flags, sldns_buffer* buffer, time_t timenow, struct regional* region, uint16_t udpsize, int dnssec) { uint16_t ancount=0, nscount=0, arcount=0; struct compress_tree_node* tree = 0; int r; - size_t rr_offset; + size_t rr_offset; sldns_buffer_clear(buffer); if(udpsize < sldns_buffer_limit(buffer)) @@ -663,7 +685,7 @@ reply_info_encode(struct query_info* qinfo, struct reply_info* rep, /* insert query section */ if(rep->qdcount) { - if((r=insert_query(qinfo, &tree, buffer, region)) != + if((r=insert_query(qinfo, &tree, buffer, region)) != RETVAL_OK) { if(r == RETVAL_TRUNC) { /* create truncated message */ @@ -707,8 +729,8 @@ reply_info_encode(struct query_info* qinfo, struct reply_info* rep, } /* insert answer section */ - if((r=insert_section(rep, rep->an_numrrsets, &ancount, buffer, - 0, timenow, region, &tree, LDNS_SECTION_ANSWER, qinfo->qtype, + if((r=insert_section(rep, rep->an_numrrsets, &ancount, buffer, + 0, timenow, region, &tree, LDNS_SECTION_ANSWER, qinfo->qtype, dnssec, rr_offset)) != RETVAL_OK) { if(r == RETVAL_TRUNC) { /* create truncated message */ @@ -724,7 +746,7 @@ reply_info_encode(struct query_info* qinfo, struct reply_info* rep, /* if response is positive answer, auth/add sections are not required */ if( ! (MINIMAL_RESPONSES && positive_answer(rep, qinfo->qtype)) ) { /* insert auth section */ - if((r=insert_section(rep, rep->ns_numrrsets, &nscount, buffer, + if((r=insert_section(rep, rep->ns_numrrsets, &nscount, buffer, rep->an_numrrsets, timenow, region, &tree, LDNS_SECTION_AUTHORITY, qinfo->qtype, dnssec, rr_offset)) != RETVAL_OK) { @@ -739,20 +761,22 @@ reply_info_encode(struct query_info* qinfo, struct reply_info* rep, } sldns_buffer_write_u16_at(buffer, 8, nscount); - /* insert add section */ - if((r=insert_section(rep, rep->ar_numrrsets, &arcount, buffer, - rep->an_numrrsets + rep->ns_numrrsets, timenow, region, - &tree, LDNS_SECTION_ADDITIONAL, qinfo->qtype, - dnssec, rr_offset)) != RETVAL_OK) { - if(r == RETVAL_TRUNC) { - /* no need to set TC bit, this is the additional */ - sldns_buffer_write_u16_at(buffer, 10, arcount); - sldns_buffer_flip(buffer); - return 1; + if(! (MINIMAL_RESPONSES && negative_answer(rep))) { + /* insert add section */ + if((r=insert_section(rep, rep->ar_numrrsets, &arcount, buffer, + rep->an_numrrsets + rep->ns_numrrsets, timenow, region, + &tree, LDNS_SECTION_ADDITIONAL, qinfo->qtype, + dnssec, rr_offset)) != RETVAL_OK) { + if(r == RETVAL_TRUNC) { + /* no need to set TC bit, this is the additional */ + sldns_buffer_write_u16_at(buffer, 10, arcount); + sldns_buffer_flip(buffer); + return 1; + } + return 0; } - return 0; + sldns_buffer_write_u16_at(buffer, 10, arcount); } - sldns_buffer_write_u16_at(buffer, 10, arcount); } sldns_buffer_flip(buffer); return 1; @@ -763,7 +787,7 @@ calc_edns_field_size(struct edns_data* edns) { size_t rdatalen = 0; struct edns_option* opt; - if(!edns || !edns->edns_present) + if(!edns || !edns->edns_present) return 0; for(opt = edns->opt_list; opt; opt = opt->next) { rdatalen += 4 + opt->opt_len;