Authorize signed plaintext remote admin in licensed mode#10972
Conversation
Preserve and publish identity keys in licensed mode so existing XEdDSA signatures can authenticate plaintext traffic. Sign licensed broadcasts and direct messages when they fit, while keeping PKI encryption disabled and normal routing behavior unchanged.
Stack the packet authenticity routing gate with licensed plaintext signing, then expose the exact verified signer key only during authenticated module dispatch. Licensed remote Admin requests now require directed signed plaintext and an allowlisted admin key, while responses remain signed plaintext and normal PKI administration is unchanged. Close the uninitialized zero-session window and add focused authorization/session regression coverage.
Remove the process-global signer context and carry only the exact Router-verified key on the authenticated packet. Clear serialized authentication metadata before every untrusted decode, require both the verified-signature marker and packet-local key in licensed Admin authorization, and expand authorization/session/isolation coverage while cleaning routing test ownership.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change centralizes XEdDSA authentication before routing state updates, supports opaque packet relay, adds licensed identity migration and channel sanitization handling, authorizes signed plaintext remote administration, and expands policy, persistence, and pipeline test coverage. ChangesLicensed XEdDSA flow
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Radio
participant Router
participant AdminModule
participant NodeDB
Radio->>Router: receive signed plaintext admin packet
Router->>Router: verify XEdDSA and apply routing-auth gate
Router->>AdminModule: deliver accepted request
AdminModule->>NodeDB: validate licensed identity and session state
NodeDB-->>AdminModule: return authorization and migration state
AdminModule-->>Router: send directed plaintext reply
Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
⚡ Try this PR in the Web Flasher
Warning This is an automated, unreviewed CI test build. Back up your device configuration Supported boards built by this PR (27)
Build artifacts expire on 2026-08-10. Updated for |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
CI label check requires an |
Make the routing authentication gate input const and collapse licensed Admin authorization into a single branch so legacy response, local, channel, and PKI checks run only for non-licensed-remote traffic.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
CI dependency status: the current Native Simulator, WASM, and Docker failures stop at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/mesh/FloodingRouter.cpp (1)
71-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep this rationale to two lines.
This three-line comment exceeds the repository’s comment limit.
Proposed cleanup
- // Verify the replacement before deleting the valid lower-hop copy waiting in the TX queue. - // This is intentionally redundant with ReliableRouter's ingress gate: it keeps this helper - // safe if another caller is introduced later. + // Re-authenticate before replacing the queued lower-hop copy so future callers remain safe.As per coding guidelines, “Keep code comments minimal: one or two lines maximum.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mesh/FloodingRouter.cpp` around lines 71 - 73, Shorten the explanatory comment above the replacement verification in FloodingRouter to no more than two lines while preserving its key rationale: verify the replacement before deleting the valid lower-hop TX-queued copy, even though ReliableRouter already enforces this at ingress.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mesh/Router.cpp`:
- Around line 98-105: Guard storeRoutingAuthCache with an early return when
routingAuthCacheLock is null, matching routingAuthCacheMatches,
applyRoutingAuthCache, and clearRoutingAuthCache, before constructing the
LockGuard.
- Around line 655-671: Sanitize the already-decoded fallback packet before
returning from perhapsDecode(), because the routing-auth cache may be clobbered
between passesRoutingAuthGate() and handleReceived(). In the
authCandidate.which_payload_variant == meshtastic_MeshPacket_decoded_tag path,
clear serialized authentication fields directly on *p (including xeddsa_signed,
public_key, and pki_encrypted) before copying or caching the packet, or
otherwise make the gate-to-handle handoff atomic.
In `@test/test_admin_radio/test_main.cpp`:
- Around line 1029-1066: Restore all modified global test state before each test
returns. In test_bootDefense_sanitizesStaleLicensedChannelsOnce and
test_restorePreferences_sanitizesLicensedBackupBeforeReturn, save the original
owner and relevant channel state at the start, then restore them on every exit
path; ensure the restore test also cleans up its temporary NodeDB and backup
resources. Use the existing test setup/cleanup patterns so later tests do not
inherit licensed or sanitized state.
---
Nitpick comments:
In `@src/mesh/FloodingRouter.cpp`:
- Around line 71-73: Shorten the explanatory comment above the replacement
verification in FloodingRouter to no more than two lines while preserving its
key rationale: verify the replacement before deleting the valid lower-hop
TX-queued copy, even though ReliableRouter already enforces this at ingress.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ec360569-c027-4563-9843-1875992d71d6
📒 Files selected for processing (24)
src/main.cppsrc/mesh/Channels.cppsrc/mesh/FloodingRouter.cppsrc/mesh/FloodingRouter.hsrc/mesh/NextHopRouter.cppsrc/mesh/NextHopRouter.hsrc/mesh/NodeDB.cppsrc/mesh/NodeDB.hsrc/mesh/Router.cppsrc/mesh/Router.hsrc/mesh/WarmNodeStore.hsrc/mesh/udp/UdpMulticastHandler.hsrc/modules/AdminModule.cppsrc/modules/AdminModule.hsrc/modules/NodeInfoModule.cppsrc/mqtt/MQTT.cpptest/support/AdminModuleTestShim.htest/support/MockMeshService.htest/test_admin_radio/test_main.cpptest/test_mqtt/MQTT.cpptest/test_packet_signing/test_main.cppvariants/stm32/CDEBYTE_E77-MBL/platformio.inivariants/stm32/rak3172/platformio.inivariants/stm32/stm32.ini
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Closes #10971
Design: meshtastic/design#123
Stack dependencies
This is a stacked draft that combines and depends on:
Review and merge the prerequisite drafts first. This PR intentionally targets
developbecause both prerequisite heads are contributor branches, so its current diff contains their changes as merge parents.Summary
security.admin_key[]entry;TOO_LARGEwhen the packet cannot carry its required signature;from | id | portnum | payloadtuple.This does not introduce an authentication version, new signature format, certificate system, new protobuf field, or encryption in licensed mode.
Security properties
MeshPacket.public_keyandxeddsa_signedare cleared at untrusted ingress and become authoritative only after successful Router verification for that packet.Validation
git diff --check: clean;4cd335d03fd9498cc3c8fa12cfc51b5834d65e5a.Local full native execution is host-blocked by existing macOS/Linux build incompatibilities: NotificationRenderer initialized VLAs, Linux input headers, and the coverage environment's GCC-only
-fprofile-abs-pathunder Apple Clang. GitHub Linux CI is the executable validation source for this stacked draft.Draft / hardware status
No RF hardware validation has been performed. Before this leaves draft:
pki_encrypted=falseand uses verified-signature shield semantics rather than an encryption lock.Operators remain responsible for their licence conditions, jurisdiction, permitted modes, and station-identification requirements.
Summary by CodeRabbit
🤝 Attestations