Sign explicit acks under Strict; bind request_id/reply_id in the XEdDSA signing buffer

[jp-bennett]

What this does

Explicit acks/naks (ROUTING_APP unicasts) are never signed today, and ROUTING_APP is PKC-excluded, so ack authenticity rests entirely on the channel PSK — which is public knowledge on default LongFast. A forged ack fakes delivery confirmation and, worse, makes honest relayers stopRetransmission() for the real packet. Strict-policy meshes have it worst: Strict drops unsigned non-PKI packets, acks are unsigned, so two Strict peers can never complete a reliable delivery at all.

Two changes:

1. Strict signs explicit acks/naks. The perhapsEncode sign gate gains a third class: self-originated ROUTING_APP unicasts are signed when packet_signature_policy == STRICT (licensed/ham signing is unchanged; Balanced/Compatible keep today's unsigned acks for interop). The Balanced downgrade mirror deliberately does not mirror this clause — a receiver can't know the sender's policy, so unsigned unicast acks are never treated as downgrades.

2. The signing buffer binds request_id/reply_id when present. Channel crypto is AES-CTR with no MAC, so without this a signed ack's request_id is malleable in flight — a captured "delivered" ack could be retargeted at a different outstanding request (and a signed tapback re-pointed at a different message). Packets where either field is nonzero now sign over [from | id | portnum | request_id | reply_id | payload].

Backwards compatibility

Packets with no request/reply linkage (broadcasts, NodeInfo, position/telemetry, licensed traffic without request ids — the overwhelming majority of signed traffic) keep the existing [from | id | portnum | payload] layout byte-for-byte, so their signatures verify across the upgrade boundary in both directions. The only pre-existing traffic whose signatures change are signed tapbacks/replies — exactly the retargetable class. Explicit acks were never signed before, so nothing existing breaks there. Both sides derive the layout from the packet's own decoded fields; no wire-format flag is added and no protobuf changes are needed. The conditional layout's theoretical ambiguity (and why it isn't exploitable for acks) is documented at buildSigningBuffer.

XEdDSA has shipped in no published release (2.8.0 is draft-only), so the tapback-signature change affects draft-alpha testers only.

Also in this PR

• test_packet_signing B11/B12 (PKC unicast tests) failed at the branch base: the native harness boots with -s, and wouldEncryptWithPKC() hard-disables PKC under force_simradio. The suite now models a real device in setUp, the same documented workaround test_admin_session_repro uses.

Known limitations

• Strict↔Balanced asymmetry is by design: a Balanced sender's acks to a Strict receiver are still dropped; this fixes Strict↔Strict (and Strict-receiving-licensed).
• Pre-existing quirk, unchanged: MeshService::sendRoutingErrorResponse acks mp->from raw, so a phone-originated packet yields a to=0 ack, which Strict now signs harmlessly. Worth a follow-up issue.
• Each explicit ack under Strict costs one Ed25519 sign at the sender and one verify at the receiver — ms-class on nRF52/ESP32, and acks are low-rate.

Testing

• ./bin/run-tests.sh — GREEN 44/44 suites, 352/352 test cases, all CLEAN.
• New coverage: crypto-primitive negative flips for both new fields; a four-direction base/extended format compatibility test (test_XEdDSA_base_format_backwards_compat); B14/B15 (Strict signs a real allocAckNak product, Balanced/Compatible don't); E14/E15 (retargeted request_id/reply_id dropped); A14b (solicited first-contact NodeInfo with nonzero request_id still bootstraps); MQTT ingress signed-ack verify + retarget-drop.
• Not tested on hardware — native test suite only. Community testing on real devices (especially a Strict↔Strict pair exchanging DMs) would be appreciated.

🤝 Attestations

• I have tested that my proposed changes behave as described.
• I have tested that my proposed changes do not cause any obvious regressions on the following devices:
  • Heltec (Lora32) V3
  • LilyGo T-Deck
  • LilyGo T-Beam
  • RAK WisBlock 4631
  • Seeed Studio T-1000E tracker card
  • Other (please specify below): native (portduino) test suite only; no physical hardware available to this environment.

🤖 Generated with Claude Code

https://claude.ai/code/session_012hLcVif8GDEmA2k77hmFG8

---

Generated by Claude Code

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 824cc67b-a82e-42a5-9d87-2165984eb167

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

⚡ Try this PR in the Web Flasher

Note

Building this pull request… the flash button, badges and supported-board
list will appear here automatically once CI finishes.