packages/bind
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix CVEs

Browse Source
This commit is contained in:
chengyechun 2024-08-02 14:13:37 +08:00
parent 34c65e95b9
commit 9a7d2022b6
6 changed files with 2977 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

981
backport-CVE-2024-0760.patch Normal file
View File

@ -0,0 +1,981 @@
From c33b3d26f695d342af3fa81ab404a366bb8ce873 Mon Sep 17 00:00:00 2001
From: Artem Boldariev <artem@boldariev.com>
Date: Wed, 3 Jul 2024 13:58:32 +0300
Subject: [PATCH] TCP/TLS DNS: unthrottle only when all input data processing
This commit ensures that we restart reading only when all DNS data in
the input buffer is processed so the we will not get into the
situation when the buffer is overrun.
Conflict:NA
Reference:https://downloads.isc.org/isc/bind9/9.18.28/patches/0001-CVE-2024-0760.patch
---
lib/isc/netmgr/netmgr-int.h | 27 +++++--
lib/isc/netmgr/netmgr.c | 79 ++++++++++++++----
lib/isc/netmgr/tcp.c | 71 +++++++++++++++-
lib/isc/netmgr/tcpdns.c | 59 +++++++++++++-
lib/isc/netmgr/tlsdns.c | 120 ++++++++++++++++++++-------
lib/ns/client.c | 156 +++++++++++++++++-------------------
lib/ns/include/ns/client.h | 6 +-
7 files changed, 379 insertions(+), 139 deletions(-)
diff --git a/lib/isc/netmgr/netmgr-int.h b/lib/isc/netmgr/netmgr-int.h
index 6aca9ab..bc1ba73 100644
--- a/lib/isc/netmgr/netmgr-int.h
+++ b/lib/isc/netmgr/netmgr-int.h
@@ -62,9 +62,10 @@
#endif
/*
- * The TCP receive buffer can fit one maximum sized DNS message plus its size,
- * the receive buffer here affects TCP, DoT and DoH.
+ * The TCP send and receive buffers can fit one maximum sized DNS message plus
+ * its size, the receive buffer here affects TCP, DoT and DoH.
*/
+#define ISC_NETMGR_TCP_SENDBUF_SIZE (sizeof(uint16_t) + UINT16_MAX)
#define ISC_NETMGR_TCP_RECVBUF_SIZE (sizeof(uint16_t) + UINT16_MAX)
/* Pick the larger buffer */
@@ -377,9 +378,10 @@ struct isc__nm_uvreq {
int magic;
isc_nmsocket_t *sock;
isc_nmhandle_t *handle;
- char tcplen[2]; /* The TCP DNS message length */
- uv_buf_t uvbuf; /* translated isc_region_t, to be
- * sent or received */
+ char tcplen[2]; /* The TCP DNS message length */
+ uv_buf_t uvbuf; /* translated isc_region_t, to be
+ * sent or received */
+ isc_region_t userbuf;
isc_sockaddr_t local; /* local address */
isc_sockaddr_t peer; /* peer address */
isc__nm_cb_t cb; /* callback */
@@ -998,7 +1000,6 @@ struct isc_nmsocket {
TLS_STATE_ERROR,
TLS_STATE_CLOSING
} state;
- isc_region_t senddata;
ISC_LIST(isc__nm_uvreq_t) sendreqs;
bool cycle;
isc_result_t pending_error;
@@ -1063,6 +1064,12 @@ struct isc_nmsocket {
*/
uint64_t write_timeout;
+ /*
+ * Reading was throttled over TCP as the peer does not read the
+ * data we are sending back.
+ */
+ bool reading_throttled;
+
/*% outer socket is for 'wrapped' sockets - e.g. tcpdns in tcp */
isc_nmsocket_t *outer;
@@ -2265,6 +2272,14 @@ isc__nmsocket_readtimeout_cb(uv_timer_t *timer);
void
isc__nmsocket_writetimeout_cb(void *data, isc_result_t eresult);
+/*%<
+ *
+ * Maximum number of simultaneous handles in flight supported for a single
+ * connected TCPDNS socket. This value was chosen arbitrarily, and may be
+ * changed in the future.
+ */
+#define STREAM_CLIENTS_PER_CONN 23
+
#define UV_RUNTIME_CHECK(func, ret) \
if (ret != 0) { \
FATAL_ERROR("%s failed: %s\n", #func, uv_strerror(ret)); \
diff --git a/lib/isc/netmgr/netmgr.c b/lib/isc/netmgr/netmgr.c
index 2310b4b..f9e3b70 100644
--- a/lib/isc/netmgr/netmgr.c
+++ b/lib/isc/netmgr/netmgr.c
@@ -49,8 +49,15 @@
* How many isc_nmhandles and isc_nm_uvreqs will we be
* caching for reuse in a socket.
*/
-#define ISC_NM_HANDLES_STACK_SIZE 600
-#define ISC_NM_REQS_STACK_SIZE 600
+#define ISC_NM_HANDLES_STACK_SIZE 16
+#define ISC_NM_REQS_STACK_SIZE 16
+
+/*%
+ * Same, but for UDP sockets which tend to need larger values as they
+ * process many requests per socket.
+ */
+#define ISC_NM_HANDLES_STACK_SIZE_UDP 64
+#define ISC_NM_REQS_STACK_SIZE_UDP 64
/*%
* Shortcut index arrays to get access to statistics counters.
@@ -1506,16 +1513,25 @@ void
isc___nmsocket_init(isc_nmsocket_t *sock, isc_nm_t *mgr, isc_nmsocket_type type,
isc_sockaddr_t *iface FLARG) {
uint16_t family;
+ size_t inactive_handles_stack_size = ISC_NM_HANDLES_STACK_SIZE;
+ size_t inactive_reqs_stack_size = ISC_NM_REQS_STACK_SIZE;
REQUIRE(sock != NULL);
REQUIRE(mgr != NULL);
- *sock = (isc_nmsocket_t){ .type = type,
- .fd = -1,
- .inactivehandles = isc_astack_new(
- mgr->mctx, ISC_NM_HANDLES_STACK_SIZE),
- .inactivereqs = isc_astack_new(
- mgr->mctx, ISC_NM_REQS_STACK_SIZE) };
+ if (type == isc_nm_udpsocket) {
+ inactive_handles_stack_size = ISC_NM_HANDLES_STACK_SIZE_UDP;
+ inactive_reqs_stack_size = ISC_NM_REQS_STACK_SIZE_UDP;
+ }
+
+ *sock = (isc_nmsocket_t){
+ .type = type,
+ .fd = -1,
+ .inactivehandles = isc_astack_new(mgr->mctx,
+ inactive_handles_stack_size),
+ .inactivereqs = isc_astack_new(mgr->mctx,
+ inactive_reqs_stack_size)
+ };
ISC_LIST_INIT(sock->tls.sendreqs);
@@ -2084,6 +2100,7 @@ isc__nmsocket_writetimeout_cb(void *data, isc_result_t eresult) {
sock = req->sock;
+ isc__nm_start_reading(sock);
isc__nmsocket_reset(sock);
}
@@ -2093,7 +2110,6 @@ isc__nmsocket_readtimeout_cb(uv_timer_t *timer) {
REQUIRE(VALID_NMSOCK(sock));
REQUIRE(sock->tid == isc_nm_tid());
- REQUIRE(atomic_load(&sock->reading));
if (atomic_load(&sock->client)) {
uv_timer_stop(timer);
@@ -2340,8 +2356,10 @@ processbuffer(isc_nmsocket_t *sock) {
* timers. If we do have a full message, reset the timer.
*
* Stop reading if this is a client socket, or if the server socket
- * has been set to sequential mode. In this case we'll be called again
- * later by isc__nm_resume_processing().
+ * has been set to sequential mode, or the number of queries we are
+ * processing simultaneously has reached the clients-per-connection
+ * limit. In this case we'll be called again later by
+ * isc__nm_resume_processing().
*/
isc_result_t
isc__nm_process_sock_buffer(isc_nmsocket_t *sock) {
@@ -2349,14 +2367,41 @@ isc__nm_process_sock_buffer(isc_nmsocket_t *sock) {
int_fast32_t ah = atomic_load(&sock->ah);
isc_result_t result = processbuffer(sock);
switch (result) {
- case ISC_R_NOMORE:
+ case ISC_R_NOMORE: {
/*
* Don't reset the timer until we have a
* full DNS message.
*/
- result = isc__nm_start_reading(sock);
- if (result != ISC_R_SUCCESS) {
- return (result);
+
+ /*
+ * Restart reading if we have less data in the send
+ * queue than the send buffer size, this means that the
+ * TCP client has started reading some data again.
+ * Starting reading when we go under the limit instead
+ * of waiting for all data has been flushed allows
+ * faster recovery (in case there was a congestion and
+ * now there isn't).
+ */
+ size_t write_queue_size =
+ uv_stream_get_write_queue_size(
+ &sock->uv_handle.stream);
+ if (write_queue_size < ISC_NETMGR_TCP_SENDBUF_SIZE) {
+ if (sock->reading_throttled) {
+ isc_log_write(isc_lctx,
+ ISC_LOGCATEGORY_GENERAL,
+ ISC_LOGMODULE_NETMGR,
+ ISC_LOG_DEBUG(3),
+ "resuming TCP "
+ "connection, the other "
+ "side is reading the "
+ "data again (%zu)",
+ write_queue_size);
+ sock->reading_throttled = false;
+ }
+ result = isc__nm_start_reading(sock);
+ if (result != ISC_R_SUCCESS) {
+ return (result);
+ }
}
/*
* Start the timer only if there are no externally used
@@ -2368,6 +2413,7 @@ isc__nm_process_sock_buffer(isc_nmsocket_t *sock) {
isc__nmsocket_timer_start(sock);
}
goto done;
+ }
case ISC_R_CANCELED:
isc__nmsocket_timer_stop(sock);
isc__nm_stop_reading(sock);
@@ -2381,7 +2427,8 @@ isc__nm_process_sock_buffer(isc_nmsocket_t *sock) {
isc__nmsocket_timer_stop(sock);
if (atomic_load(&sock->client) ||
- atomic_load(&sock->sequential))
+ atomic_load(&sock->sequential) ||
+ atomic_load(&sock->ah) >= STREAM_CLIENTS_PER_CONN)
{
isc__nm_stop_reading(sock);
goto done;
diff --git a/lib/isc/netmgr/tcp.c b/lib/isc/netmgr/tcp.c
index 16b53cc..37d44bd 100644
--- a/lib/isc/netmgr/tcp.c
+++ b/lib/isc/netmgr/tcp.c
@@ -766,7 +766,7 @@ isc__nm_async_tcpstartread(isc__networker_t *worker, isc__netievent_t *ev0) {
isc__netievent_tcpstartread_t *ievent =
(isc__netievent_tcpstartread_t *)ev0;
isc_nmsocket_t *sock = ievent->sock;
- isc_result_t result;
+ isc_result_t result = ISC_R_SUCCESS;
REQUIRE(VALID_NMSOCK(sock));
REQUIRE(sock->tid == isc_nm_tid());
@@ -774,7 +774,7 @@ isc__nm_async_tcpstartread(isc__networker_t *worker, isc__netievent_t *ev0) {
if (isc__nmsocket_closing(sock)) {
result = ISC_R_CANCELED;
- } else {
+ } else if (!sock->reading_throttled) {
result = isc__nm_start_reading(sock);
}
@@ -905,6 +905,32 @@ isc__nm_tcp_read_cb(uv_stream_t *stream, ssize_t nread, const uv_buf_t *buf) {
/* The readcb could have paused the reading */
if (atomic_load(&sock->reading)) {
+ if (!sock->client) {
+ /*
+ * Stop reading if we have accumulated enough bytes in
+ * the send queue; this means that the TCP client is not
+ * reading back the data we sending to it, and there's
+ * no reason to continue processing more incoming DNS
+ * messages, if the client is not reading back the
+ * responses.
+ */
+ size_t write_queue_size =
+ uv_stream_get_write_queue_size(
+ &sock->uv_handle.stream);
+
+ if (write_queue_size >= ISC_NETMGR_TCP_SENDBUF_SIZE) {
+ isc_log_write(isc_lctx, ISC_LOGCATEGORY_GENERAL,
+ ISC_LOGMODULE_NETMGR,
+ ISC_LOG_DEBUG(3),
+ "throttling TCP connection, "
+ "the other side is "
+ "not reading the data (%zu)",
+ write_queue_size);
+ sock->reading_throttled = true;
+ isc__nm_stop_reading(sock);
+ }
+ }
+
/* The timer will be updated */
isc__nmsocket_timer_restart(sock);
}
@@ -1095,6 +1121,34 @@ isc__nm_tcp_send(isc_nmhandle_t *handle, const isc_region_t *region,
return;
}
+static void
+tcp_maybe_restart_reading(isc_nmsocket_t *sock) {
+ if (!sock->client && sock->reading_throttled &&
+ !uv_is_active(&sock->uv_handle.handle))
+ {
+ /*
+ * Restart reading if we have less data in the send queue than
+ * the send buffer size, this means that the TCP client has
+ * started reading some data again. Starting reading when we go
+ * under the limit instead of waiting for all data has been
+ * flushed allows faster recovery (in case there was a
+ * congestion and now there isn't).
+ */
+ size_t write_queue_size =
+ uv_stream_get_write_queue_size(&sock->uv_handle.stream);
+ if (write_queue_size < ISC_NETMGR_TCP_SENDBUF_SIZE) {
+ isc_log_write(
+ isc_lctx, ISC_LOGCATEGORY_GENERAL,
+ ISC_LOGMODULE_NETMGR, ISC_LOG_DEBUG(3),
+ "resuming TCP connection, the other side "
+ "is reading the data again (%zu)",
+ write_queue_size);
+ sock->reading_throttled = false;
+ isc__nm_start_reading(sock);
+ }
+ }
+}
+
static void
tcp_send_cb(uv_write_t *req, int status) {
isc__nm_uvreq_t *uvreq = (isc__nm_uvreq_t *)req->data;
@@ -1112,10 +1166,23 @@ tcp_send_cb(uv_write_t *req, int status) {
isc__nm_incstats(sock, STATID_SENDFAIL);
isc__nm_failed_send_cb(sock, uvreq,
isc__nm_uverr2result(status));
+
+ if (!sock->client &&
+ (atomic_load(&sock->reading) || sock->reading_throttled))
+ {
+ /*
+ * As we are resuming reading, it is not throttled
+ * anymore (technically).
+ */
+ sock->reading_throttled = false;
+ isc__nm_start_reading(sock);
+ isc__nmsocket_reset(sock);
+ }
return;
}
isc__nm_sendcb(sock, uvreq, ISC_R_SUCCESS, false);
+ tcp_maybe_restart_reading(sock);
}
/*
diff --git a/lib/isc/netmgr/tcpdns.c b/lib/isc/netmgr/tcpdns.c
index 46958d0..6d417f7 100644
--- a/lib/isc/netmgr/tcpdns.c
+++ b/lib/isc/netmgr/tcpdns.c
@@ -733,7 +733,7 @@ isc__nm_async_tcpdnsread(isc__networker_t *worker, isc__netievent_t *ev0) {
isc__netievent_tcpdnsread_t *ievent =
(isc__netievent_tcpdnsread_t *)ev0;
isc_nmsocket_t *sock = ievent->sock;
- isc_result_t result;
+ isc_result_t result = ISC_R_SUCCESS;
UNUSED(worker);
@@ -742,7 +742,7 @@ isc__nm_async_tcpdnsread(isc__networker_t *worker, isc__netievent_t *ev0) {
if (isc__nmsocket_closing(sock)) {
result = ISC_R_CANCELED;
- } else {
+ } else if (!sock->reading_throttled) {
result = isc__nm_process_sock_buffer(sock);
}
@@ -905,6 +905,28 @@ isc__nm_tcpdns_read_cb(uv_stream_t *stream, ssize_t nread,
result = isc__nm_process_sock_buffer(sock);
if (result != ISC_R_SUCCESS) {
isc__nm_failed_read_cb(sock, result, true);
+ } else if (!sock->client) {
+ /*
+ * Stop reading if we have accumulated enough bytes in
+ * the send queue; this means that the TCP client is not
+ * reading back the data we sending to it, and there's
+ * no reason to continue processing more incoming DNS
+ * messages, if the client is not reading back the
+ * responses.
+ */
+ size_t write_queue_size =
+ uv_stream_get_write_queue_size(&sock->uv_handle.stream);
+
+ if (write_queue_size >= ISC_NETMGR_TCP_SENDBUF_SIZE) {
+ isc_log_write(isc_lctx, ISC_LOGCATEGORY_GENERAL,
+ ISC_LOGMODULE_NETMGR, ISC_LOG_DEBUG(3),
+ "throttling TCP connection, "
+ "the other side is "
+ "not reading the data (%zu)",
+ write_queue_size);
+ sock->reading_throttled = true;
+ isc__nm_stop_reading(sock);
+ }
}
free:
if (nread < 0) {
@@ -1125,6 +1147,19 @@ isc__nm_tcpdns_send(isc_nmhandle_t *handle, isc_region_t *region,
return;
}
+static void
+tcpdns_maybe_restart_reading(isc_nmsocket_t *sock) {
+ if (!sock->client && sock->reading_throttled &&
+ !uv_is_active(&sock->uv_handle.handle))
+ {
+ isc_result_t result = isc__nm_process_sock_buffer(sock);
+ if (result != ISC_R_SUCCESS) {
+ atomic_store(&sock->reading, true);
+ isc__nm_failed_read_cb(sock, result, false);
+ }
+ }
+}
+
static void
tcpdns_send_cb(uv_write_t *req, int status) {
isc__nm_uvreq_t *uvreq = (isc__nm_uvreq_t *)req->data;
@@ -1142,10 +1177,23 @@ tcpdns_send_cb(uv_write_t *req, int status) {
isc__nm_incstats(sock, STATID_SENDFAIL);
isc__nm_failed_send_cb(sock, uvreq,
isc__nm_uverr2result(status));
+
+ if (!sock->client &&
+ (atomic_load(&sock->reading) || sock->reading_throttled))
+ {
+ /*
+ * As we are resuming reading, it is not throttled
+ * anymore (technically).
+ */
+ sock->reading_throttled = false;
+ isc__nm_start_reading(sock);
+ isc__nmsocket_reset(sock);
+ }
return;
}
isc__nm_sendcb(sock, uvreq, ISC_R_SUCCESS, false);
+ tcpdns_maybe_restart_reading(sock);
}
/*
@@ -1211,6 +1259,13 @@ isc__nm_async_tcpdnssend(isc__networker_t *worker, isc__netievent_t *ev0) {
goto fail;
}
+ isc_log_write(isc_lctx, 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);
+
r = uv_write(&uvreq->uv_req.write, &sock->uv_handle.stream, bufs, nbufs,
tcpdns_send_cb);
if (r < 0) {
diff --git a/lib/isc/netmgr/tlsdns.c b/lib/isc/netmgr/tlsdns.c
index 40e6fc8..f62dfd4 100644
--- a/lib/isc/netmgr/tlsdns.c
+++ b/lib/isc/netmgr/tlsdns.c
@@ -88,6 +88,9 @@ tlsdns_set_tls_shutdown(isc_tls_t *tls) {
(void)SSL_set_shutdown(tls, SSL_SENT_SHUTDOWN);
}
+static void
+tlsdns_maybe_restart_reading(isc_nmsocket_t *sock);
+
static bool
peer_verification_has_failed(isc_nmsocket_t *sock) {
if (sock->tls.tls != NULL && sock->tls.state == TLS_STATE_HANDSHAKE &&
@@ -1076,6 +1079,19 @@ tls_cycle_input(isc_nmsocket_t *sock) {
size_t len;
for (;;) {
+ /*
+ * There is a similar branch in
+ * isc__nm_process_sock_buffer() which is sufficient to
+ * stop excessive processing in TCP. However, as we wrap
+ * this call in a loop, we need to have it here in order
+ * to limit the number of loop iterations (and,
+ * consequently, the number of messages processed).
+ */
+ if (atomic_load(&sock->ah) >= STREAM_CLIENTS_PER_CONN) {
+ isc__nm_stop_reading(sock);
+ break;
+ }
+
(void)SSL_peek(sock->tls.tls, &(char){ '\0' }, 0);
int pending = SSL_pending(sock->tls.tls);
@@ -1253,17 +1269,17 @@ call_pending_send_callbacks(isc_nmsocket_t *sock, const isc_result_t result) {
}
static void
-free_senddata(isc_nmsocket_t *sock, const isc_result_t result) {
+free_senddata(isc_nmsocket_t *sock, isc__nm_uvreq_t *req,
+ const isc_result_t result) {
REQUIRE(VALID_NMSOCK(sock));
- REQUIRE(sock->tls.senddata.base != NULL);
- REQUIRE(sock->tls.senddata.length > 0);
+ REQUIRE(req != NULL && req->userbuf.base != NULL &&
+ req->userbuf.length > 0);
- isc_mem_put(sock->mgr->mctx, sock->tls.senddata.base,
- sock->tls.senddata.length);
- sock->tls.senddata.base = NULL;
- sock->tls.senddata.length = 0;
+ isc_mem_put(sock->mgr->mctx, req->userbuf.base, req->userbuf.length);
call_pending_send_callbacks(sock, result);
+
+ isc__nm_uvreq_put(&req, sock);
}
static void
@@ -1276,11 +1292,19 @@ tls_write_cb(uv_write_t *req, int status) {
isc_nm_timer_stop(uvreq->timer);
isc_nm_timer_detach(&uvreq->timer);
- free_senddata(sock, result);
-
- isc__nm_uvreq_put(&uvreq, sock);
+ free_senddata(sock, uvreq, result);
if (status != 0) {
+ if (!sock->client &&
+ (atomic_load(&sock->reading) || sock->reading_throttled))
+ {
+ /*
+ * As we are resuming reading, it is not throttled
+ * anymore (technically).
+ */
+ sock->reading_throttled = false;
+ isc__nm_start_reading(sock);
+ }
tls_error(sock, result);
return;
}
@@ -1290,6 +1314,8 @@ tls_write_cb(uv_write_t *req, int status) {
tls_error(sock, result);
return;
}
+
+ tlsdns_maybe_restart_reading(sock);
}
static isc_result_t
@@ -1303,23 +1329,18 @@ tls_cycle_output(isc_nmsocket_t *sock) {
int rv;
int r;
- if (sock->tls.senddata.base != NULL ||
- sock->tls.senddata.length > 0)
- {
- break;
- }
-
if (pending > (int)ISC_NETMGR_TCP_RECVBUF_SIZE) {
pending = (int)ISC_NETMGR_TCP_RECVBUF_SIZE;
}
- sock->tls.senddata.base = isc_mem_get(sock->mgr->mctx, pending);
- sock->tls.senddata.length = pending;
-
/* It's a bit misnomer here, but it does the right thing */
req = isc__nm_get_read_req(sock, NULL);
- req->uvbuf.base = (char *)sock->tls.senddata.base;
- req->uvbuf.len = sock->tls.senddata.length;
+
+ req->userbuf.base = isc_mem_get(sock->mgr->mctx, pending);
+ req->userbuf.length = (size_t)pending;
+
+ req->uvbuf.base = (char *)req->userbuf.base;
+ req->uvbuf.len = (size_t)req->userbuf.length;
rv = BIO_read_ex(sock->tls.app_rbio, req->uvbuf.base,
req->uvbuf.len, &bytes);
@@ -1331,32 +1352,36 @@ tls_cycle_output(isc_nmsocket_t *sock) {
if (r == pending) {
/* Wrote everything, restart */
- isc__nm_uvreq_put(&req, sock);
- free_senddata(sock, ISC_R_SUCCESS);
+ free_senddata(sock, req, ISC_R_SUCCESS);
continue;
}
if (r > 0) {
/* Partial write, send rest asynchronously */
- memmove(req->uvbuf.base, req->uvbuf.base + r,
- req->uvbuf.len - r);
- req->uvbuf.len = req->uvbuf.len - r;
+ req->uvbuf.base += r;
+ req->uvbuf.len -= r;
} else if (r == UV_ENOSYS || r == UV_EAGAIN) {
/* uv_try_write is not supported, send
* asynchronously */
} else {
result = isc__nm_uverr2result(r);
- isc__nm_uvreq_put(&req, sock);
- free_senddata(sock, result);
+ free_senddata(sock, req, result);
break;
}
+ isc_log_write(
+ isc_lctx, 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);
+
r = uv_write(&req->uv_req.write, &sock->uv_handle.stream,
&req->uvbuf, 1, tls_write_cb);
if (r < 0) {
result = isc__nm_uverr2result(r);
- isc__nm_uvreq_put(&req, sock);
- free_senddata(sock, result);
+ free_senddata(sock, req, result);
break;
}
@@ -1525,6 +1550,28 @@ isc__nm_tlsdns_read_cb(uv_stream_t *stream, ssize_t nread,
result = tls_cycle(sock);
if (result != ISC_R_SUCCESS) {
isc__nm_failed_read_cb(sock, result, true);
+ } else if (!sock->client) {
+ /*
+ * Stop reading if we have accumulated enough bytes in
+ * the send queue; this means that the TCP client is not
+ * reading back the data we sending to it, and there's
+ * no reason to continue processing more incoming DNS
+ * messages, if the client is not reading back the
+ * responses.
+ */
+ size_t write_queue_size =
+ uv_stream_get_write_queue_size(&sock->uv_handle.stream);
+
+ if (write_queue_size >= ISC_NETMGR_TCP_SENDBUF_SIZE) {
+ isc_log_write(isc_lctx, ISC_LOGCATEGORY_GENERAL,
+ ISC_LOGMODULE_NETMGR, ISC_LOG_DEBUG(3),
+ "throttling TCP connection, "
+ "the other side is "
+ "not reading the data (%zu)",
+ write_queue_size);
+ sock->reading_throttled = true;
+ isc__nm_stop_reading(sock);
+ }
}
free:
async_tlsdns_cycle(sock);
@@ -1766,6 +1813,19 @@ isc__nm_tlsdns_send(isc_nmhandle_t *handle, isc_region_t *region,
return;
}
+static void
+tlsdns_maybe_restart_reading(isc_nmsocket_t *sock) {
+ if (!sock->client && sock->reading_throttled &&
+ !uv_is_active(&sock->uv_handle.handle))
+ {
+ isc_result_t result = isc__nm_process_sock_buffer(sock);
+ if (result != ISC_R_SUCCESS) {
+ atomic_store(&sock->reading, true);
+ isc__nm_failed_read_cb(sock, result, false);
+ }
+ }
+}
+
/*
* Handle 'tcpsend' async event - send a packet on the socket
*/
diff --git a/lib/ns/client.c b/lib/ns/client.c
index a62343b..8981222 100644
--- a/lib/ns/client.c
+++ b/lib/ns/client.c
@@ -101,6 +101,9 @@
#define COOKIE_SIZE 24U /* 8 + 4 + 4 + 8 */
#define ECS_SIZE 20U /* 2 + 1 + 1 + [0..16] */
+#define TCPBUFFERS_FILLCOUNT 1U
+#define TCPBUFFERS_FREEMAX 8U
+
#define WANTNSID(x) (((x)->attributes & NS_CLIENTATTR_WANTNSID) != 0)
#define WANTEXPIRE(x) (((x)->attributes & NS_CLIENTATTR_WANTEXPIRE) != 0)
#define WANTPAD(x) (((x)->attributes & NS_CLIENTATTR_WANTPAD) != 0)
@@ -330,12 +333,36 @@ client_senddone(isc_nmhandle_t *handle, isc_result_t result, void *cbarg) {
NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(3),
"send failed: %s",
isc_result_totext(result));
+ isc_nm_bad_request(handle);
}
}
isc_nmhandle_detach(&handle);
}
+static void
+client_setup_tcp_buffer(ns_client_t *client) {
+ REQUIRE(client->tcpbuf == NULL);
+
+ client->tcpbuf = client->manager->tcp_buffer;
+ client->tcpbuf_size = NS_CLIENT_TCP_BUFFER_SIZE;
+}
+
+static void
+client_put_tcp_buffer(ns_client_t *client) {
+ if (client->tcpbuf == NULL) {
+ return;
+ }
+
+ if (client->tcpbuf != client->manager->tcp_buffer) {
+ isc_mem_put(client->manager->mctx, client->tcpbuf,
+ client->tcpbuf_size);
+ }
+
+ client->tcpbuf = NULL;
+ client->tcpbuf_size = 0;
+}
+
static void
client_allocsendbuf(ns_client_t *client, isc_buffer_t *buffer,
unsigned char **datap) {
@@ -345,12 +372,9 @@ client_allocsendbuf(ns_client_t *client, isc_buffer_t *buffer,
REQUIRE(datap != NULL);
if (TCP_CLIENT(client)) {
- INSIST(client->tcpbuf == NULL);
- client->tcpbuf = isc_mem_get(client->manager->send_mctx,
- NS_CLIENT_TCP_BUFFER_SIZE);
- client->tcpbuf_size = NS_CLIENT_TCP_BUFFER_SIZE;
+ client_setup_tcp_buffer(client);
data = client->tcpbuf;
- isc_buffer_init(buffer, data, NS_CLIENT_TCP_BUFFER_SIZE);
+ isc_buffer_init(buffer, data, client->tcpbuf_size);
} else {
data = client->sendbuf;
if ((client->attributes & NS_CLIENTATTR_HAVECOOKIE) == 0) {
@@ -383,11 +407,49 @@ client_sendpkg(ns_client_t *client, isc_buffer_t *buffer) {
if (isc_buffer_base(buffer) == client->tcpbuf) {
size_t used = isc_buffer_usedlength(buffer);
- client->tcpbuf = isc_mem_reget(client->manager->send_mctx,
- client->tcpbuf,
- client->tcpbuf_size, used);
- client->tcpbuf_size = used;
- r.base = client->tcpbuf;
+ INSIST(client->tcpbuf_size == NS_CLIENT_TCP_BUFFER_SIZE);
+
+ /*
+ * Copy the data into a smaller buffer before sending,
+ * and keep the original big TCP send buffer for reuse
+ * by other clients.
+ */
+ if (used > NS_CLIENT_SEND_BUFFER_SIZE) {
+ /*
+ * We can save space by allocating a new buffer with a
+ * correct size and freeing the big buffer.
+ */
+ unsigned char *new_tcpbuf =
+ isc_mem_get(client->manager->mctx, used);
+ memmove(new_tcpbuf, buffer->base, used);
+
+ /*
+ * Put the big buffer so we can replace the pointer
+ * and the size with the new ones.
+ */
+ client_put_tcp_buffer(client);
+
+ /*
+ * Keep the new buffer's information so it can be freed.
+ */
+ client->tcpbuf = new_tcpbuf;
+ client->tcpbuf_size = used;
+
+ r.base = new_tcpbuf;
+ } else {
+ /*
+ * The data fits in the available space in
+ * 'sendbuf', there is no need for a new buffer.
+ */
+ memmove(client->sendbuf, buffer->base, used);
+
+ /*
+ * Put the big buffer, we don't need a dynamic buffer.
+ */
+ client_put_tcp_buffer(client);
+
+ r.base = client->sendbuf;
+ }
r.length = used;
} else {
isc_buffer_usedregion(buffer, &r);
@@ -461,8 +523,7 @@ ns_client_sendraw(ns_client_t *client, dns_message_t *message) {
return;
done:
if (client->tcpbuf != NULL) {
- isc_mem_put(client->manager->send_mctx, client->tcpbuf,
- client->tcpbuf_size);
+ client_put_tcp_buffer(client);
}
ns_client_drop(client, result);
@@ -746,8 +807,7 @@ renderend:
cleanup:
if (client->tcpbuf != NULL) {
- isc_mem_put(client->manager->send_mctx, client->tcpbuf,
- client->tcpbuf_size);
+ client_put_tcp_buffer(client);
}
if (cleanup_cctx) {
@@ -1629,8 +1689,7 @@ ns__client_reset_cb(void *client0) {
ns_client_endrequest(client);
if (client->tcpbuf != NULL) {
- isc_mem_put(client->manager->send_mctx, client->tcpbuf,
- client->tcpbuf_size);
+ client_put_tcp_buffer(client);
}
if (client->keytag != NULL) {
@@ -1661,8 +1720,6 @@ ns__client_put_cb(void *client0) {
client->magic = 0;
client->shuttingdown = true;
- isc_mem_put(client->manager->send_mctx, client->sendbuf,
- NS_CLIENT_SEND_BUFFER_SIZE);
if (client->opt != NULL) {
INSIST(dns_rdataset_isassociated(client->opt));
dns_rdataset_disassociate(client->opt);
@@ -2339,8 +2396,6 @@ ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) {
dns_message_create(client->mctx, DNS_MESSAGE_INTENTPARSE,
&client->message);
- client->sendbuf = isc_mem_get(client->manager->send_mctx,
- NS_CLIENT_SEND_BUFFER_SIZE);
/*
* Set magic earlier than usual because ns_query_init()
* and the functions it calls will require it.
@@ -2357,7 +2412,6 @@ ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) {
ns_clientmgr_t *oldmgr = client->manager;
ns_server_t *sctx = client->sctx;
isc_task_t *task = client->task;
- unsigned char *sendbuf = client->sendbuf;
dns_message_t *message = client->message;
isc_mem_t *oldmctx = client->mctx;
ns_query_t query = client->query;
@@ -2372,7 +2426,6 @@ ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) {
.manager = oldmgr,
.sctx = sctx,
.task = task,
- .sendbuf = sendbuf,
.message = message,
.query = query,
.tid = tid };
@@ -2397,8 +2450,6 @@ ns__client_setup(ns_client_t *client, ns_clientmgr_t *mgr, bool new) {
return (ISC_R_SUCCESS);
cleanup:
- isc_mem_put(client->manager->send_mctx, client->sendbuf,
- NS_CLIENT_SEND_BUFFER_SIZE);
dns_message_detach(&client->message);
isc_task_detach(&client->task);
ns_clientmgr_detach(&client->manager);
@@ -2461,8 +2512,6 @@ clientmgr_destroy(ns_clientmgr_t *manager) {
isc_task_detach(&manager->task);
ns_server_detach(&manager->sctx);
- isc_mem_detach(&manager->send_mctx);
-
isc_mem_putanddetach(&manager->mctx, manager, sizeof(*manager));
}
@@ -2499,61 +2548,6 @@ ns_clientmgr_create(ns_server_t *sctx, isc_taskmgr_t *taskmgr,
ISC_LIST_INIT(manager->recursing);
- /*
- * We create specialised per-worker memory context specifically
- * dedicated and tuned for allocating send buffers as it is a very
- * common operation. Not doing so may result in excessive memory
- * use in certain workloads.
- *
- * Please see this thread for more details:
- *
- * https://github.com/jemalloc/jemalloc/issues/2483
- *
- * In particular, this information from the jemalloc developers is
- * of the most interest:
- *
- * https://github.com/jemalloc/jemalloc/issues/2483#issuecomment-1639019699
- * https://github.com/jemalloc/jemalloc/issues/2483#issuecomment-1698173849
- *
- * In essence, we use the following memory management strategy:
- *
- * 1. We use a per-worker memory arena for send buffers memory
- * allocation to reduce lock contention (In reality, we create a
- * per-client manager arena, but we have one client manager per
- * worker).
- *
- * 2. The automatically created arenas settings remain unchanged
- * and may be controlled by users (e.g. by setting the
- * "MALLOC_CONF" variable).
- *
- * 3. We attune the arenas to not use dirty pages cache as the
- * cache would have a poor reuse rate, and that is known to
- * significantly contribute to excessive memory use.
- *
- * 4. There is no strict need for the dirty cache, as there is a
- * per arena bin for each allocation size, so because we initially
- * allocate strictly 64K per send buffer (enough for a DNS
- * message), allocations would get directed to one bin (an "object
- * pool" or a "slab") maintained within an arena. That is, there
- * is an object pool already, specifically to optimise for the
- * case of frequent allocations of objects of the given size. The
- * object pool should suffice our needs, as we will end up
- * recycling the objects from there without the need to back it by
- * an additional layer of dirty pages cache. The dirty pages cache
- * would have worked better in the case when there are more
- * allocation bins involved due to a higher reuse rate (the case
- * of a more "generic" memory management).
- */
- isc_mem_create_arena(&manager->send_mctx);
- isc_mem_setname(manager->send_mctx, "sendbufs");
- (void)isc_mem_arena_set_dirty_decay_ms(manager->send_mctx, 0);
- /*
- * Disable muzzy pages cache too, as versions < 5.2.0 have it
- * enabled by default. The muzzy pages cache goes right below the
- * dirty pages cache and backs it.
- */
- (void)isc_mem_arena_set_muzzy_decay_ms(manager->send_mctx, 0);
-
manager->magic = MANAGER_MAGIC;
MTRACE("create");
diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h
index 7a7196f..ea2d83e 100644
--- a/lib/ns/include/ns/client.h
+++ b/lib/ns/include/ns/client.h
@@ -144,7 +144,6 @@ struct ns_clientmgr {
unsigned int magic;
isc_mem_t *mctx;
- isc_mem_t *send_mctx;
ns_server_t *sctx;
isc_taskmgr_t *taskmgr;
isc_timermgr_t *timermgr;
@@ -159,6 +158,8 @@ struct ns_clientmgr {
/* Lock covers the recursing list */
isc_mutex_t reclock;
client_list_t recursing; /*%< Recursing clients */
+
+ uint8_t tcp_buffer[NS_CLIENT_TCP_BUFFER_SIZE];
};
/*% nameserver client structure */
@@ -187,7 +188,6 @@ struct ns_client {
unsigned char *tcpbuf;
size_t tcpbuf_size;
dns_message_t *message;
- unsigned char *sendbuf;
dns_rdataset_t *opt;
dns_ednsopt_t *ede;
uint16_t udpsize;
@@ -240,6 +240,8 @@ struct ns_client {
* bits will be used as the rcode in the response message.
*/
int32_t rcode_override;
+
+ uint8_t sendbuf[NS_CLIENT_SEND_BUFFER_SIZE];
};
#define NS_CLIENT_MAGIC ISC_MAGIC('N', 'S', 'C', 'c')
--
2.33.0

1500
backport-CVE-2024-1737.patch Normal file
View File

File diff suppressed because it is too large Load Diff

352
backport-CVE-2024-1975.patch Normal file
View File

@ -0,0 +1,352 @@
From bef3d2cca3552100bbe44790c8c1a4f5bef06798 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20=C5=A0pa=C4=8Dek?= <pspacek@isc.org>
Date: Thu, 16 May 2024 12:10:41 +0200
Subject: [PATCH] Remove support for SIG(0) message verification
Conflict:Case adaptation
Reference:https://downloads.isc.org/isc/bind9/9.18.28/patches/0003-CVE-2024-1975.patch
---
bin/tests/system/tsiggss/authsock.pl | 5 ++
bin/tests/system/tsiggss/tests.sh | 12 ++--
bin/tests/system/upforwd/tests.sh | 9 ++-
doc/arm/general.rst | 6 +-
doc/arm/intro-security.inc.rst | 2 +-
doc/arm/reference.rst | 4 +-
doc/arm/security.inc.rst | 4 +-
doc/arm/sig0.inc.rst | 16 +----
lib/dns/message.c | 99 ++--------------------------
lib/ns/client.c | 7 ++
10 files changed, 40 insertions(+), 124 deletions(-)
diff --git a/bin/tests/system/tsiggss/authsock.pl b/bin/tests/system/tsiggss/authsock.pl
index 4c76bf8..972252a 100644
--- a/bin/tests/system/tsiggss/authsock.pl
+++ b/bin/tests/system/tsiggss/authsock.pl
@@ -33,6 +33,10 @@ if (!defined($path)) {
exit(1);
}
+# Enable output autoflush so that it's not lost when the parent sends TERM.
+select STDOUT;
+$| = 1;
+
unlink($path);
my $server = IO::Socket::UNIX->new(Local => $path, Type => SOCK_STREAM, Listen => 8) or
die "unable to create socket $path";
@@ -50,6 +54,7 @@ if ($timeout != 0) {
}
while (my $client = $server->accept()) {
+ printf("accept()\n");
$client->recv(my $buf, 8, 0);
my ($version, $req_len) = unpack('N N', $buf);
diff --git a/bin/tests/system/tsiggss/tests.sh b/bin/tests/system/tsiggss/tests.sh
index c37f32e..004ad83 100644
--- a/bin/tests/system/tsiggss/tests.sh
+++ b/bin/tests/system/tsiggss/tests.sh
@@ -117,7 +117,7 @@ status=$((status + ret))
echo_i "testing external update policy (CNAME) with auth sock ($n)"
ret=0
-$PERL ./authsock.pl --type=CNAME --path=ns1/auth.sock --pidfile=authsock.pid --timeout=120 >/dev/null 2>&1 &
+$PERL ./authsock.pl --type=CNAME --path=ns1/auth.sock --pidfile=authsock.pid --timeout=120 >authsock.log 2>&1 &
sleep 1
test_update $n testcname.example.nil. CNAME "86400 CNAME testdenied.example.nil" "testdenied" || ret=1
n=$((n + 1))
@@ -131,17 +131,19 @@ n=$((n + 1))
if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
status=$((status + ret))
-echo_i "testing external policy with SIG(0) key ($n)"
+echo_i "testing external policy with unsupported SIG(0) key ($n)"
ret=0
-$NSUPDATE -k ns1/Kkey.example.nil.*.private <<END >/dev/null 2>&1 || ret=1
+$NSUPDATE -d -k ns1/Kkey.example.nil.*.private <<END >nsupdate.out${n} 2>&1 || true
+debug
server 10.53.0.1 ${PORT}
zone example.nil
update add fred.example.nil 120 cname foo.bar.
send
END
output=$($DIG $DIGOPTS +short cname fred.example.nil.)
-[ -n "$output" ] || ret=1
-[ $ret -eq 0 ] || echo_i "failed"
+# update must have failed - SIG(0) signer is not supported
+[ -n "$output" ] && ret=1
+grep -F "signer=key.example.nil" authsock.log >/dev/null && ret=1
n=$((n + 1))
if [ "$ret" -ne 0 ]; then echo_i "failed"; fi
status=$((status + ret))
diff --git a/bin/tests/system/upforwd/tests.sh b/bin/tests/system/upforwd/tests.sh
index 518eac6..d231d0f 100644
--- a/bin/tests/system/upforwd/tests.sh
+++ b/bin/tests/system/upforwd/tests.sh
@@ -229,10 +229,12 @@ fi
n=$((n + 1))
if test -f keyname; then
- echo_i "checking update forwarding to with sig0 ($n)"
+ echo_i "checking update forwarding to with sig0 (expected to fail) ($n)"
ret=0
keyname=$(cat keyname)
- $NSUPDATE -k $keyname.private -- - <<EOF
+ # SIG(0) is removed, update is expected to fail.
+ {
+ $NSUPDATE -k $keyname.private -- - <<EOF
local 10.53.0.1
server 10.53.0.3 ${PORT}
zone example2
@@ -240,8 +242,9 @@ if test -f keyname; then
update add unsigned.example2. 600 TXT Foo
send
EOF
+ } >nsupdate.out.$n 2>&1 && ret=1
$DIG -p ${PORT} unsigned.example2 A @10.53.0.1 >dig.out.ns1.test$n
- grep "status: NOERROR" dig.out.ns1.test$n >/dev/null || ret=1
+ grep "status: NOERROR" dig.out.ns1.test$n >/dev/null && ret=1
if [ $ret != 0 ]; then echo_i "failed"; fi
status=$((status + ret))
n=$((n + 1))
diff --git a/doc/arm/general.rst b/doc/arm/general.rst
index 5b65f6a..35f74b3 100644
--- a/doc/arm/general.rst
+++ b/doc/arm/general.rst
@@ -379,10 +379,8 @@ Notes
.. [#rfc1035_2] CLASS ANY queries are not supported. This is considered a
feature.
-.. [#rfc2931] When receiving a query signed with a SIG(0), the server is
- only able to verify the signature if it has the key in its local
- authoritative data; it cannot do recursion or validation to
- retrieve unknown keys.
+.. [#rfc2931] Support for SIG(0) message verification was removed
+ as part of the mitigation of CVE-2024-1975.
.. [#rfc2874] Compliance is with loading and serving of A6 records only.
A6 records were moved to the experimental category by :rfc:`3363`.
diff --git a/doc/arm/intro-security.inc.rst b/doc/arm/intro-security.inc.rst
index 87db970..996e910 100644
--- a/doc/arm/intro-security.inc.rst
+++ b/doc/arm/intro-security.inc.rst
@@ -47,7 +47,7 @@ or ports come preconfigured with local (loopback address) security preconfigured
If ``rndc`` is being invoked from a remote host, further configuration is required.
The ``nsupdate`` tool uses **Dynamic DNS (DDNS)** features and allows users to dynamically
change the contents of the zone file(s). ``nsupdate`` access and security may be controlled
-using ``named.conf`` :ref:`statements or using TSIG or SIG(0) cryptographic methods <dynamic_update_security>`.
+using ``named.conf`` :ref:`statements or via the TSIG cryptographic method <dynamic_update_security>`.
Clearly, if the remote hosts used for either ``rndc`` or DDNS lie within a network entirely
under the user's control, the security threat may be regarded as non-existent. Any implementation requirements,
therefore, depend on the site's security policy.
diff --git a/doc/arm/reference.rst b/doc/arm/reference.rst
index 29e246b..157ab30 100644
--- a/doc/arm/reference.rst
+++ b/doc/arm/reference.rst
@@ -7417,7 +7417,7 @@ the zone's filename, unless :any:`inline-signing` is enabled.
updates are allowed. It specifies a set of rules, in which each rule
either grants or denies permission for one or more names in the zone to
be updated by one or more identities. Identity is determined by the key
- that signed the update request, using either TSIG or SIG(0). In most
+ that signed the update request, using TSIG. In most
cases, :any:`update-policy` rules only apply to key-based identities. There
is no way to specify update permissions based on the client source address.
@@ -7474,7 +7474,7 @@ the zone's filename, unless :any:`inline-signing` is enabled.
field. Details for each rule type are described below.
The ``identity`` field must be set to a fully qualified domain name. In
- most cases, this represents the name of the TSIG or SIG(0) key that
+ most cases, this represents the name of the TSIG key that
must be used to sign the update request. If the specified name is a
wildcard, it is subject to DNS wildcard expansion, and the rule may
apply to multiple identities. When a TKEY exchange has been used to
diff --git a/doc/arm/security.inc.rst b/doc/arm/security.inc.rst
index 878fa37..8fc65d3 100644
--- a/doc/arm/security.inc.rst
+++ b/doc/arm/security.inc.rst
@@ -85,7 +85,7 @@ Limiting access to the server by outside parties can help prevent
spoofing and denial of service (DoS) attacks against the server.
ACLs match clients on the basis of up to three characteristics: 1) The
-client's IP address; 2) the TSIG or SIG(0) key that was used to sign the
+client's IP address; 2) the TSIG key that was used to sign the
request, if any; and 3) an address prefix encoded in an EDNS
Client-Subnet option, if any.
@@ -126,7 +126,7 @@ and no queries at all from the networks specified in ``bogusnets``.
In addition to network addresses and prefixes, which are matched against
the source address of the DNS request, ACLs may include ``key``
-elements, which specify the name of a TSIG or SIG(0) key.
+elements, which specify the name of a TSIG key.
When BIND 9 is built with GeoIP support, ACLs can also be used for
geographic access restrictions. This is done by specifying an ACL
diff --git a/doc/arm/sig0.inc.rst b/doc/arm/sig0.inc.rst
index 048dbea..6e6fc32 100644
--- a/doc/arm/sig0.inc.rst
+++ b/doc/arm/sig0.inc.rst
@@ -12,17 +12,5 @@
SIG(0)
------
-BIND partially supports DNSSEC SIG(0) transaction signatures as
-specified in :rfc:`2535` and :rfc:`2931`. SIG(0) uses public/private keys to
-authenticate messages. Access control is performed in the same manner as with
-TSIG keys; privileges can be granted or denied in ACL directives based
-on the key name.
-
-When a SIG(0) signed message is received, it is only verified if
-the key is known and trusted by the server. The server does not attempt
-to recursively fetch or validate the key.
-
-SIG(0) signing of multiple-message TCP streams is not supported.
-
-The only tool shipped with BIND 9 that generates SIG(0) signed messages
-is :iscman:`nsupdate`.
+Support for DNSSEC SIG(0) transaction signatures has been removed.
+This is a countermeasure for CVE-2024-1975.
diff --git a/lib/dns/message.c b/lib/dns/message.c
index 8654e92..a379125 100644
--- a/lib/dns/message.c
+++ b/lib/dns/message.c
@@ -3288,111 +3288,24 @@ dns_message_dumpsig(dns_message_t *msg, char *txt1) {
isc_result_t
dns_message_checksig(dns_message_t *msg, dns_view_t *view) {
- isc_buffer_t b, msgb;
+ isc_buffer_t msgb;
REQUIRE(DNS_MESSAGE_VALID(msg));
- if (msg->tsigkey == NULL && msg->tsig == NULL && msg->sig0 == NULL) {
+ if (msg->tsigkey == NULL && msg->tsig == NULL) {
return (ISC_R_SUCCESS);
}
INSIST(msg->saved.base != NULL);
isc_buffer_init(&msgb, msg->saved.base, msg->saved.length);
isc_buffer_add(&msgb, msg->saved.length);
- if (msg->tsigkey != NULL || msg->tsig != NULL) {
#ifdef SKAN_MSG_DEBUG
- dns_message_dumpsig(msg, "dns_message_checksig#1");
+ dns_message_dumpsig(msg, "dns_message_checksig#1");
#endif /* ifdef SKAN_MSG_DEBUG */
- if (view != NULL) {
- return (dns_view_checksig(view, &msgb, msg));
- } else {
- return (dns_tsig_verify(&msgb, msg, NULL, NULL));
- }
+ if (view != NULL) {
+ return (dns_view_checksig(view, &msgb, msg));
} else {
- dns_rdata_t rdata = DNS_RDATA_INIT;
- dns_rdata_sig_t sig;
- dns_rdataset_t keyset;
- isc_result_t result;
-
- result = dns_rdataset_first(msg->sig0);
- INSIST(result == ISC_R_SUCCESS);
- dns_rdataset_current(msg->sig0, &rdata);
-
- /*
- * This can occur when the message is a dynamic update, since
- * the rdata length checking is relaxed. This should not
- * happen in a well-formed message, since the SIG(0) is only
- * looked for in the additional section, and the dynamic update
- * meta-records are in the prerequisite and update sections.
- */
- if (rdata.length == 0) {
- return (ISC_R_UNEXPECTEDEND);
- }
-
- result = dns_rdata_tostruct(&rdata, &sig, NULL);
- if (result != ISC_R_SUCCESS) {
- return (result);
- }
-
- dns_rdataset_init(&keyset);
- if (view == NULL) {
- result = DNS_R_KEYUNAUTHORIZED;
- goto freesig;
- }
- result = dns_view_simplefind(view, &sig.signer,
- dns_rdatatype_key /* SIG(0) */, 0,
- 0, false, &keyset, NULL);
-
- if (result != ISC_R_SUCCESS) {
- /* XXXBEW Should possibly create a fetch here */
- result = DNS_R_KEYUNAUTHORIZED;
- goto freesig;
- } else if (keyset.trust < dns_trust_secure) {
- /* XXXBEW Should call a validator here */
- result = DNS_R_KEYUNAUTHORIZED;
- goto freesig;
- }
- result = dns_rdataset_first(&keyset);
- INSIST(result == ISC_R_SUCCESS);
- for (; result == ISC_R_SUCCESS;
- result = dns_rdataset_next(&keyset))
- {
- dst_key_t *key = NULL;
-
- dns_rdata_reset(&rdata);
- dns_rdataset_current(&keyset, &rdata);
- isc_buffer_init(&b, rdata.data, rdata.length);
- isc_buffer_add(&b, rdata.length);
-
- result = dst_key_fromdns(&sig.signer, rdata.rdclass, &b,
- view->mctx, &key);
- if (result != ISC_R_SUCCESS) {
- continue;
- }
- if (dst_key_alg(key) != sig.algorithm ||
- dst_key_id(key) != sig.keyid ||
- !(dst_key_proto(key) == DNS_KEYPROTO_DNSSEC ||
- dst_key_proto(key) == DNS_KEYPROTO_ANY))
- {
- dst_key_free(&key);
- continue;
- }
- result = dns_dnssec_verifymessage(&msgb, msg, key);
- dst_key_free(&key);
- if (result == ISC_R_SUCCESS) {
- break;
- }
- }
- if (result == ISC_R_NOMORE) {
- result = DNS_R_KEYUNAUTHORIZED;
- }
-
- freesig:
- if (dns_rdataset_isassociated(&keyset)) {
- dns_rdataset_disassociate(&keyset);
- }
- dns_rdata_freestruct(&sig);
- return (result);
+ return (dns_tsig_verify(&msgb, msg, NULL, NULL));
}
}
diff --git a/lib/ns/client.c b/lib/ns/client.c
index 8981222..5d2ad0b 100644
--- a/lib/ns/client.c
+++ b/lib/ns/client.c
@@ -2168,6 +2168,13 @@ ns__client_request(isc_nmhandle_t *handle, isc_result_t eresult,
ns_client_log(client, DNS_LOGCATEGORY_SECURITY,
NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(3),
"request is signed by a nonauthoritative key");
+ } else if (result == DNS_R_NOTVERIFIEDYET &&
+ client->message->sig0 != NULL)
+ {
+ ns_client_log(client, DNS_LOGCATEGORY_SECURITY,
+ NS_LOGMODULE_CLIENT, ISC_LOG_DEBUG(3),
+ "request has a SIG(0) signature but its support "
+ "was removed (CVE-2024-1975)");
} else {
char tsigrcode[64];
isc_buffer_t b;
--
2.33.0

34
backport-CVE-2024-4076.patch Normal file
View File

@ -0,0 +1,34 @@
From 9cfd20cd90fab4c97fe91f68555b7a2e05b808e8 Mon Sep 17 00:00:00 2001
From: Mark Andrews <marka@isc.org>
Date: Tue, 16 Jan 2024 14:25:27 +1100
Subject: [PATCH] Clear qctx->zversion
Clear qctx->zversion when clearing qctx->zrdataset et al in
lib/ns/query.c:qctx_freedata. The uncleared pointer could lead to
an assertion failure if zone data needed to be re-saved which could
happen with stale data support enabled.
(cherry picked from commit 179fb3532ab8d4898ab070b2db54c0ce872ef709)
Conflict:NA
Reference:https://downloads.isc.org/isc/bind9/9.18.28/patches/0004-CVE-2024-4076.patch
---
lib/ns/query.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/ns/query.c b/lib/ns/query.c
index 40e1232..7884514 100644
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -5323,6 +5323,7 @@ qctx_freedata(query_ctx_t *qctx) {
ns_client_releasename(qctx->client, &qctx->zfname);
dns_db_detachnode(qctx->zdb, &qctx->znode);
dns_db_detach(&qctx->zdb);
+ qctx->zversion = NULL;
}
if (qctx->event != NULL && !qctx->client->nodetach) {
--
2.33.0

98
backport-optimize-the-slabheader-placement-for-certain-RRtypes.patch Normal file
View File

@ -0,0 +1,98 @@
From 8ef414a7f38a04cfc11df44adaedaf3126fa3878 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Mon, 29 Jan 2024 16:36:30 +0100
Subject: [PATCH] Optimize the slabheader placement for certain RRTypes
Mark the infrastructure RRTypes as "priority" types and place them at
the beginning of the rdataslab header data graph. The non-priority
types either go right after the priority types (if any).
(cherry picked from commit 3ac482be7fd058d284e89873021339579fad0615)
Conflict:NA
Reference:https://gitlab.isc.org/isc-projects/bind9/-/commit/8ef414a7f38a04cfc11df44adaedaf3126fa3878
---
lib/dns/rbtdb.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 42 insertions(+), 2 deletions(-)
diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
index 7793be8..bc0f8d8 100644
--- a/lib/dns/rbtdb.c
+++ b/lib/dns/rbtdb.c
@@ -906,6 +906,30 @@ set_ttl(dns_rbtdb_t *rbtdb, rdatasetheader_t *header, dns_ttl_t newttl) {
}
}
+static bool
+prio_type(rbtdb_rdatatype_t type) {
+ switch (type) {
+ case dns_rdatatype_soa:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_soa):
+ case dns_rdatatype_a:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_a):
+ case dns_rdatatype_aaaa:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_aaaa):
+ case dns_rdatatype_nsec:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_nsec):
+ case dns_rdatatype_nsec3:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_nsec3):
+ case dns_rdatatype_ns:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_ns):
+ case dns_rdatatype_ds:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_ds):
+ case dns_rdatatype_cname:
+ case RBTDB_RDATATYPE_VALUE(dns_rdatatype_rrsig, dns_rdatatype_cname):
+ return (true);
+ }
+ return (false);
+}
+
/*%
* These functions allow the heap code to rank the priority of each
* element. It returns true if v1 happens "sooner" than v2.
@@ -6167,6 +6191,7 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, const dns_name_t *nodename,
rbtdb_changed_t *changed = NULL;
rdatasetheader_t *topheader = NULL, *topheader_prev = NULL;
rdatasetheader_t *header = NULL, *sigheader = NULL;
+ rdatasetheader_t *prioheader = NULL;
unsigned char *merged = NULL;
isc_result_t result;
bool header_nx;
@@ -6313,6 +6338,9 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, const dns_name_t *nodename,
for (topheader = rbtnode->data; topheader != NULL;
topheader = topheader->next)
{
+ if (prio_type(topheader->type)) {
+ prioheader = topheader;
+ }
if (topheader->type == newheader->type ||
topheader->type == negtype)
{
@@ -6679,9 +6707,21 @@ find_header:
/*
* No rdatasets of the given type exist at the node.
*/
- newheader->next = rbtnode->data;
newheader->down = NULL;
- rbtnode->data = newheader;
+
+ if (prio_type(newheader->type)) {
+ /* This is a priority type, prepend it */
+ newheader->next = rbtnode->data;
+ rbtnode->data = newheader;
+ } else if (prioheader != NULL) {
+ /* Append after the priority headers */
+ newheader->next = prioheader->next;
+ prioheader->next = newheader;
+ } else {
+ /* There were no priority headers */
+ newheader->next = rbtnode->data;
+ rbtnode->data = newheader;
+ }
}
}
--
2.33.0

13
bind.spec
View File

@ -29,7 +29,7 @@ Summary: The Berkeley Internet Name Domain (BIND) DNS (Domain Name System) serv
Name: bind Name: bind
License: MPLv2.0 License: MPLv2.0
Version: 9.18.21 Version: 9.18.21
Release: 2 Release: 3
Epoch: 32 Epoch: 32
Url: https://www.isc.org/downloads/bind/ Url: https://www.isc.org/downloads/bind/
# #
@ -64,6 +64,11 @@ Patch6000:backport-CVE-2023-4408.patch
Patch6001:backport-CVE-2023-5517.patch Patch6001:backport-CVE-2023-5517.patch
Patch6002:backport-CVE-2023-5679.patch Patch6002:backport-CVE-2023-5679.patch
Patch6003:backport-CVE-2023-50387-CVE-2023-50868.patch Patch6003:backport-CVE-2023-50387-CVE-2023-50868.patch
Patch6004:backport-CVE-2024-0760.patch
Patch6005:backport-optimize-the-slabheader-placement-for-certain-RRtypes.patch
Patch6006:backport-CVE-2024-1737.patch
Patch6007:backport-CVE-2024-1975.patch
Patch6008:backport-CVE-2024-4076.patch
# Common patches # Common patches
%{?systemd_ordering} %{?systemd_ordering}
@ -903,6 +908,12 @@ fi;
%endif %endif
%changelog %changelog
* Fri Aug 02 2024 chengyechun<chengyechun1@huawei.com> - 32:9.18.21-3
- Type:CVE
- CVE:CVE-2024-0760,CVE-2024-1737,CVE-2024-1975,CVE-2024-4076
- SUG:NA
- DESC:fix CVE-2024-0760,CVE-2024-1737,CVE-2024-1975,CVE-2024-4076
* Tue Mar 19 2024 chengyechun<chengyechun1@huawei.com> - 32:9.18.21-2 * Tue Mar 19 2024 chengyechun<chengyechun1@huawei.com> - 32:9.18.21-2
- Type:CVE - Type:CVE
- CVE:CVE-2023-4408 CVE-2023-5517 CVE-2023-5679 CVE-2023-50387 CVE-2023-50868 - CVE:CVE-2023-4408 CVE-2023-5517 CVE-2023-5679 CVE-2023-50387 CVE-2023-50868