fix: bound and validate stream reassembly on the transport level - #31
Open
Woralem wants to merge 1 commit into
Open
fix: bound and validate stream reassembly on the transport level#31Woralem wants to merge 1 commit into
Woralem wants to merge 1 commit into
Conversation
Owner
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

fix: bound and validate stream reassembly on the transport level
Связано с: refs #18 (пункт про транспортный уровень)
Кратко
Транспорт лежит ниже проверки подписи, поэтому он обрабатывает пакеты от кого угодно, ещё до того как станет понятно, кто их прислал. Сейчас он верит заголовку пакета на слово: таблица незавершённых потоков не ограничена и никогда не чистится,
chunk_count/chunk_id/sizeне проверяются, а окно по времени проверяется только в одну сторону. Одиночного UDP-пакета достаточно, чтобы навсегда занять память узлаPR закрывает три из четырёх транспортных дефектов из #18: границы, валидация заголовка и симметричное окно времени. Аутентификация служебных пакетов (пинги, ACK) в этот PR не входит. Я могу его сделать здесь же, но там придётся менять формат кадра
Причина
WAITING_STREAMS[stream_id]создаётся по первому же чанку и удаляется только когда поток собран полностью. Если прислать один чанк из ста и замолчать, запись останется в памяти до перезапуска процесса. Ни таймаута, ни лимита на количество потоков нет, аstream_id- один байт, то есть 256 записей можно занять 256 пакетами.chunk_count- 2 байта, то есть до 65535 чанков,sizeтоже приходит из пакета.chunk_count == 0навсегда оставляет поток недособранным,chunk_id >= chunk_countпишет мусор в словарь, а большойsizeдаёт неограниченный рост памяти на одинstream_id. Плюс ACK отправляется до проверок, то есть узел ещё и подтверждает мусорdifference_seconds >= 300отбрасывает старые пакеты, но пакет с временем из будущего даёт отрицательную разницу и проходит всегда. Такой пакет можно переиспользовать бесконечно (replay-окно ограничено толькоSEEN_PACKETS_WINDOW)Что сделано
Все лимиты - константы класса, их видно в одном месте и можно переопределить в тестах
STREAM_TIMEOUTACK_TIMEOUT * ACK_RETRIES(30s)MAX_WAITING_STREAMS8MAX_STREAM_CHUNKS4096chunk_countиз заголовкаMAX_STREAM_BYTES1 MiBPACKET_MAX_AGE300rworkerCLOCK_SKEW_TOLERANCE60Валидация заголовка -
valid_stream_header(packet)Вызывается в
rworkerдоsend_acknowledgment, чтобы не подтверждать заведомо битый пакет. Отбрасываетchunk_count == 0,chunk_count > MAX_STREAM_CHUNKS,chunk_id >= chunk_countиsize > MAX_STREAM_BYTES, каждый случай пишется в логИстечение потоков -
expire_waiting_streams()У каждого потока есть
deadlineнаtime.monotonic(). Дедлайн продлевается на каждом новом чанке, просроченные потоки выбрасываются перед обработкой очередного пакета.Учёт потоков -
stream_for(packet)chunk_countне совпал с уже известным для этогоstream_id- старая запись считается устаревшей и выбрасывается (переиспользованиеstream_idбольше не склеивает два разных сообщения);count,packets,bytes,deadline.Бюджет по байтам
stream["bytes"]растёт только на новыхchunk_id, поэтому честные повторы (retransmit) бюджет не тратят. При превышенииMAX_STREAM_BYTESчанк отбрасывается.Симметричное окно времени
difference_seconds >= PACKET_MAX_AGE or difference_seconds < -CLOCK_SKEW_TOLERANCE- теперь пакет из будущего дальше 60s тоже отбрасывается.Проверка
Новый файл
tests/test_transport.py- 7 тестов, только stdlib +levels.*, без сети и без внешних зависимостей, в том же стиле, чтоtests/test_pipeline.py:Cуществующий
tests/test_pipeline.pyна этой же ветке:Сборка потоков, повторы при 30% потерь и переиспользование
stream_idработают как раньше. Лимиты специально выставлены с запасом относительно рабочих сценариев: 14 KiB сообщение приCHUNK_SIZE = 100- это ~140 чанков против лимита 4096, а отправитель шлёт один поток за раз, так чтоMAX_WAITING_STREAMS = 8не мешает нормальному трафикуШо осталось с #18
MAC, а на пинг узел отвечает ещё до
READY- то есть до того, как известно,с кем он вообще говорит. Фикс меняет формат кадра и задевает и транспорт, и
рукопожатие, поэтому не хочу тащить его сюда молча: давайте сначала обсудим
подход в Предупреждение о безопасности: критические уязвимости в текущей версии протокола v1.6.3 #18, а потом я уже сделаю это отдельным PR