Skip to content

Some fixes for xeddsa pr#9610

Merged
jp-bennett merged 10 commits into
meshtastic:XEdDSAfrom
weebl2000:fix/xeddsa-review
May 13, 2026
Merged

Some fixes for xeddsa pr#9610
jp-bennett merged 10 commits into
meshtastic:XEdDSAfrom
weebl2000:fix/xeddsa-review

Conversation

@weebl2000

@weebl2000 weebl2000 commented Feb 11, 2026

Copy link
Copy Markdown
Contributor
  • fix null dereference crash in NodeInfoModule when receiving NodeInfo from unknown nodes
  • signature verification failure didn't drop the packet — now it does
  • HAS_XEDDSA_SIGNED bitfield was never set — now set on successful verification, so unsigned packets from previously-signing nodes are rejected
  • Signature only covered payload bytes — now covers metadata too: fromNode, packetId, and portnum to prevent replay, reattribution, and portnum redirection
  • xeddsa_sign() had no key initialization check — now returns false if keys are all zeros
  • AdminModule didn't load keys when user provided both private and public key
  • curve_to_ed_pub() field inversion ran on every signed packet — now cached per sender
  • createNewIdentity() ran unnecessarily on every boot when the key hadn't changed
  • Hardcoded 120 byte signing limit replaced with derivation from XEDDSA_SIGNATURE_SIZE and DATA_PAYLOAD_LEN
  • Added const qualifiers to input-only parameters in xeddsa_verify and curve_to_ed_pub

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.
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
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.
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.
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.
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.
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.
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.
@github-actions github-actions Bot added needs-review Needs human review bugfix Pull request that fixes bugs labels Feb 11, 2026
Comment thread arch/portduino/portduino.ini
Comment thread src/mesh/NodeDB.cpp
Comment thread src/mesh/Router.cpp
Comment thread src/modules/NodeInfoModule.cpp
Comment thread src/modules/AdminModule.cpp Outdated
@jp-bennett jp-bennett merged commit 27dd628 into meshtastic:XEdDSA May 13, 2026
4 checks passed
thebentern added a commit that referenced this pull request Jun 13, 2026
* Test commit for XEdDSA support

* Update to Crypto lib in Meshtatic org

* Generate a new node identity on key generation (#7628)

* 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>

* Update crypto commit hash

* Some fixes for xeddsa pr (#9610)

* 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>

* trunk

* protobuf re-update

* Protobufs

* Merge resolution fix

* Put XEDDSA on the right bit

* NodeDB update to new nodeInfoLite accessors, etc

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Refine unsigned packet rejection logic in Router (#10534)

* use hardware random to fill the first 32 signature bytes with entropy prior to signing.

* Add XEdDSA packet-signing policy tests and update dependencies for macos

* Minor fixes

* integrate XEdDSA support and update dependencies across multiple modules

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Wessel <github@weebl.me>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com>
oscgonfer pushed a commit that referenced this pull request Jun 14, 2026
* Test commit for XEdDSA support

* Update to Crypto lib in Meshtatic org

* Generate a new node identity on key generation (#7628)

* 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>

* Update crypto commit hash

* Some fixes for xeddsa pr (#9610)

* 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>

* trunk

* protobuf re-update

* Protobufs

* Merge resolution fix

* Put XEDDSA on the right bit

* NodeDB update to new nodeInfoLite accessors, etc

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Refine unsigned packet rejection logic in Router (#10534)

* use hardware random to fill the first 32 signature bytes with entropy prior to signing.

* Add XEdDSA packet-signing policy tests and update dependencies for macos

* Minor fixes

* integrate XEdDSA support and update dependencies across multiple modules

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Wessel <github@weebl.me>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com>
raghumad pushed a commit to raghumad/mezulla-firmware that referenced this pull request Jun 25, 2026
* Test commit for XEdDSA support

* Update to Crypto lib in Meshtatic org

* Generate a new node identity on key generation (meshtastic#7628)

* 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>

* Update crypto commit hash

* Some fixes for xeddsa pr (meshtastic#9610)

* 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>

* trunk

* protobuf re-update

* Protobufs

* Merge resolution fix

* Put XEDDSA on the right bit

* NodeDB update to new nodeInfoLite accessors, etc

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Potential fix for pull request finding

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

* Refine unsigned packet rejection logic in Router (meshtastic#10534)

* use hardware random to fill the first 32 signature bytes with entropy prior to signing.

* Add XEdDSA packet-signing policy tests and update dependencies for macos

* Minor fixes

* integrate XEdDSA support and update dependencies across multiple modules

---------

Co-authored-by: Ben Meadors <benmmeadors@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Wessel <github@weebl.me>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: thebentern <9000580+thebentern@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Pull request that fixes bugs needs-review Needs human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants