Skip to content

fix: advance handshake stage only after the core accepts a packet - #30

Open
Woralem wants to merge 2 commits into
igmunv:devfrom
Woralem:fix/handshake-stage-before-validation
Open

fix: advance handshake stage only after the core accepts a packet#30
Woralem wants to merge 2 commits into
igmunv:devfrom
Woralem:fix/handshake-stage-before-validation

Conversation

@Woralem

@Woralem Woralem commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Fix: handshake deadlock on malformed service packets

Короче

Один некорректный служебный пакет, отправленный до честного собеседника, необратимо
вешал рукопожатие навсегда: поток инициализации оставался на Event.wait() без
таймаута. Проще говоря дедлок

Причина

На уровне приложения этап рукопожатия HANDSHAKE_STAGE переключался до того,
как ядро проверит содержимое пакета. Методы receive_node_id / receive_sign /
receive_public_key ничего не возвращали, поэтому уровень приложения не мог
отличить принятые данные от отброшенных

  • MY_NODE_ID: этап уходил в WAIT_SIGN до receive_node_id. Если node id не проходил
    check_node_id, корректный node id, пришедший следующим, отбрасывался уже
    проверкой этапа
  • MY_SIGN: этап уходил в SIGN_RECEIVED до receive_sign. Разбор мусорной точки
    кривой падал исключением (его глотал Base._pump), этап оставался сдвинутым,
    и корректная подпись уже не принималась
  • MY_PUBLIC_KEY: та же схема с переходом в READY

Соответствующий Event при этом не выставлялся, а ожидание было бессрочным, поэтому
init() вставал навсегда

Хде

  • src/levels/application.py - ветки MY_NODE_ID, MY_SIGN, MY_PUBLIC_KEY в
    handle_packet: присвоение HANDSHAKE_STAGE стояло перед вызовом receive_*
  • src/crypto_layer.py -COMPANION_NODE_ID_RECEIVED.wait(),
    COMPANION_SIGN_RECEIVED.wait(), COMPANION_PUBLIC_KEY_RECEIVED.wait() без таймаута
  • src/crypto_layer.py - receive_node_id / receive_sign / receive_public_key
    не сообщали результат наверх; разбор точки кривой не был изолирован

Что сделано

src/levels/application.py - этап только после подтверждения приёма

  • 126, 141, 158 - receive_node_id / receive_sign / receive_public_key
    вызываются до смены этапа, и этап меняется только при их успехе
    (if not ...: return)
  • 120-124 - полезная нагрузка MY_NODE_ID декодируется безопасно: невалидный UTF-8
    логируется и отбрасывается без смены этапа

src/crypto_layer.py - контролируемый разбор и конечное ожидание

  • 34 - EC_COMPRESSED_POINT_LENGTH = 33: ожидаемая длина точки SECP256R1 в сжатом
    формате X9.62. Обе стороны отправляют ключи только в этом виде
  • 104, 109 - таймауты шагов рукопожатия берутся из config.py:
    HANDSHAKE_TIMEOUT и отдельный HANDSHAKE_USER_CHECK_TIMEOUT
  • 133-149 - рукопожатие внутри init() обёрнуто в try/except: при сбое ошибка
    уходит в UI со статусом error, вызывается abort_init(), исключение
    пробрасывается вызывающему. on_ready() в этом случае не вызывается
  • 164 - новый abort_init(): пароль стирается из RAM, потоки уровней и модуля
    останавливаются (Base.stop_event, BaseModule.stop_event). DISCONNECT не
    отправляем - рукопожатие не состоялось и подписывать пакет нечем
  • 311, 334, 430-434 - три бесконечных ожидания заменены на wait_handshake_step.
    Шаг ECDH-ключа ждёт по HANDSHAKE_USER_CHECK_TIMEOUT, остальные - по
    HANDSHAKE_TIMEOUT
  • 491 - wait_handshake_step(event, step_name, timeout=None): ждёт шаг не дольше
    лимита, при истечении логирует и бросает TimeoutError с именем шага
  • 501 - receive_node_id(...) -> bool: при провале check_node_id возвращает False
  • 516 - receive_sign(...) -> bool: сначала проверка длины payload, затем разбор точки
    в try/except (ValueError, TypeError). При отказе Event не выставляется и
    COMPANION_SIGN не перезаписывается
  • 539 - receive_public_key(...) -> bool: то же самое для ECDH-ключа

src/config.py - лимиты рядом с остальными настройками

  • 24 - HANDSHAKE_TIMEOUT = 60: обычный шаг рукопожатия. На медленном модуле
    (редкий опрос мессенджера, повторы транспорта) значение стоит увеличить
  • 29 - HANDSHAKE_USER_CHECK_TIMEOUT = 900: шаг, который ждёт действия человека.
    Собеседник отправляет ECDH-ключ только после того, как вручную сверит отпечаток
    подписи по доверенному каналу, поэтому единый минутный лимит рвал бы честное
    рукопожатие

