fix: compute signed-post push ACK over the full signed span

push_post_to_client sent posts as TXT_TYPE_SIGNED_PLAIN (plaintext =
timestamp + flags + author_prefix + text) but computed its expected ACK
over the plain-DM span (timestamp + attempt byte + text). Signed-plain
receivers (firmware BaseChatMesh::onPeerDataRecv, and openhop_core's
text handler since it gained signed-message ACKs) reply with
sha256(decrypted[0 : 9 + strlen(text)] || client_pubkey)[:4], so the
server never recognized any push ACK: every push timed out and the same
posts were re-pushed forever (issue #286's repeating 'Push timed out'
log and duplicate unread floods).

Hash the exact plaintext span that is encrypted and sent, plus the
client pubkey. Verified against openhop_core's TextMessageHandler: the
ACK it emits for a pushed post now matches the stored pending_ack_crc.

Fixes #286
This commit is contained in:
agessaman
2026-07-07 10:35:28 -07:00
parent 0404b3ab44
commit 9c6ea0151e
2 changed files with 78 additions and 8 deletions
+5 -4
View File
@@ -357,10 +357,11 @@ class RoomServer:
timestamp.to_bytes(4, "little") + bytes([flags]) + author_prefix + message_bytes
)
# Calculate expected ACK (same algorithm as openhop_core)
attempt = 0
pack_data = PacketBuilder._pack_timestamp_data(timestamp, attempt, message_bytes)
ack_hash = CryptoUtils.sha256(pack_data + client_info.id.get_public_key())[:4]
# Expected ACK: signed-plain receivers (firmware
# BaseChatMesh::onPeerDataRecv, and openhop_core's text handler)
# ack with sha256(decrypted[0 : 9 + strlen(text)] || client_pubkey)[:4]
# — exactly the timestamp+flags+author_prefix+text span built above.
ack_hash = CryptoUtils.sha256(plaintext + client_info.id.get_public_key())[:4]
expected_ack_crc = int.from_bytes(ack_hash, "little")
# Determine routing based on stored out_path
+73 -4
View File
@@ -8,6 +8,7 @@ from repeater.handler_helpers.room_server import (
MAX_UNSYNCED_POSTS,
RoomServer,
TXT_TYPE_PLAIN,
TXT_TYPE_SIGNED_PLAIN,
)
@@ -131,10 +132,6 @@ async def test_room_server_push_post_to_client_success_direct_route_sets_path_an
packet = SimpleNamespace(path=bytearray(), path_len=0)
with (
patch(
"repeater.handler_helpers.room_server.PacketBuilder._pack_timestamp_data",
return_value=b"pk",
),
patch(
"repeater.handler_helpers.room_server.CryptoUtils.sha256",
return_value=b"\x01\x02\x03\x04abcd",
@@ -156,6 +153,78 @@ async def test_room_server_push_post_to_client_success_direct_route_sets_path_an
rs.global_limiter.release.assert_called_once()
@pytest.mark.asyncio
async def test_room_server_push_expected_ack_matches_firmware_signed_ack():
"""The pending ACK CRC must match what a signed-plain receiver sends back.
Firmware clients (BaseChatMesh::onPeerDataRecv) and openhop_core's text
handler ack TXT_TYPE_SIGNED_PLAIN with
``sha256(decrypted[0 : 9 + strlen(text)] || client_pubkey)[:4]``.
Regression test for the room server expecting a plain-DM style hash
(timestamp + attempt + text) instead: every push timed out and posts were
re-pushed forever (issue #286).
"""
from openhop_core import LocalIdentity
from openhop_core.protocol import CryptoUtils, Identity
room_id = LocalIdentity()
client_id = LocalIdentity()
client_pk = client_id.get_public_key()
shared = Identity(client_pk).calc_shared_secret(room_id.get_private_key())
db = _FakeDB()
sent = []
async def injector(packet, wait_for_ack=False):
sent.append(packet)
return False # no ACK: leaves the pending upsert as the only db write
rs = RoomServer(
room_hash=room_id.get_public_key()[0],
room_name="parity",
local_identity=room_id,
sqlite_handler=db,
packet_injector=injector,
acl=_FakeACL(),
)
rs.global_limiter = SimpleNamespace(acquire=AsyncMock(), release=MagicMock())
author = LocalIdentity().get_public_key()
client = _FakeClient(pubkey=client_pk, shared_secret=shared, out_path=b"\x01", out_path_len=1)
post = {
"author_pubkey": author.hex(),
"message_text": "parity check",
"post_timestamp": 1234.5,
}
ok = await rs.push_post_to_client(client, post)
assert ok is False
assert len(sent) == 1
upserts = [c.kwargs for c in db.upsert_client_sync.call_args_list if "pending_ack_crc" in c.kwargs]
assert len(upserts) == 1
expected_ack_crc = upserts[0]["pending_ack_crc"]
# Decrypt the pushed datagram and verify the signed-plain layout.
pkt = sent[0]
encrypted = bytes(pkt.payload[2 : pkt.payload_len])
decrypted = CryptoUtils.mac_then_decrypt(shared[:16], shared, encrypted)
assert decrypted is not None
flags = decrypted[4]
assert (flags >> 2) & 0x3F == TXT_TYPE_SIGNED_PLAIN
assert bytes(decrypted[5:9]) == author[:4]
body = bytes(decrypted[9:])
nul = body.find(b"\x00")
text_len = nul if nul >= 0 else len(body)
assert body[:text_len] == b"parity check"
# Recompute the ACK exactly as the receiving client does.
client_ack_crc = int.from_bytes(
CryptoUtils.sha256(bytes(decrypted[: 9 + text_len]) + client_pk)[:4], "little"
)
assert client_ack_crc == expected_ack_crc
@pytest.mark.asyncio
async def test_room_server_push_post_to_client_backoff_skip_and_timeout_path():
db = _FakeDB()