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 066ad7ce0785fe7304eaadf903cfb685a8080d42..d90a8070add9f6781c5a7f58406974d04ef65c7e 100644 (file)
--- 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 05b1fc04a629c05c0e31b8aa1a5ddbaa7fe33a11..f339f184e1c93f809787f0a458ef36c876efac61 100644 (file)
--- 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));
                }
        }