Merge remote-tracking branch 'nlnet/master'

* nlnet/master:
  - Fix question section mismatch in local zone redirect.
  Fixup space in error message.
  - Fix NLnetLabs#49: Set no renegotiation on the SSL context to stop client   session renegotiation.
  - Fix NLnetLabs#48: Unbound returns additional records on NODATA response,   if minimal-responses is enabled, also the additional for negative   responses is removed.
  -  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 too    specific.
  - Fix for possible assertion failure when answering respip CNAME from cache.

daemon/worker.c

@@ -721,8 +721,6 @@ answer_from_cache(struct worker* worker, struct query_info* qinfo,
 	if(encode_rep != rep)
 		secure = 0; /* if rewritten, it can't be considered "secure" */
 	if(!encode_rep || *alias_rrset) {
-		sldns_buffer_clear(repinfo->c->buffer);
-		sldns_buffer_flip(repinfo->c->buffer);
 		if(!encode_rep)
 			*need_drop = 1;
 		else {
@@ -762,17 +760,14 @@ answer_from_cache(struct worker* worker, struct query_info* qinfo,
 	return 0;
 }
 
-/** Reply to client and perform prefetch to keep cache up to date.
- * If the buffer for the reply is empty, it indicates that only prefetch is
- * necessary and the reply should be suppressed (because it's dropped or
- * being deferred). */
+/** Reply to client and perform prefetch to keep cache up to date. */
 static void
 reply_and_prefetch(struct worker* worker, struct query_info* qinfo, 
-	uint16_t flags, struct comm_reply* repinfo, time_t leeway)
+	uint16_t flags, struct comm_reply* repinfo, time_t leeway, int noreply)
 {
 	/* first send answer to client to keep its latency 
 	 * as small as a cachereply */
-	if(sldns_buffer_limit(repinfo->c->buffer) != 0) {
+	if(!noreply) {
 		if(repinfo->c->tcp_req_info) {
 			sldns_buffer_copy(
 				repinfo->c->tcp_req_info->spool_buffer,
@@ -1484,7 +1479,8 @@ worker_handle_request(struct comm_point* c, void* arg, int error,
 					lock_rw_unlock(&e->lock);
 					reply_and_prefetch(worker, lookup_qinfo,
 						sldns_buffer_read_u16_at(c->buffer, 2),
-						repinfo, leeway);
+						repinfo, leeway,
+						(partial_rep || need_drop));
 					if(!partial_rep) {
 						rc = 0;
 						regional_free_all(worker->scratchpad);

doc/Changelog

@@ -1,3 +1,24 @@
+23 July 2019: Wouter
+	- Fix question section mismatch in local zone redirect.
+
+19 July 2019: Wouter
+	- Fix #49: Set no renegotiation on the SSL context to stop client
+	  session renegotiation.
+
+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
+	  too specific.
+
+5 July 2019: Ralph
+	- Fix for possible assertion failure when answering respip CNAME from
+	  cache.
+
 25 June 2019: Wouter
 	- For #45, check that 127.0.0.1 and ::1 are not used in unbound.conf
 	  when do-not-query-localhost is turned on, or at default on,

respip/respip.c

@@ -361,6 +361,7 @@ respip_set_apply_cfg(struct respip_set* set, char* const* tagname, int num_tags,
 		free(pd);
 		pd = np;
 	}
+	addr_tree_init_parents(&set->ip_tree);
 
 	return 1;
 }

services/mesh.c

@@ -1340,14 +1340,15 @@ int mesh_state_add_reply(struct mesh_state* s, struct edns_data* edns,
 		log_assert(!qinfo->local_alias->next && dsrc->count == 1 &&
 			qinfo->local_alias->rrset->rk.type ==
 			htons(LDNS_RR_TYPE_CNAME));
-		/* Technically, we should make a local copy for the owner
-		 * name of the RRset, but in the case of the first (and
-		 * currently only) local alias RRset, the owner name should
-		 * point to the qname of the corresponding query, which should
-		 * be valid throughout the lifetime of this mesh_reply.  So
-		 * we can skip copying. */
-		log_assert(qinfo->local_alias->rrset->rk.dname ==
-			sldns_buffer_at(rep->c->buffer, LDNS_HEADER_SIZE));
+		/* we should make a local copy for the owner name of
+		 * the RRset */
+		r->local_alias->rrset->rk.dname_len =
+			qinfo->local_alias->rrset->rk.dname_len;
+		r->local_alias->rrset->rk.dname = regional_alloc_init(
+			s->s.region, qinfo->local_alias->rrset->rk.dname,
+			qinfo->local_alias->rrset->rk.dname_len);
+		if(!r->local_alias->rrset->rk.dname)
+			return 0;
 
 		/* the rrset is not packed, like in the cache, but it is
 		 * individualy allocated with an allocator from localzone. */

smallapp/unbound-control.c

@@ -499,6 +499,12 @@ setup_ctx(struct config_file* cfg)
 	if((SSL_CTX_set_options(ctx, SSL_OP_NO_SSLv3) & SSL_OP_NO_SSLv3)
 		!= SSL_OP_NO_SSLv3)
 		ssl_err("could not set SSL_OP_NO_SSLv3");
+#if defined(SSL_OP_NO_RENEGOTIATION)
+	/* disable client renegotiation */
+	if((SSL_CTX_set_options(ctx, SSL_OP_NO_RENEGOTIATION) &
+		SSL_OP_NO_RENEGOTIATION) != SSL_OP_NO_RENEGOTIATION)
+		ssl_err("could not set SSL_OP_NO_RENEGOTIATION");
+#endif
 	if(!SSL_CTX_use_certificate_chain_file(ctx,c_cert))
 		ssl_path_err("Error setting up SSL_CTX client cert", c_cert);
 	if (!SSL_CTX_use_PrivateKey_file(ctx,c_key,SSL_FILETYPE_PEM))

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

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; i<rep->an_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;

util/net_help.c

@@ -744,6 +744,14 @@ listen_sslctx_setup(void* ctxt)
 		return 0;
 	}
 #endif
+#if defined(SSL_OP_NO_RENEGOTIATION)
+	/* disable client renegotiation */
+	if((SSL_CTX_set_options(ctx, SSL_OP_NO_RENEGOTIATION) &
+		SSL_OP_NO_RENEGOTIATION) != SSL_OP_NO_RENEGOTIATION) {
+		log_crypto_err("could not set SSL_OP_NO_RENEGOTIATION");
+		return 0;
+	}
+#endif
 #if defined(SHA256_DIGEST_LENGTH) && defined(USE_ECDSA)
 	/* if we have sha256, set the cipher list to have no known vulns */
 	if(!SSL_CTX_set_cipher_list(ctx, "TLS13-CHACHA20-POLY1305-SHA256:TLS13-AES-256-GCM-SHA384:TLS13-AES-128-GCM-SHA256:ECDHE-ECDSA-CHACHA20-POLY1305:ECDHE-RSA-CHACHA20-POLY1305:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256"))
@@ -962,6 +970,14 @@ void* connect_sslctx_create(char* key, char* pem, char* verifypem, int wincert)
 		SSL_CTX_free(ctx);
 		return NULL;
 	}
+#if defined(SSL_OP_NO_RENEGOTIATION)
+	/* disable client renegotiation */
+	if((SSL_CTX_set_options(ctx, SSL_OP_NO_RENEGOTIATION) &
+		SSL_OP_NO_RENEGOTIATION) != SSL_OP_NO_RENEGOTIATION) {
+		log_crypto_err("could not set SSL_OP_NO_RENEGOTIATION");
+		return 0;
+	}
+#endif
 	if(key && key[0]) {
 		if(!SSL_CTX_use_certificate_chain_file(ctx, pem)) {
 			log_err("error in client certificate %s", pem);