diff options
| -rw-r--r-- | NEWS | 8 | ||||
| -rw-r--r-- | daemon/io.c | 38 | ||||
| -rw-r--r-- | daemon/session.c | 26 | ||||
| -rw-r--r-- | daemon/session.h | 5 |
4 files changed, 42 insertions, 35 deletions
@@ -1,6 +1,14 @@ Knot Resolver 5.7.0 (2023-0m-dd) ================================ +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, diff --git a/daemon/io.c b/daemon/io.c index 27119adb..6d548d77 100644 --- a/daemon/io.c +++ b/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; } diff --git a/daemon/session.c b/daemon/session.c index a1f22072..ed0ff686 100644 --- a/daemon/session.c +++ b/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,6 +630,7 @@ 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) { @@ -624,6 +638,7 @@ knot_pkt_t *session_produce_packet(struct session *session, knot_mm_t *mm) } if (msg_size == 0) { session->sflags.wirebuf_error = true; + session_tcp_penalize(session); return NULL; } msg_start += 2; @@ -631,6 +646,7 @@ knot_pkt_t *session_produce_packet(struct session *session, knot_mm_t *mm) msg_size = wirebuf_msg_data_size; } else { session->sflags.wirebuf_error = true; + session_tcp_penalize(session); return NULL; } diff --git a/daemon/session.h b/daemon/session.h index 603d7cb4..1f95ac5b 100644 --- a/daemon/session.h +++ b/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. */ |