diff --git a/CHANGES b/CHANGES index 907ff0de34..d684fa9774 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6074. [func] Refactor the isc_nm_xfr_allowed() function to return + isc_result_t instead of boolean. [GL #3808] + 6072. [bug] Avoid the OpenSSL lock contention when initializing Message Digest Contexts by using explicit algorithm fetching, initializing static contexts for every diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 1f9d430bf6..84cdda915e 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -3487,20 +3487,19 @@ launch_next_query(dig_query_t *query) { xfr = query->lookup->rdtype == dns_rdatatype_ixfr || query->lookup->rdtype == dns_rdatatype_axfr; - if (xfr && isc_nm_socket_type(query->handle) == isc_nm_tlsdnssocket && - !isc_nm_xfr_allowed(query->handle)) - { - 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; + if (xfr && isc_nm_socket_type(query->handle) == isc_nm_tlsdnssocket) { + 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 a8d6833236..dad53e28e1 100644 --- a/lib/dns/xfrin.c +++ b/lib/dns/xfrin.c @@ -1233,11 +1233,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 b8e67b6135..eff33f6acb 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -692,10 +692,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 dd03b9c00c..1b5941178c 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, @@ -221,6 +222,7 @@ typedef enum isc_result { DNS_R_NSEC3BADALG, DNS_R_NSEC3RESALT, DNS_R_INCONSISTENTRR, + DNS_R_NOALPN, DST_R_UNSUPPORTEDALG = 2 << 16, DST_R_CRYPTOFAILURE, diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index d5269753ac..50528af480 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -1715,12 +1715,17 @@ isc__nm_async_tlsdns_set_tlsctx(isc_nmsocket_t *listener, isc_tlsctx_t *tlsctx, * Callback handlers for asynchronous TLSDNS events. */ -bool -isc__nm_tlsdns_xfr_allowed(isc_nmsocket_t *sock); +isc_result_t +isc__nm_tlsdns_xfr_checkperm(isc_nmsocket_t *sock); /*%< - * Check if it is possible to do a zone transfer over the given TLSDNS + * Check if it is permitted to do a zone transfer over the given TLSDNS * socket. * + * Returns: + * \li #ISC_R_SUCCESS Success, permission check passed successfully + * \li #ISC_R_DOTALPNERROR No permission because of ALPN tag mismatch + * \li any other result indicates failure (i.e. no permission) + * * Requires: * \li 'sock' is a valid TLSDNS socket. */ diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c index 301d5e10f8..01d5f168a8 100644 --- a/lib/isc/netmgr/netmgr.c +++ b/lib/isc/netmgr/netmgr.c @@ -3640,9 +3640,10 @@ isc_nm_bad_request(isc_nmhandle_t *handle) { } } -bool -isc_nm_xfr_allowed(isc_nmhandle_t *handle) { - isc_nmsocket_t *sock; +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)); @@ -3651,16 +3652,16 @@ isc_nm_xfr_allowed(isc_nmhandle_t *handle) { switch (sock->type) { case isc_nm_tcpdnssocket: - return (true); + result = ISC_R_SUCCESS; + break; case isc_nm_tlsdnssocket: - return (isc__nm_tlsdns_xfr_allowed(sock)); + result = isc__nm_tlsdns_xfr_checkperm(sock); + break; default: - return (false); + break; } - UNREACHABLE(); - - return (false); + return (result); } bool diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c index 19da3353e3..52c72e0fca 100644 --- a/lib/isc/netmgr/tlsdns.c +++ b/lib/isc/netmgr/tlsdns.c @@ -2216,12 +2216,16 @@ isc__nm_async_tlsdnscancel(isc__networker_t *worker, isc__netievent_t *ev0) { * The ones requiring strict compatibility with the specification * could disable TLSv1.2 in the configuration file. */ -bool -isc__nm_tlsdns_xfr_allowed(isc_nmsocket_t *sock) { +isc_result_t +isc__nm_tlsdns_xfr_checkperm(isc_nmsocket_t *sock) { REQUIRE(VALID_NMSOCK(sock)); REQUIRE(sock->type == isc_nm_tlsdnssocket); - return (sock->tls.alpn_negotiated); + if (!sock->tls.alpn_negotiated) { + return (ISC_R_DOTALPNERROR); + } + + return (ISC_R_SUCCESS); } const char * diff --git a/lib/isc/result.c b/lib/isc/result.c index 9e089dff07..24a0053511 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", @@ -220,6 +221,7 @@ static const char *description[ISC_R_NRESULTS] = { [DNS_R_NSEC3BADALG] = "cannot use NSEC3 with key algorithm", [DNS_R_NSEC3RESALT] = "NSEC3 resalt", [DNS_R_INCONSISTENTRR] = "inconsistent resource record", + [DNS_R_NOALPN] = "no ALPN", [DST_R_UNSUPPORTEDALG] = "algorithm is unsupported", [DST_R_CRYPTOFAILURE] = "crypto failure", @@ -340,6 +342,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", @@ -466,6 +469,7 @@ static const char *identifier[ISC_R_NRESULTS] = { [DNS_R_NSEC3BADALG] = "DNS_R_NSEC3BADALG", [DNS_R_NSEC3RESALT] = "DNS_R_NSEC3RESALT", [DNS_R_INCONSISTENTRR] = "DNS_R_INCONSISTENTRR", + [DNS_R_NOALPN] = "DNS_R_NOALPN", [DST_R_UNSUPPORTEDALG] = "DST_R_UNSUPPORTEDALG", [DST_R_CRYPTOFAILURE] = "DST_R_CRYPTOFAILURE", diff --git a/lib/ns/query.c b/lib/ns/query.c index 36f3c40dc1..be15f36180 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -12251,17 +12251,25 @@ ns_query_start(ns_client_t *client, isc_nmhandle_t *handle) { query_error(client, DNS_R_NOTIMP, __LINE__); return; } - if (isc_nm_socket_type(handle) == isc_nm_tlsdnssocket && - !isc_nm_xfr_allowed(handle)) - { + if (isc_nm_socket_type(handle) == isc_nm_tlsdnssocket) { /* * Currently this code is here for DoT, which * has more complex requirements for zone * 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; diff --git a/tests/isc/netmgr_test.c b/tests/isc/netmgr_test.c index cb2c1debab..1527c68112 100644 --- a/tests/isc/netmgr_test.c +++ b/tests/isc/netmgr_test.c @@ -2644,7 +2644,7 @@ tlsdns_connect_connect_noalpn(isc_nmhandle_t *handle, isc_result_t eresult, isc_refcount_decrement(&active_cconnects); if (eresult != ISC_R_SUCCESS || connect_readcb == NULL || - !isc_nm_xfr_allowed(handle)) + isc_nm_xfr_checkperm(handle) != ISC_R_SUCCESS) { return; } @@ -2715,7 +2715,7 @@ tls_accept_cb_noalpn(isc_nmhandle_t *handle, isc_result_t eresult, atomic_fetch_add(&saccepts, 1); - if (!isc_nm_xfr_allowed(handle)) { + if (isc_nm_xfr_checkperm(handle) != ISC_R_SUCCESS) { return (ISC_R_FAILURE); }