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.
The phone app interprets path_len with an unconditional `& 63`, so
the OUT_PATH_SENT (0xFE) sentinel we were writing into the
offline-queue frame became "62 hops" + 4 bogus path bytes in the
app's UI. There are 0 LoRa hops between the sender (us) and the
phone viewing the message — path_len = 0 is both correct and
renders cleanly as "direct / 0 hops". The "(>>✓) " / "(>>✗) " body
prefix still distinguishes wio-originated messages from incoming.
OUT_PATH_SENT remains the marker for the joystick UI's own local
entries (UnreadScreen, _ch_previews) — formatHopCount handles it
explicitly so it's safe there.
After broadcasting a group message, wait 5s to see if any neighbor
repeated the flood and use the outcome to mark the on-device entry
and the BLE-app mirror.
- ContentionTracker gets extractDupeCount(hash): finds the tracked
entry, captures dupe_count, finalizes (folds into EMA, marks
inactive), returns the count or -1.
- BaseChatMesh::sendGroupMessage gains an optional out_hash param;
when set, the FNV-1a packet hash is also pre-registered with the
contention tracker so heard retransmits get counted (originated
floods weren't tracked before, only relays).
- Mesh::getContentionTracker() promoted to public so the UI can
query after the feedback window.
JoystickUITask grows a 4-slot pending-channel table with per-slot
k_timer (5s one-shot). startPendingChannel() broadcasts, adds the
local _ch_previews entry with path_len = OUT_PATH_SENT, and starts
the feedback timer. The timer ISR sets a feedback_due flag; the
loop's processPendingChannelFeedback() picks it up, calls
extractDupeCount(), and rewrites the preview's path_len to
OUT_PATH_SENT_HEARD (0xFD) or OUT_PATH_SENT_UNHEARD (0xFC) — which
formatHopCount renders as "sent+" / "sent?".
The deferred BLE-app mirror queues only on outcome with body prefix
"(>>✓) " (heard) or "(>>✗) " (not heard). queueLocalSentChannelMessage
gains a heard_repeat parameter for the selection.
Both channel send entry points (sendComposedMessage's channel branch
and sendChannelMessage) now route through startPendingChannel().
Outgoing DMs now go through a 4-slot pending-send table with per-slot
one-shot k_timer. On no-ACK the message retries (up to 5 attempts,
0-indexed); attempt 4 clears recipient.out_path_len + markContactsDirty
so the last try forces flood and future DMs re-discover the path.
ACK dispatch: CompanionMesh::processAck tries _ack_table first
(phone-initiated sends), then ui_joystick_try_match_ack() for joystick-
initiated sends, then falls through to connection-keepalive ACKs.
BLE-app mirror is now deferred until outcome is known. The body prefix
in the offline-queue frame is "(>>✓) " on delivery or "(>>✗) " on
failure, replacing the previous unconditional "(>>) ". UnreadScreen's
sent-entry origin prefix is updated in place via markSentEntryStatus()
to "(>>+) " / "(>>X) " (ASCII for the OLED font).
queueLocalSentContactMessage gains a 'delivered' parameter to pick the
prefix; markContactsDirtyPublic() exposes the lazy-write trigger so
the joystick's path-clear persists.
BaseChatMesh::sendGroupMessage wraps the body as "<sender_name>: <body>"
before transmitting (the receiver's onChannelMessageRecv sees the full
"USER: MSG" string, which CompanionMesh queues as-is for the phone).
queueLocalSentChannelMessage was queuing just the raw body, so the
phone parsed an empty sender and lost the body. Prepend
"<prefs.node_name>: " to mirror the wire format.
joystick UI: prefix DM mirrors to phone with sent-marker
DM offline-queue frames identify the sender by pubkey field, not by
in-text prefix. queueLocalSentContactMessage was queuing with the
contact's pubkey + raw body, so the phone app rendered wio-originated
DMs identically to incoming ones from that contact. Prepend "(>>) "
to the body so sent messages are visually distinguishable; matches
the joystick UI's own sent indicator (UnreadScreen::addPreview).
liquidraver