]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Refactor isc_nm_xfr_allowed()
authorAram Sargsyan <aram@isc.org>
Wed, 18 Jan 2023 10:36:34 +0000 (10:36 +0000)
committerAram Sargsyan <aram@isc.org>
Thu, 19 Jan 2023 10:24:08 +0000 (10:24 +0000)
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.

bin/dig/dighost.c
lib/dns/xfrin.c
lib/isc/include/isc/netmgr.h
lib/isc/include/isc/result.h
lib/isc/netmgr/netmgr-int.h
lib/isc/netmgr/netmgr.c
lib/isc/netmgr/streamdns.c
lib/isc/result.c
lib/ns/query.c

index e07867143730edcd49c843ae6944bd76afcfbb42..1a1c14feca146234e1d405bd0991e21910d0610b 100644 (file)
@@ -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);
index c4723925c2446b4f6e2f2430325ff2696bfd763c..05b612f850751ad9ea815d7879f3827d9ef3ecf7 100644 (file)
@@ -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) {
index f6cf2b7d824dc8caf3247ebb6599899ccce45ef5..bd81b7bb9d7b0c155e39a470d06781b6ea207d23 100644 (file)
@@ -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.
index 6e7ba1b68f8d6ba8e75c65bbeccfe57a3dde1203..ac7273ce52149034348b661f40313c2b2210ef18 100644 (file)
@@ -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,
index 4a722130fa32253d4efa90cbb303010aee8a9ac2..082bba22b0c8f76bb1273f1db61b2ddade167861 100644 (file)
@@ -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);
index 10eac867adc5b4d44a69b0ba7e5ce8fdd31ff0a6..392bf3ceaea211315da9109fd4542241c34a88a1 100644 (file)
@@ -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
index a2395dd82026228d5f44064c621c9c708a657b65..ef5fd6874ce71dcab1a537e8567b584f1d55b330 100644 (file)
@@ -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
index 372dd590e987e21d4fd89b31655e26317bd1ab54..b41f760fa453a4c07a1fafe4d09ac13e9c610823 100644 (file)
@@ -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",
index faab1340ed58bf02634962511e827f50c2191a5d..946591009cc190eac8f12e5334b505af34a58890 100644 (file)
@@ -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;