]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Make BIND refuse to serve XFRs over DoH
authorArtem Boldariev <artem@boldariev.com>
Mon, 7 Jun 2021 13:38:39 +0000 (16:38 +0300)
committerArtem Boldariev <artem@boldariev.com>
Mon, 14 Jun 2021 08:37:36 +0000 (11:37 +0300)
We cannot use DoH for zone transfers.  According to RFC8484 a DoH
request contains exactly one DNS message (see Section 6: Definition of
the "application/dns-message" Media Type,
https://datatracker.ietf.org/doc/html/rfc8484#section-6).  This makes
DoH unsuitable for zone transfers as often (and usually!) these need
more than one DNS message, especially for larger zones.

As zone transfers over DoH are not (yet) standardised, nor discussed
in RFC8484, the best thing we can do is to return "not implemented."

Technically DoH can be used to transfer small zones which fit in one
message, but that is not enough for the generic case.

Also, this commit makes the server-side DoH code ensure that no
multiple responses could be attempted to be sent over one HTTP/2
stream. In HTTP/2 one stream is mapped to one request/response
transaction. Now the write callback will be called with failure error
code in such a case.

bin/tests/system/doth/tests.sh
lib/isc/include/isc/netmgr.h
lib/isc/netmgr/http.c
lib/isc/netmgr/netmgr-int.h
lib/ns/query.c

index de1fa2ffd339d7e5fb719ad3933c0766a7f3ac2e..3ef4016ac318e96023fca7d1058682a851bdc0fe 100644 (file)
@@ -114,7 +114,7 @@ n=$((n + 1))
 echo_i "checking DoH XFR (POST) (failure expected) ($n)"
 ret=0
 dig_with_https_opts +comm @10.53.0.1 . AXFR > dig.out.test$n
-grep "status: FORMERR" dig.out.test$n > /dev/null || ret=1
+grep "; Transfer failed." dig.out.test$n > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=$((status + ret))
 
@@ -154,7 +154,7 @@ n=$((n + 1))
 echo_i "checking DoH XFR (GET) (failure expected) ($n)"
 ret=0
 dig_with_https_opts +https-get +comm @10.53.0.1 . AXFR > dig.out.test$n
-grep "status: FORMERR" dig.out.test$n > /dev/null || ret=1
+grep "; Transfer failed." dig.out.test$n > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=$((status + ret))
 
@@ -178,7 +178,7 @@ n=$((n + 1))
 echo_i "checking unencrypted DoH XFR (failure expected) ($n)"
 ret=0
 dig_with_http_opts +comm @10.53.0.1 . AXFR > dig.out.test$n
-grep "status: FORMERR" dig.out.test$n > /dev/null || ret=1
+grep "; Transfer failed." dig.out.test$n > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=$((status + ret))
 
index b8f19bf15a9fe19ec9ec6e0c319abe0e9a380a11..e2ce614df469a62158c3c9bf1d6b9714bd28c541 100644 (file)
@@ -522,6 +522,9 @@ isc_result_t
 isc_nm_http_endpoint(isc_nmsocket_t *sock, const char *uri, isc_nm_recv_cb_t cb,
                     void *cbarg, size_t extrahandlesize);
 
+bool
+isc_nm_is_http_handle(isc_nmhandle_t *handle);
+
 void
 isc_nm_task_enqueue(isc_nm_t *mgr, isc_task_t *task, int threadid);
 /*%<
index e38ff130923548f7e57caaa2fcced153ab4d46ad..d087507d4747cd2f6e1cbc75a9f52cb8bae2f660 100644 (file)
@@ -1910,6 +1910,15 @@ server_send_response(nghttp2_session *ngsession, int32_t stream_id,
        nghttp2_data_provider data_prd;
        int rv;
 
+       if (socket->h2.response_submitted) {
+               /* NGHTTP2 will gladly accept new response (write request)
+                * from us even though we cannot send more than one over the
+                * same HTTP/2 stream. Thus, we need to handle this case
+                * manually. We will return failure code so that it will be
+                * passed to the write callback. */
+               return (ISC_R_FAILURE);
+       }
+
        data_prd.source.ptr = socket;
        data_prd.read_callback = server_read_callback;
 
@@ -1918,6 +1927,8 @@ server_send_response(nghttp2_session *ngsession, int32_t stream_id,
        if (rv != 0) {
                return (ISC_R_FAILURE);
        }
+
+       socket->h2.response_submitted = true;
        return (ISC_R_SUCCESS);
 }
 
@@ -2695,6 +2706,14 @@ failed_read_cb(isc_result_t result, isc_nm_http_session_t *session) {
        }
 }
 
+bool
+isc_nm_is_http_handle(isc_nmhandle_t *handle) {
+       REQUIRE(VALID_NMHANDLE(handle));
+       REQUIRE(VALID_NMSOCK(handle->sock));
+
+       return (handle->sock->type == isc_nm_httpsocket);
+}
+
 static const bool base64url_validation_table[256] = {
        false, false, false, false, false, false, false, false, false, false,
        false, false, false, false, false, false, false, false, false, false,
index 03cf31667753c34bd12e23535990d67a74908499..b61c925bc34de964730bd3c0432478107e576a3c 100644 (file)
@@ -823,6 +823,7 @@ typedef struct isc_nmsocket_h2 {
        ISC_LIST(isc_nm_httpcbarg_t) handler_cbargs;
        isc_rwlock_t lock;
 
+       bool response_submitted;
        struct {
                char *uri;
                bool post;
index 6639a754518cbc20b81b7842f676ef59479e4e66..c5dd5cae3cc541b8f7a7fd8f1e6af1c7683fd09f 100644 (file)
@@ -12043,7 +12043,24 @@ ns_query_start(ns_client_t *client, isc_nmhandle_t *handle) {
                        break; /* Let the query logic handle it. */
                case dns_rdatatype_ixfr:
                case dns_rdatatype_axfr:
-                       ns_xfr_start(client, rdataset->type);
+                       if (isc_nm_is_http_handle(handle)) {
+                               /* We cannot use DoH for zone transfers.
+                                * According to RFC8484 a DoH request contains
+                                * exactly one DNS message (see Section 6:
+                                * Definition of the "application/dns-message"
+                                * Media Type,
+                                * https://datatracker.ietf.org/doc/html/rfc8484#section-6).
+                                * This makes DoH unsuitable for zone transfers
+                                * as often (and usually!) these need more than
+                                * one DNS message, especially for larger zones.
+                                * As zone transfers over DoH are not (yet)
+                                * standardised, nor discussed in the RFC8484,
+                                * the best thing we can do is to return "not
+                                * implemented". */
+                               query_error(client, DNS_R_NOTIMP, __LINE__);
+                       } else {
+                               ns_xfr_start(client, rdataset->type);
+                       }
                        return;
                case dns_rdatatype_maila:
                case dns_rdatatype_mailb: