From 536e439c79fc65de6743af96ee8648e720481f7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 3 Apr 2023 14:56:17 +0200 Subject: [PATCH 1/2] Fix xfrin_connect_done() error paths The xfrin_connect_done() had several problems: - it would not add the server to unreachable table in case of the failure coming from the dispatch [GL #3989] - if dns_dispatch_checkperm() disallowed the connection, the xfr would be left undetached - if xfrin_send_request() failed to send the request, the xfr would be left undetached All of these have been fixed in this commit. --- lib/dns/xfrin.c | 60 +++++++++++++++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/lib/dns/xfrin.c b/lib/dns/xfrin.c index a13967e7a0..9f347772b7 100644 --- a/lib/dns/xfrin.c +++ b/lib/dns/xfrin.c @@ -963,15 +963,13 @@ failure: * A connection has been established. */ static void -xfrin_connect_done(isc_result_t result, isc_region_t *region, void *arg) { +xfrin_connect_done(isc_result_t result, isc_region_t *region ISC_ATTR_UNUSED, + void *arg) { dns_xfrin_t *xfr = (dns_xfrin_t *)arg; char addrtext[ISC_SOCKADDR_FORMATSIZE]; char signerbuf[DNS_NAME_FORMATSIZE]; const char *signer = "", *sep = ""; dns_zonemgr_t *zmgr = NULL; - isc_time_t now; - - UNUSED(region); REQUIRE(VALID_XFRIN(xfr)); @@ -981,23 +979,19 @@ xfrin_connect_done(isc_result_t result, isc_region_t *region, void *arg) { if (result != ISC_R_SUCCESS) { xfrin_fail(xfr, result, "failed to connect"); - dns_xfrin_unref(xfr); - return; + goto failure; } - CHECK(dns_dispatch_checkperm(xfr->disp)); + result = dns_dispatch_checkperm(xfr->disp); + if (result != ISC_R_SUCCESS) { + xfrin_fail(xfr, result, "connected but unable to transfer"); + goto failure; + } zmgr = dns_zone_getmgr(xfr->zone); if (zmgr != NULL) { - if (result != ISC_R_SUCCESS) { - now = isc_time_now(); - dns_zonemgr_unreachableadd(zmgr, &xfr->primaryaddr, - &xfr->sourceaddr, &now); - CHECK(result); - } else { - dns_zonemgr_unreachabledel(zmgr, &xfr->primaryaddr, - &xfr->sourceaddr); - } + dns_zonemgr_unreachabledel(zmgr, &xfr->primaryaddr, + &xfr->sourceaddr); } if (xfr->tsigkey != NULL && xfr->tsigkey->key != NULL) { @@ -1011,12 +1005,40 @@ xfrin_connect_done(isc_result_t result, isc_region_t *region, void *arg) { xfrin_log(xfr, ISC_LOG_INFO, "connected using %s%s%s", addrtext, sep, signer); - CHECK(xfrin_send_request(xfr)); + result = xfrin_send_request(xfr); + if (result != ISC_R_SUCCESS) { + xfrin_fail(xfr, result, "connected but unable to send"); + goto detach; + } + + return; failure: - if (result != ISC_R_SUCCESS) { - xfrin_fail(xfr, result, "connected but unable to transfer"); + switch (result) { + case ISC_R_NETDOWN: + case ISC_R_HOSTDOWN: + case ISC_R_NETUNREACH: + case ISC_R_HOSTUNREACH: + case ISC_R_CONNREFUSED: + /* + * Add the server to unreachable primaries table only if + * the server has a permanent networking error. + */ + zmgr = dns_zone_getmgr(xfr->zone); + if (zmgr != NULL) { + isc_time_t now = isc_time_now(); + + dns_zonemgr_unreachableadd(zmgr, &xfr->primaryaddr, + &xfr->sourceaddr, &now); + } + break; + default: + /* Retry sooner than in 10 minutes */ + break; } + +detach: + dns_xfrin_unref(xfr); } /* From 04b851342b0244d1783870665164e839f9a20191 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 3 Apr 2023 15:07:45 +0200 Subject: [PATCH 2/2] Add CHANGES note for [GL #3989] --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 9c3d4bdfb3..f7f8bdd6b2 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +6143. [bug] A reference counting problem on the error path in + the xfrin_connect_done() might cause an assertion + failure on shutdown. [GL #3989] + 6142. [bug] Reduce the number of dns_dnssec_verify calls made determining if revoked keys needs to be removed from the trust anchors. [GL #3981]