From: Artem Boldariev <artem@boldariev.com>
Date: Wed, 15 Jun 2022 10:57:52 +0000 (+0300)
Subject: TLS DNS: do not call accept callback twice
X-Git-Tag: v9.19.3~41^2
X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=e616d7f240aece9b43d96fca1b63bc5b874daa42;p=thirdparty%2Fbind9.git

TLS DNS: do not call accept callback twice

Before the changes from this commit were introduced, the accept
callback function will get called twice when accepting connection
during two of these stages:

* when accepting the TCP connection;
* when handshake has completed.

That is clearly an error, as it should have been called only once. As
far as I understand it the mistake is a result of TLS DNS transport
being essentially a fork of TCP transport, where calling the accept
callback immediately after accepting TCP connection makes sense.

This commit fixes this mistake. It did not have any very serious
consequences because in BIND the accept callback only checks an ACL
and updates stats.
---

diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c
index 74b66ac0a87..05b71e8b7d8 100644
--- a/lib/isc/netmgr/tlsdns.c
+++ b/lib/isc/netmgr/tlsdns.c
@@ -1530,7 +1530,6 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
 	struct sockaddr_storage peer_ss;
 	struct sockaddr_storage local_ss;
 	isc_sockaddr_t local;
-	isc_nmhandle_t *handle = NULL;
 
 	REQUIRE(VALID_NMSOCK(ssock));
 	REQUIRE(ssock->tid == isc_nm_tid());
@@ -1600,18 +1599,6 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
 		goto failure;
 	}
 
-	/*
-	 * The handle will be either detached on acceptcb failure or in
-	 * the readcb.
-	 */
-	handle = isc__nmhandle_get(csock, NULL, &local);
-
-	result = ssock->accept_cb(handle, ISC_R_SUCCESS, ssock->accept_cbarg);
-	if (result != ISC_R_SUCCESS) {
-		isc_nmhandle_detach(&handle);
-		goto failure;
-	}
-
 	csock->tls.state = TLS_STATE_NONE;
 
 	csock->tls.tls = isc_tls_create(ssock->tls.ctx);
@@ -1655,8 +1642,11 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
 	 * We need to keep the handle alive until we fail to read or
 	 * connection is closed by the other side, it will be detached
 	 * via prep_destroy()->tlsdns_close_direct().
+	 *
+	 * The handle will be either detached on acceptcb failure or in
+	 * the readcb.
 	 */
-	isc_nmhandle_attach(handle, &csock->recv_handle);
+	csock->recv_handle = isc__nmhandle_get(csock, NULL, &local);
 
 	/*
 	 * The initial timer has been set, update the read timeout for
@@ -1666,8 +1656,6 @@ accept_connection(isc_nmsocket_t *ssock, isc_quota_t *quota) {
 				       ? atomic_load(&csock->mgr->keepalive)
 				       : atomic_load(&csock->mgr->idle));
 
-	isc_nmhandle_detach(&handle);
-
 	result = isc__nm_process_sock_buffer(csock);
 	if (result != ISC_R_SUCCESS) {
 		goto failure;