sec: usr: Fix outgoing zone transfers' quota issue

Unauthorized clients could consume outgoing zone transfers quota
and block authorized zone transfer clients. This has been fixed.

Fixes isc-projects/bind9#3589

Merge branch '3859-security-xfrout-quota-fix' into 'security-main'

See merge request isc-private/bind9!971

bin/tests/system/xferquota/ns1/named.conf.j2

@@ -10,6 +10,8 @@ options {
 	recursion no;
 	dnssec-validation no;
 	notify yes;
+
+	transfers-out 3;
 };
 
 key rndc_key {

bin/tests/system/xferquota/ns3/named.conf.j2

@@ -0,0 +1,46 @@
+/*
+ * 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.
+ */
+
+options {
+	query-source address 10.53.0.3;
+	notify-source 10.53.0.3;
+	transfer-source 10.53.0.3;
+	port @PORT@;
+	pid-file "named.pid";
+	listen-on { 10.53.0.3; };
+	listen-on-v6 { none; };
+	recursion no;
+	dnssec-validation no;
+
+	transfers-out 1;
+	allow-transfer { 10.53.0.2; };
+};
+
+key rndc_key {
+	secret "1234abcd8765";
+	algorithm @DEFAULT_HMAC@;
+};
+
+controls {
+	inet 10.53.0.3 port @CONTROLPORT@ allow { any; } keys { rndc_key; };
+};
+
+zone "." {
+	type primary;
+	file "root.db";
+};
+
+zone "quota." {
+	type primary;
+	file "quota.db";
+};

bin/tests/system/xferquota/ns3/quota.db

@@ -0,0 +1,22 @@
+; 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.
+
+$TTL 300
+@	IN SOA	ns1.quota. hostmaster.quota. (
+			1		; serial
+			3600		; refresh
+			1800		; retry
+			604800		; expire
+			600		; minimum
+			)
+	IN NS	ns1.quota.
+ns1	IN A	10.53.0.3
+www	IN A	10.0.0.1

bin/tests/system/xferquota/ns3/root.db

@@ -0,0 +1,21 @@
+; 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.
+
+$TTL 300
+. 			IN SOA	ns.root. hostmaster.root. (
+				1   		; serial
+				3600         	; refresh
+				1800         	; retry
+				604800    	; expire
+				600       	; minimum
+				)
+.			NS	a.root-servers.nil.
+a.root-servers.nil.	A	10.53.0.3

bin/tests/system/xferquota/tests_xferquota.py

@@ -12,12 +12,15 @@
 from re import compile as Re
 
 import glob
+import multiprocessing
 import os
 import re
 import shutil
 import signal
 import time
 
+import dns.message
+import dns.query
 import dns.zone
 import pytest
 
@@ -60,6 +63,9 @@ def test_xferquota(named_port, ns1, ns2):
                     matching_line_count += 1
         return matching_line_count == 300
 
+    # The primary has 'transfers-out 3;', while the secondary has
+    # 'transfers-in 5; transfer-per-ns 5;'. This will allow all the zones
+    # to be eventually transferred, hitting the quotas now and then.
     isctest.run.retry_with_timeout(check_line_count, timeout=360)
 
     axfr_msg = isctest.query.create("zone000099.example.", "AXFR")
@@ -80,3 +86,39 @@ def test_xferquota(named_port, ns1, ns2):
     with ns2.watch_log_from_start(timeout=30) as watcher:
         watcher.wait_for_line(pattern)
     query_and_compare(a_msg)
+
+
+def _flood_unauthorized_axfrs(port, duration):
+    """Child process: send unauthorized AXFR requests for `duration` seconds."""
+    deadline = time.monotonic() + duration
+    while time.monotonic() < deadline:
+        try:
+            msg = dns.message.make_query("quota.", "AXFR")
+            dns.query.tcp(msg, "10.53.0.3", port=port, timeout=2, source="10.53.0.1")
+        except Exception:  # pylint: disable=broad-exception-caught
+            pass
+
+
+def test_xfrquota_unauthorized_no_starve(named_port):
+    """Unauthorized AXFR clients must not consume XFR-out quota (GL #3859).
+
+    ns3 is configured with transfers-out 1 and allow-transfer { 10.53.0.2; }.
+    We flood AXFR requests from unauthorized source processes (10.53.0.1) and
+    verify that an authorized client (10.53.0.2) can still transfer.
+    """
+    with multiprocessing.Pool(10) as pool:
+        pool.starmap_async(_flood_unauthorized_axfrs, [(named_port, 5)] * 10)
+
+        # Give the flood a moment to saturate
+        time.sleep(1)
+
+        # Try an authorized AXFR from 10.53.0.2 multiple times to increase
+        # the chance of hitting the race window where quota is consumed.
+        zone = dns.zone.Zone("quota.")
+        dns.query.inbound_xfr(
+            "10.53.0.3",
+            zone,
+            port=named_port,
+            timeout=10,
+            source="10.53.0.2",
+        )

lib/ns/xfrout.c

@@ -738,6 +738,7 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) {
 	bool is_poll = false;
 	bool is_dlz = false;
 	bool is_ixfr = false;
+	bool is_quota_applied = false;
 	bool useviewacl = false;
 	uint32_t begin_serial = 0, current_serial;
 
@@ -754,16 +755,6 @@ ns_xfr_start(ns_client_t *client, dns_rdatatype_t reqtype) {
 
 	ns_client_log(client, DNS_LOGCATEGORY_XFER_OUT, NS_LOGMODULE_XFER_OUT,
 		      ISC_LOG_DEBUG(6), "%s request", mnemonic);
-	/*
-	 * Apply quota.
-	 */
-	result = isc_quota_acquire(&client->manager->sctx->xfroutquota);
-	if (result != ISC_R_SUCCESS) {
-		isc_log_write(DNS_LOGCATEGORY_XFER_OUT, NS_LOGMODULE_XFER_OUT,
-			      ISC_LOG_WARNING, "%s request denied: %s",
-			      mnemonic, isc_result_totext(result));
-		goto max_quota;
-	}
 
 	/*
 	 * Interpret the question section.
@@ -922,6 +913,19 @@ got_soa:
 		FAILC(DNS_R_FORMERR, "attempted AXFR over UDP");
 	}
 
+	/*
+	 * Apply quota after ACL is checked, so that unauthorized clients
+	 * can not starve the authorized clients.
+	 */
+	result = isc_quota_acquire(&client->manager->sctx->xfroutquota);
+	if (result != ISC_R_SUCCESS) {
+		isc_log_write(DNS_LOGCATEGORY_XFER_OUT, NS_LOGMODULE_XFER_OUT,
+			      ISC_LOG_WARNING, "%s request denied: %s",
+			      mnemonic, isc_result_totext(result));
+		goto cleanup;
+	}
+	is_quota_applied = true;
+
 	/*
 	 * Look up the requesting server in the peer table.
 	 */
@@ -1060,7 +1064,7 @@ have_stream:
 	CHECK(dns_message_getquerytsig(request, mctx, &tsigbuf));
 	/*
 	 * Create the xfrout context object.  This transfers the ownership
-	 * of "stream", "db", "ver", and "quota" to the xfrout context object.
+	 * of "stream", "db" and "ver" to the xfrout context object.
 	 */
 
 	if (is_dlz) {
@@ -1179,10 +1183,13 @@ cleanup:
 	}
 
 	if (xfr != NULL) {
+		/* The quota will be released in xfrout_ctx_destroy(). */
+		INSIST(is_quota_applied);
 		xfrout_fail(xfr, result, "setting up zone transfer");
 	} else if (result != ISC_R_SUCCESS) {
-		isc_quota_release(&client->manager->sctx->xfroutquota);
-	max_quota:
+		if (is_quota_applied) {
+			isc_quota_release(&client->manager->sctx->xfroutquota);
+		}
 		ns_client_log(client, DNS_LOGCATEGORY_XFER_OUT,
 			      NS_LOGMODULE_XFER_OUT, ISC_LOG_DEBUG(3),
 			      "zone transfer setup failed");