diff --git a/CHANGES b/CHANGES index f5598d1691..0bf0969fd8 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +5811. [bug] Reimplement the maximum and idle timeouts for outgoing + zone tranfers. [GL #1897] + 5810. [func] New option '-J' for dnssec-signzone and dnssec-verify allows loading journal files. [GL #2486] diff --git a/bin/tests/system/run.sh.in b/bin/tests/system/run.sh.in index 9c529d0286..c8467e6b94 100644 --- a/bin/tests/system/run.sh.in +++ b/bin/tests/system/run.sh.in @@ -88,6 +88,7 @@ if ! $do_run; then LOG_DRIVER_FLAGS="--verbose yes --color-tests yes" \ LOG_FLAGS="$log_flags" \ TEST_LARGE_MAP="${TEST_LARGE_MAP}" \ + CI_ENABLE_ALL_TESTS="${CI_ENABLE_ALL_TESTS}" \ make -e check exit $? fi diff --git a/bin/tests/system/timeouts/conftest.py b/bin/tests/system/timeouts/conftest.py index 0ce749b3fd..d92d87f72c 100644 --- a/bin/tests/system/timeouts/conftest.py +++ b/bin/tests/system/timeouts/conftest.py @@ -20,6 +20,9 @@ def pytest_configure(config): config.addinivalue_line( "markers", "dnspython2: mark tests that need dnspython >= 2.0.0" ) + config.addinivalue_line( + "markers", "long: mark tests that take a long time to run" + ) def pytest_collection_modifyitems(config, items): @@ -46,6 +49,13 @@ def pytest_collection_modifyitems(config, items): if "dnspython2" in item.keywords: item.add_marker(skip_dnspython2) + skip_long_tests = pytest.mark.skip( + reason="need CI_ENABLE_ALL_TESTS environment variable") + if not os.environ.get("CI_ENABLE_ALL_TESTS"): + for item in items: + if "long" in item.keywords: + item.add_marker(skip_long_tests) + @pytest.fixture def port(request): diff --git a/bin/tests/system/timeouts/ns1/named.conf.in b/bin/tests/system/timeouts/ns1/named.conf.in index c9a7a6b42e..4b422e22b2 100644 --- a/bin/tests/system/timeouts/ns1/named.conf.in +++ b/bin/tests/system/timeouts/ns1/named.conf.in @@ -30,6 +30,8 @@ options { tcp-initial-timeout 20; tcp-idle-timeout 50; tcp-keepalive-timeout 70; + max-transfer-time-out 5; /* minutes */ + max-transfer-idle-out 1; /* minutes */ }; zone "." { diff --git a/bin/tests/system/timeouts/tests-tcp.py b/bin/tests/system/timeouts/tests-tcp.py index e1f19608c8..a1c94839ea 100644 --- a/bin/tests/system/timeouts/tests-tcp.py +++ b/bin/tests/system/timeouts/tests-tcp.py @@ -220,3 +220,76 @@ def test_send_timeout(port): (response, rtime) = dns.query.receive_tcp(sock, timeout()) except ConnectionError as e: raise EOFError from e + + +@pytest.mark.dnspython +@pytest.mark.dnspython2 +@pytest.mark.long +def test_max_transfer_idle_out(port): + import dns.query + import dns.rdataclass + import dns.rdatatype + + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + sock.connect(("10.53.0.1", port)) + + name = dns.name.from_text("example.") + msg = create_msg("example.", "AXFR") + (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout()) + + # Receive the initial DNS message with SOA + (response, rtime) = dns.query.receive_tcp(sock, timeout(), + one_rr_per_rrset=True) + soa = response.get_rrset(dns.message.ANSWER, name, + dns.rdataclass.IN, dns.rdatatype.SOA) + assert soa is not None + + time.sleep(61) # max-transfer-idle-out is 1 minute + + with pytest.raises(ConnectionResetError): + # Process queued TCP messages + while True: + (response, rtime) = \ + dns.query.receive_tcp(sock, timeout(), + one_rr_per_rrset=True) + soa = response.get_rrset(dns.message.ANSWER, name, + dns.rdataclass.IN, dns.rdatatype.SOA) + if soa is not None: + break + assert soa is None + + +@pytest.mark.dnspython +@pytest.mark.dnspython2 +@pytest.mark.long +def test_max_transfer_time_out(port): + import dns.query + import dns.rdataclass + import dns.rdatatype + + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + sock.connect(("10.53.0.1", port)) + + name = dns.name.from_text("example.") + msg = create_msg("example.", "AXFR") + (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout()) + + # Receive the initial DNS message with SOA + (response, rtime) = dns.query.receive_tcp(sock, timeout(), + one_rr_per_rrset=True) + soa = response.get_rrset(dns.message.ANSWER, name, + dns.rdataclass.IN, dns.rdatatype.SOA) + assert soa is not None + + # The loop should timeout at the 5 minutes (max-transfer-time-out) + with pytest.raises(EOFError): + while True: + time.sleep(1) + (response, rtime) = \ + dns.query.receive_tcp(sock, timeout(), + one_rr_per_rrset=True) + soa = response.get_rrset(dns.message.ANSWER, name, + dns.rdataclass.IN, dns.rdatatype.SOA) + if soa is not None: + break + assert soa is None diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index bfe85f1f4d..075b68f5be 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -81,3 +81,8 @@ Bug Fixes - Client TCP connections are now closed immediately when data received cannot be parsed as a valid DNS request. :gl:`#3149` + +- The ``max-transfer-time-out`` and ``max-transfer-idle-out`` options were + not implemented when the BIND 9 networking stack was refactored in 9.16. + The missing functionality has been re-implemented and outgoing zone + transfers now time out properly when not progressing. :gl:`#1897` diff --git a/lib/isc/Makefile.am b/lib/isc/Makefile.am index c6f800d5c6..3d37127fe9 100644 --- a/lib/isc/Makefile.am +++ b/lib/isc/Makefile.am @@ -109,6 +109,7 @@ libisc_la_SOURCES = \ netmgr/netmgr.c \ netmgr/tcp.c \ netmgr/tcpdns.c \ + netmgr/timer.c \ netmgr/tlsdns.c \ netmgr/udp.c \ netmgr/uv-compat.c \ diff --git a/lib/isc/include/isc/netmgr.h b/lib/isc/include/isc/netmgr.h index 671880137d..16eedb708f 100644 --- a/lib/isc/include/isc/netmgr.h +++ b/lib/isc/include/isc/netmgr.h @@ -700,3 +700,27 @@ isc__nm_force_tid(int tid); void isc_nmhandle_setwritetimeout(isc_nmhandle_t *handle, uint64_t write_timeout); + +/* + * Timer related functions + */ + +typedef struct isc_nm_timer isc_nm_timer_t; + +typedef void (*isc_nm_timer_cb)(void *, isc_result_t); + +void +isc_nm_timer_create(isc_nmhandle_t *, isc_nm_timer_cb, void *, + isc_nm_timer_t **); + +void +isc_nm_timer_attach(isc_nm_timer_t *, isc_nm_timer_t **); + +void +isc_nm_timer_detach(isc_nm_timer_t **); + +void +isc_nm_timer_start(isc_nm_timer_t *, uint64_t); + +void +isc_nm_timer_stop(isc_nm_timer_t *); diff --git a/lib/isc/netmgr/timer.c b/lib/isc/netmgr/timer.c new file mode 100644 index 0000000000..3016387b0f --- /dev/null +++ b/lib/isc/netmgr/timer.c @@ -0,0 +1,118 @@ +/* + * Copyright (C) Internet Systems Consortium, Inc. ("ISC") + * + * SPDX-License-Identifier: MPL-2.0 + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, you can obtain one at https://mozilla.org/MPL/2.0/. + * + * See the COPYRIGHT file distributed with this work for additional + * information regarding copyright ownership. + */ + +#include + +#include +#include + +#include "netmgr-int.h" + +struct isc_nm_timer { + isc_refcount_t references; + uv_timer_t timer; + isc_nmhandle_t *handle; + isc_nm_timer_cb cb; + void *cbarg; +}; + +void +isc_nm_timer_create(isc_nmhandle_t *handle, isc_nm_timer_cb cb, void *cbarg, + isc_nm_timer_t **timerp) { + isc__networker_t *worker = NULL; + isc_nmsocket_t *sock = NULL; + isc_nm_timer_t *timer = NULL; + int r; + + REQUIRE(isc__nm_in_netthread()); + REQUIRE(VALID_NMHANDLE(handle)); + REQUIRE(VALID_NMSOCK(handle->sock)); + + sock = handle->sock; + worker = &sock->mgr->workers[isc_nm_tid()]; + + timer = isc_mem_get(sock->mgr->mctx, sizeof(*timer)); + *timer = (isc_nm_timer_t){ .cb = cb, .cbarg = cbarg }; + isc_refcount_init(&timer->references, 1); + isc_nmhandle_attach(handle, &timer->handle); + + r = uv_timer_init(&worker->loop, &timer->timer); + UV_RUNTIME_CHECK(uv_timer_init, r); + + uv_handle_set_data((uv_handle_t *)&timer->timer, timer); + + *timerp = timer; +} + +void +isc_nm_timer_attach(isc_nm_timer_t *timer, isc_nm_timer_t **timerp) { + REQUIRE(timer != NULL); + REQUIRE(timerp != NULL && *timerp == NULL); + + isc_refcount_increment(&timer->references); + *timerp = timer; +} + +static void +timer_destroy(uv_handle_t *uvhandle) { + isc_nm_timer_t *timer = uv_handle_get_data(uvhandle); + isc_nmhandle_t *handle = timer->handle; + isc_mem_t *mctx = timer->handle->sock->mgr->mctx; + + isc_mem_put(mctx, timer, sizeof(*timer)); + + isc_nmhandle_detach(&handle); +} + +void +isc_nm_timer_detach(isc_nm_timer_t **timerp) { + isc_nm_timer_t *timer = NULL; + isc_nmhandle_t *handle = NULL; + + REQUIRE(timerp != NULL && *timerp != NULL); + + timer = *timerp; + *timerp = NULL; + + handle = timer->handle; + + REQUIRE(isc__nm_in_netthread()); + REQUIRE(VALID_NMHANDLE(handle)); + REQUIRE(VALID_NMSOCK(handle->sock)); + + if (isc_refcount_decrement(&timer->references) == 1) { + uv_timer_stop(&timer->timer); + uv_close((uv_handle_t *)&timer->timer, timer_destroy); + } +} + +static void +timer_cb(uv_timer_t *uvtimer) { + isc_nm_timer_t *timer = uv_handle_get_data((uv_handle_t *)uvtimer); + + REQUIRE(timer->cb != NULL); + + timer->cb(timer->cbarg, ISC_R_TIMEDOUT); +} + +void +isc_nm_timer_start(isc_nm_timer_t *timer, uint64_t timeout) { + int r = uv_timer_start(&timer->timer, timer_cb, timeout, 0); + UV_RUNTIME_CHECK(uv_timer_start, r); +} + +void +isc_nm_timer_stop(isc_nm_timer_t *timer) { + int r = uv_timer_stop(&timer->timer); + UV_RUNTIME_CHECK(uv_timer_stop, r); +} diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h index d456bc83af..a5c1f0072e 100644 --- a/lib/ns/include/ns/client.h +++ b/lib/ns/include/ns/client.h @@ -198,8 +198,6 @@ struct ns_client { int16_t ednsversion; /* -1 noedns */ uint16_t additionaldepth; void (*cleanup)(ns_client_t *); - void (*shutdown)(void *arg, isc_result_t result); - void *shutdown_arg; ns_query_t query; isc_time_t requesttime; isc_stdtime_t now; diff --git a/lib/ns/xfrout.c b/lib/ns/xfrout.c index fdbd4029b7..e32493f8bf 100644 --- a/lib/ns/xfrout.c +++ b/lib/ns/xfrout.c @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -668,6 +669,12 @@ typedef struct { const char *mnemonic; /* Style of transfer */ uint32_t end_serial; /* Serial number after XFR is done */ struct xfr_stats stats; /*%< Transfer statistics */ + + /* Timeouts */ + uint64_t maxtime; /*%< Maximum XFR timeout (in ms) */ + isc_nm_timer_t *maxtime_timer; + + uint64_t idletime; /*%< XFR idle timeout (in ms) */ } xfrout_ctx_t; static void @@ -696,7 +703,7 @@ static void xfrout_ctx_destroy(xfrout_ctx_t **xfrp); static void -xfrout_client_shutdown(void *arg, isc_result_t result); +xfrout_client_timeout(void *arg, isc_result_t result); static void xfrout_log1(ns_client_t *client, dns_name_t *zonename, dns_rdataclass_t rdclass, @@ -1146,6 +1153,14 @@ have_stream: } } + /* Start the timers */ + if (xfr->maxtime > 0) { + xfrout_log(xfr, ISC_LOG_ERROR, + "starting maxtime timer %" PRIu64 " ms", + xfr->maxtime); + isc_nm_timer_start(xfr->maxtime_timer, xfr->maxtime); + } + /* * Hand the context over to sendstream(). Set xfr to NULL; * sendstream() is responsible for either passing the @@ -1205,64 +1220,52 @@ xfrout_ctx_create(isc_mem_t *mctx, ns_client_t *client, unsigned int id, bool verified_tsig, unsigned int maxtime, unsigned int idletime, bool many_answers, xfrout_ctx_t **xfrp) { - xfrout_ctx_t *xfr; - unsigned int len; - void *mem; + xfrout_ctx_t *xfr = NULL; + unsigned int len = NS_CLIENT_TCP_BUFFER_SIZE; + void *mem = NULL; REQUIRE(xfrp != NULL && *xfrp == NULL); - UNUSED(maxtime); - UNUSED(idletime); - xfr = isc_mem_get(mctx, sizeof(*xfr)); - xfr->mctx = NULL; + *xfr = (xfrout_ctx_t){ + .client = client, + .id = id, + .qname = qname, + .qtype = qtype, + .qclass = qclass, + .maxtime = maxtime * 1000, /* in milliseconds */ + .idletime = idletime * 1000, /* In milliseconds */ + .tsigkey = tsigkey, + .lasttsig = lasttsig, + .verified_tsig = verified_tsig, + .many_answers = many_answers, + }; + isc_mem_attach(mctx, &xfr->mctx); - xfr->client = client; - xfr->id = id; - xfr->qname = qname; - xfr->qtype = qtype; - xfr->qclass = qclass; - xfr->zone = NULL; - xfr->db = NULL; - xfr->ver = NULL; + if (zone != NULL) { /* zone will be NULL if it's DLZ */ dns_zone_attach(zone, &xfr->zone); } dns_db_attach(db, &xfr->db); dns_db_attachversion(db, ver, &xfr->ver); - xfr->question_added = false; - xfr->end_of_stream = false; - xfr->tsigkey = tsigkey; - xfr->lasttsig = lasttsig; - xfr->verified_tsig = verified_tsig; - xfr->many_answers = many_answers; - xfr->sends = 0; - xfr->shuttingdown = false; - xfr->poll = false; - xfr->mnemonic = NULL; - xfr->buf.base = NULL; - xfr->buf.length = 0; - xfr->txmem = NULL; - xfr->txmemlen = 0; - xfr->stream = NULL; - xfr->quota = NULL; - xfr->stats.nmsg = 0; - xfr->stats.nrecs = 0; - xfr->stats.nbytes = 0; isc_time_now(&xfr->stats.start); + isc_nm_timer_create(xfr->client->handle, xfrout_client_timeout, xfr, + &xfr->maxtime_timer); + /* * Allocate a temporary buffer for the uncompressed response - * message data. The size should be no more than 65535 bytes - * so that the compressed data will fit in a TCP message, - * and no less than 65535 bytes so that an almost maximum-sized - * RR will fit. Note that although 65535-byte RRs are allowed - * in principle, they cannot be zone-transferred (at least not - * if uncompressible), because the message and RR headers would - * push the size of the TCP message over the 65536 byte limit. + * message data. The buffer size must be 65535 bytes + * (NS_CLIENT_TCP_BUFFER_SIZE): small enough that compressed + * data will fit in a single TCP message, and big enough to + * hold a maximum-sized RR. + * + * Note that although 65535-byte RRs are allowed in principle, they + * cannot be zone-transferred (at least not if uncompressible), + * because the message and RR headers would push the size of the + * TCP message over the 65536 byte limit. */ - len = 65535; mem = isc_mem_get(mctx, len); isc_buffer_init(&xfr->buf, mem, len); @@ -1270,19 +1273,11 @@ xfrout_ctx_create(isc_mem_t *mctx, ns_client_t *client, unsigned int id, * Allocate another temporary buffer for the compressed * response message. */ - len = NS_CLIENT_TCP_BUFFER_SIZE; mem = isc_mem_get(mctx, len); isc_buffer_init(&xfr->txbuf, (char *)mem, len); xfr->txmem = mem; xfr->txmemlen = len; - /* - * Register a shutdown callback with the client, so that we - * can stop the transfer immediately when the client task - * gets a shutdown event. - */ - xfr->client->shutdown = xfrout_client_shutdown; - xfr->client->shutdown_arg = xfr; /* * These MUST be after the last "goto failure;" / CHECK to * prevent a double free by the caller. @@ -1567,6 +1562,10 @@ sendstream(xfrout_ctx_t *xfr) { isc_nmhandle_attach(xfr->client->handle, &xfr->client->sendhandle); + if (xfr->idletime > 0) { + isc_nmhandle_setwritetimeout(xfr->client->sendhandle, + xfr->idletime); + } isc_nm_send(xfr->client->sendhandle, &used, xfrout_senddone, xfr); xfr->sends++; @@ -1632,8 +1631,8 @@ xfrout_ctx_destroy(xfrout_ctx_t **xfrp) { INSIST(xfr->sends == 0); - xfr->client->shutdown = NULL; - xfr->client->shutdown_arg = NULL; + isc_nm_timer_stop(xfr->maxtime_timer); + isc_nm_timer_detach(&xfr->maxtime_timer); if (xfr->stream != NULL) { xfr->stream->methods->destroy(&xfr->stream); @@ -1740,9 +1739,12 @@ xfrout_maybe_destroy(xfrout_ctx_t *xfr) { } static void -xfrout_client_shutdown(void *arg, isc_result_t result) { +xfrout_client_timeout(void *arg, isc_result_t result) { xfrout_ctx_t *xfr = (xfrout_ctx_t *)arg; - xfrout_fail(xfr, result, "aborted"); + + xfr->shuttingdown = true; + xfrout_log(xfr, ISC_LOG_ERROR, "%s: %s", "aborted", + isc_result_totext(result)); } /*