From: Artem Boldariev Date: Mon, 7 Jun 2021 13:38:39 +0000 (+0300) Subject: Make BIND refuse to serve XFRs over DoH X-Git-Tag: v9.17.16~39^2~2 X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=b84fa122ced4a58cdbf25166d9fc7ebae7aa9223;p=thirdparty%2Fbind9.git Make BIND refuse to serve XFRs over DoH 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. --- diff --git a/bin/tests/system/doth/tests.sh b/bin/tests/system/doth/tests.sh index de1fa2ffd33..3ef4016ac31 100644 --- a/bin/tests/system/doth/tests.sh +++ b/bin/tests/system/doth/tests.sh @@ -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)) diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index b8f19bf15a9..e2ce614df46 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -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); /*%< diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c index e38ff130923..d087507d474 100644 --- a/lib/isc/netmgr/http.c +++ b/lib/isc/netmgr/http.c @@ -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, diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h index 03cf3166775..b61c925bc34 100644 --- a/lib/isc/netmgr/netmgr-int.h +++ b/lib/isc/netmgr/netmgr-int.h @@ -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; diff --git a/lib/ns/query.c b/lib/ns/query.c index 6639a754518..c5dd5cae3cc 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -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: