]> git.ipfire.org Git - thirdparty/FORT-validator.git/commit
RTR server maintenance
authorAlberto Leiva Popper <ydahhrk@gmail.com>
Fri, 20 Oct 2023 22:18:39 +0000 (16:18 -0600)
committerAlberto Leiva Popper <ydahhrk@gmail.com>
Sat, 21 Oct 2023 02:56:25 +0000 (20:56 -0600)
commiteea8e4b4ed18019efeed72315a43c4b41f4fd0db
tree9ff25afe7750c88e11cb6fd3eca10ad32f668dc1
parent6125d017f3a480bb5b6f007de3f6b8819b7cb2ce
RTR server maintenance

Someone reported a security vulnerability in the server, but the details
are muddy, and clarifications have not arrived yet. I haven't been able
to reproduce it, but the review did yield room for improvement:

1. Buffer request bytes better

The old code seemed to assume each socket read consumed exactly one
(nonempty) TCP packet, and each such packet contained exactly one PDU.

I'm scratching my head at this, but I guess for most intents and
purposes, this assumption is not as lunatic as it seems. Benign RTR PDUs
are very small, and it doesn't make sense for a request packet to
contain multiple of them. Error Reports aside, it doesn't even make
sense for the client to send multiple PDUs in quick succession at all.

Regardless, I'm flushing that assumption down the toilet:

- If read() yields multiple PDUs, queue and handle them in sequence.

Although as I'm writing this I'm realizing that queuing PDUs is a dumb
idea, because Serial Queries and Reset Queries are alternate means to
achieve the same goal. If the client sent a new request, it's most
likely given up on the old one. Plus, queuing PDUs brings additional
complexity and risks. I'm going to have to change this in the next
commit.

- If a read() yields a fragmented PDU, buffer and prepend it to the next
  successful read.

This will probably never happen, but it's nice to handle it properly
anyway.

2. Drop unused PDU parsers

An RTR server only needs to handle PDU types Serial Query, Reset Query
and Error Report. Fort also had dead code meant for the other PDU types.

I'm guessing they were intended for the Error Report internal PDU field,
but it turns out that's also unused.

3. Improve PDU validation

Since Serial Queries and Reset Queries are supposed to have constant
length, Fort was often ignoring the PDU header length field.

Fort now punishes incorrect lengths more aggressively.
26 files changed:
src/Makefile.am
src/notify.c
src/rtr/err_pdu.c
src/rtr/err_pdu.h
src/rtr/pdu.c
src/rtr/pdu.h
src/rtr/pdu_handler.c
src/rtr/pdu_handler.h
src/rtr/pdu_sender.c
src/rtr/pdu_sender.h
src/rtr/pdu_serializer.c [deleted file]
src/rtr/pdu_serializer.h [deleted file]
src/rtr/pdu_stream.c [new file with mode: 0644]
src/rtr/pdu_stream.h [new file with mode: 0644]
src/rtr/primitive_reader.c [deleted file]
src/rtr/primitive_reader.h [deleted file]
src/rtr/primitive_writer.c
src/rtr/primitive_writer.h
src/rtr/rtr.c
src/rtr/rtr.h
test/Makefile.am
test/rtr/db/rtr_db_mock.c
test/rtr/pdu_handler_test.c
test/rtr/pdu_stream_test.c [new file with mode: 0644]
test/rtr/pdu_test.c [deleted file]
test/rtr/primitive_reader_test.c [deleted file]