feat(security): surface XEdDSA packet signing in node & messaging UI#1993
Open
bruschill wants to merge 8 commits into
Open
feat(security): surface XEdDSA packet signing in node & messaging UI#1993bruschill wants to merge 8 commits into
bruschill wants to merge 8 commits into
Conversation
Affirm when a node and its broadcast packets are cryptographically signed and verified (XEdDSA), per design#113 and mirroring Android #5976/#5980. Read-only: the radio drops bad signatures before the client sees them, so there is no error state — clients only ever affirm the good state. - Messaging: green checkmark.shield.fill on verified signed broadcast bubbles; "Signed · verified" + "Verified with the sender's key." in the message context menu. Never on DMs. - Node details: green "Signed node / Verified automatically" row driven by has_xeddsa_signed, above the public-key row (most-trusted-first), phrased as observed. - Wiring: MeshPacket.xeddsa_signed (22) -> MessageEntity.xeddsaSigned (gated on broadcast addr); NodeInfo.has_xeddsa_signed (14) -> NodeInfoEntity.hasXeddsaSigned (latched, "persists / >=1 verified") across all node-info ingest paths. - Consolidated the bubble corner badges (lock/shield/envelope) into one side-by-side row so multiple badges no longer overlap. Protobuf note: the two fields are hand-added to the generated mesh.pb.swift because the pinned protobufs submodule predates 2.8. Bump the submodule to a version carrying these fields before merge; gen_protos.sh against the current pin would erase the hand-edit. Closes meshtastic#1992
…-ui-1992 # Conflicts: # MeshtasticProtobufs/Sources/meshtastic/mesh.pb.swift
Update docs/user/messages.md (signing shield + Message Details copy), docs/user/nodes.md (Signed node row), and whats-new; regenerate the bundled in-app HTML docs via scripts/build-docs.sh.
Adds behavioral tests driving the actual MeshPackets ingestion paths: - nodeInfoPacket sets/leaves NodeInfoEntity.hasXeddsaSigned and latches it across updates (a later NodeInfo omitting the bit must not downgrade). - textMessageAppPacket sets MessageEntity.xeddsaSigned on a signed broadcast but never on a signed direct message (DM-safety gate).
Contributor
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for surfacing radio-verified XEdDSA signing (signed/verified broadcasts only) across persistence, ingestion, UI affordances, tests, and user docs.
Changes:
- Persist XEdDSA signing flags onto
MessageEntity(per-message) andNodeInfoEntity(node-level, latched) during packet ingestion. - Update Messages UI (bubble corner badges + Message Details context) and Node Detail UI to affirm verified signing state.
- Add Swift Testing coverage plus regenerated/updated user documentation and docs index metadata.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| MeshtasticTests/XEdDSASigningTests.swift | Adds wire round-trip + ingestion tests for signing flags. |
| Meshtastic/Views/Nodes/Helpers/NodeDetail.swift | Shows “Signed node / Verified automatically” row when observed. |
| Meshtastic/Views/Messages/MessageText.swift | Adds shield badge and fixes corner badge overlap by laying out side-by-side. |
| Meshtastic/Views/Messages/MessageContextMenuItems.swift | Adds “Signed · verified” + explanatory line in message details. |
| Meshtastic/Resources/docs/user/whats-new.html | Documents new signing UI in What’s New (HTML bundle). |
| Meshtastic/Resources/docs/user/nodes.html | Documents Signed Node behavior (HTML bundle). |
| Meshtastic/Resources/docs/user/messages.html | Documents signing vs encryption distinction (HTML bundle). |
| Meshtastic/Resources/docs/markdown/user/whats-new.md | What’s New entry (bundled markdown source). |
| Meshtastic/Resources/docs/markdown/user/nodes.md | Signed Node section (bundled markdown source). |
| Meshtastic/Resources/docs/markdown/user/messages.md | Signing section (bundled markdown source). |
| Meshtastic/Resources/docs/index.json | Updates docs search index metadata/keywords/char counts. |
| Meshtastic/Persistence/UpdateSwiftData.swift | Persists/latches hasXeddsaSigned during NodeInfo upsert paths. |
| Meshtastic/Model/NodeInfoEntity.swift | Adds hasXeddsaSigned SwiftData property. |
| Meshtastic/Model/MessageEntity.swift | Adds xeddsaSigned SwiftData property. |
| Meshtastic/Helpers/MeshPackets.swift | Sets xeddsaSigned for broadcasts only; persists/latches node flag. |
| Meshtastic.xcodeproj/project.pbxproj | Adds new test file to the test target. |
| Localizable.xcstrings | Adds new localized string keys for signing UI copy. |
| docs/user/whats-new.md | Updates user docs (repo markdown source). |
| docs/user/nodes.md | Updates user docs (repo markdown source). |
| docs/user/messages.md | Updates user docs (repo markdown source). |
Comment on lines
+4
to
+8
| // Covers the XEdDSA packet-signing flags surfaced in the UI (design#113 / issue #1992): | ||
| // - MeshPacket.xeddsa_signed (field 22) → MessageEntity.xeddsaSigned | ||
| // - NodeInfo.has_xeddsa_signed (field 14) → NodeInfoEntity.hasXeddsaSigned | ||
| // The protobuf fields are hand-added to the generated sources, so these tests also guard the | ||
| // binary wire round-trip for both fields. |
- Add node-row signing shield to NodeListItem + NodeListItemCompact so the
checkmark.shield.fill ("Signed node") appears in the Nodes list rows
alongside the other security icons, fulfilling issue meshtastic#1992 (shield was
previously only in Node Detail and Messages UI).
- Give the four new Strings Catalog entries explicit `en` localization units
instead of empty objects, so they're tracked for translation.
- Replace the non-hex pbxproj object IDs for XEdDSASigningTests.swift
(…XEDDSA…) with valid 24-char hex IDs.
- Reword test header/MARK comments that wrongly described the protobuf fields
as hand-edited generated code; they come from the upstream 2.8 protos.
Contributor
Author
|
@thebentern all cleaned up 👍🏻 |
…-ui-1992 # Conflicts: # Meshtastic.xcodeproj/project.pbxproj # Meshtastic/Resources/docs/index.json # Meshtastic/Resources/docs/markdown/user/whats-new.md # Meshtastic/Resources/docs/user/whats-new.html # docs/user/whats-new.md
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds XEdDSA signing state to message and node models, persists it from incoming packets, displays signed indicators in message and node UI, and updates tests, localized strings, and docs. ChangesXEdDSA Signing UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@docs/user/messages.md`:
- Line 80: The user-facing text is attributing “Signed · verified” to the wrong
UI surface. Update the copy in the message docs so it reflects that
MessageContextMenuItems shows “Signed · verified” in the top-level context menu,
while Message Details only contains “Verified with the sender's key.” Keep the
rest of the guidance about broadcast/channel messages and direct messages
aligned with that distinction.
In `@Meshtastic/Helpers/MeshPackets.swift`:
- Around line 1142-1145: The signing gate in MeshPackets.swift is too strict
because it only treats packets addressed to Constants.maximumNodeNum as
broadcasts, while the same receive path later already recognizes
storeForwardBroadcast as a broadcast-like message. Update the xeddsaSigned
assignment in the received packet handling logic to include the
storeForwardBroadcast case alongside the broadcast address check, using the
existing packet classification in the same method so valid signed
store-and-forward broadcasts keep their verified state.
In `@Meshtastic/Views/Nodes/Helpers/NodeListItem.swift`:
- Around line 220-226: The signed-node trust signal is only shown visually in
NodeListItem, so VoiceOver misses it because the row uses
accessibilityDescription(...) as its custom label. Update the accessibility text
path used by NodeListItem (and any shared helper it relies on) to append
node.hasXeddsaSigned with a clear “Signed node” or equivalent phrase, matching
the existing shield icon state so the spoken output mirrors the visual row.
In `@Meshtastic/Views/Nodes/Helpers/NodeListItemCompact.swift`:
- Around line 217-223: The compact row height logic in NodeListItemCompact is
missing the new signed-node display line, so signed nodes can be sized too
short. Update the height calculation that derives circleSize from lineNums to
also account for the node.hasXeddsaSigned branch, keeping it in sync with the
rendered rows in NodeListItemCompact and IconAndText so compact mode reserves
space whenever the “Signed node” row appears.
In `@MeshtasticTests/XEdDSASigningTests.swift`:
- Around line 21-58: The current tests only verify self round-tripping, so they
can miss protobuf wire-format regressions. Update the MeshPacket and NodeInfo
tests to assert the raw serialized bytes produced by `serializedData()` for the
xeddsaSigned fields, specifically checking the expected field tags for field 22
and field 14, and verify the default-false cases emit no payload on the wire.
Use the existing `MeshPacket`, `NodeInfo`, `serializedData()`, and
`serializedData(serializedData:)` paths to compare exact emitted bytes rather
than relying only on encode→decode assertions.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0f72e673-dc3b-4833-9dbc-10293ef83f12
📒 Files selected for processing (22)
Localizable.xcstringsMeshtastic.xcodeproj/project.pbxprojMeshtastic/Helpers/MeshPackets.swiftMeshtastic/Model/MessageEntity.swiftMeshtastic/Model/NodeInfoEntity.swiftMeshtastic/Persistence/UpdateSwiftData.swiftMeshtastic/Resources/docs/index.jsonMeshtastic/Resources/docs/markdown/user/messages.mdMeshtastic/Resources/docs/markdown/user/nodes.mdMeshtastic/Resources/docs/markdown/user/whats-new.mdMeshtastic/Resources/docs/user/messages.htmlMeshtastic/Resources/docs/user/nodes.htmlMeshtastic/Resources/docs/user/whats-new.htmlMeshtastic/Views/Messages/MessageContextMenuItems.swiftMeshtastic/Views/Messages/MessageText.swiftMeshtastic/Views/Nodes/Helpers/NodeDetail.swiftMeshtastic/Views/Nodes/Helpers/NodeListItem.swiftMeshtastic/Views/Nodes/Helpers/NodeListItemCompact.swiftMeshtasticTests/XEdDSASigningTests.swiftdocs/user/messages.mddocs/user/nodes.mddocs/user/whats-new.md
CodeRabbit feedback: - Treat store-and-forward router broadcasts as broadcasts in the xeddsaSigned gate so a valid signed S&F broadcast keeps its verified shield (they are addressed to the local node but classified as channel broadcasts). - Announce the 'Signed node' trust signal to VoiceOver in the node list (NodeListItem) and the compact list (NodeListItemCompact), mirroring the shield. - Count the signed-node row in the compact avatar height (lineNums). - Correct the docs: 'Signed · verified' is the context-menu label; Message Details only adds 'Verified with the sender's key.' - Assert the raw protobuf wire bytes for MeshPacket field 22 / NodeInfo field 14 and their default omission, not just self round-trips. Also (code-review): classify S&F broadcasts as broadcasts in the unified-log redaction too (was logged as '(DM)'); add an S&F signed-broadcast regression test; dedupe the test packet scaffold. Addresses CodeRabbit + code-review feedback on meshtastic#1992.
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
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 #1992 · Spec: design#113 · Reference: Android #5976 + #5980
What changed?
Surfaces when a node and its broadcast packets are cryptographically signed and verified (XEdDSA). Read-only — the radio does all crypto and drops invalid/stripped signatures before the client sees them, so there is no error state; we only ever affirm the good state. 🛡️ shield = authentic (verified signed broadcast); 🔒 lock = private (PKI-encrypted DM) is left untouched.
checkmark.shield.fillonxeddsa_signedbubbles. Per-message. Never on DMs. The lock/shield/store-forward corner badges are now laid out side-by-side so they no longer overlap when more than one applies.has_xeddsa_signed, placed above the public-key (has-key) row so the security area reads most-trusted-first. Phrased as observed ("Signed node"), labelled as automatic trust — distinct from user-asserted key verification.Wiring
MeshPacket.xeddsa_signed(field 22) →MessageEntity.xeddsaSigned, set inMeshPackets.textMessageAppPacket. Gated on the broadcast address as defense-in-depth so the shield can never appear on a DM even if a stray/spoofed packet carries the flag.NodeInfo.has_xeddsa_signed(field 14) →NodeInfoEntity.hasXeddsaSigned, set inMeshPackets.nodeInfoPacketandUpdateSwiftData.upsertNodeInfoPacket. Latched (x = x || new) on update paths because the field means "≥1 verified" and persists — a later NodeInfo that omits the bit must not downgrade a node we've seen sign.Why did it change?
Firmware 2.8 signs unencrypted broadcasts with XEdDSA and verifies them on-radio. This is the iOS client side of the cross-platform tracking issue (design#113); Android already shipped it. Per the spec's v1 decisions: two affordances (shield + lock, not merged), unsigned traffic is strictly silent, no red/error styling, and the "unverifiable" state is dropped (verified-bool only).
Important
Protobuf scope. The two fields are hand-added to the generated
mesh.pb.swift(accessor, storage, decode/traverse/equality, and the_NameMapbytecode token) because the pinnedprotobufssubmodule predates 2.8 and doesn't define them. This keeps the PR focused on XEdDSA and avoids dragging in the unrelated, build-breaking churn (TrafficManagementConfig redesign, newmeshBeaconAppportnum) that rides along with a full submodule bump. Resolved: upstreammainhas since landed the real 2.8 protobuf bump (submodulef680aace). This branch was merged withmainand the hand-edit was dropped —mesh.pb.swiftnow matches upstream's generated code, and the feature's UI/ingestion/tests merged cleanly and build green. No hand-edited protobuf remains.How is this tested?
MeshtasticTests/XEdDSASigningTests.swift(6 tests, all green): binary wire round-trip for both fields (guards the hand-edited generated code), proto3 default-omission, and entity defaults.Screenshots/Videos (when applicable)
Signing flags only populate when connected to a 2.8 node that signs; screenshots to follow once the protobufs submodule is bumped.
Checklist
docs/user/ordocs/developer/, and updated accordingly. Updateddocs/user/messages.md,docs/user/nodes.md, andwhats-new, and regenerated the bundled HTML.Summary by CodeRabbit
New Features
Bug Fixes
Documentation / Tests