From 3adabb4f893be218b68676fd3651924190b10fca Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Fri, 1 Nov 2024 13:23:20 +0100 Subject: [PATCH 1/2] Add test case for nsupdate hangs on large update This test case hangs, despite the update being performed on the name server. --- .../system/nsupdate/ns3/many-updates.test.db | 14 +++++++++++++ bin/tests/system/nsupdate/ns3/named.conf.in | 7 +++++++ bin/tests/system/nsupdate/tests.sh | 20 +++++++++++++++++++ 3 files changed, 41 insertions(+) create mode 100644 bin/tests/system/nsupdate/ns3/many-updates.test.db diff --git a/bin/tests/system/nsupdate/ns3/many-updates.test.db b/bin/tests/system/nsupdate/ns3/many-updates.test.db new file mode 100644 index 00000000000..bb0805b53f5 --- /dev/null +++ b/bin/tests/system/nsupdate/ns3/many-updates.test.db @@ -0,0 +1,14 @@ +; 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. + +many-updates.test. 10 IN SOA many-updates.test. hostmaster.many-updates.test. 1 3600 900 2419200 3600 +many-updates.test. 10 IN NS many-updates.test. +many-updates.test. 10 IN A 10.53.0.3 diff --git a/bin/tests/system/nsupdate/ns3/named.conf.in b/bin/tests/system/nsupdate/ns3/named.conf.in index b62885111a5..180ce700bf5 100644 --- a/bin/tests/system/nsupdate/ns3/named.conf.in +++ b/bin/tests/system/nsupdate/ns3/named.conf.in @@ -91,6 +91,13 @@ zone "too-big.test" { file "too-big.test.db"; }; +zone "many-updates.test" { + type primary; + allow-update { any; }; + file "many-updates.test.db"; +}; + + /* Zone for testing CDS and CDNSKEY updates from other provider */ zone "multisigner.test" { type primary; diff --git a/bin/tests/system/nsupdate/tests.sh b/bin/tests/system/nsupdate/tests.sh index 035d44d0378..2995bb5b69d 100755 --- a/bin/tests/system/nsupdate/tests.sh +++ b/bin/tests/system/nsupdate/tests.sh @@ -732,6 +732,26 @@ EOF status=1 } +n=$((n + 1)) +ret=0 +i=0 +echo_i "check that nsupdate does not hang when processing a large number of updates interactively ($n)" +{ + echo "server 10.53.0.3 ${PORT}" + echo "zone many-updates.test." + while [ $i -le 2000 ]; do + echo "update add host$i.many-updates.test. 3600 IN TXT \"host $i\"" + i=$((i + 1)) + done + echo "send" +} | $NSUPDATE +echo_i "query for host2000.many-updates.test ($n)" +retry_quiet 5 has_positive_response host2000.many-updates.test TXT 10.53.0.3 || ret=1 +[ $ret = 0 ] || { + echo_i "failed" + status=1 +} + n=$((n + 1)) ret=0 echo_i "start NSEC3PARAM changes via UPDATE on a unsigned zone test ($n)" From aa24b77d8ba9ba2c55b71f18f54e19f71a200491 Mon Sep 17 00:00:00 2001 From: Matthijs Mekking Date: Fri, 1 Nov 2024 13:25:26 +0100 Subject: [PATCH 2/2] Fix nsupdate hang when processing a large update The root cause is the fix for CVE-2024-0760 (part 3), which resets the TCP connection on a failed send. Specifically commit 4b7c61381f186e20a476c35032a871295ebbd385 stops reading on the socket because the TCP connection is throttling. When the tcpdns_send_cb callback thinks about restarting reading on the socket, this fails because the socket is a client socket. And nsupdate is a client and is using the same netmgr code. This commit removes the requirement that the socket must be a server socket, allowing reading on the socket again after being throttled. --- lib/isc/netmgr/tcp.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c index 1e21c6fe735..ca3ed8b7f4f 100644 --- a/lib/isc/netmgr/tcp.c +++ b/lib/isc/netmgr/tcp.c @@ -1208,12 +1208,17 @@ tcp_send_direct(isc_nmsocket_t *sock, isc__nm_uvreq_t *req) { } } - isc_log_write(ISC_LOGCATEGORY_GENERAL, ISC_LOGMODULE_NETMGR, - ISC_LOG_DEBUG(3), - "throttling TCP connection, the other side is not " - "reading the data, switching to uv_write()"); - sock->reading_throttled = true; - isc__nm_stop_reading(sock); + if (!sock->client && sock->reading) { + sock->reading_throttled = true; + isc__nm_stop_reading(sock); + } + + isc__nmsocket_log(sock, ISC_LOG_DEBUG(3), + "%sthe other side is not " + "reading the data, switching to uv_write()", + !sock->client && sock->reading + ? "throttling TCP connection, " + : ""); r = uv_write(&req->uv_req.write, &sock->uv_handle.stream, bufs, nbufs, tcp_send_cb);