Skip to content

Packet Signing via XEdDSA#10478

Open
jp-bennett wants to merge 26 commits into
developfrom
XEdDSA-merge
Open

Packet Signing via XEdDSA#10478
jp-bennett wants to merge 26 commits into
developfrom
XEdDSA-merge

Conversation

@jp-bennett
Copy link
Copy Markdown
Collaborator

New PR after a particularly ugly merge.

jp-bennett and others added 12 commits May 13, 2026 12:02
* Generate a new node identity on key generation

* Fixes

* Fixes

* Fixes

* Messed up

* Fixes

* Update src/modules/AdminModule.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/mesh/NodeDB.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Figured it out!

* Cleanup

* Update src/mesh/NodeDB.h

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/mesh/NodeDB.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/modules/AdminModule.cpp

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* fix: add null check for getMeshNode() in NodeInfoModule

getMeshNode() can return nullptr for unknown nodes. Dereferencing
without a check crashes the firmware when receiving NodeInfo from
a node not yet in the database.

* fix: enforce XEdDSA signature verification and prevent stripping

Previously, failed signature verification still allowed the packet
through, making signatures purely cosmetic. Now:

- Failed verification drops the packet (DECODE_FAILURE)
- Successfully verified nodes get HAS_XEDDSA_SIGNED bitfield set
- Unsigned packets from previously-signing nodes are rejected
- Log levels reduced from WARN/ERROR to DEBUG/WARN as appropriate

* fix: include packet metadata in XEdDSA signature

The signature now covers [fromNode | packetId | portnum | payload]
instead of just the payload bytes. This prevents:
- Replay attacks (different packetId fails verification)
- Reattribution (different fromNode fails verification)
- Portnum redirection (different portnum fails verification)

Also adds a key initialization check to xeddsa_sign (returns false
if XEdDSA keys are all zeros) and checks the return value in the
encode path.

* fix: handle existing key pair in AdminModule security config

When a user provides both a valid private key and public key via
admin config, the crypto engine's DH private key and owner public
key were never loaded. DMs and XEdDSA signing would silently break.

Add an else branch to load both keys into the crypto engine.

* perf: cache Ed25519 public key conversion in xeddsa_verify

curve_to_ed_pub() performs field element parsing, inversion, and
multiplication on every call. Since packets from the same node
tend to arrive in bursts, a single-entry cache avoids repeating
this expensive conversion for consecutive packets from one sender.

* fix: skip identity cleanup when node number is unchanged

createNewIdentity() was called on every generateCryptoKeyPair(),
including normal boots where the same key is regenerated. This
caused unnecessary NodeDB writes and old-node cleanup logic to
run when the node number hadn't actually changed.

Also fixes only zeroing byte[0] of the old node's public key
instead of clearing the entire array.

* fix: replace hardcoded 120 with derived XEDDSA_SIGNATURE_SIZE constant

The payload size check for XEdDSA signing used a magic number (120).
Replace with a derivation from DATA_PAYLOAD_LEN and XEDDSA_SIGNATURE_SIZE
so the limit adjusts automatically if constants change. This also
increases the max signable payload from 120 to 169 bytes, which is
still safe since the actual encoded size is checked after pb_encode.

* fix: add const qualifiers to XEdDSA verify and curve_to_ed_pub inputs

pubKey, payload, and signature parameters in xeddsa_verify are
input-only and should not be modified. Same for curve_pubkey in
curve_to_ed_pub.

* chore: remove commented-out old Crypto dependency in portduino.ini

* Leave out the admin module change for now

---------

Co-authored-by: Jonathan Bennett <jbennett@incomsystems.biz>
@github-actions github-actions Bot added needs-review Needs human review enhancement New feature or request labels May 14, 2026
@thebentern thebentern requested a review from Copilot May 14, 2026 19:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces XEdDSA-based packet signing support across the firmware (signing on transmit, verification on receive, and persistence of “signer” state per node), adds unit tests for the signing primitives, and switches multiple PlatformIO environments to a Meshtastic-hosted Crypto library fork to pick up the required crypto APIs.

Changes:

  • Switch platform lib_deps from rweather/Crypto (and STM32’s previous Crypto source) to a pinned meshtastic/Crypto GitHub zip across several variants.
  • Add XEdDSA sign/verify support in CryptoEngine and integrate signature verify + “previously signed” enforcement in Router.
  • Consolidate PKI key generation in NodeDB and add XEdDSA unit tests.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 15 comments.

