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>

arch/portduino/portduino.ini

@@ -24,7 +24,6 @@ lib_deps =
   ${radiolib_base.lib_deps}
   ${environmental_base.lib_deps}
   # renovate: datasource=custom.pio depName=rweather/Crypto packageName=rweather/library/Crypto
-  #rweather/Crypto@0.4.0
   https://github.com/meshtastic/Crypto/archive/1aa30eb536bd52a576fde6dfa393bf7349cf102d.zip
   # renovate: datasource=custom.pio depName=LovyanGFX packageName=lovyan03/library/LovyanGFX
   lovyan03/LovyanGFX@^1.2.0

src/mesh/CryptoEngine.cpp

@@ -68,21 +68,54 @@ bool CryptoEngine::regeneratePublicKey(uint8_t *pubKey, uint8_t *privKey)
     return true;
 }
 
-bool CryptoEngine::xeddsa_sign(uint8_t *message, size_t len, uint8_t *signature)
+/**
+ * Build a signing buffer that covers packet metadata and payload:
+ *   [fromNode(4) | packetId(4) | portnum(4) | payload(N)]
+ * This prevents replay, reattribution, and portnum redirection attacks.
+ */
+static size_t buildSigningBuffer(uint8_t *buf, size_t bufSize, uint32_t fromNode, uint32_t packetId, uint32_t portnum,
+                                 const uint8_t *payload, size_t payloadLen)
 {
-    XEdDSA::sign(signature, xeddsa_private_key, xeddsa_public_key, message,
-                 len); // sign will need modified to use the raw secret scalar, and not hash it first.
+    const size_t headerLen = sizeof(uint32_t) * 3;
+    size_t totalLen = headerLen + payloadLen;
+    if (totalLen > bufSize)
+        return 0;
+    memcpy(buf, &fromNode, sizeof(uint32_t));
+    memcpy(buf + sizeof(uint32_t), &packetId, sizeof(uint32_t));
+    memcpy(buf + sizeof(uint32_t) * 2, &portnum, sizeof(uint32_t));
+    memcpy(buf + headerLen, payload, payloadLen);
+    return totalLen;
+}
+
+bool CryptoEngine::xeddsa_sign(uint32_t fromNode, uint32_t packetId, uint32_t portnum, const uint8_t *payload, size_t payloadLen,
+                               uint8_t *signature)
+{
+    if (memfll(xeddsa_private_key, 0, sizeof(xeddsa_private_key)))
+        return false;
+    uint8_t sigBuf[MAX_BLOCKSIZE];
+    size_t sigLen = buildSigningBuffer(sigBuf, sizeof(sigBuf), fromNode, packetId, portnum, payload, payloadLen);
+    if (sigLen == 0)
+        return false;
+    XEdDSA::sign(signature, xeddsa_private_key, xeddsa_public_key, sigBuf, sigLen);
     return true;
 }
-bool CryptoEngine::xeddsa_verify(uint8_t *pubKey, uint8_t *message, size_t len, uint8_t *signature)
-{
-    uint8_t publicKey[32] = {0};
-    curve_to_ed_pub(pubKey, publicKey);
 
-    return XEdDSA::verify(signature, publicKey, message, len);
+bool CryptoEngine::xeddsa_verify(const uint8_t *pubKey, uint32_t fromNode, uint32_t packetId, uint32_t portnum,
+                                 const uint8_t *payload, size_t payloadLen, const uint8_t *signature)
+{
+    // Use cached Ed25519 key if the Curve25519 key matches, avoiding expensive field inversion
+    if (memcmp(pubKey, cached_curve_pubkey, 32) != 0) {
+        curve_to_ed_pub(pubKey, cached_ed_pubkey);
+        memcpy(cached_curve_pubkey, pubKey, 32);
+    }
+    uint8_t sigBuf[MAX_BLOCKSIZE];
+    size_t sigLen = buildSigningBuffer(sigBuf, sizeof(sigBuf), fromNode, packetId, portnum, payload, payloadLen);
+    if (sigLen == 0)
+        return false;
+    return XEdDSA::verify(signature, cached_ed_pubkey, sigBuf, sigLen);
 }
 
-void CryptoEngine::curve_to_ed_pub(uint8_t *curve_pubkey, uint8_t *ed_pubkey)
+void CryptoEngine::curve_to_ed_pub(const uint8_t *curve_pubkey, uint8_t *ed_pubkey)
 {
 
     // Apply the birational map defined in RFC 7748, section 4.1 "Curve25519" to calculate an Ed25519 public

src/mesh/CryptoEngine.h

@@ -22,6 +22,7 @@ struct CryptoKey {
 
 #define MAX_BLOCKSIZE 256
 #define TEST_CURVE25519_FIELD_OPS // Exposes Curve25519::isWeakPoint() for testing keys
+#define XEDDSA_SIGNATURE_SIZE 64
 
 class CryptoEngine
 {
@@ -35,8 +36,10 @@ class CryptoEngine
 #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN)
     virtual void generateKeyPair(uint8_t *pubKey, uint8_t *privKey);
     virtual bool regeneratePublicKey(uint8_t *pubKey, uint8_t *privKey);
-    bool xeddsa_sign(uint8_t *message, size_t len, uint8_t *signature);
-    bool xeddsa_verify(uint8_t *pubKey, uint8_t *message, size_t len, uint8_t *signature);
+    bool xeddsa_sign(uint32_t fromNode, uint32_t packetId, uint32_t portnum, const uint8_t *payload, size_t payloadLen,
+                     uint8_t *signature);
+    bool xeddsa_verify(const uint8_t *pubKey, uint32_t fromNode, uint32_t packetId, uint32_t portnum, const uint8_t *payload,
+                       size_t payloadLen, const uint8_t *signature);
 
 #endif
     void clearKeys();
@@ -86,7 +89,10 @@ class CryptoEngine
     uint8_t private_key[32] = {0};
     uint8_t xeddsa_public_key[32] = {0};
     uint8_t xeddsa_private_key[32] = {0};
-    void curve_to_ed_pub(uint8_t *curve_pubkey, uint8_t *ed_pubkey);
+    void curve_to_ed_pub(const uint8_t *curve_pubkey, uint8_t *ed_pubkey);
+    // Single-entry cache for curve_to_ed_pub conversion (avoids expensive field inversion per packet)
+    uint8_t cached_curve_pubkey[32] = {0};
+    uint8_t cached_ed_pubkey[32] = {0};
 #endif
     /**
      * Init our 128 bit nonce for a new packet

src/mesh/NodeDB.cpp

@@ -2079,22 +2079,26 @@ bool NodeDB::generateCryptoKeyPair(const uint8_t *privateKey)
 
 bool NodeDB::createNewIdentity()
 {
-    // Remove the old node from the NodeDB
     uint32_t oldNodeNum = getNodeNum();
+    uint32_t newNodeNum = crc32Buffer(config.security.public_key.bytes, config.security.public_key.size);
+
+    // If the key hasn't changed, nothing to do
+    if (newNodeNum == oldNodeNum)
+        return false;
+
+    // Retire the old node entry
     meshtastic_NodeInfoLite *node = getMeshNode(oldNodeNum);
-
-    // Set my node num uint32 value to bytes from the new public key
-    myNodeInfo.my_node_num = crc32Buffer(config.security.public_key.bytes, config.security.public_key.size);
-
-    if (node != NULL && myNodeInfo.my_node_num != oldNodeNum) {
-        LOG_DEBUG("Old node num %u is now %u", oldNodeNum, myNodeInfo.my_node_num);
+    if (node != NULL) {
+        LOG_DEBUG("Old node num %u is now %u", oldNodeNum, newNodeNum);
         node->is_ignored = true;
         node->has_device_metrics = false;
         node->has_position = false;
         node->user.public_key.size = 0;
-        node->user.public_key.bytes[0] = 0;
+        memset(node->user.public_key.bytes, 0, sizeof(node->user.public_key.bytes));
     }
 
+    myNodeInfo.my_node_num = newNodeNum;
+
     meshtastic_NodeInfoLite *info = getOrCreateMeshNode(getNodeNum());
     info->user = TypeConversions::ConvertToUserLite(owner);
     info->has_user = true;

src/mesh/Router.cpp

@@ -501,22 +501,29 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p)
             p->decoded.want_response |= p->decoded.bitfield & BITFIELD_WANT_RESPONSE_MASK;
 
         if (p->decoded.has_xeddsa_signature) {
-            LOG_WARN("packet shows XEdDSA");
             meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(p->from);
             if (node && node->user.public_key.size == 32) {
-                LOG_WARN("attempting to verify");
-                p->xeddsa_signed = crypto->xeddsa_verify(node->user.public_key.bytes, p->decoded.payload.bytes,
-                                                         p->decoded.payload.size, p->decoded.xeddsa_signature.bytes);
+                p->xeddsa_signed =
+                    crypto->xeddsa_verify(node->user.public_key.bytes, p->from, p->id, p->decoded.portnum,
+                                          p->decoded.payload.bytes, p->decoded.payload.size, p->decoded.xeddsa_signature.bytes);
+                if (p->xeddsa_signed) {
+                    // Mark this node as a signer so future unsigned packets from it are rejected
+                    node->bitfield |= NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK;
+                    LOG_DEBUG("Verified XEdDSA signature from 0x%08x", p->from);
+                } else {
+                    LOG_WARN("XEdDSA signature verification failed from 0x%08x, dropping", p->from);
+                    return DecodeState::DECODE_FAILURE;
+                }
             } else {
-                LOG_WARN("Don't have key to verify");
+                LOG_DEBUG("No public key for 0x%08x, cannot verify XEdDSA signature", p->from);
             }
-        }
-        if (p->xeddsa_signed) {
-            LOG_WARN("Received XEdDSA Signed Packet!");
-        } else if (p->decoded.has_xeddsa_signature) {
-            LOG_ERROR("Node sent signed packet, but cannot verify!");
         } else {
-            LOG_WARN("Received Unsigned Packet!");
+            // Unsigned packet — reject if this node previously sent signed packets
+            meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(p->from);
+            if (node && (node->bitfield & NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK)) {
+                LOG_WARN("Dropping unsigned packet from 0x%08x that previously signed", p->from);
+                return DecodeState::DECODE_FAILURE;
+            }
         }
 
         /* Not actually ever used.
@@ -568,11 +575,16 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p)
             p->decoded.has_bitfield = true;
             p->decoded.bitfield |= (config.lora.config_ok_to_mqtt << BITFIELD_OK_TO_MQTT_SHIFT);
             p->decoded.bitfield |= (p->decoded.want_response << BITFIELD_WANT_RESPONSE_SHIFT);
-            if (p->pki_encrypted == false && isBroadcast(p->to) && p->decoded.payload.size < 120) {
-                crypto->xeddsa_sign(p->decoded.payload.bytes, p->decoded.payload.size, p->decoded.xeddsa_signature.bytes);
-                p->decoded.xeddsa_signature.size = 64;
-                p->decoded.has_xeddsa_signature = true;
-                LOG_WARN("XEDDSA Signed!");
+            // Sign broadcast packets if payload + signature fits within the max Data payload.
+            // The actual encoded size is checked after pb_encode (TOO_LARGE).
+            if (!p->pki_encrypted && isBroadcast(p->to) &&
+                p->decoded.payload.size + XEDDSA_SIGNATURE_SIZE < meshtastic_Constants_DATA_PAYLOAD_LEN) {
+                if (crypto->xeddsa_sign(p->from, p->id, p->decoded.portnum, p->decoded.payload.bytes, p->decoded.payload.size,
+                                        p->decoded.xeddsa_signature.bytes)) {
+                    p->decoded.xeddsa_signature.size = XEDDSA_SIGNATURE_SIZE;
+                    p->decoded.has_xeddsa_signature = true;
+                    LOG_DEBUG("XEdDSA signed packet 0x%08x", p->id);
+                }
             }
         }
 

src/modules/NodeInfoModule.cpp

@@ -24,8 +24,10 @@ bool NodeInfoModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, mes
     }
     NodeNum sourceNum = getFrom(&mp);
     auto node = nodeDB->getMeshNode(sourceNum);
-    if ((node->bitfield & NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK) && !mp.xeddsa_signed)
+    if (node && (node->bitfield & NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK) && !mp.xeddsa_signed) {
+        LOG_WARN("Dropping unsigned NodeInfo from node 0x%08x that previously signed", sourceNum);
         return true;
+    }
 
     // Coerce user.id to be derived from the node number
     snprintf(p.id, sizeof(p.id), "!%08x", getFrom(&mp));

test/test_crypto/test_main.cpp

@@ -164,17 +164,35 @@ void test_XEdDSA(void)
     uint8_t message[] = "This is a test!";
     uint8_t message2[] = "This is a test.";
     uint8_t signature[64];
+    uint32_t fromNode = 0x1234;
+    uint32_t packetId = 0xDEADBEEF;
+    uint32_t portnum = 1;
     for (int times = 0; times < 10; times++) {
         printf("Start of time %u\n", times);
         crypto->generateKeyPair(x_public_key, private_key);
-        // crypto->setDHPrivateKey(private_key);
         XEdDSA::priv_curve_to_ed_keys(private_key, ed_private_key, ed_public_key);
         crypto->curve_to_ed_pub(x_public_key, ed_public_key2);
         TEST_ASSERT_EQUAL_MEMORY(ed_public_key, ed_public_key2, 32);
 
-        crypto->xeddsa_sign(message, sizeof(message), signature);
-        TEST_ASSERT(crypto->xeddsa_verify(x_public_key, message, sizeof(message), signature));
-        TEST_ASSERT_FALSE(crypto->xeddsa_verify(x_public_key, message2, sizeof(message), signature));
+        // Sign and verify with metadata
+        TEST_ASSERT(crypto->xeddsa_sign(fromNode, packetId, portnum, message, sizeof(message), signature));
+        TEST_ASSERT(crypto->xeddsa_verify(x_public_key, fromNode, packetId, portnum, message, sizeof(message), signature));
+
+        // Different payload fails
+        TEST_ASSERT_FALSE(
+            crypto->xeddsa_verify(x_public_key, fromNode, packetId, portnum, message2, sizeof(message2), signature));
+
+        // Different fromNode fails
+        TEST_ASSERT_FALSE(
+            crypto->xeddsa_verify(x_public_key, fromNode + 1, packetId, portnum, message, sizeof(message), signature));
+
+        // Different packetId fails
+        TEST_ASSERT_FALSE(
+            crypto->xeddsa_verify(x_public_key, fromNode, packetId + 1, portnum, message, sizeof(message), signature));
+
+        // Different portnum fails
+        TEST_ASSERT_FALSE(
+            crypto->xeddsa_verify(x_public_key, fromNode, packetId, portnum + 1, message, sizeof(message), signature));
     }
 }