fix(anon-req): encode the reply path with its hash mode and hop order

An anon request tells the server how to route its answer back. The leading
byte of that reply path packs two fields, which the server unpacks as:

    reply_path_len       = byte & 63
    reply_path_hash_size = (byte >> 6) + 1

Three defects in producing it:

1. The hash mode was never written into the top two bits, so the server always
   read a hash size of 1 whatever the contact's real mode was.
2. The path was reversed byte-wise (out_path[::-1]) rather than hop-wise. A
   return path visits the same hops in reverse order with each hop's
   multi-byte hash intact.
3. reader.py built out_path by stripping every NUL from the fixed 64-byte
   field. That trims the padding but also eats a legitimate 0x00 inside a hop
   hash, shortening the path and shifting every hop after it. It now takes
   out_path_len * hash_size bytes, as PATH_DISCOVERY_RESPONSE already did.

Worked example at hash mode 2 (3 bytes per hop), for a contact two hops away
via aabbcc then ddeeff:

    before:  lenbyte 0x02, path ffeeddccbbaa
             -> server reads 2 hops of 1 byte, replies via ['ff', 'ee']
    after:   lenbyte 0x82, path ddeeffaabbcc
             -> server reads 2 hops of 3 bytes, replies via ['ddeeff', 'aabbcc']

The old form routes the response to hops that do not exist, so it is dropped
and the request times out.

At hash mode 0 both encodings are byte-identical -- the mode contributes
nothing to the high bits and byte-wise reversal equals hop-wise reversal for
single-byte hops -- which is why this stayed latent: mode 0 is the default.
Confirmed by the mode-0 and zero-hop tests passing unchanged against the old
code while the mode-1/mode-2 tests fail.

Scope: only anon requests routed direct to a contact with a known multi-hop
path. Flood requests are unaffected (the server answers via createPathReturn
and ignores the supplied reply path), as is login (handleLoginReq never sets
reply_path_len, so its reply always goes out flood). The neighbors zero-hop
probe is unaffected: length 0 makes hash size irrelevant.