Show a summary per file
File Description
variants/stm32/stm32.ini Switch STM32 Crypto dependency to Meshtastic fork (pinned zip).
variants/rp2350/rp2350.ini Switch RP2350 Crypto dependency to Meshtastic fork (pinned zip).
variants/rp2040/rp2040.ini Switch RP2040 Crypto dependency to Meshtastic fork (pinned zip).
variants/nrf52840/nrf52.ini Switch nRF52 Crypto dependency to Meshtastic fork (pinned zip).
variants/native/portduino.ini Switch Portduino Crypto dependency to Meshtastic fork (pinned zip).
variants/esp32c6/esp32c6.ini Switch ESP32-C6 Crypto dependency to Meshtastic fork (pinned zip).
variants/esp32/esp32-common.ini Switch ESP32 common Crypto dependency to Meshtastic fork (pinned zip).
test/test_crypto/test_main.cpp Add XEdDSA sign/verify unit tests.
src/modules/NodeInfoModule.cpp Reject unsigned NodeInfo from nodes previously observed signing.
src/modules/AdminModule.cpp Rework security config handling to use NodeDB keypair generation + force reboot/save segments.
src/mesh/TypeConversions.cpp Expose per-node “has_xeddsa_signed” status in NodeInfo conversion.
src/mesh/Router.cpp Verify signatures on decode; sign outgoing broadcasts when size allows; enforce “previously signed” policy.
src/mesh/NodeDB.h Add NodeDB crypto keypair helper + new bitfield flag/accessor for XEdDSA signing.
src/mesh/NodeDB.cpp Implement consolidated crypto key generation and identity switching logic.
src/mesh/CryptoEngine.h Add XEdDSA signing API + cache fields and Curve25519→Ed25519 pubkey conversion hook.
src/mesh/CryptoEngine.cpp Implement XEdDSA signing/verification and Curve25519→Ed25519 pubkey conversion.

Comment thread src/mesh/Router.cpp
Comment on lines +544 to +548
// Unsigned packet — reject if this node previously sent signed packets
meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(p->from);
if (node && nodeInfoLiteHasXeddsaSigned(node)) {
LOG_WARN("Dropping unsigned packet from 0x%08x that previously signed", p->from);
return DecodeState::DECODE_FAILURE;
Comment thread src/mesh/Router.cpp Outdated
Comment thread src/modules/NodeInfoModule.cpp Outdated
Comment thread src/mesh/NodeDB.cpp
}

bool keygenSuccess = false;
bool lowEntropy = checkLowEntropyPublicKey(config.security.public_key);
Comment thread src/mesh/NodeDB.cpp
Comment on lines +2783 to +2787
// Retire the old node entry
meshtastic_NodeInfoLite *node = getMeshNode(oldNodeNum);
if (node != NULL) {
LOG_DEBUG("Old node num %u is now %u", oldNodeNum, newNodeNum);
nodeInfoLiteSetBit(node, NODEINFO_BITFIELD_IS_IGNORED_MASK, true);
Comment thread variants/native/portduino.ini Outdated
Comment thread variants/esp32c6/esp32c6.ini Outdated
Comment thread variants/esp32/esp32-common.ini Outdated
Comment thread test/test_crypto/test_main.cpp Outdated
Comment thread src/mesh/CryptoEngine.cpp
Comment on lines +136 to +137
// format of Ed25519 public keys only in a single sign bit. This function assumes that the sign bit is
// known to the user and is passed accordingly.
@Xaositek
Copy link
Copy Markdown
Contributor

Should #7602 be closed in favor of this?

thebentern and others added 11 commits May 15, 2026 16:01
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@thebentern
Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented May 19, 2026

@copilot resolve the merge conflicts in this pull request

Resolved the merge conflict and pushed the merge commit in 80727e8.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • api.registry.nm1.platformio.org
    • Triggering command: /home/REDACTED/.local/bin/pio /home/REDACTED/.local/bin/pio test -e native -f test_crypto (dns block)
    • Triggering command: /home/REDACTED/.local/bin/pio /home/REDACTED/.local/bin/pio test -e native -f test_crypto -v ndor/bin/git (dns block)
  • api.registry.platformio.org
    • Triggering command: /home/REDACTED/.local/bin/pio /home/REDACTED/.local/bin/pio test -e native -f test_crypto (dns block)
    • Triggering command: /home/REDACTED/.local/bin/pio /home/REDACTED/.local/bin/pio test -e native -f test_crypto -v ndor/bin/git (dns block)
  • collector.platformio.org
    • Triggering command: /home/REDACTED/.local/bin/pio /home/REDACTED/.local/bin/pio test -e native -f test_crypto (dns block)
    • Triggering command: /home/REDACTED/.local/bin/pio /home/REDACTED/.local/bin/pio project config --environment tlora_c6 --json-output (dns block)
    • Triggering command: /home/REDACTED/.local/bin/pio /home/REDACTED/.local/bin/pio project config -h (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI requested a review from thebentern May 19, 2026 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request needs-review Needs human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants