Merge remote-tracking branch 'nlnet/master'

* nlnet/master: - For NLnetLabs#1207: [FR] Support for RESINFO RRType 261 (RFC9606), add LDNS_RR_TYPE_RESINFO similar to LDNS_RR_TYPE_TXT. Changelog entry for NLnetLabs#1204: - Merge NLnetLabs#1204: ci: set persist-credentials: false for actions/checkout per zizmor suggestion. set persist-credentials: false per zizmor suggestion - Fix typo in log_servfail.tdir test. Changelog entry for NLnetLabs#1187: - Merge NLnetLabs#1187: Create the SSL_CTX for QUIC before chroot and privilege drop. Create the SSL_CTX for QUIC before chroot and privilege drop (NLnetLabs#1187) - Safeguard alias loop while looking in the cache for expired answers. - Merge NLnetLabs#1198: Fix log-servfail with serve expired and no useful cache contents. - For NLnetLabs#1175, the default value of serve-expired-ttl is set to 86400 (1 day) as suggested by RFC8767. Changelog entry for NLnetLabs#1189, NLnetLabs#1197: - Merge NLnetLabs#1189: Fix the dname_str method to cause conversion errors when the domain name length is 255. - Merge NLnetLabs#1197: dname_str() fixes. - For NLnetLabs#1193, introduce log-servfail.tdir and cleanup the log-servfail setting from other tests. - Fix NLnetLabs#1193: log-servfail fails to log host SERVFAIL responses in Unbound 1.19.2 on Ubuntu 24.04.1 LTS, by not considering cached failures when trying to reply with expired data. - For NLnetLabs#1189, homogenize the input buffer size for dname_str(). - For NLnetLabs#1189, add unit tests for dname_str() and debug check the input buffer size. Fix the dname_str method to cause conversion errors when the domain name length is 255
jedisct1 · Dec 26, 2024 · 074ee24 · 074ee24
2 parents 59d241b + e57e537
commit 074ee24
Show file tree

Hide file tree

Showing 63 changed files with 529 additions and 260 deletions.
diff --git a/.github/workflows/analysis_ports.yml b/.github/workflows/analysis_ports.yml
@@ -193,6 +193,7 @@ jobs:
       - uses: actions/checkout@v4
         with:
           submodules: false
+          persist-credentials: false
       - name: test_windows
         if: ${{ matrix.test_windows == 'yes' }}
         shell: bash

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -13,6 +13,8 @@ jobs:
 
     steps:
     - uses: actions/checkout@v4
+      with:
+        persist-credentials: false
     - name: configure
       run: ./configure --enable-debug
     - name: make

diff --git a/daemon/cachedump.c b/daemon/cachedump.c
@@ -836,7 +836,7 @@ int print_deleg_lookup(RES* ssl, struct worker* worker, uint8_t* nm,
 	struct delegpt* dp;
 	struct dns_msg* msg;
 	struct regional* region = worker->scratchpad;
-	char b[260];
+	char b[LDNS_MAX_DOMAINLEN];
 	struct query_info qinfo;
 	struct iter_hints_stub* stub;
 	int nolock = 0;

diff --git a/daemon/daemon.c b/daemon/daemon.c
@@ -956,6 +956,9 @@ daemon_delete(struct daemon* daemon)
 	listen_sslctx_delete_ticket_keys();
 	SSL_CTX_free((SSL_CTX*)daemon->listen_sslctx);
 	SSL_CTX_free((SSL_CTX*)daemon->connect_sslctx);
+#endif
+#ifdef HAVE_NGTCP2
+	SSL_CTX_free((SSL_CTX*)daemon->quic_sslctx);
 #endif
 	free(daemon);
 	/* lex cleanup */

diff --git a/daemon/daemon.h b/daemon/daemon.h
@@ -99,6 +99,8 @@ struct daemon {
 	struct daemon_remote* rc;
 	/** ssl context for listening to dnstcp over ssl, and connecting ssl */
 	void* listen_sslctx, *connect_sslctx;
+	/** ssl context for listening to quic */
+	void* quic_sslctx;
 	/** num threads allocated */
 	int num;
 	/** num threads allocated in the previous config or 0 at first */

diff --git a/daemon/remote.c b/daemon/remote.c
@@ -2141,7 +2141,7 @@ static int
 ssl_print_name_dp(RES* ssl, const char* str, uint8_t* nm, uint16_t dclass,
 	struct delegpt* dp)
 {
-	char buf[257];
+	char buf[LDNS_MAX_DOMAINLEN];
 	struct delegpt_ns* ns;
 	struct delegpt_addr* a;
 	int f = 0;
@@ -2509,7 +2509,7 @@ do_insecure_remove(RES* ssl, struct worker* worker, char* arg)
 static void
 do_insecure_list(RES* ssl, struct worker* worker)
 {
-	char buf[257];
+	char buf[LDNS_MAX_DOMAINLEN];
 	struct trust_anchor* a;
 	if(worker->env.anchors) {
 		RBTREE_FOR(a, struct trust_anchor*, worker->env.anchors->tree) {
@@ -2606,7 +2606,7 @@ get_mesh_status(struct mesh_area* mesh, struct mesh_state* m,
 		}
 	} else if(s == module_wait_subquery) {
 		/* look in subs from mesh state to see what */
-		char nm[257];
+		char nm[LDNS_MAX_DOMAINLEN];
 		struct mesh_state_ref* sub;
 		snprintf(buf, len, "%s wants", modname);
 		l = strlen(buf);
@@ -2636,7 +2636,7 @@ do_dump_requestlist(RES* ssl, struct worker* worker)
 	struct mesh_area* mesh;
 	struct mesh_state* m;
 	int num = 0;
-	char buf[257];
+	char buf[LDNS_MAX_DOMAINLEN];
 	char timebuf[32];
 	char statbuf[10240];
 	if(!ssl_printf(ssl, "thread #%d\n", worker->thread_num))
@@ -2686,7 +2686,7 @@ dump_infra_host(struct lruhash_entry* e, void* arg)
 	struct infra_key* k = (struct infra_key*)e->key;
 	struct infra_data* d = (struct infra_data*)e->data;
 	char ip_str[1024];
-	char name[257];
+	char name[LDNS_MAX_DOMAINLEN];
 	int port;
 	if(a->ssl_failed)
 		return;
@@ -2953,7 +2953,7 @@ static void
 do_list_auth_zones(RES* ssl, struct auth_zones* az)
 {
 	struct auth_zone* z;
-	char buf[257], buf2[256];
+	char buf[LDNS_MAX_DOMAINLEN], buf2[256];
 	lock_rw_rdlock(&az->lock);
 	RBTREE_FOR(z, struct auth_zone*, &az->ztree) {
 		lock_rw_rdlock(&z->lock);
@@ -2983,7 +2983,7 @@ static void
 do_list_local_zones(RES* ssl, struct local_zones* zones)
 {
 	struct local_zone* z;
-	char buf[257];
+	char buf[LDNS_MAX_DOMAINLEN];
 	lock_rw_rdlock(&zones->lock);
 	RBTREE_FOR(z, struct local_zone*, &zones->ztree) {
 		lock_rw_rdlock(&z->lock);
@@ -3094,7 +3094,7 @@ rate_list(struct lruhash_entry* e, void* arg)
 	struct ratelimit_list_arg* a = (struct ratelimit_list_arg*)arg;
 	struct rate_key* k = (struct rate_key*)e->key;
 	struct rate_data* d = (struct rate_data*)e->data;
-	char buf[257];
+	char buf[LDNS_MAX_DOMAINLEN];
 	int lim = infra_find_ratelimit(a->infra, k->name, k->namelen);
 	int max = infra_rate_max(d, a->now, a->backoff);
 	if(a->all == 0) {

diff --git a/daemon/unbound.c b/daemon/unbound.c
@@ -494,8 +494,9 @@ perform_setup(struct daemon* daemon, struct config_file* cfg, int debug_mode,
 		fatal_exit("could not set up remote-control");
 	if(cfg->ssl_service_key && cfg->ssl_service_key[0]) {
 		if(!(daemon->listen_sslctx = listen_sslctx_create(
-			cfg->ssl_service_key, cfg->ssl_service_pem, NULL)))
+			cfg->ssl_service_key, cfg->ssl_service_pem, NULL))) {
 			fatal_exit("could not set up listen SSL_CTX");
+		}
 		if(cfg->tls_ciphers && cfg->tls_ciphers[0]) {
 			if (!SSL_CTX_set_cipher_list(daemon->listen_sslctx, cfg->tls_ciphers)) {
 				fatal_exit("failed to set tls-cipher %s", cfg->tls_ciphers);
@@ -507,18 +508,24 @@ perform_setup(struct daemon* daemon, struct config_file* cfg, int debug_mode,
 				fatal_exit("failed to set tls-ciphersuites %s", cfg->tls_ciphersuites);
 			}
 		}
-#endif
+#endif /* HAVE_SSL_CTX_SET_CIPHERSUITES */
 		if(cfg->tls_session_ticket_keys.first &&
 			cfg->tls_session_ticket_keys.first->str[0] != 0) {
 			if(!listen_sslctx_setup_ticket_keys(daemon->listen_sslctx, cfg->tls_session_ticket_keys.first)) {
 				fatal_exit("could not set session ticket SSL_CTX");
 			}
 		}
+#ifdef HAVE_NGTCP2
+		if(!(daemon->quic_sslctx = quic_sslctx_create(
+			cfg->ssl_service_key, cfg->ssl_service_pem, NULL))) {
+			fatal_exit("could not set up quic SSL_CTX");
+		}
+#endif /* HAVE_NGTCP2 */
 	}
 	if(!(daemon->connect_sslctx = connect_sslctx_create(NULL, NULL,
 		cfg->tls_cert_bundle, cfg->tls_win_cert)))
 		fatal_exit("could not set up connect SSL_CTX");
-#endif
+#endif /* HAVE_SSL */
 
 	/* init syslog (as root) if needed, before daemonize, otherwise
 	 * a fork error could not be printed since daemonize closed stderr.*/

diff --git a/daemon/worker.c b/daemon/worker.c
@@ -1082,7 +1082,7 @@ answer_notify(struct worker* w, struct query_info* qinfo,
 
 	if(verbosity >= VERB_DETAIL) {
 		char buf[380];
-		char zname[255+1];
+		char zname[LDNS_MAX_DOMAINLEN];
 		char sr[25];
 		dname_str(qinfo->qname, zname);
 		sr[0]=0;
@@ -1413,7 +1413,7 @@ worker_handle_request(struct comm_point* c, void* arg, int error,
 		return 0;
 	}
 	if(c->dnscrypt && !repinfo->is_dnscrypted) {
-		char buf[LDNS_MAX_DOMAINLEN+1];
+		char buf[LDNS_MAX_DOMAINLEN];
 		/* Check if this is unencrypted and asking for certs */
 		worker_check_request(c->buffer, worker, &check_result);
 		if(check_result.value != 0) {
@@ -2174,9 +2174,9 @@ worker_init(struct worker* worker, struct config_file *cfg,
 		cfg->harden_large_queries, cfg->http_max_streams,
 		cfg->http_endpoint, cfg->http_notls_downstream,
 		worker->daemon->tcl, worker->daemon->listen_sslctx,
+		worker->daemon->quic_sslctx,
 		dtenv, worker->daemon->doq_table, worker->env.rnd,
-		cfg->ssl_service_key, cfg->ssl_service_pem, cfg,
-		worker_handle_request, worker);
+		cfg, worker_handle_request, worker);
 	if(!worker->front) {
 		log_err("could not create listening sockets");
 		worker_delete(worker);

diff --git a/doc/Changelog b/doc/Changelog
@@ -1,3 +1,24 @@
+20 December 2024: Yorgos
+	- For #1207: [FR] Support for RESINFO RRType 261 (RFC9606), add
+	  LDNS_RR_TYPE_RESINFO similar to LDNS_RR_TYPE_TXT.
+
+13 December 2024: Yorgos
+	- Merge #1204: ci: set persist-credentials: false for actions/checkout
+	  per zizmor suggestion.
+
+3 December 2024: Yorgos
+	- Merge #1189: Fix the dname_str method to cause conversion errors
+	  when the domain name length is 255.
+	- Merge #1197: dname_str() fixes.
+	- For #1175, the default value of serve-expired-ttl is set to 86400
+	  (1 day) as suggested by RFC8767.
+	- Merge #1198: Fix log-servfail with serve expired and no useful cache
+	  contents.
+	- Safeguard alias loop while looking in the cache for expired answers.
+	- Merge #1187: Create the SSL_CTX for QUIC before chroot and privilege
+	  drop.
+	- Fix typo in log_servfail.tdir test.
+
 22 November 2024: Yorgos
 	- Fix #1175: serve-expired does not adhere to secure-by-default
 	  principle. The default value of serve-expired-client-timeout

diff --git a/doc/example.conf.in b/doc/example.conf.in
@@ -737,7 +737,7 @@ server:
 	#
 	# Limit serving of expired responses to configured seconds after
 	# expiration. 0 disables the limit.
-	# serve-expired-ttl: 0
+	# serve-expired-ttl: 86400
 	#
 	# Set the TTL of expired records to the serve-expired-ttl value after a
 	# failed attempt to retrieve the record from upstream. This makes sure

diff --git a/doc/unbound.conf.5.in b/doc/unbound.conf.5.in
@@ -1408,10 +1408,11 @@ out or is taking more than serve\-expired\-client\-timeout to resolve.
 Default is "no".
 .TP
 .B serve\-expired\-ttl: \fI<seconds>
-Limit serving of expired responses to configured seconds after expiration. 0
-disables the limit.  This option only applies when \fBserve\-expired\fR is
-enabled.  A suggested value per RFC 8767 is between
-86400 (1 day) and 259200 (3 days).  The default is 0.
+Limit serving of expired responses to configured seconds after expiration.
+0 disables the limit.
+This option only applies when \fBserve\-expired\fR is enabled.
+A suggested value per RFC 8767 is between 86400 (1 day) and 259200 (3 days).
+The default is 86400.
 .TP
 .B serve\-expired\-ttl\-reset: \fI<yes or no>
 Set the TTL of expired records to the \fBserve\-expired\-ttl\fR value after a

diff --git a/iterator/iter_delegpt.c b/iterator/iter_delegpt.c
@@ -278,7 +278,7 @@ delegpt_count_addr(struct delegpt* dp, size_t* numaddr, size_t* numres,
 
 void delegpt_log(enum verbosity_value v, struct delegpt* dp)
 {
-	char buf[LDNS_MAX_DOMAINLEN+1];
+	char buf[LDNS_MAX_DOMAINLEN];
 	struct delegpt_ns* ns;
 	struct delegpt_addr* a;
 	size_t missing=0, numns=0, numaddr=0, numres=0, numavail=0;

diff --git a/iterator/iter_fwd.c b/iterator/iter_fwd.c
@@ -129,7 +129,7 @@ forwards_insert_data(struct iter_forwards* fwd, uint16_t c, uint8_t* nm,
 	node->namelabs = nmlabs;
 	node->dp = dp;
 	if(!rbtree_insert(fwd->tree, &node->node)) {
-		char buf[257];
+		char buf[LDNS_MAX_DOMAINLEN];
 		dname_str(nm, buf);
 		log_err("duplicate forward zone %s ignored.", buf);
 		delegpt_free_mlc(dp);

diff --git a/iterator/iter_hints.c b/iterator/iter_hints.c
@@ -181,7 +181,7 @@ hints_insert(struct iter_hints* hints, uint16_t c, struct delegpt* dp,
 	node->noprime = (uint8_t)noprime;
 	if(!name_tree_insert(&hints->tree, &node->node, dp->name, dp->namelen,
 		dp->namelabs, c)) {
-		char buf[257];
+		char buf[LDNS_MAX_DOMAINLEN];
 		dname_str(dp->name, buf);
 		log_err("second hints for zone %s ignored.", buf);
 		delegpt_free_mlc(dp);

diff --git a/iterator/iter_utils.c b/iterator/iter_utils.c
@@ -1492,8 +1492,8 @@ iter_stub_fwd_no_cache(struct module_qstate *qstate, struct query_info *qinf,
 		int stub_no_cache = stub->dp->no_cache;
 		lock_rw_unlock(&qstate->env->fwds->lock);
 		if(stub_no_cache) {
-			char qname[255+1];
-			char dpname[255+1];
+			char qname[LDNS_MAX_DOMAINLEN];
+			char dpname[LDNS_MAX_DOMAINLEN];
 			dname_str(qinf->qname, qname);
 			dname_str(stub->dp->name, dpname);
 			verbose(VERB_ALGO, "stub for %s %s has no_cache", qname, dpname);
@@ -1520,8 +1520,8 @@ iter_stub_fwd_no_cache(struct module_qstate *qstate, struct query_info *qinf,
 		int dp_no_cache = dp->no_cache;
 		lock_rw_unlock(&qstate->env->hints->lock);
 		if(dp_no_cache) {
-			char qname[255+1];
-			char dpname[255+1];
+			char qname[LDNS_MAX_DOMAINLEN];
+			char dpname[LDNS_MAX_DOMAINLEN];
 			dname_str(qinf->qname, qname);
 			dname_str(dp->name, dpname);
 			verbose(VERB_ALGO, "forward for %s %s has no_cache", qname, dpname);

diff --git a/iterator/iterator.c b/iterator/iterator.c
@@ -1092,7 +1092,7 @@ auth_zone_delegpt(struct module_qstate* qstate, struct iter_qstate* iq,
 			/* cache is blacklisted and fallback, and we
 			 * already have an auth_zone dp */
 			if(verbosity>=VERB_ALGO) {
-				char buf[255+1];
+				char buf[LDNS_MAX_DOMAINLEN];
 				dname_str(z->name, buf);
 				verbose(VERB_ALGO, "auth_zone %s "
 				  "fallback because cache blacklisted",
@@ -1109,7 +1109,7 @@ auth_zone_delegpt(struct module_qstate* qstate, struct iter_qstate* iq,
 				 * validation failure, and the zone allows
 				 * fallback to the internet, query there. */
 				if(verbosity>=VERB_ALGO) {
-					char buf[255+1];
+					char buf[LDNS_MAX_DOMAINLEN];
 					dname_str(z->name, buf);
 					verbose(VERB_ALGO, "auth_zone %s "
 					  "fallback because cache blacklisted",
@@ -2033,15 +2033,15 @@ query_for_targets(struct module_qstate* qstate, struct iter_qstate* iq,
 		return 1;
 	if(iq->depth > 0 && iq->target_count &&
 		iq->target_count[TARGET_COUNT_QUERIES] > MAX_TARGET_COUNT) {
-		char s[LDNS_MAX_DOMAINLEN+1];
+		char s[LDNS_MAX_DOMAINLEN];
 		dname_str(qstate->qinfo.qname, s);
 		verbose(VERB_QUERY, "request %s has exceeded the maximum "
 			"number of glue fetches %d", s,
 			iq->target_count[TARGET_COUNT_QUERIES]);
 		return 2;
 	}
 	if(iq->dp_target_count > MAX_DP_TARGET_COUNT) {
-		char s[LDNS_MAX_DOMAINLEN+1];
+		char s[LDNS_MAX_DOMAINLEN];
 		dname_str(qstate->qinfo.qname, s);
 		verbose(VERB_QUERY, "request %s has exceeded the maximum "
 			"number of glue fetches %d to a single delegation point",
@@ -2252,7 +2252,7 @@ processLastResort(struct module_qstate* qstate, struct iter_qstate* iq,
 	}
 	if(iq->depth > 0 && iq->target_count &&
 		iq->target_count[TARGET_COUNT_QUERIES] > MAX_TARGET_COUNT) {
-		char s[LDNS_MAX_DOMAINLEN+1];
+		char s[LDNS_MAX_DOMAINLEN];
 		dname_str(qstate->qinfo.qname, s);
 		verbose(VERB_QUERY, "request %s has exceeded the maximum "
 			"number of glue fetches %d", s,
@@ -3044,7 +3044,7 @@ processQueryTargets(struct module_qstate* qstate, struct iter_qstate* iq,
 	target_count_increase_global_quota(iq, 1);
 	if(iq->target_count && iq->target_count[TARGET_COUNT_GLOBAL_QUOTA]
 		> MAX_GLOBAL_QUOTA) {
-		char s[LDNS_MAX_DOMAINLEN+1];
+		char s[LDNS_MAX_DOMAINLEN];
 		dname_str(qstate->qinfo.qname, s);
 		verbose(VERB_QUERY, "request %s has exceeded the maximum "
 			"global quota on number of upstream queries %d", s,

diff --git a/libunbound/libworker.c b/libunbound/libworker.c
@@ -423,7 +423,7 @@ int libworker_bg(struct ub_ctx* ctx)
 static int
 fill_canon(struct ub_result* res, uint8_t* s)
 {
-	char buf[255+2];
+	char buf[LDNS_MAX_DOMAINLEN];
 	dname_str(s, buf);
 	res->canonname = strdup(buf);
 	return res->canonname != 0;

diff --git a/pythonmod/interface.i b/pythonmod/interface.i
@@ -204,7 +204,7 @@ struct query_info {
 
 %inline %{
    PyObject* dnameAsStr(PyObject* dname) {
-       char buf[LDNS_MAX_DOMAINLEN+1];
+       char buf[LDNS_MAX_DOMAINLEN];
        buf[0] = '\0';
        dname_str((uint8_t*)PyBytes_AsString(dname), buf);
        return PyString_FromString(buf);

diff --git a/respip/respip.c b/respip/respip.c
@@ -961,7 +961,7 @@ respip_rewrite_reply(const struct query_info* qinfo,
 						struct sockaddr_storage ss;
 						socklen_t ss_len = 0;
 						char nm[256], ip[256];
-						char qn[255+1];
+						char qn[LDNS_MAX_DOMAINLEN];
 						if(!rdata2sockaddr(rep->rrsets[rrset_id]->entry.data, ntohs(rep->rrsets[rrset_id]->rk.type), rr_id, &ss, &ss_len))
 							snprintf(ip, sizeof(ip), "invalidRRdata");
 						else