WIP: Add broken max-transfer-{time,idle}-in test

The test is totally broken, but it just shows how I think it should be
done.  Now it needs somebody with Python-foo to help finish this.

CHANGES

@@ -1,3 +1,6 @@
+6148.	[bug]		Reimplement the maximum and idle timeouts for incoming
+			zone tranfers. [GL #4004]
+
 6147.	[performance]	Fix the TCP server parent quota use. [GL #3985]
 
 6146.	[performance]	Replace the zone table red-black tree and associated

bin/tests/system/timeouts/ns1/named.conf.in

@@ -32,6 +32,8 @@ options {
 	tcp-keepalive-timeout 70;
 	max-transfer-time-out 5; /* minutes */
 	max-transfer-idle-out 1; /* minutes */
+	max-transfer-time-in 5; /* minutes */
+	max-transfer-idle-in 1; /* minutes */
 };
 
 zone "." {

bin/tests/system/timeouts/tests_tcp_timeouts.py

@@ -246,33 +246,81 @@ def test_max_transfer_idle_out(named_port):
 
 
 @pytest_custom_markers.long_test
-def test_max_transfer_time_out(named_port):
-    with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
-        sock.connect(("10.53.0.1", named_port))
+def test_max_transfer_time_in(named_port):
+    udp = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
+    udp.bind(("10.53.0.2", port))
 
-        name = dns.name.from_text("example.")
-        msg = create_msg("example.", "AXFR")
-        (sbytes, stime) = dns.query.send_tcp(sock, msg, timeout())
+    tcp = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    tcp.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+    tcp.bind("10.53.0.2", named_port)
+    tcp.listen(100)
 
-        # 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
+    input = [udp, tcp]
+    while True:
+        try:
+            inputready, outputready, exceptready = select.select(input, [], [])
+        except select.error:
+            break
+        except socket.error:
+            break
+        except KeyboardInterrupt:
+            break
 
-        # 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
+        for s in inputready:
+            if s == udp:
+                qrybytes, clientaddr = udp.recvfrom(65535)
+                qry = dns.message.from_wire(qrybytes)
+                answ = dns.message.make_response(qry)
+                answbytes = answ.to_wire()
+                udp.sendto(answbytes, clientaddr)
+            if s == tcp:
+                csock, clientaddr = tcp.accept()
+                qrylen = csock.recv(2)
+                qry = csock.recv(socket.ntoh(qrylen))
+                # Make a long loop here, each record sent in less than 1 minute
+                # so, every 30 seconds?
+                name = dns.name.from_text("example.")
+                answ = create_msg("example", "SOA")
+                csock.send(answ.to_wire())
+    tcp.close()
+    udp.close()
+
+
+@pytest_custom_markers.long_test
+def test_max_transfer_idle_in(named_port):
+    udp = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
+    udp.bind(("10.53.0.2", port))
+
+    tcp = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    tcp.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+    tcp.bind("10.53.0.2", named_port)
+    tcp.listen(100)
+
+    input = [udp, tcp]
+    while True:
+        try:
+            inputready, outputready, exceptready = select.select(input, [], [])
+        except select.error:
+            break
+        except socket.error:
+            break
+        except KeyboardInterrupt:
+            break
+
+        for s in inputready:
+            if s == udp:
+                qrybytes, clientaddr = udp.recvfrom(65535)
+                qry = dns.message.from_wire(qrybytes)
+                answ = dns.message.make_response(qry)
+                answbytes = answ.to_wire()
+                udp.sendto(answbytes, clientaddr)
+            if s == tcp:
+                csock, clientaddr = tcp.accept()
+                qrylen = csock.recv(2)
+                qry = csock.recv(socket.ntoh(qrylen))
+                # Send SOA than wait for 1+ minute
+                name = dns.name.from_text("example.")
+                answ = create_msg("example", "SOA")
+                csock.send(answ.to_wire())
+    tcp.close()
+    udp.close()

doc/notes/notes-current.rst

@@ -67,6 +67,12 @@ Bug Fixes
 - Performance of DNSSEC validation in zones with many DNSKEY records
   has been improved. :gl:`#3981`
 
+- The :any:`max-transfer-time-in` and :any:`max-transfer-idle-in` options
+  were not implemented when the BIND 9 networking stack was refactored
+  in 9.16. The missing functionality has been re-implemented and
+  incoming zone transfers now time out properly when not progressing.
+  :gl:`#4004`
+
 Known Issues
 ~~~~~~~~~~~~
 

lib/dns/include/dns/xfrin.h

@@ -27,6 +27,7 @@
  ***/
 
 #include <isc/lang.h>
+#include <isc/refcount.h>
 #include <isc/tls.h>
 
 #include <dns/transport.h>

lib/dns/xfrin.c

@@ -177,6 +177,9 @@ struct dns_xfrin {
 	unsigned char *firstsoa_data;
 
 	isc_tlsctx_cache_t *tlsctx_cache;
+
+	isc_timer_t *max_time_timer;
+	isc_timer_t *max_idle_timer;
 };
 
 #define XFRIN_MAGIC    ISC_MAGIC('X', 'f', 'r', 'I')
@@ -237,6 +240,10 @@ xfrin_recv_done(isc_result_t result, isc_region_t *region, void *arg);
 static void
 xfrin_destroy(dns_xfrin_t *xfr);
 
+static void
+xfrin_timedout(void *);
+static void
+xfrin_idledout(void *);
 static void
 xfrin_fail(dns_xfrin_t *xfr, isc_result_t result, const char *msg);
 static isc_result_t
@@ -693,6 +700,7 @@ dns_xfrin_create(dns_zone_t *zone, dns_rdatatype_t xfrtype,
 	REQUIRE(isc_sockaddr_getport(primaryaddr) != 0);
 	REQUIRE(zone != NULL);
 	REQUIRE(dns_zone_getview(zone) != NULL);
+	REQUIRE(dns_zone_gettid(zone) == isc_tid());
 
 	(void)dns_zone_getdb(zone, &db);
 
@@ -737,9 +745,24 @@ dns_xfrin_create(dns_zone_t *zone, dns_rdatatype_t xfrtype,
 	return (result);
 }
 
+static void
+xfrin_timedout(void *xfr) {
+	REQUIRE(VALID_XFRIN(xfr));
+
+	xfrin_fail(xfr, ISC_R_TIMEDOUT, "maximum transfer time exceeded");
+}
+
+static void
+xfrin_idledout(void *xfr) {
+	REQUIRE(VALID_XFRIN(xfr));
+
+	xfrin_fail(xfr, ISC_R_TIMEDOUT, "maximum idle time exceeded");
+}
+
 void
 dns_xfrin_shutdown(dns_xfrin_t *xfr) {
 	REQUIRE(VALID_XFRIN(xfr));
+	REQUIRE(dns_zone_gettid(xfr->zone) == isc_tid());
 
 	xfrin_fail(xfr, ISC_R_CANCELED, "shut down");
 }
@@ -790,6 +813,9 @@ xfrin_fail(dns_xfrin_t *xfr, isc_result_t result, const char *msg) {
 	if (atomic_compare_exchange_strong(&xfr->shuttingdown, &(bool){ false },
 					   true))
 	{
+		isc_timer_stop(xfr->max_time_timer);
+		isc_timer_stop(xfr->max_idle_timer);
+
 		if (result != DNS_R_UPTODATE && result != DNS_R_TOOMANYRECORDS)
 		{
 			xfrin_log(xfr, ISC_LOG_ERROR, "%s: %s", msg,
@@ -829,13 +855,16 @@ xfrin_create(isc_mem_t *mctx, dns_zone_t *zone, dns_db_t *db,
 	dns_xfrin_t *xfr = NULL;
 
 	xfr = isc_mem_get(mctx, sizeof(*xfr));
-	*xfr = (dns_xfrin_t){ .shutdown_result = ISC_R_UNSET,
-			      .rdclass = rdclass,
-			      .reqtype = reqtype,
-			      .maxrecords = dns_zone_getmaxrecords(zone),
-			      .primaryaddr = *primaryaddr,
-			      .sourceaddr = *sourceaddr,
-			      .firstsoa = DNS_RDATA_INIT };
+	*xfr = (dns_xfrin_t){
+		.shutdown_result = ISC_R_UNSET,
+		.rdclass = rdclass,
+		.reqtype = reqtype,
+		.maxrecords = dns_zone_getmaxrecords(zone),
+		.primaryaddr = *primaryaddr,
+		.sourceaddr = *sourceaddr,
+		.firstsoa = DNS_RDATA_INIT,
+		.magic = XFRIN_MAGIC,
+	};
 
 	isc_mem_attach(mctx, &xfr->mctx);
 	dns_zone_iattach(zone, &xfr->zone);
@@ -879,7 +908,10 @@ xfrin_create(isc_mem_t *mctx, dns_zone_t *zone, dns_db_t *db,
 
 	isc_tlsctx_cache_attach(tlsctx_cache, &xfr->tlsctx_cache);
 
-	xfr->magic = XFRIN_MAGIC;
+	isc_timer_create(dns_zone_getloop(zone), xfrin_timedout, xfr,
+			 &xfr->max_time_timer);
+	isc_timer_create(dns_zone_getloop(zone), xfrin_idledout, xfr,
+			 &xfr->max_idle_timer);
 
 	*xfrp = xfr;
 }
@@ -887,6 +919,7 @@ xfrin_create(isc_mem_t *mctx, dns_zone_t *zone, dns_db_t *db,
 static isc_result_t
 xfrin_start(dns_xfrin_t *xfr) {
 	isc_result_t result = ISC_R_FAILURE;
+	isc_interval_t interval;
 
 	dns_xfrin_ref(xfr);
 
@@ -922,6 +955,15 @@ xfrin_start(dns_xfrin_t *xfr) {
 		xfr->tlsctx_cache, xfrin_connect_done, xfrin_send_done,
 		xfrin_recv_done, xfr, &xfr->id, &xfr->dispentry));
 	CHECK(dns_dispatch_connect(xfr->dispentry));
+
+	/* Set the maximum timer */
+	isc_interval_set(&interval, dns_zone_getmaxxfrin(xfr->zone), 0);
+	isc_timer_start(xfr->max_time_timer, isc_timertype_once, &interval);
+
+	/* Set the idle timer */
+	isc_interval_set(&interval, dns_zone_getidlein(xfr->zone), 0);
+	isc_timer_start(xfr->max_time_timer, isc_timertype_once, &interval);
+
 	return (ISC_R_SUCCESS);
 
 failure:
@@ -1216,6 +1258,9 @@ xfrin_recv_done(isc_result_t result, isc_region_t *region, void *arg) {
 		result = ISC_R_SHUTTINGDOWN;
 	}
 
+	/* Stop the idle timer */
+	isc_timer_stop(xfr->max_idle_timer);
+
 	CHECK(result);
 
 	xfrin_log(xfr, ISC_LOG_DEBUG(7), "received %u bytes", region->length);
@@ -1467,6 +1512,7 @@ xfrin_recv_done(isc_result_t result, isc_region_t *region, void *arg) {
 		}
 
 		atomic_store(&xfr->shuttingdown, true);
+		isc_timer_stop(xfr->max_time_timer);
 		xfr->shutdown_result = ISC_R_SUCCESS;
 		break;
 	default:
@@ -1475,6 +1521,11 @@ xfrin_recv_done(isc_result_t result, isc_region_t *region, void *arg) {
 		 */
 		dns_message_detach(&msg);
 		dns_dispatch_getnext(xfr->dispentry);
+
+		isc_interval_t interval;
+		isc_interval_set(&interval, dns_zone_getidlein(xfr->zone), 0);
+		isc_timer_start(xfr->max_time_timer, isc_timertype_once,
+				&interval);
 		return;
 	}
 
@@ -1494,6 +1545,7 @@ xfrin_destroy(dns_xfrin_t *xfr) {
 	uint64_t msecs, persec;
 
 	REQUIRE(VALID_XFRIN(xfr));
+	REQUIRE(dns_zone_gettid(xfr->zone) == isc_tid());
 
 	/* Safe-guards */
 	REQUIRE(atomic_load(&xfr->shuttingdown));
@@ -1599,6 +1651,9 @@ xfrin_destroy(dns_xfrin_t *xfr) {
 		isc_tlsctx_cache_detach(&xfr->tlsctx_cache);
 	}
 
+	isc_timer_destroy(&xfr->max_idle_timer);
+	isc_timer_destroy(&xfr->max_time_timer);
+
 	isc_mem_putanddetach(&xfr->mctx, xfr, sizeof(*xfr));
 }