From: Aram Sargsyan Date: Wed, 18 Jan 2023 10:36:34 +0000 (+0000) Subject: Refactor isc_nm_xfr_allowed() X-Git-Tag: v9.19.10~28^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=41dc48bfd735b1fc21943bb9a636986cc5278662;p=thirdparty%2Fbind9.git Refactor isc_nm_xfr_allowed() Return 'isc_result_t' type value instead of 'bool' to indicate the actual failure. Rename the function to something not suggesting a boolean type result. Make changes in the places where the API function is being used to check for the result code instead of a boolean value. --- diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index e0786714373..1a1c14feca1 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -3452,19 +3452,20 @@ launch_next_query(dig_query_t *query) { query->lookup->rdtype == dns_rdatatype_axfr; if (xfr && isc_nm_socket_type(query->handle) == isc_nm_streamdnssocket && - query->lookup->tls_mode && !isc_nm_xfr_allowed(query->handle)) + query->lookup->tls_mode) { - dighost_error("zone transfers over the " - "established TLS connection are not allowed"); - dighost_error("as the " - "connection does not meet the requirements " - "enforced by the RFC 9103"); - isc_refcount_decrement0(&recvcount); - isc_nmhandle_detach(&query->readhandle); - cancel_lookup(l); - lookup_detach(&l); - clear_current_lookup(); - return; + isc_result_t result = isc_nm_xfr_checkperm(query->handle); + if (result != ISC_R_SUCCESS) { + dighost_error("zone transfers over the established TLS " + "connection are not allowed: %s", + isc_result_totext(result)); + isc_refcount_decrement0(&recvcount); + isc_nmhandle_detach(&query->readhandle); + cancel_lookup(l); + lookup_detach(&l); + clear_current_lookup(); + return; + } } query_attach(query, &readquery); diff --git a/lib/dns/xfrin.c b/lib/dns/xfrin.c index c4723925c24..05b612f8507 100644 --- a/lib/dns/xfrin.c +++ b/lib/dns/xfrin.c @@ -1020,11 +1020,7 @@ xfrin_connect_done(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) { CHECK(result); - if (!isc_nm_xfr_allowed(handle)) { - /* set the error code so that XFER will fail */ - result = ISC_R_NOPERM; - goto failure; - } + CHECK(isc_nm_xfr_checkperm(handle)); zmgr = dns_zone_getmgr(xfr->zone); if (zmgr != NULL) { diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index f6cf2b7d824..bd81b7bb9d7 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -661,10 +661,16 @@ isc_nm_bad_request(isc_nmhandle_t *handle); * \li 'handle' is a valid netmgr handle object. */ -bool -isc_nm_xfr_allowed(isc_nmhandle_t *handle); +isc_result_t +isc_nm_xfr_checkperm(isc_nmhandle_t *handle); /*%< - * Check if it is possible to do a zone transfer over the given handle. + * Check if it is permitted to do a zone transfer over the given handle. + * + * Returns: + * \li #ISC_R_SUCCESS Success, permission check passed successfully + * \li #ISC_R_DOTALPNERROR No permission because of ALPN tag mismatch + * \li #ISC_R_NOPERM No permission because of other restrictions + * \li any other result indicates failure (i.e. no permission) * * Requires: * \li 'handle' is a valid connection handle. diff --git a/lib/isc/include/isc/result.h b/lib/isc/include/isc/result.h index 6e7ba1b68f8..ac7273ce521 100644 --- a/lib/isc/include/isc/result.h +++ b/lib/isc/include/isc/result.h @@ -94,6 +94,7 @@ typedef enum isc_result { ISC_R_TLSERROR, /*%< TLS error */ ISC_R_TLSBADPEERCERT, /*%< TLS peer certificate verification failed */ ISC_R_HTTP2ALPNERROR, /*%< ALPN for HTTP/2 failed */ + ISC_R_DOTALPNERROR, /*%< ALPN for DoT failed */ DNS_R_LABELTOOLONG = 1 << 16, DNS_R_BADESCAPE, diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 4a722130fa3..082bba22b0c 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -1666,8 +1666,8 @@ isc__nm_streamdns_verify_tls_peer_result_string(const isc_nmhandle_t *handle); void isc__nm_streamdns_set_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *tlsctx); -bool -isc__nm_streamdns_xfr_allowed(isc_nmsocket_t *sock); +isc_result_t +isc__nm_streamdns_xfr_checkperm(isc_nmsocket_t *sock); void isc__nmsocket_streamdns_reset(isc_nmsocket_t *sock); diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 10eac867adc..392bf3ceaea 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -2385,9 +2385,10 @@ isc_nm_bad_request(isc_nmhandle_t *handle) { } } -bool -isc_nm_xfr_allowed(isc_nmhandle_t *handle) { +isc_result_t +isc_nm_xfr_checkperm(isc_nmhandle_t *handle) { isc_nmsocket_t *sock = NULL; + isc_result_t result = ISC_R_NOPERM; REQUIRE(VALID_NMHANDLE(handle)); REQUIRE(VALID_NMSOCK(handle->sock)); @@ -2396,14 +2397,13 @@ isc_nm_xfr_allowed(isc_nmhandle_t *handle) { switch (sock->type) { case isc_nm_streamdnssocket: - return (isc__nm_streamdns_xfr_allowed(sock)); + result = isc__nm_streamdns_xfr_checkperm(sock); + break; default: - return (false); + break; } - UNREACHABLE(); - - return (false); + return (result); } bool diff --git a/lib/isc/netmgr/streamdns.c b/lib/isc/netmgr/streamdns.c index a2395dd8202..ef5fd6874ce 100644 --- a/lib/isc/netmgr/streamdns.c +++ b/lib/isc/netmgr/streamdns.c @@ -1099,18 +1099,24 @@ isc__nm_streamdns_set_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *tlsctx) { } } -bool -isc__nm_streamdns_xfr_allowed(isc_nmsocket_t *sock) { +isc_result_t +isc__nm_streamdns_xfr_checkperm(isc_nmsocket_t *sock) { + isc_result_t result = ISC_R_NOPERM; + REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->type == isc_nm_streamdnssocket); - if (sock->outerhandle == NULL) { - return (false); - } else if (!isc_nm_has_encryption(sock->outerhandle)) { - return (true); + if (sock->outerhandle != NULL) { + if (isc_nm_has_encryption(sock->outerhandle) && + !sock->streamdns.dot_alpn_negotiated) + { + result = ISC_R_DOTALPNERROR; + } else { + result = ISC_R_SUCCESS; + } } - return (sock->streamdns.dot_alpn_negotiated); + return (result); } void diff --git a/lib/isc/result.c b/lib/isc/result.c index 372dd590e98..b41f760fa45 100644 --- a/lib/isc/result.c +++ b/lib/isc/result.c @@ -93,6 +93,7 @@ static const char *description[ISC_R_NRESULTS] = { [ISC_R_TLSERROR] = "TLS error", [ISC_R_TLSBADPEERCERT] = "TLS peer certificate verification failed", [ISC_R_HTTP2ALPNERROR] = "ALPN for HTTP/2 failed", + [ISC_R_DOTALPNERROR] = "ALPN for DoT failed", [DNS_R_LABELTOOLONG] = "label too long", [DNS_R_BADESCAPE] = "bad escape", @@ -343,6 +344,7 @@ static const char *identifier[ISC_R_NRESULTS] = { [ISC_R_TLSERROR] = "ISC_R_TLSERROR", [ISC_R_TLSBADPEERCERT] = "ISC_R_TLSBADPEERCERT", [ISC_R_HTTP2ALPNERROR] = "ISC_R_HTTP2ALPNERROR", + [ISC_R_DOTALPNERROR] = "ISC_R_DOTALPNERROR", [DNS_R_LABELTOOLONG] = "DNS_R_LABELTOOLONG", [DNS_R_BADESCAPE] = "DNS_R_BADESCAPE", [DNS_R_EMPTYLABEL] = "DNS_R_EMPTYLABEL", diff --git a/lib/ns/query.c b/lib/ns/query.c index faab1340ed5..946591009cc 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -11973,9 +11973,7 @@ ns_query_start(ns_client_t *client, isc_nmhandle_t *handle) { return; } if (isc_nm_socket_type(handle) == - isc_nm_streamdnssocket && - isc_nm_has_encryption(handle) && - !isc_nm_xfr_allowed(handle)) + isc_nm_streamdnssocket) { /* * Currently this code is here for DoT, which @@ -11983,8 +11981,18 @@ ns_query_start(ns_client_t *client, isc_nmhandle_t *handle) { * transfers compared to other stream * protocols. See RFC 9103 for details. */ - query_error(client, DNS_R_REFUSED, __LINE__); - return; + switch (isc_nm_xfr_checkperm(handle)) { + case ISC_R_SUCCESS: + break; + case ISC_R_DOTALPNERROR: + query_error(client, DNS_R_NOALPN, + __LINE__); + return; + default: + query_error(client, DNS_R_REFUSED, + __LINE__); + return; + } } ns_xfr_start(client, rdataset->type); return;