A room server is an endpoint, not a repeater. Strip the repeater
machinery the role inherited from the RepeaterMesh clone:
- allowPacketForward() returns false unconditionally — never relays
transit traffic. (Upstream's simple_room_server gates this on
disable_fwd; hard-off here since the role should never repeat.)
- neighbour tracking: NeighbourInfo/neighbours[], putNeighbour,
onAdvertRecv, removeNeighbor; formatNeighborsReply -> "not supported"
- node discovery: onControlDataRecv, sendNodeDiscoverReq, the
discover.neighbors CLI, discover rate-limiter + pending state
- loop detection: isLooped + max_loop_* tables + loop_detect pref
- dead code: RepeaterStats struct, GET_NEIGHBOURS handler, simple_sort
-344 lines net. Builds clean (FLASH 30.7% / RAM 32.2%); login, post,
push/sync, read-only guest and admin remote-management all
hardware-verified on the GAT562 kit.
The room server's onAnonDataRecv handles login inline (the room login
protocol carries a sync_since cursor the repeater's doesn't), so the
cloned repeater helpers handleLoginReq + handleAnonRegionsReq /
handleAnonOwnerReq / handleAnonClockReq — and the anon_limiter they used
— were dead after the port. Remove them (~160 lines). No behavior change.
The inherited neighbor-tracking / node-discovery machinery is also unused
on a room server; left in place for now (can be trimmed in a follow-up).
StrHelper::strncpy null-terminates but does NOT zero-pad the 16-byte
password buffer, so setting a shorter password over a longer previous
value (e.g. one inherited from a prior repeater config) leaves trailing
garbage. onAnonDataRecv's constant-time compare runs over the full
buffer width, so a correct password stopped matching — admin/guest
logins were silently rejected, or downgraded to a read-only guest when
allow_read_only was on (the login looked identical to read-only).
Fix: copy both stored passwords into zeroed buffers (up to strnlen)
before the constant-time compare, so the comparison reflects the actual
string while staying constant-time over the full width.
Hardware-verified on the kit: admin login with the correct password now
grants ADMIN (post + remote management), confirmed server-side via
get acl (perms 03).
Note: the repeater's handleLoginReq shares this latent issue.
Two hardware-found fixes after on-air testing on the GAT562 kit:
- Add a 500ms push timer in main_room_server.cpp so the post-sync
engine advances at its intended cadence. ZephCore is event-driven
(no Arduino superloop), so without this the engine only ran on the
5s housekeeping tick — posts dripped out every ~5s and transmits
bunched up, causing timeouts/resends.
- Lower the post-sync hold from upstream's conservative defaults
(POST_SYNC_DELAY 6s -> 2s, PUSH_NOTIFY 2000ms -> 1000ms). These are
server-side timing only (no wire-format change), and take measured
delivery from ~6-7s down to ~2s.
Verified on hardware (910.525/62.5/SF7/CR8, two clients): normal-pace
messages deliver in ~2s with clean ACKs. Rapid-fire bursts can still
drop out-of-order messages via the per-client timestamp replay check
(unchanged from upstream) — left as-is to stay upstream-compatible.
Add the MeshCore Room Server role to ZephCore — a store-and-forward
shared message room. Clients log in with an admin or guest password
and post messages; the server pushes each new post to all other
logged-in clients (round-robin, per-client sync cursor, ACK + retry,
3-strike eviction).
Ported from upstream MeshCore's simple_room_server, structured as a
near-clone of RepeaterMesh so it reuses the proven ACL, region
filtering, CLI, adverts and telemetry; the post buffer + push engine
are the only net-new pieces:
- RoomServerMesh: PostInfo ring (MAX_UNSYNCED_POSTS=32), addPost,
pushPostToClient, getUnsyncedCount, processAck, onAckRecv, the
loop() push driver; room login (onAnonDataRecv parses the
sync_since cursor); posts/admin-CLI/keep-alive (onPeerDataRecv);
ADV_TYPE_ROOM advert; ServerStats wire layout; disable_fwd=1.
- main_room_server.cpp: event-loop entry (USB serial CLI, no BLE).
- Kconfig: ZEPHCORE_ROLE_ROOM_SERVER + ZEPHCORE_MAX_UNSYNCED_POSTS.
- CMakeLists role gating; boards/common/room_server.conf.
Post frame, SHA-256 ACK and login-reply layouts match upstream for
MeshCore app compatibility. Builds for gat562_30s
(FLASH 30.95% / RAM 32.46%). Not yet hardware-tested.
Swap the vendored orlp/ed25519 (frozen ~2017 ref10) for Monocypher
4.0.2, an actively maintained, audited, single-file implementation.
The persisted private key keeps its 64-byte expanded layout
(clamped SHA-512(seed) scalar a || nonce prefix), so identities
written by older firmware load, sign, verify and key-exchange
unchanged -- no re-key, no storage migration, full wire
compatibility with the existing mesh and Arduino MeshCore.
Because the stored key carries no seed for Monocypher's high-level
EdDSA API, Identity drives signing from the low-level primitives
(crypto_sha512 + crypto_eddsa_reduce/scalarbase/mul_add); verify
uses crypto_ed25519_check and ECDH uses crypto_eddsa_to_x25519 +
crypto_x25519. Nonce material is now wiped after signing.
Validated byte-for-byte against the previous orlp output via a
known-answer harness (keygen, sign-from-expanded-key, verify
accept/reject, X25519-over-Ed25519 shared secret) before the swap.
Frees ~42 KB of flash: orlp linked ~55 KB (dominated by its ~30 KB
ref10 precomputed tables); the Monocypher Ed25519/X25519/SHA-512
paths link ~14 KB, with --gc-sections dropping all unused algos.
wio_tracker_l1 pristine build: FLASH 54.14%, links clean.
Monocypher is CC0-1.0 OR BSD-2-Clause.
The three anon handlers ignored copyPath's return value: on a rejected
(over-long) path, reply_path_len kept the attacker byte while reply_path
stayed stale, so the reply went out with a corrupt direct path. Reset to
OUT_PATH_UNKNOWN so it floods instead; keep the legit zero-length case.
Audit-driven sweep found additional compiler-optimization-sensitive
patterns beyond the login password compare just fixed:
P4.F3 (HIGH) — Utils::MACThenDecrypt verified packet MACs with
plain memcmp. Runs on EVERY encrypted-then-MAC'd packet in the
mesh; a timing oracle here lets attackers forge MACs byte-by-byte
across the whole mesh layer. Replaced with constantTimeEqual.
P4.F4 (MEDIUM) — Multiple memset(secret, 0, ...) calls on
stack-resident crypto buffers (Ed25519 seed, ADC noise pool, AES
key derived in extract_via_aes_ctr, HWINFO unique ID) were
subject to dead-store elimination under -Os. GCC/Clang routinely
elide these when the buffer is never read after; the wipe vanishes
and the secret persists on stack until next call overwrites.
Replaced with secureZeroize using volatile pointer writes.
P4.F5 (LOW) — Identity::validatePrivateKey boot self-test compared
shared secrets with plain memcmp. Boot-only, no attacker
observation channel, but hygiene matters and the fix is one line.
Also added secret-wipe for ss1/ss2 on all return paths.
Promoted the local ct_memeq() previously added to RepeaterMesh.cpp
into Utils::constantTimeEqual + Utils::secureZeroize (Utils.h/cpp)
so the login compare and MAC compare share the same audited helper.
Both helpers verified by Thumb-2 disassembly on rak3401_1watt:
- constantTimeEqual: loop branches on iterator, accumulator
load-modify-stored to stack every iteration, final return uses
CLZ+LSR (no conditional branch on result).
- secureZeroize: STRB.W to memory in a counted loop, not replaced
with memset builtin and not eliminated.
mbedtls_ct_memcmp is declared in the tf-psa-crypto header but its
implementation isn't compiled into the current Zephyr mbedtls build
(would require enabling additional TLS features). Use a local
ct_memeq() with `volatile uint8_t` accumulator instead — pattern
matches rweather/arduinolibs Crypto.cpp secure_compare().
Disassembly verified on rak3401_1watt (Thumb-2): loop branches on
the iterator pointer not the accumulator, result is load-modify-
stored to stack every iteration (volatile preserved), final return
uses clz+shift instead of a conditional branch on the value.
Spotted by nextgens during review of meshcore-dev/MeshCore#2556
mitigations.
Tracks upstream meshcore-dev/MeshCore#2556 — passwords sent as
plaintext over encrypted links are vulnerable to evil-twin/phishing
attacks (attacker advertises a repeater with the same name but a
different pubkey; user picks the wrong one and types the password).
The structural fix is a protocol change (PAKE adoption — SPAKE2,
OPAQUE, or HMAC-with-both-pubkeys) and must land synchronously
across all implementations. Diverging unilaterally would break
interop with Arduino-based companions and repeaters, so we wait
for upstream.
Two within-protocol mitigations applied server-side:
1. Constant-time password comparison. Replaced strcmp() in
RepeaterMesh::handleLoginReq with a local ct_memeq() helper.
Pads the received password to the full 16-byte storage size,
XOR-accumulates byte differences with no early exit. Compares
both admin and guest passwords unconditionally so timing is
identical regardless of which (if any) the attempt resembled.
Eliminates the timing oracle that lets an already-MITM
attacker recover the stored password byte-by-byte.
2. Failed-login rate limit. New login_fail_limiter(4, 180)
RateLimiter — 4 wrong-password attempts per 180s, matching the
existing anon_limiter pattern. Hitting the cap trips a distinct
LOG_WRN so operators see active brute-force attempts in logs.
Global rate (not per-sender) — simpler, no ACL state bloat;
trade-off documented in CRYPTO_AUDIT_INDEX.md.
What's NOT fixed: the wire protocol still carries plaintext
passwords. The evil-twin attack itself remains possible; these
mitigations raise the attacker's cost (no timing leak, no
brute-force at line rate) but don't replace the structural fix.
UI-side defenses (TOFU warnings on duplicate names, pubkey
fingerprint display) are valuable companion-side mitigations
but out of scope for this audit's server-side commit.
- track real RTC sync source on the joystick Time screen
- gate joystick-only UI helpers behind stub headers so non-joystick builds skip the extra code without #ifdef at every call site.
liquidraver