Skip to content

Commit

Permalink
fix: usr: Fix nsupdate hang when processing a large update
Browse files Browse the repository at this point in the history
To mitigate DNS flood attacks over a single TCP connection, we throttle the connection when the other side does not read the data. Throttling should only occur on server-side sockets, but erroneously also happened for nsupdate, which acts as a client. When nsupdate started throttling the connection, it never attempts to read again. This has been fixed.
 
Closes #4910

Merge branch '4910-nsupdate-hangs-when-processing-large-update' into 'main'

See merge request isc-projects/bind9!9709
  • Loading branch information
matje committed Dec 5, 2024
2 parents 80a5745 + aa24b77 commit fa56e0d
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 6 deletions.
14 changes: 14 additions & 0 deletions bin/tests/system/nsupdate/ns3/many-updates.test.db
Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions bin/tests/system/nsupdate/ns3/named.conf.in
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
20 changes: 20 additions & 0 deletions bin/tests/system/nsupdate/tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand Down
17 changes: 11 additions & 6 deletions lib/isc/netmgr/tcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit fa56e0d

Please sign in to comment.