Изменение контракта init()

Раньше init() при неудачном рукопожатии не возвращался никогда. Теперь он может
бросить исключение (TimeoutError по таймауту шага, TypeError при отказе от
отпечатка). К этому моменту уровни и модуль уже остановлены, пароль стёрт, объект
повторному использованию не подлежит - нужен новый CryptoLayer. Вызывающему коду
(CLI, WebUI, и прочие) следует обработать исключение и показать ошибку
вместо бесконечного «Loading...»

Всех обнял, поцеловал <3

@igmunv

igmunv commented Aug 26, 2026

Copy link
Copy Markdown
Owner
  • 24 - HANDSHAKE_TIMEOUT = 60: обычный шаг рукопожатия. На медленном модуле
    (редкий опрос мессенджера, повторы транспорта) значение стоит увеличить

как его увеличивать пользователю или разработчику написавший модуль? может не стоит добавлять таймаут?
Либо сделать возможность модулю задавать таймаут

@Woralem

Woralem commented Aug 29, 2026

Copy link
Copy Markdown
Contributor Author
  • 24 - HANDSHAKE_TIMEOUT = 60: обычный шаг рукопожатия. На медленном модуле
    (редкий опрос мессенджера, повторы транспорта) значение стоит увеличить

как его увеличивать пользователю или разработчику написавший модуль? может не стоит добавлять таймаут? Либо сделать возможность модулю задавать таймаут

Убирать таймаут не стоит: бесконечный event.wait() - это и есть тот дедлок,
который мой PR лечит. Согласен с третьим вариантом: значение должен задавать
модуль, потому что только он знает свою частоту опроса; config остаётся
дефолтом и последним словом пользователя

Предлагаю условно так, что:

self.HANDSHAKE_TIMEOUT = (
    getattr(module_class, "handshake_timeout", None) or config.HANDSHAKE_TIMEOUT
)
  • Разработчик модуля выставляет handshake_timeout у своего класса, если опрос
    редкий (например, 300 для модуля, который читает мессенджер раз в минуту)
  • getattr с дефолтом - старые модули без этого атрибута продолжают работать,
    При желании потом добавим атрибут в интерфейс как опциональный
  • Пользователь может переопределить через config, если модуль ошибся с оценкой

HANDSHAKE_USER_CHECK_TIMEOUT = 900 оставляю отдельным и не связанным с
модулем: там мы ждём юзера, а не сеть

Про «(CLI, WebUI и прочие) обработать исключение» - согласен полностью.
wait_handshake_step бросает TimeoutError с именем шага, так что UI есть что
показать. Сделать это в этом же PR или отдельным - как скажете; если в этом,
скажите, какой слой UI считается точкой обработки, и я допишу.

Сорянчик еще что пару дней отсутствовал, мне будет довольно сложно заниматься параллельно этим проектом пусть мне он и нравиться, в основном из-за того что сейчас я нахожусь в другом городе на отдыхе с родными, и продолжать активную разработку здесь смогу только ближе к 2-3 сентября

@igmunv

igmunv commented Aug 30, 2026

Copy link
Copy Markdown
Owner

Предлагаю условно так, что:

self.HANDSHAKE_TIMEOUT = (
    getattr(module_class, "handshake_timeout", None) or config.HANDSHAKE_TIMEOUT
)

Да, давай. Еще надо в документацию про это написать

@Woralem

Woralem commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Сделал, 1f6d3c1 + новый коммит.

crypto_layer.py:104 - ровно тот вариант, что обсуждали:

self.HANDSHAKE_TIMEOUT = (
    getattr(module_class, "handshake_timeout", None) or config.HANDSHAKE_TIMEOUT
)

Документация - подраздел «9. Необязательное поле handshake_timeout» в конце
5.2, в docs/README.md и docs/README_en.md: пример с handshake_timeout = 300,
поведение по умолчанию, возможность пользователя переопределить через config и
почему шаг с ручной сверкой отпечатка идёт по отдельному лимиту. Нумерацию
соседних подразделов и оглавление не сдвигал.

Проверял на реальном CryptoLayer с двумя классами модулей: без атрибута - 60,
с handshake_timeout = 300 - 300, при config.HANDSHAKE_TIMEOUT = 45 модуль
со своим значением остаётся на 300, а модуль без атрибута берёт 45.
HANDSHAKE_USER_CHECK_TIMEOUT во всех случаях остаётся 900.
tests/test_pipeline.py - 6/6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants