From d01de9d14d671cff9951d5afd4097837abf1515a Mon Sep 17 00:00:00 2001 From: Stephen Waits Date: Wed, 1 Apr 2026 11:57:32 -0600 Subject: [PATCH] fix: MeshCore spec compliance audit and packet quality warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audited against meshcore-spec and fixed all compliance gaps: - Fix trace hash size encoding: use 1<<(flags&3) not (flags&3)+1 - Reject reserved path hash_size_code 3 per spec Section 3 - Minimum packet size 3 bytes (was 2) - Add MAX_PACKET_PAYLOAD (184) and min payload (1) validation - Tighten payload minimums: ANON_REQ≥51, encrypted≥20, group≥19, multipart≥2 - Add node_type 0 ("none") to NODE_TYPES - Fix node_type fallback display (was "node=type=14", now "node=0x0e") Add packet quality warnings for likely-corrupt frames: - decode_meshcore_packet returns (str, list[str]) with quality warnings - ADVERT: flag bad node_type, bad UTF-8 in name, future/bad timestamps - Combine content warnings with low-SNR signal at display layer - Warnings stack: one flag = maybe, multiple = high confidence corrupt Update test corpus and generators for tightened validation rules. Co-Authored-By: Claude Opus 4.6 (1M context) --- examples/meshcore/corpus/build_hand.py | 70 ++++++++--- examples/meshcore/corpus/generate.py | 18 ++- .../meshcore/corpus/hand/bad_path_len.json | 4 +- .../hand/bad_path_len_reserved_hash_size.json | 16 +++ .../meshcore/corpus/hand/path_4byte_hash.json | 22 +--- .../{too_short.json => too_short_1byte.json} | 0 .../corpus/hand/too_short_2bytes.json | 14 +++ .../corpus/hand/trace_4byte_hash.json | 24 ++++ examples/meshcore/corpus/validate.py | 2 +- examples/meshcore/meshcore_rx.py | 110 +++++++++++------- 10 files changed, 198 insertions(+), 82 deletions(-) create mode 100644 examples/meshcore/corpus/hand/bad_path_len_reserved_hash_size.json rename examples/meshcore/corpus/hand/{too_short.json => too_short_1byte.json} (100%) create mode 100644 examples/meshcore/corpus/hand/too_short_2bytes.json create mode 100644 examples/meshcore/corpus/hand/trace_4byte_hash.json diff --git a/examples/meshcore/corpus/build_hand.py b/examples/meshcore/corpus/build_hand.py index 064591db..b611e577 100755 --- a/examples/meshcore/corpus/build_hand.py +++ b/examples/meshcore/corpus/build_hand.py @@ -458,6 +458,24 @@ write_case("trace_basic", { } }) +# TRACE: flags=2 means hash_size=4 (power of 2 encoding: 1 << 2 = 4) +write_case("trace_4byte_hash", { + "description": "TRACE flood with 4-byte trace hashes (flags=0x02, hash_size=4)", + "tags": ["trace", "flood", "4byte_hash"], + "packet_hex": trace_packet( + 0xAABBCCDD, 0x11223344, 0x02, b"\xde\xad\xbe\xef\xca\xfe\xba\xbe" + ).hex(), + "valid": True, + "expected": { + "route_type": "flood", + "payload_type": "TRACE", + "payload_ver": 0, + "transport_codes": None, + "path": {"hash_size": 1, "hops": []}, + "payload": {"trace_tag": 0xAABBCCDD, "trace_hops": 2} + } +}) + # MULTIPART: ACK inner type write_case("multipart_ack", { "description": "MULTIPART flood, remaining=3, inner type=ACK", @@ -513,29 +531,26 @@ write_case("path_3byte_hash", { } }) -# PATH: 4-byte hash, 1 hop +# PATH: 4-byte hash (hash_size_code 3 = RESERVED, must be rejected) write_case("path_4byte_hash", { - "description": "PATH flood, 1 hop with 4-byte hash", - "tags": ["path", "flood", "4byte_hash", "encrypted"], + "description": "PATH flood, 4-byte hash (reserved hash_size_code 3, invalid)", + "tags": ["invalid", "path", "flood", "4byte_hash", "reserved_hash_size"], "packet_hex": peer_packet( 0x08, 1, 0xe6, 0xe9, b"\x2e\x47", bytes(16), path_hops=[b"\xaa\xbb\xcc\xdd"], hash_size=4 ).hex(), - "valid": True, + "valid": False, "expected": { "route_type": "flood", "payload_type": "PATH", - "payload_ver": 0, - "transport_codes": None, - "path": {"hash_size": 4, "hops": ["aabbccdd"]}, - "payload": {"dst": "e6", "src": "e9", "mac": "2e47", "ciphertext_len": 16} + "error": "bad_framing" } }) # ── Invalid packets ─────────────────────────────────────────────── # Too short: 1 byte -write_case("too_short", { +write_case("too_short_1byte", { "description": "Packet too short (1 byte)", "tags": ["invalid", "too_short"], "packet_hex": "11", @@ -547,11 +562,24 @@ write_case("too_short", { } }) -# Bad path_len: claims 33 hops but only 24 bytes remain -bad_pkt = bytes([mc_header(0x02, 1), 0xe1]) + bytes(24) -write_case("bad_path_len", { - "description": "TXT_MSG flood, path_len claims 33 hops (4B hash) but data is too short", - "tags": ["invalid", "bad_framing", "path"], +# Too short: 2 bytes (need at least 3: header + path_len + 1 byte payload) +write_case("too_short_2bytes", { + "description": "Packet too short (2 bytes, need >= 3)", + "tags": ["invalid", "too_short"], + "packet_hex": "1100", + "valid": False, + "expected": { + "route_type": "flood", + "payload_type": "ADVERT", + "error": "too_short" + } +}) + +# Bad path_len: reserved hash_size_code 3 (bits 6-7 = 0b11) +bad_pkt = bytes([mc_header(0x02, 1), 0xC1]) + bytes(24) +write_case("bad_path_len_reserved_hash_size", { + "description": "TXT_MSG flood, path_len has reserved hash_size_code 3", + "tags": ["invalid", "bad_framing", "path", "reserved_hash_size"], "packet_hex": bad_pkt.hex(), "valid": False, "expected": { @@ -561,6 +589,20 @@ write_case("bad_path_len", { } }) +# Bad path_len: claims 33 hops (2B hash) but only 24 bytes remain +bad_pkt2 = bytes([mc_header(0x02, 1), 0x61]) + bytes(24) +write_case("bad_path_len", { + "description": "TXT_MSG flood, path_len claims 33 hops (2B hash) but data is too short", + "tags": ["invalid", "bad_framing", "path"], + "packet_hex": bad_pkt2.hex(), + "valid": False, + "expected": { + "route_type": "flood", + "payload_type": "TXT_MSG", + "error": "bad_framing" + } +}) + # Noise: random bytes that don't parse as meshcore write_case("noise_packet", { "description": "Random bytes, not a valid MeshCore packet", diff --git a/examples/meshcore/corpus/generate.py b/examples/meshcore/corpus/generate.py index 583664ed..756883c5 100755 --- a/examples/meshcore/corpus/generate.py +++ b/examples/meshcore/corpus/generate.py @@ -94,7 +94,7 @@ def advert_appdata(draw): name = data[8 if has_loc else 0:].decode("ascii", errors="replace") app_data = bytes([flags]) + data - node_label = NODE_TYPES.get(node_type, f"type={node_type}") + node_label = NODE_TYPES.get(node_type, f"0x{node_type:02x}") return app_data, {"flags": flags, "node_type": node_label, "location": loc, "name": name} @@ -158,7 +158,7 @@ def valid_encrypted_peer_packet(draw): dst = draw(st.integers(min_value=0, max_value=255)) src = draw(st.integers(min_value=0, max_value=255)) mac = draw(st.binary(min_size=2, max_size=2)) - ct_len = draw(st.integers(min_value=0, max_value=64)) + ct_len = draw(st.integers(min_value=16, max_value=64).filter(lambda n: n % 16 == 0)) ct = draw(st.binary(min_size=ct_len, max_size=ct_len)) hdr = bytes([mc_header(ptype, route_type)]) @@ -188,7 +188,7 @@ def valid_group_packet(draw): hs, hops = draw(path_strategy()) ch = draw(st.integers(min_value=0, max_value=255)) mac = draw(st.binary(min_size=2, max_size=2)) - ct_len = draw(st.integers(min_value=0, max_value=64)) + ct_len = draw(st.integers(min_value=16, max_value=64).filter(lambda n: n % 16 == 0)) ct = draw(st.binary(min_size=ct_len, max_size=ct_len)) hdr = bytes([mc_header(ptype, route_type)]) @@ -220,7 +220,7 @@ def invalid_packet(draw): pkt, _ = draw(valid_ack_packet()) cut = draw(st.integers(min_value=0, max_value=max(0, len(pkt) - 2))) packet = pkt[:cut] - if len(packet) < 2: + if len(packet) < 3: error = "too_short" else: error = "bad_framing" @@ -230,12 +230,18 @@ def invalid_packet(draw): ptype = draw(st.sampled_from(list(PAYLOAD_TYPES.keys()))) hdr = bytes([mc_header(ptype, route_type)]) # path_len claiming huge path - bad_pl = draw(st.integers(min_value=0xC0, max_value=0xFF)) + # Use hash_size_code 3 (reserved, 0xC0-0xFF) or valid codes with huge counts + bad_pl = draw(st.sampled_from([ + *range(0xC0, 0x100), # reserved hash_size_code 3 + 0x3F, # 63 hops × 1 byte = 63 (valid encoding, will overflow data) + 0x60, # 32 hops × 2 bytes = 64 (valid encoding, will overflow data) + 0x95, # 21 hops × 3 bytes = 63 (valid encoding, will overflow data) + ])) tail = draw(st.binary(min_size=1, max_size=20)) packet = hdr + bytes([bad_pl]) + tail error = "bad_framing" else: # random - packet = draw(st.binary(min_size=2, max_size=50)) + packet = draw(st.binary(min_size=3, max_size=50)) error = "not_meshcore" header = packet[0] if packet else 0 diff --git a/examples/meshcore/corpus/hand/bad_path_len.json b/examples/meshcore/corpus/hand/bad_path_len.json index a2e7fa63..1b1c68b3 100644 --- a/examples/meshcore/corpus/hand/bad_path_len.json +++ b/examples/meshcore/corpus/hand/bad_path_len.json @@ -1,11 +1,11 @@ { - "description": "TXT_MSG flood, path_len claims 33 hops (4B hash) but data is too short", + "description": "TXT_MSG flood, path_len claims 33 hops (2B hash) but data is too short", "tags": [ "invalid", "bad_framing", "path" ], - "packet_hex": "09e1000000000000000000000000000000000000000000000000", + "packet_hex": "0961000000000000000000000000000000000000000000000000", "valid": false, "expected": { "route_type": "flood", diff --git a/examples/meshcore/corpus/hand/bad_path_len_reserved_hash_size.json b/examples/meshcore/corpus/hand/bad_path_len_reserved_hash_size.json new file mode 100644 index 00000000..f4f30d77 --- /dev/null +++ b/examples/meshcore/corpus/hand/bad_path_len_reserved_hash_size.json @@ -0,0 +1,16 @@ +{ + "description": "TXT_MSG flood, path_len has reserved hash_size_code 3", + "tags": [ + "invalid", + "bad_framing", + "path", + "reserved_hash_size" + ], + "packet_hex": "09c1000000000000000000000000000000000000000000000000", + "valid": false, + "expected": { + "route_type": "flood", + "payload_type": "TXT_MSG", + "error": "bad_framing" + } +} diff --git a/examples/meshcore/corpus/hand/path_4byte_hash.json b/examples/meshcore/corpus/hand/path_4byte_hash.json index e1d3a29b..dd4a99cc 100644 --- a/examples/meshcore/corpus/hand/path_4byte_hash.json +++ b/examples/meshcore/corpus/hand/path_4byte_hash.json @@ -1,29 +1,17 @@ { - "description": "PATH flood, 1 hop with 4-byte hash", + "description": "PATH flood, 4-byte hash (reserved hash_size_code 3, invalid)", "tags": [ + "invalid", "path", "flood", "4byte_hash", - "encrypted" + "reserved_hash_size" ], "packet_hex": "21c1aabbccdde6e92e4700000000000000000000000000000000", - "valid": true, + "valid": false, "expected": { "route_type": "flood", "payload_type": "PATH", - "payload_ver": 0, - "transport_codes": null, - "path": { - "hash_size": 4, - "hops": [ - "aabbccdd" - ] - }, - "payload": { - "dst": "e6", - "src": "e9", - "mac": "2e47", - "ciphertext_len": 16 - } + "error": "bad_framing" } } diff --git a/examples/meshcore/corpus/hand/too_short.json b/examples/meshcore/corpus/hand/too_short_1byte.json similarity index 100% rename from examples/meshcore/corpus/hand/too_short.json rename to examples/meshcore/corpus/hand/too_short_1byte.json diff --git a/examples/meshcore/corpus/hand/too_short_2bytes.json b/examples/meshcore/corpus/hand/too_short_2bytes.json new file mode 100644 index 00000000..a80f66c6 --- /dev/null +++ b/examples/meshcore/corpus/hand/too_short_2bytes.json @@ -0,0 +1,14 @@ +{ + "description": "Packet too short (2 bytes, need >= 3)", + "tags": [ + "invalid", + "too_short" + ], + "packet_hex": "1100", + "valid": false, + "expected": { + "route_type": "flood", + "payload_type": "ADVERT", + "error": "too_short" + } +} diff --git a/examples/meshcore/corpus/hand/trace_4byte_hash.json b/examples/meshcore/corpus/hand/trace_4byte_hash.json new file mode 100644 index 00000000..00bc543e --- /dev/null +++ b/examples/meshcore/corpus/hand/trace_4byte_hash.json @@ -0,0 +1,24 @@ +{ + "description": "TRACE flood with 4-byte trace hashes (flags=0x02, hash_size=4)", + "tags": [ + "trace", + "flood", + "4byte_hash" + ], + "packet_hex": "2500ddccbbaa4433221102deadbeefcafebabe", + "valid": true, + "expected": { + "route_type": "flood", + "payload_type": "TRACE", + "payload_ver": 0, + "transport_codes": null, + "path": { + "hash_size": 1, + "hops": [] + }, + "payload": { + "trace_tag": 2864434397, + "trace_hops": 2 + } + } +} diff --git a/examples/meshcore/corpus/validate.py b/examples/meshcore/corpus/validate.py index 192ac580..0308a561 100755 --- a/examples/meshcore/corpus/validate.py +++ b/examples/meshcore/corpus/validate.py @@ -58,7 +58,7 @@ def validate_decode(case: dict, path: Path) -> list[str]: # Run decoder — must not crash try: - raw_output = decode_meshcore_packet(packet) + raw_output, _warnings = decode_meshcore_packet(packet) except Exception as e: errors.append(f"Decoder crashed: {e}") return errors diff --git a/examples/meshcore/meshcore_rx.py b/examples/meshcore/meshcore_rx.py index 0183f5e6..9fb0ad85 100755 --- a/examples/meshcore/meshcore_rx.py +++ b/examples/meshcore/meshcore_rx.py @@ -514,7 +514,7 @@ PAYLOAD_NAMES = { 0x0F: "RAW_CUSTOM", } -NODE_TYPES = {0x01: "chat", 0x02: "repeater", 0x03: "room", 0x04: "sensor"} +NODE_TYPES = {0x00: "none", 0x01: "chat", 0x02: "repeater", 0x03: "room", 0x04: "sensor"} MAX_PATH_SIZE = 64 @@ -574,14 +574,18 @@ def _best_name(candidates: list[bytes]) -> bytes: return min(candidates, key=lambda d: (_utf8_score(d), -len(d))) -def _decode_advert_appdata(app_data: bytes) -> tuple[str, str]: - """Decode advert app_data. Returns (display_string, node_name).""" +def _decode_advert_appdata(app_data: bytes) -> tuple[str, str, list[str]]: + """Decode advert app_data. Returns (display_string, node_name, quality_warnings).""" if not app_data: - return "", "" + return "", "", [] flags = app_data[0] node_type = flags & 0x0F - node_label = NODE_TYPES.get(node_type, f"type={node_type}") + node_label = NODE_TYPES.get(node_type, f"0x{node_type:02x}") parts = [f"{DIM}flags={RST}{flags:02x} {DIM}node={RST}{node_label}"] + warnings: list[str] = [] + + if node_type > 4: + warnings.append("bad node_type") # Parse location if flagged and coords are valid off = 1 @@ -614,12 +618,14 @@ def _decode_advert_appdata(app_data: bytes) -> tuple[str, str]: if candidates: best = _best_name(candidates) name = best.decode("utf-8", errors="replace") + if "\ufffd" in name: + warnings.append("bad UTF-8") if loc_str: parts.append(loc_str) if name: parts.append(f"{BWHT}{name}{RST}") - return " ".join(parts), name + return " ".join(parts), name, warnings LOOP_INDENT = " " * 38 # align under decoded packet text (after RSSI/SNR/len) @@ -673,18 +679,21 @@ def _try_decode_body( payload_ver: int, route_name: str, tc_str: str, -) -> str | None: - """Try to parse path + payload starting at pos. Returns decoded string or None.""" +) -> tuple[str | None, list[str]]: + """Try to parse path + payload starting at pos. Returns (decoded_string, warnings) or (None, []).""" if pos >= len(data): - return None + return None, [] path_len_byte = data[pos] pos += 1 - hash_size = (path_len_byte >> 6) + 1 + hash_size_code = path_len_byte >> 6 + if hash_size_code == 3: + return None, [] # reserved hash size code per spec Section 3 + hash_size = hash_size_code + 1 hop_count = path_len_byte & 0x3F path_bytes = hop_count * hash_size if path_bytes > MAX_PATH_SIZE or pos + path_bytes > len(data): - return None + return None, [] hops_str = "" loop_warn = "" @@ -700,10 +709,13 @@ def _try_decode_body( pos += path_bytes payload = data[pos:] + if not payload or len(payload) > 184: + return None, [] prefix = f"{type_color}{type_name}{RST} {route_name}{tc_str}{hops_str}" + warnings: list[str] = [] if payload_type == 0x04: - result = _decode_advert(prefix, payload) + result, warnings = _decode_advert(prefix, payload) elif payload_type == 0x03: result = _decode_ack(prefix, payload) elif payload_type in (0x00, 0x01, 0x02, 0x08): @@ -721,17 +733,17 @@ def _try_decode_body( else: result = f"{prefix} {DIM}{payload.hex()}{RST}" if payload else prefix - return result + loop_warn + return result + loop_warn, warnings -def decode_meshcore_packet(data: bytes) -> str: - """Decode a MeshCore packet into a human-readable string.""" - if len(data) < 2: - return f"{RED}{RST} {DIM}{data.hex()}{RST}" +def decode_meshcore_packet(data: bytes) -> tuple[str, list[str]]: + """Decode a MeshCore packet. Returns (display_string, quality_warnings).""" + if len(data) < 3: + return f"{RED}{RST} {DIM}{data.hex()}{RST}", [] header = data[0] if header == 0xFF: - return f"{DIM} {data[1:].hex()}{RST}" + return f"{DIM} {data[1:].hex()}{RST}", [] route_type = header & 0x03 payload_type = (header >> 2) & 0x0F @@ -740,11 +752,10 @@ def decode_meshcore_packet(data: bytes) -> str: # MeshCore only implements PAYLOAD_VER_1 (0x00) — versions 1-3 are reserved # and rejected by real nodes. Packets with ver > 0 are noise or non-MeshCore. if payload_ver != 0: - route_name = ROUTE_NAMES.get(route_type, f"rt{route_type}") - type_name = PAYLOAD_NAMES.get(payload_type, f"0x{payload_type:02x}") return ( f"{RED}{RST} {DIM}hdr=0x{header:02x} " - f"(ver={payload_ver} — only v0 exists) {data[1:].hex()}{RST}" + f"(ver={payload_ver} — only v0 exists) {data[1:].hex()}{RST}", + [], ) route_name = ROUTE_NAMES.get(route_type, f"rt{route_type}") @@ -757,7 +768,7 @@ def decode_meshcore_packet(data: bytes) -> str: if has_tc and len(data) >= 6: tc1, tc2 = struct.unpack_from(" str: tc_str, ) if result is not None: - return result + return result, warnings # TC parse failed — retry without (older firmware compatibility) - result = _try_decode_body( + result, warnings = _try_decode_body( data, 1, type_name, type_color, payload_type, payload_ver, route_name, "" ) if result is not None: - return result + return result, warnings else: - result = _try_decode_body( + result, warnings = _try_decode_body( data, 1, type_name, type_color, payload_type, payload_ver, route_name, "" ) if result is not None: - return result + return result, warnings # Both attempts failed — show what we know from the header return ( f"{RED}{RST} {type_color}{type_name}{RST} " - f"{route_name} {DIM}{data[1:].hex()}{RST}" + f"{route_name} {DIM}{data[1:].hex()}{RST}", + [], ) -def _decode_advert(prefix: str, payload: bytes) -> str: +def _decode_advert(prefix: str, payload: bytes) -> tuple[str, list[str]]: if len(payload) < 100: - return f"{prefix} {RED}=100>{RST} {DIM}{payload.hex()}{RST}" + return f"{prefix} {RED}=100>{RST} {DIM}{payload.hex()}{RST}", [] pubkey = payload[0:32] timestamp = struct.unpack_from(" time.time() + 3600: + warnings.append("future ts") except (OSError, OverflowError, ValueError): ts = f"epoch={timestamp}" - app_str, name = _decode_advert_appdata(app_data) - _register_node(pubkey, name) - return f"{prefix} {DIM}pub={RST}{pubkey[:4].hex()}.. {DIM}ts={RST}{ts}Z {app_str}" + warnings.append("bad ts") + app_str, name, app_warnings = _decode_advert_appdata(app_data) + warnings.extend(app_warnings) + if not warnings: + _register_node(pubkey, name) + return f"{prefix} {DIM}pub={RST}{pubkey[:4].hex()}.. {DIM}ts={RST}{ts}Z {app_str}", warnings def _decode_ack(prefix: str, payload: bytes) -> str: @@ -816,7 +834,7 @@ def _decode_ack(prefix: str, payload: bytes) -> str: def _decode_encrypted_peer(prefix: str, type_name: str, payload: bytes) -> str: - if len(payload) < 4: + if len(payload) < 20: return f"{prefix} {RED}{RST} {DIM}{payload.hex()}{RST}" dst = payload[0] src = payload[1] @@ -826,7 +844,7 @@ def _decode_encrypted_peer(prefix: str, type_name: str, payload: bytes) -> str: def _decode_encrypted_group(prefix: str, payload: bytes) -> str: - if len(payload) < 3: + if len(payload) < 19: return ( f"{prefix} {RED}{RST} {DIM}{payload.hex()}{RST}" ) @@ -865,7 +883,7 @@ def _decode_encrypted_group(prefix: str, payload: bytes) -> str: def _decode_anon_req(prefix: str, payload: bytes) -> str: - if len(payload) < 35: + if len(payload) < 51: return f"{prefix} {RED}{RST} {DIM}{payload.hex()}{RST}" dst = payload[0] ephem_pub = payload[1:33] @@ -881,7 +899,7 @@ def _decode_trace(prefix: str, payload: bytes) -> str: ) tag = struct.unpack_from(" str: def _decode_multipart(prefix: str, payload: bytes) -> str: - if len(payload) < 1: - return f"{prefix} {RED}{RST}" + if len(payload) < 2: + return f"{prefix} {RED}{RST}" remaining = (payload[0] >> 4) & 0x0F inner_type = payload[0] & 0x0F inner_name = PAYLOAD_NAMES.get(inner_type, f"0x{inner_type:02x}") @@ -1019,9 +1037,17 @@ def configure_and_listen(ser: serial.Serial): last_activity = time.monotonic() payload = resp["payload"] - decoded = decode_meshcore_packet(payload) - rssi_snr = _format_rssi_snr(resp["rssi"], resp["snr"]) - print(f" {rssi_snr} {DIM}len:{len(payload):3d}{RST} {decoded}") + decoded, warnings = decode_meshcore_packet(payload) + snr = resp["snr"] + # SNR below LoRa demodulation floor is one quality signal + sf = RADIO_CONFIG["sf"] + if -32 <= snr <= 32 and snr < -2.5 * (sf - 4): + warnings.append("low SNR") + warn_tag = "" + if warnings: + warn_tag = f" {BYEL}[{', '.join(warnings)}]{RST}" + rssi_snr = _format_rssi_snr(resp["rssi"], snr) + print(f" {rssi_snr} {DIM}len:{len(payload):3d}{RST} {decoded}{warn_tag}") except Exception: pass