From: Alberto Leiva Popper <ydahhrk@gmail.com>
Date: Tue, 24 Oct 2023 21:28:11 +0000 (-0600)
Subject: Validate Error Reports harder
X-Git-Tag: 1.6.0~37
X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=b94e49e78e216b3c9eae17e24723267d9337e98b;p=thirdparty%2FFORT-validator.git

Validate Error Reports harder

- Reject overly small Error Reports earlier and more clearly on the logs
- Compare Length of Encapsulated PDU to header length earlier

It seems both issues caused relevant malformed Error Reports to be
considered fragmentend rather than invalid.
---

diff --git a/src/rtr/pdu_stream.c b/src/rtr/pdu_stream.c
index 066ad7ce..d90a8070 100644
--- a/src/rtr/pdu_stream.c
+++ b/src/rtr/pdu_stream.c
@@ -366,7 +366,13 @@ load_error_report(struct pdu_stream *stream, struct pdu_header *hdr)
 
 	if (hdr->len > RTRPDU_ERROR_REPORT_MAX_LEN) {
 		return pr_op_err(
-			"RTR client %s sent a large Error Report PDU (%u bytes). This looks broken, so I'm dropping the connection.",
+			"%s: Error Report PDU is too big (%u bytes).",
+			stream->addr, hdr->len
+		);
+	}
+	if (hdr->len < RTR_HDR_LEN + 8) { /* hdr + errpdu len + errmsg len */
+		return pr_op_err(
+			"%s: Error Report PDU is too small (%u bytes).",
 			stream->addr, hdr->len
 		);
 	}
@@ -390,11 +396,18 @@ load_error_report(struct pdu_stream *stream, struct pdu_header *hdr)
 		 * have sent this PDU. Looks like someone is messing with us.
 		 */
 		error = pr_op_err(
-			"RTR client %s sent an Error Report PDU containing a large error PDU (%u bytes). This looks broken/insecure; I'm dropping the connection.",
+			"%s: Error Report PDU's embedded PDU is too big (%u bytes).",
 			stream->addr, errpdu_len
 		);
 		goto revert_errpdu_len;
 	}
+	if (hdr->len < RTR_HDR_LEN + 8 + errpdu_len) {
+		error = pr_op_err(
+			"%s: Invalid Length of Encapsulated PDU (%u); PDU length is %u.",
+			stream->addr, errpdu_len, hdr->len
+		);
+		goto revert_errpdu_len;
+	}
 
 	/* Error PDU */
 	if (get_length(stream) < errpdu_len) {
@@ -415,7 +428,7 @@ load_error_report(struct pdu_stream *stream, struct pdu_header *hdr)
 	stream->start += 4;
 	if (hdr->len != rtrpdu_error_report_len(errpdu_len, errmsg_len)) {
 		error = pr_op_err(
-			"RTR client %s sent a malformed Error Report PDU; header length is %u, but effective length is %u + %u + %u + %u + %u.",
+			"%s: Error Report PDU is malformed; header length is %u, but effective length is %u + %u + %u + %u + %u.",
 			stream->addr, hdr->len,
 			RTR_HDR_LEN, 4, errpdu_len, 4, errmsg_len
 		);
diff --git a/src/rtr/rtr.c b/src/rtr/rtr.c
index 05b1fc04..f339f184 100644
--- a/src/rtr/rtr.c
+++ b/src/rtr/rtr.c
@@ -457,6 +457,8 @@ print_poll_failure(struct pollfd *pfd, char const *what, char const *addr)
 		pr_op_err("%s '%s' down: POLLERR (Generic error)", what, addr);
 	if (pfd->revents & POLLNVAL)
 		pr_op_err("%s '%s' down: POLLNVAL (fd not open)", what, addr);
+	if (!(pfd->revents & (POLLHUP | POLLERR | POLLNVAL)))
+		pr_op_info("%s '%s' down.", what, addr);
 }
 
 static void
@@ -490,9 +492,9 @@ apply_pollfds(struct pollfd *pollfds, unsigned int nclients)
 		/* PR_DEBUG_MSG("pfd:%d server:%d", pfd->fd, server->fd); */
 
 		if ((pfd->fd == -1) && (server->fd != -1)) {
+			print_poll_failure(pfd, "Server", server->addr);
 			close(server->fd);
 			server->fd = -1;
-			print_poll_failure(pfd, "Server", server->addr);
 		}
 	}
 
@@ -503,9 +505,9 @@ apply_pollfds(struct pollfd *pollfds, unsigned int nclients)
 		/* PR_DEBUG_MSG("pfd:%d client:%d", pfd->fd, client->fd); */
 
 		if ((pfd->fd == -1) && (pdustream_fd(client) != -1)) {
+			print_poll_failure(pfd, "Client", pdustream_addr(client));
 			pdustream_destroy(&client);
 			clients.array[i] = NULL;
-			print_poll_failure(pfd, "Client", pdustream_addr(client));
 		}
 	}