Sign plaintext packets in licensed mode#10969
Open
RCGV1 wants to merge 8 commits into
Open
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.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughLicensed operation now generates and publishes identity keys, signs eligible plaintext traffic, prevents PKI decryption, sanitizes channels, and persists migration changes across boot, configuration, and restore flows. ChangesLicensed operation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AdminModule
participant NodeDB
participant MeshService
participant Router
participant NodeInfoModule
AdminModule->>NodeDB: generate licensed identity key
NodeDB->>NodeDB: mark migration pending
NodeDB->>MeshService: notify pending identity migration
Router->>Router: sign eligible plaintext licensed packet
Router->>Router: skip PKI decryption for licensed traffic
NodeInfoModule->>NodeInfoModule: publish identity public key
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
⚡ 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 (30)
Build artifacts expire on 2026-08-18. Updated for |
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Member
Author
|
CI label check requires an |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/modules/AdminModule.cpp`:
- Around line 978-987: Update the licensed first-region setup branch in
AdminModule.cpp after nodeDB->generateCryptoKeyPair() to persist all newly
generated identity state by saving SEGMENT_CONFIG | SEGMENT_MODULECONFIG |
SEGMENT_DEVICESTATE | SEGMENT_NODEDATABASE, ensuring the node number and owner
identity survive reboot.
In `@test/test_admin_radio/test_main.cpp`:
- Around line 968-976: In the test setup around testAdmin, preserve the existing
global owner and config.lora.region values before assigning test-specific
values, then restore both after the test completes, preferably with a small RAII
guard to cover failures and early exits. Use the NodeDB replacement block and
the licensed test setup as the integration point, ensuring restoration occurs
before teardown.
In `@test/test_packet_signing/test_main.cpp`:
- Around line 597-600: Shorten the Group C banner comment to one or two lines,
retaining only a concise section heading and the rationale that it covers
NodeInfoModule downgrade drops for broadcast ingress paths while unicast
NodeInfo is exempt.
🪄 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: 7d239e62-5359-4d18-a5d7-5a318749999c
📒 Files selected for processing (12)
src/main.cppsrc/mesh/Channels.cppsrc/mesh/NodeDB.cppsrc/mesh/NodeDB.hsrc/mesh/Router.cppsrc/modules/AdminModule.cppsrc/modules/AdminModule.hsrc/modules/NodeInfoModule.cpptest/support/AdminModuleTestShim.htest/support/MockMeshService.htest/test_admin_radio/test_main.cpptest/test_packet_signing/test_main.cpp
Member
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/mesh/RadioInterface.h (1)
259-262: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCondense the comment to a maximum of two lines.
As per coding guidelines, code comments should be kept to a maximum of one or two lines. Please condense this explanatory block.
♻️ Proposed refactor
- // Check if a candidate region is compatible and valid, with no side effects (safe for - // speculative UI checks). prospectiveLicensedOwner is for a UI flow that requires - // confirmation before it sets the owner licensed. errBuf, if given, receives the failure reason. + // Checks region compatibility without side effects. prospectiveLicensedOwner bypasses + // the license check for confirmation flows. errBuf receives the failure reason. static bool checkConfigRegion(const meshtastic_Config_LoRaConfig &loraConfig, char *errBuf = nullptr, size_t errLen = 0, bool prospectiveLicensedOwner = false);🤖 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/RadioInterface.h` around lines 259 - 262, Condense the explanatory comment above checkConfigRegion to no more than two lines while preserving its key details about prospectiveLicensedOwner and errBuf failure reporting.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/graphics/draw/MenuHandler.cpp`:
- Around line 288-297: Safeguard the selectedRegionInfo access before reading
profile->licensedOnly by validating the region result and profile pointer,
preserving the existing invalid-region handling in
RadioInterface::checkConfigRegion. Initialize regionErr when declaring it so
failure logging never reads uninitialized data. Condense the adjacent three-line
explanatory comment to no more than two lines without changing the
prospective-owner validation behavior.
---
Nitpick comments:
In `@src/mesh/RadioInterface.h`:
- Around line 259-262: Condense the explanatory comment above checkConfigRegion
to no more than two lines while preserving its key details about
prospectiveLicensedOwner and errBuf failure reporting.
🪄 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: c2d0e6aa-de53-4454-9b7f-6497cb0eca34
📒 Files selected for processing (4)
src/graphics/draw/MenuHandler.cppsrc/mesh/RadioInterface.cppsrc/mesh/RadioInterface.htest/test_admin_radio/test_main.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- test/test_admin_radio/test_main.cpp
Member
|
Tested. Confirmed this solves the issues with ham-mode causing nodes to regenerate their IDs upon every reboot. Have not yet tested that this is still sending unencrypted messages 😬 |
This was referenced Jul 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #10966
Design: meshtastic/design#122
Related receive-policy work: #10967
Related schema: meshtastic/protobufs#983
Summary
from | id | portnum | payloadtuple;This does not add an authentication version, signature format, certificate system, or new wire-auth scheme. Normal-mode broadcast signing and PKI direct messages are unchanged. A signed callsign-bearing NodeInfo self-binds that label to a key; it does not validate licence ownership, provide confidentiality, or by itself prevent replay.
Validation
test_admin_radio: 72/72 passedtest_packet_signing: 39/39 passedrak3172: successful buildgit diff --check: clean155c4f76289e760bd789d36f3254105f6b8e1702Draft / hardware status
No RF hardware verification has been performed. Before this leaves draft:
Operators remain responsible for their licence conditions, jurisdiction, permitted modes, and station-identification requirements.
Summary by CodeRabbit
New Features
Bug Fixes
Reliability