From 1916ba4270f3a7a4ff25cabc591e130b9fddcf2e Mon Sep 17 00:00:00 2001 From: evgeny Date: Mon, 26 Oct 2026 19:23:46 +0100 Subject: [PATCH] Document admin key import failures and null transfer crash --- doc/chat_audit_findings.txt | 75 ++++++++++++++++++++++++++++ src/chat/chat_admin.h | 11 ++-- src/routing_layer/topo_node_sqlite.h | 4 +- 3 files changed, 86 insertions(+), 4 deletions(-) diff --git a/doc/chat_audit_findings.txt b/doc/chat_audit_findings.txt index 471d8e0c..54d31525 100644 --- a/doc/chat_audit_findings.txt +++ b/doc/chat_audit_findings.txt @@ -7,6 +7,8 @@ Chat: исправление приглашений и проверка конт выделения cm_invite_wrap. После согласования архитектуры исправлен весь путь команды JOIN; добавлены тесты контрактов, отказов и жизненного цикла. Полный аудит всех модулей src/chat не завершён. chat_sync.h остаётся [~]. +Продолжение26.10.2026 после ab8713d8:аудит chat_admin остановлен на двух новых дефектах; +доказательства и план исправления в конце отчёта. Runtime этого продолжения не менялся. Реализация @@ -126,3 +128,76 @@ instance_lite, старое объявление InviteDataC в JNI; затем зависимость db_sync от группы; требуемый порядок восстановлен, добавлена очистка новой группы при отказе истории/подписки. Это покрыто отрицательными сценариями. Исходные изменения аудита и итоговые правки не коммитились и не публиковались. + +Продолжение chat_admin — 26.10.2026, база ab8713d8 + +Прочитан полностью chat_admin.c/.h; сверены публичная передача/trampoline из chat_core.h, +регистрация/stop в chat_sync, compact delivery router и crypto policy, синхронные события, +SQL-get/set_privs и desktop/JNI callers. Последние прочитаны только в связанных участках. +chat_admin.h переведён в [~]; topo_node_sqlite.h снова [~] из-за ранее пропущенного SQL-дефекта. +Ранее закрытый блок member/Merkle/MESSAGES остаётся [V]. + +Подтверждённые дефекты + +1. Частичный импорт пары и ложный ADMIN_KEY_RECEIVED при отказе bind. + topo_node_sqlite_channel_set_privs (topo_node_sqlite.c:956–967) не проверяет два bind_blob, + bind_text канала и finalize; успех зависит только от step==SQLITE_DONE. + admin_key_recv_cb (chat_admin.c:84–97) доверяет 0 и публикует успех. + Временный стенд использует настоящее ядро/chat_service/router callback/SQLite/OpenSSL, + собственные канальные ключи; канал полностью подготовлен ensure_channel_ready. + Вызывается зарегистрированный локальный обработчик с готовыми расшифрованными данными; + это не сетевой обмен и не проверка защиты router/шифрования. + GNU ld wrappers вводят ровно один возврат SQLITE_NOMEM перед соответствующей реальной + привязкой; реальное исчерпание памяти и конкретный allocator SQLite не моделировались. + Публичные ключи соответствуют обоим присланным приватным ключам во всех fault-случаях. + + Случай X priv bytes Ed priv bytes Успешных событий Итог + Без нового отказа 32 32 1 PASS + Отказ bind X priv 0 32 1 FAIL + Отказ bind Ed priv 32 0 1 FAIL + Отказ bind channel_id 0 0 1 FAIL + + 0 в таблице означает SQL NULL; сам ключ/его байты в журнал не выводятся. + Независимый SELECT после callback подтверждает размеры; autocommit=1 в каждом случае. + Контроль реального UPDATE с SQL-триггером RAISE(ABORT):события0,PASS. + После снятия отказа повтор сохраняет оба ключа побайтно и даёт одно событие. + Неверный присланный Ed priv отклонён, прежняя правильная пара сохранена,PASS. + +2. NULL instance разыменовывается раньше намеренной проверки аргумента. + chat_core_transfer_admin (chat_admin.c:122–124) вычисляет CC(inst)=inst->chat_core, + затем проверяет !inst. Локальный вызов с NULL в отдельном дочернем процессе + завершается signal11/SIGSEGV, а не штатным отказом. Core dump отключён. + Этот сценарий не требует SQL-fault; внешний GUI/JNI race/restart не воспроизводился. + +Стенд /tmp/chat-admin-audit.c/.mk собран через настоящий tests/Makefile и libutun.a. +Финальный run exit1 ожидаемо фиксирует4 отказные проверки; bind_fault_hits3, +tracked allocations0->0. Консольная категория chat_sync=debug,остальные error; +все введённые SQL-отказы и размеры сохранённых ключей видны без секретных байтов. +Логи /tmp/chat-admin-audit-build.log и /tmp/chat-admin-audit.log. +Первый запуск имел посторонний teardown WARN о msg_901:фикстура создала канал только SQL; +финальный стенд вызывает ensure_channel_ready и не содержит этого постороннего отказа. + +План исправления (реализация ещё не выполнена) + +1. Установить одного владельца SQL-атомарности:channel_set_privs использует короткий savepoint, + проверяет prepare,все bind,step,число изменённых строк,finalize и RELEASE. + Ровно одна существующая строка — успех; отсутствие/пропуск UPDATE — ошибка. + Ошибка до подтверждения savepoint откатывает запись; причина/стадия логируется без ключей. + Прежнее значение пары сохраняется при отказе; повтор одинаковой пары допустим. + Helper может участвовать во внешней транзакции без публикации события. +2. Получатель обрабатывает импорт только вне внешней SQL-транзакции, чтобы RELEASE helper + действительно завершал внешний commit. Он по-прежнему сверяет обе пары ключей до write-lock; + успех и ADMIN_KEY_RECEIVED публикуются только после подтверждённого завершения helper. + Отказ не подтверждается; следующая корректная доставка может повторить импорт. + Если rollback не освободил свою транзакцию,фиксировать фатальный отказ/restart по принятому + в member_sync образцу,а не продолжать публикацию/работу с открытой транзакцией. +3. Проверять inst до CC(inst) в transfer; NULL/неинициализированный контекст отвергаются с ERROR. + Уточнить обязательность instance/uasync и результаты API; нового сетевого ACK/формата не вводить. +4. Постоянные регрессии:каждый prepare/bind/step/finalize/commit отказ, NULL, прежние ключи, + отсутствующий канал, повтор, разные/неверные ключи,внешняя транзакция, отсутствие ложных событий. + Проверять БД и события независимо,сохранность другого канала и повтор после отказа. + Затем целевой native/sanitizer-профиль и общий check.sh; платформенные прогоны отдельно. + +По AGENTS.md:10 дальнейший аудит остановлен до согласования исправления. +В этом продолжении изменены только комментарии,отчёт и список задач/отметки; +runtime,wire и штатные тесты не менялись. Новый общий check/sanitizers не запускались. diff --git a/src/chat/chat_admin.h b/src/chat/chat_admin.h index e377ff09..7e55b5d6 100644 --- a/src/chat/chat_admin.h +++ b/src/chat/chat_admin.h @@ -2,9 +2,13 @@ * chat_admin.h — передача канального приватного ключа (прав админа) другому узлу * * Владелец канала (имеющий ed25519_privkey + x25519_privkey) может передать оба ключа - * выбранному мемберу. Передача идёт через сервисный пакет etcp_router с + * выбранному узлу. Передача идёт через сервисный пакет etcp_router с * ROUTE_CRYPTO_SIGN | ROUTE_CRYPTO_ENCRYPT: X25519 ECDH + AES-128-CCM и Ed25519-подпись. - * Получатель автоматически импортирует ключи и становится совладельцем канала. + * Получатель сверяет выведенные публичные ключи с уже известными ключами канала, + * сохраняет оба privkey и синхронно публикует CHAT_EVT_ADMIN_KEY_RECEIVED. + * Известный дефект: SQL-helper игнорирует ошибки bind; возможен частичный импорт + * с ложным событием успеха. План: doc/chat_audit_findings.txt, продолжение chat_admin. + * Модуль включается/отключается chat_sync; все вызовы — в потоке uasync. */ #ifndef CHAT_ADMIN_H @@ -18,8 +22,9 @@ extern "C" { struct UTUN_INSTANCE; -/* Зарегистрировать/снять обработчик сервиса ETCP_RT_ID_ADMIN_KEY. */ +/* Зарегистрировать ETCP_RT_ID_ADMIN_KEY; 0/-1. inst обязателен, повтор заменяет callback. */ int chat_admin_init(struct UTUN_INSTANCE* inst); +/* Снять обработчик; NULL допустим, отсутствие регистрации логирует router. */ void chat_admin_destroy(struct UTUN_INSTANCE* inst); #ifdef __cplusplus diff --git a/src/routing_layer/topo_node_sqlite.h b/src/routing_layer/topo_node_sqlite.h index dadb0834..1681ff5e 100644 --- a/src/routing_layer/topo_node_sqlite.h +++ b/src/routing_layer/topo_node_sqlite.h @@ -124,7 +124,9 @@ int topo_node_sqlite_channel_get_privs(sqlite3* db, const char* channel_id, uint8_t* x25519_priv_out, uint8_t* ed25519_priv_out); /* Записать оба канальных privkey по 32 байта (импорт прав админа); 0/-1, без проверки пары/подписи. - * Отсутствующая строка даёт 0 без вставки; наличие канала проверяет вызывающий. */ + * Отсутствующая строка даёт 0 без вставки; наличие канала проверяет вызывающий. + * Известный дефект: bind/finalize не проверяются; ошибка bind допускает частичную пару/ложный 0. + * Исправление ещё не выполнено: doc/chat_audit_findings.txt, продолжение chat_admin. */ int topo_node_sqlite_channel_set_privs(sqlite3* db, const char* channel_id, const uint8_t* x25519_priv, const uint8_t* ed25519_priv);