Merge branch 'release-5.7.0' into 'master'

release 5.7.0

See merge request knot/knot-resolver!1448

AUTHORS

@@ -14,6 +14,7 @@ Ali Asad Lotia <[email protected]>
 Anbang Wen <[email protected]>
 Andreas Rammhold <[email protected]>
 Christophe Nowicki <[email protected]>
+Christopher Ng <[email protected]>
 cronfy <[email protected]>
 Daniel Kahn Gillmor <[email protected]>
 Daniel Salzman <[email protected]>

NEWS

@@ -1,6 +1,14 @@
-Knot Resolver 5.7.0 (2023-0m-dd)
+Knot Resolver 5.7.0 (2023-08-22)
 ================================
 
+Security
+--------
+- avoid excessive TCP reconnections in a few more cases (!NNNN)
+  Like before, the remote server had to behave nonsensically in order
+  to inflict this upon itself, but it might be abusable for DoS.
+
+  We thank Ivan Jedek from OryxLabs for reporting this.
+
 Improvements
 ------------
 - forwarding mode: tweak dealing with failures from forwarders,

daemon/io.c

@@ -337,33 +337,6 @@ void tcp_timeout_trigger(uv_timer_t *timer)
 	}
 }
 
-static void tcp_disconnect(struct session *s, int errcode)
-{
-	if (kr_log_is_debug(IO, NULL)) {
-		struct sockaddr *peer = session_get_peer(s);
-		char *peer_str = kr_straddr(peer);
-		kr_log_debug(IO, "=> connection to '%s' closed by peer (%s)\n",
-			       peer_str ? peer_str : "",
-			       uv_strerror(errcode));
-	}
-
-	if (!session_was_useful(s) && session_flags(s)->outgoing) {
-		/* We want to penalize the IP address, if a task is asking a query.
-		 * It might not be the right task, but that doesn't matter so much
-		 * for attributing the useless session to the IP address. */
-		struct qr_task *t = session_tasklist_get_first(s);
-		struct kr_query *qry = NULL;
-		if (t) {
-			struct kr_request *req = worker_task_request(t);
-			qry = array_tail(req->rplan.pending);
-		}
-		if (qry) /* We reuse the error for connection, as it's quite similar. */
-			qry->server_selection.error(qry, worker_task_get_transport(t),
-							KR_SELECTION_TCP_CONNECT_FAILED);
-	}
-	worker_end_tcp(s);
-}
-
 static void tcp_recv(uv_stream_t *handle, ssize_t nread, const uv_buf_t *buf)
 {
 	struct session *s = handle->data;
@@ -381,7 +354,16 @@ static void tcp_recv(uv_stream_t *handle, ssize_t nread, const uv_buf_t *buf)
 	}
 
 	if (nread < 0 || !buf->base) {
-		tcp_disconnect(s, nread);
+		if (kr_log_is_debug(IO, NULL)) {
+			struct sockaddr *peer = session_get_peer(s);
+			char *peer_str = kr_straddr(peer);
+			kr_log_debug(IO, "=> connection to '%s' closed by peer (%s)\n",
+				       peer_str ? peer_str : "",
+				       uv_strerror(nread));
+		}
+
+		session_tcp_penalize(s);
+		worker_end_tcp(s);
 		return;
 	}
 

daemon/session.c

@@ -123,11 +123,6 @@ void session_close(struct session *session)
 	}
 }
 
-bool session_was_useful(const struct session *session)
-{
-	return session->was_useful;
-}
-
 int session_start_read(struct session *session)
 {
 	return io_start_read(session->handle);
@@ -582,6 +577,24 @@ ssize_t session_wirebuf_trim(struct session *session, ssize_t len)
 	return len;
 }
 
+void session_tcp_penalize(struct session *s)
+{
+	if (s->was_useful || !s->sflags.outgoing)
+		return;
+	/* We want to penalize the IP address, if a task is asking a query.
+	 * It might not be the right task, but that doesn't matter so much
+	 * for attributing the useless session to the IP address. */
+	struct qr_task *t = session_tasklist_get_first(s);
+	struct kr_query *qry = NULL;
+	if (t) {
+		struct kr_request *req = worker_task_request(t);
+		qry = array_tail(req->rplan.pending);
+	}
+	if (qry) /* We reuse the error for connection, as it's quite similar. */
+		qry->server_selection.error(qry, worker_task_get_transport(t),
+						KR_SELECTION_TCP_CONNECT_FAILED);
+}
+
 knot_pkt_t *session_produce_packet(struct session *session, knot_mm_t *mm)
 {
 	session->sflags.wirebuf_error = false;
@@ -617,20 +630,23 @@ knot_pkt_t *session_produce_packet(struct session *session, knot_mm_t *mm)
 		msg_size = knot_wire_read_u16(msg_start);
 		if (msg_size >= session->wire_buf_size) {
 			session->sflags.wirebuf_error = true;
+			session_tcp_penalize(session);
 			return NULL;
 		}
 		if (msg_size + 2 > wirebuf_msg_data_size) {
 			return NULL;
 		}
 		if (msg_size == 0) {
 			session->sflags.wirebuf_error = true;
+			session_tcp_penalize(session);
 			return NULL;
 		}
 		msg_start += 2;
 	} else if (wirebuf_msg_data_size < UINT16_MAX) {
 		msg_size = wirebuf_msg_data_size;
 	} else {
 		session->sflags.wirebuf_error = true;
+		session_tcp_penalize(session);
 		return NULL;
 	}
 

daemon/session.h

@@ -91,8 +91,9 @@ int session_tasklist_finalize_expired(struct session *session);
 /** Both of task lists (associated & waiting). */
 /** Check if empty. */
 bool session_is_empty(const struct session *session);
-/** Return whether session seems to have done something useful. */
-bool session_was_useful(const struct session *session);
+/** Penalize this server if the session hasn't been useful (and is outgoing). */
+void session_tcp_penalize(struct session *session);
+
 /** Get pointer to session flags */
 struct session_flags *session_flags(struct session *session);
 /** Get pointer to peer address. */

meson.build

@@ -4,7 +4,7 @@ project(
   'knot-resolver',
   ['c', 'cpp'],
   license: 'GPLv3+',
-  version: '5.6.0',
+  version: '5.7.0',
   default_options: ['c_std=gnu11', 'b_ndebug=true'],
   meson_version: '>=0.49',
 )