Encoding is extracted into encode_reply_path() so it can be tested directly.
Verified on hardware only for the zero-hop case, which still works; the
multi-hop paths are covered by unit tests, as the test radio has no multi-hop
contacts to exercise on air.
This commit is contained in:
agessaman
2026-07-26 19:38:33 -07:00
parent 00135bbb95
commit 30446ed093
3 changed files with 169 additions and 5 deletions
+39 -4
View File
@@ -57,6 +57,38 @@ def _validate_destination(dst: DestinationType, prefix_length: int = 6) -> bytes
)
def encode_reply_path(out_path_len: int, out_path_hex: str, out_path_hash_mode: int) -> bytes:
"""Encode the reply path a server should use when answering us.
The leading byte packs two fields, which the server unpacks as:
reply_path_len = byte & 63
reply_path_hash_size = (byte >> 6) + 1
so the hash mode has to travel in the top two bits. Omitting it makes the
server read a hash size of 1 regardless of the real mode, take the wrong
number of bytes per hop, and route its reply to hops that do not exist.
The path itself is reversed by *hop*, not by byte: a return path visits the
same hops in the opposite order, and each hop's multi-byte hash must stay
intact. (At hash mode 0 the two are indistinguishable, which is why this
went unnoticed - mode 0 is the default.)
"""
hash_mode = max(out_path_hash_mode, 0) # -1 means "flood", i.e. no path
hash_size = hash_mode + 1
hops = max(out_path_len, 0) & 63
raw = bytes.fromhex(out_path_hex or "")
# Never read past what the contact actually carries; a truncated or padded
# field would otherwise yield short trailing hops.
hops = min(hops, len(raw) // hash_size)
path = b"".join(
raw[i * hash_size:(i + 1) * hash_size] for i in range(hops - 1, -1, -1)
)
return bytes([hops | (hash_mode << 6)]) + path
class CommandHandlerBase:
"""Base class for command handlers.
@@ -325,7 +357,7 @@ class CommandHandlerBase:
if contact is None:
logger.debug("No contact found, requesting a zero-hop direct reply path")
out_path_len = 0
out_path = b""
reply_path = encode_reply_path(0, "", 0)
else:
if contact["out_path_len"] == -1:
logger.info("No path set trying zero hop")
@@ -339,10 +371,13 @@ class CommandHandlerBase:
# zero-hop on the device. Zero is the right value to send regardless,
# since zero-hop is exactly what we just asked for.
out_path_len = max(contact["out_path_len"], 0)
out_path = bytes.fromhex(contact["out_path"])[::-1]
reply_path = encode_reply_path(
out_path_len,
contact["out_path"],
contact.get("out_path_hash_mode", 0),
)
data = out_path_len.to_bytes(1, "little") + out_path
data = b"\x39" + dst_bytes + request_type.value.to_bytes(1, "little", signed=False) + (data if data else b"")
data = b"\x39" + dst_bytes + request_type.value.to_bytes(1, "little", signed=False) + reply_path
result = await self.send(data, [EventType.MSG_SENT, EventType.ERROR])
+11 -1
View File
@@ -112,7 +112,17 @@ class MessageReader:
else:
c["out_path_hash_mode"] = plen >> 6
c["out_path_len"] = plen & 0x3F # 6 LSB
c["out_path"] = dbuf.read(64).replace(b"\0", b"").hex()
# The field is a fixed 64 bytes, NUL-padded past the real path.
# Take exactly the bytes the path occupies rather than stripping
# NULs: a hop hash may legitimately contain 0x00, and dropping
# those shortens the path and shifts every hop after it.
# (PATH_DISCOVERY_RESPONSE below already reads opl*opl_hlen.)
path_bytes = dbuf.read(64)
if c["out_path_len"] > 0:
used = c["out_path_len"] * (c["out_path_hash_mode"] + 1)
c["out_path"] = path_bytes[:used].hex()
else:
c["out_path"] = ""
c["adv_name"] = dbuf.read(32).decode("utf-8", "ignore").replace("\0", "")
c["last_advert"] = int.from_bytes(dbuf.read(4), byteorder="little")
c["adv_lat"] = (
+119
View File
@@ -476,3 +476,122 @@ async def test_ble_send_releases_lock_on_write_failure():
# Second write must still be able to acquire the lock.
await asyncio.wait_for(conn.send(b"\x02"), timeout=1.0)
assert conn.client.calls == 2
# ── reply-path encoding (hash mode + hop-wise reversal) ──────
from meshcore.commands.base import encode_reply_path # noqa: E402
async def test_encode_reply_path_zero_hop():
# len 0, mode 0 -> a single 0x00 byte, the zero-hop direct request.
assert encode_reply_path(0, "", 0) == b"\x00"
async def test_encode_reply_path_mode0_reverses_hops():
# hops aa, bb -> reply visits bb then aa. Mode 0 leaves the top bits clear.
assert encode_reply_path(2, "aabb", 0) == b"\x02" + bytes.fromhex("bbaa")
async def test_encode_reply_path_carries_hash_mode_in_top_bits():
"""The server reads hash size from bits 6-7; omitting it truncates each hop.
Mode 2 = 3 bytes per hop. Without the mode the server would read hash_size 1
and reply to two 1-byte hops that do not exist.
"""
out = encode_reply_path(2, "aabbccddeeff", 2)
assert out[0] == 0x82 # 2 hops | (mode 2 << 6)
assert out[0] & 63 == 2 # server: reply_path_len
assert (out[0] >> 6) + 1 == 3 # server: reply_path_hash_size
# Hop order reversed, each 3-byte hash intact.
assert out[1:] == bytes.fromhex("ddeeff") + bytes.fromhex("aabbcc")
async def test_encode_reply_path_mode1_two_byte_hops():
out = encode_reply_path(3, "aabbccddeeff", 1)
assert out[0] == (3 | (1 << 6))
assert out[1:] == bytes.fromhex("eeff") + bytes.fromhex("ccdd") + bytes.fromhex("aabb")
async def test_encode_reply_path_flood_mode_is_clamped():
# out_path_hash_mode is -1 for a flood contact; must not produce a negative shift.
assert encode_reply_path(0, "", -1) == b"\x00"
async def test_encode_reply_path_ignores_padding_beyond_the_path():
# Real device fields are NUL-padded to 64 bytes; only the used hops count.
padded = "aabb" + "00" * 60
assert encode_reply_path(2, padded, 0) == b"\x02" + bytes.fromhex("bbaa")
async def test_encode_reply_path_truncated_field_does_not_emit_short_hops():
# Claims 4 hops of 3 bytes but only 6 bytes present -> emit the 2 it has.
out = encode_reply_path(4, "aabbccddeeff", 2)
assert out[0] & 63 == 2
assert out[1:] == bytes.fromhex("ddeeff") + bytes.fromhex("aabbcc")
async def test_send_anon_req_reply_path_uses_hash_mode(
command_handler, mock_connection, mock_dispatcher
):
"""End-to-end: a mode-2 multi-hop contact must get a correct reply path."""
command_handler._get_contact_by_prefix = MagicMock(return_value={
"public_key": VALID_PUBKEY_HEX,
"out_path_len": 2,
"out_path": "aabbccddeeff",
"out_path_hash_mode": 2,
})
command_handler.change_contact_path = AsyncMock()
command_handler.reset_path = AsyncMock()
setup_event_response(
mock_dispatcher, EventType.MSG_SENT,
{"expected_ack": b"\x01\x02\x03\x04", "suggested_timeout": 4000},
)
await command_handler.send_anon_req(VALID_PUBKEY_HEX, MagicMock(value=1))
sent = mock_connection.send.await_args.args[0]
assert sent == (
b"\x39" + bytes.fromhex(VALID_PUBKEY_HEX) + b"\x01"
+ b"\x82" + bytes.fromhex("ddeeff") + bytes.fromhex("aabbcc")
)
async def test_reader_contact_out_path_keeps_zero_bytes():
"""A hop hash containing 0x00 must survive parsing by the real reader.
The 64-byte field is NUL-padded, but stripping every NUL also eats
legitimate hash bytes, shortening the path and shifting every hop after it.
"""
from meshcore.reader import MessageReader
from meshcore.packets import PacketType
hops = bytes.fromhex("aa00bb") # middle byte is a legitimate 0x00
frame = (
bytes([PacketType.CONTACT.value])
+ bytes(32) # public_key
+ b"\x02" # type
+ b"\x00" # flags
+ bytes([1 | (2 << 6)]) # 1 hop, hash mode 2 -> 3 bytes/hop
+ hops + bytes(64 - len(hops)) # out_path, NUL-padded to 64
+ b"name".ljust(32, b"\0") # adv_name
+ (0).to_bytes(4, "little") # last_advert
+ (0).to_bytes(4, "little", signed=True) # adv_lat
+ (0).to_bytes(4, "little", signed=True) # adv_lon
+ (0).to_bytes(4, "little") # lastmod
)
seen = []
class Dispatcher:
async def dispatch(self, event):
seen.append(event)
reader = MessageReader(Dispatcher())
reader.contacts = {}
await reader.handle_rx(frame)
contact = next(e.payload for e in seen if e.type == EventType.NEXT_CONTACT)
assert contact["out_path_len"] == 1
assert contact["out_path_hash_mode"] == 2
assert contact["out_path"] == "aa00bb", "the 0x00 inside the hop hash was lost"