diff --git a/src/mesh/CryptoEngine.cpp b/src/mesh/CryptoEngine.cpp index 302d96745..91fb59b9d 100644 --- a/src/mesh/CryptoEngine.cpp +++ b/src/mesh/CryptoEngine.cpp @@ -79,21 +79,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 diff --git a/src/mesh/CryptoEngine.h b/src/mesh/CryptoEngine.h index fba3dd018..8730c9519 100644 --- a/src/mesh/CryptoEngine.h +++ b/src/mesh/CryptoEngine.h @@ -23,6 +23,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 { @@ -37,8 +38,10 @@ class CryptoEngine virtual void generateKeyPair(uint8_t *pubKey, uint8_t *privKey); virtual bool regeneratePublicKey(uint8_t *pubKey, uint8_t *privKey); virtual bool ensurePkiKeys(meshtastic_Config_SecurityConfig &security, meshtastic_User &user); - 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 setDHPrivateKey(uint8_t *_private_key); @@ -89,7 +92,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 diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index fe5141af1..03d15e300 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -2773,22 +2773,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; diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 65bd475e1..944d0b869 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -524,22 +524,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. @@ -614,11 +621,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); + } } } diff --git a/src/modules/NodeInfoModule.cpp b/src/modules/NodeInfoModule.cpp index 0c7454645..b052ff43b 100644 --- a/src/modules/NodeInfoModule.cpp +++ b/src/modules/NodeInfoModule.cpp @@ -51,8 +51,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)); diff --git a/test/test_crypto/test_main.cpp b/test/test_crypto/test_main.cpp index 778f5ccbb..ed5467ad8 100644 --- a/test/test_crypto/test_main.cpp +++ b/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)); } } diff --git a/variants/native/portduino.ini b/variants/native/portduino.ini index 9ab45d1ab..a0f3d997d 100644 --- a/variants/native/portduino.ini +++ b/variants/native/portduino.ini @@ -25,7 +25,7 @@ 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.21 ; # renovate: datasource=git-refs depName=libch341-spi-userspace packageName=https://github.com/pine64/libch341-spi-userspace gitBranch=main