fix: advance handshake stage only after the core accepts a packet - #30
Open
Woralem wants to merge 1 commit into
Open
fix: advance handshake stage only after the core accepts a packet#30Woralem 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: handshake deadlock on malformed service packets
Короче
Один некорректный служебный пакет, отправленный до честного собеседника, необратимо
вешал рукопожатие навсегда: поток инициализации оставался на
Event.wait()безтаймаута. Проще говоря дедлок
Причина
На уровне приложения этап рукопожатия
HANDSHAKE_STAGEпереключался до того,как ядро проверит содержимое пакета. Методы
receive_node_id/receive_sign/receive_public_keyничего не возвращали, поэтому уровень приложения не моготличить принятые данные от отброшенных
WAIT_SIGNдоreceive_node_id. Если node id не проходилcheck_node_id, корректный node id, пришедший следующим, отбрасывался ужепроверкой этапа
SIGN_RECEIVEDдоreceive_sign. Разбор мусорной точкикривой падал исключением (его глотал
Base._pump), этап оставался сдвинутым,и корректная подпись уже не принималась
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_TIMEOUT133-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_TIMEOUT491-wait_handshake_step(event, step_name, timeout=None): ждёт шаг не дольшелимита, при истечении логирует и бросает
TimeoutErrorс именем шага501-receive_node_id(...) -> bool: при провалеcheck_node_idвозвращаетFalse516-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