From 5bc17a99112e6695dce63e95be48fb4f2f886361 Mon Sep 17 00:00:00 2001 From: Jonathan Bennett Date: Thu, 29 Aug 2024 16:28:03 -0500 Subject: [PATCH] Key regen and MQTT fix (#4585) * Add public key regen * Properly label and handle PKI MQTT packets * Extra debug message to indicate PKI_UNKNOWN_PUBKEY * Ternary! * Don't call non-existant function on stm32 * Actually fix STM32 compilation --- src/mesh/CryptoEngine.cpp | 25 +++++++++++++++++++++++++ src/mesh/CryptoEngine.h | 3 +++ src/mesh/NodeDB.cpp | 26 ++++++++++++++++++-------- src/mesh/ReliableRouter.cpp | 2 +- src/modules/AdminModule.cpp | 9 +++++++++ src/mqtt/MQTT.cpp | 22 ++++++---------------- 6 files changed, 62 insertions(+), 25 deletions(-) diff --git a/src/mesh/CryptoEngine.cpp b/src/mesh/CryptoEngine.cpp index eef9f74b1..a5322a65a 100644 --- a/src/mesh/CryptoEngine.cpp +++ b/src/mesh/CryptoEngine.cpp @@ -11,6 +11,7 @@ #include #include #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN) + /** * Create a public/private key pair with Curve25519. * @@ -24,6 +25,30 @@ void CryptoEngine::generateKeyPair(uint8_t *pubKey, uint8_t *privKey) memcpy(pubKey, public_key, sizeof(public_key)); memcpy(privKey, private_key, sizeof(private_key)); } + +/** + * regenerate a public key with Curve25519. + * + * @param pubKey The destination for the public key. + * @param privKey The source for the private key. + */ +bool CryptoEngine::regeneratePublicKey(uint8_t *pubKey, uint8_t *privKey) +{ + if (!memfll(privKey, 0, sizeof(private_key))) { + Curve25519::eval(pubKey, privKey, 0); + if (Curve25519::isWeakPoint(pubKey)) { + LOG_ERROR("PKI key generation failed. Specified private key results in a weak\n"); + memset(pubKey, 0, 32); + return false; + } + memcpy(private_key, privKey, sizeof(private_key)); + memcpy(public_key, pubKey, sizeof(public_key)); + } else { + LOG_WARN("X25519 key generation failed due to blank private key\n"); + return false; + } + return true; +} #endif void CryptoEngine::clearKeys() { diff --git a/src/mesh/CryptoEngine.h b/src/mesh/CryptoEngine.h index 64382f6a7..4c2fc19d9 100644 --- a/src/mesh/CryptoEngine.h +++ b/src/mesh/CryptoEngine.h @@ -21,6 +21,7 @@ struct CryptoKey { */ #define MAX_BLOCKSIZE 256 +#define TEST_CURVE25519_FIELD_OPS // Exposes Curve25519::isWeakPoint() for testing keys class CryptoEngine { @@ -33,6 +34,8 @@ class CryptoEngine #if !(MESHTASTIC_EXCLUDE_PKI) #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN) virtual void generateKeyPair(uint8_t *pubKey, uint8_t *privKey); + virtual bool regeneratePublicKey(uint8_t *pubKey, uint8_t *privKey); + #endif void clearKeys(); void setDHPrivateKey(uint8_t *_private_key); diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index bf8596423..ba7671dc5 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -139,14 +139,24 @@ NodeDB::NodeDB() crypto->setDHPrivateKey(config.security.private_key.bytes); } else { #if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN) - LOG_INFO("Generating new PKI keys\n"); - crypto->generateKeyPair(config.security.public_key.bytes, config.security.private_key.bytes); - config.security.public_key.size = 32; - config.security.private_key.size = 32; - - printBytes("New Pubkey", config.security.public_key.bytes, 32); - owner.public_key.size = 32; - memcpy(owner.public_key.bytes, config.security.public_key.bytes, 32); + bool keygenSuccess = false; + if (config.security.private_key.size == 32) { + LOG_INFO("Calculating PKI Public Key\n"); + if (crypto->regeneratePublicKey(config.security.public_key.bytes, config.security.private_key.bytes)) { + keygenSuccess = true; + } + } else { + LOG_INFO("Generating new PKI keys\n"); + crypto->generateKeyPair(config.security.public_key.bytes, config.security.private_key.bytes); + keygenSuccess = true; + } + if (keygenSuccess) { + config.security.public_key.size = 32; + config.security.private_key.size = 32; + printBytes("New Pubkey", config.security.public_key.bytes, 32); + owner.public_key.size = 32; + memcpy(owner.public_key.bytes, config.security.public_key.bytes, 32); + } #else LOG_INFO("No PKI keys set, and generation disabled!\n"); #endif diff --git a/src/mesh/ReliableRouter.cpp b/src/mesh/ReliableRouter.cpp index 4c8c1e1e7..1f2c01473 100644 --- a/src/mesh/ReliableRouter.cpp +++ b/src/mesh/ReliableRouter.cpp @@ -112,7 +112,7 @@ void ReliableRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtas sendAckNak(meshtastic_Routing_Error_NONE, getFrom(p), p->id, p->channel, p->hop_start, p->hop_limit); } else if (p->which_payload_variant == meshtastic_MeshPacket_encrypted_tag && p->channel == 0 && (nodeDB->getMeshNode(p->from) == nullptr || nodeDB->getMeshNode(p->from)->user.public_key.size == 0)) { - // This looks like it might be a PKI packet from an unknown node, so send PKI_UNKNOWN_PUBKEY + LOG_INFO("This looks like it might be a PKI packet from an unknown node, so send PKI_UNKNOWN_PUBKEY\n"); sendAckNak(meshtastic_Routing_Error_PKI_UNKNOWN_PUBKEY, getFrom(p), p->id, channels.getPrimaryIndex(), p->hop_start, p->hop_limit); } else { diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index b63eca396..c9b875bd1 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -537,6 +537,15 @@ void AdminModule::handleSetConfig(const meshtastic_Config &c) case meshtastic_Config_security_tag: LOG_INFO("Setting config: Security\n"); config.security = c.payload_variant.security; +#if !(MESHTASTIC_EXCLUDE_PKI_KEYGEN) && !(MESHTASTIC_EXCLUDE_PKI) + // We check for a potentially valid private key, and a blank public key, and regen the public key if needed. + if (config.security.private_key.size == 32 && !memfll(config.security.private_key.bytes, 0, 32) && + (config.security.public_key.size == 0 || memfll(config.security.public_key.bytes, 0, 32))) { + if (crypto->regeneratePublicKey(config.security.public_key.bytes, config.security.private_key.bytes)) { + config.security.public_key.size = 32; + } + } +#endif owner.public_key.size = config.security.public_key.size; memcpy(owner.public_key.bytes, config.security.public_key.bytes, config.security.public_key.size); #if !MESHTASTIC_EXCLUDE_PKI diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index 2f7e82e3d..797fc7dc3 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -167,9 +167,9 @@ void MQTT::onReceive(char *topic, byte *payload, size_t length) strcmp(e.channel_id, "PKI") == 0) { meshtastic_NodeInfoLite *tx = nodeDB->getMeshNode(getFrom(p)); meshtastic_NodeInfoLite *rx = nodeDB->getMeshNode(p->to); - // Only accept PKI messages if we have both the sender and receiver in our nodeDB, as then it's likely - // they discovered each other via a channel we have downlink enabled for - if (tx && tx->has_user && rx && rx->has_user) + // Only accept PKI messages to us, or if we have both the sender and receiver in our nodeDB, as then it's + // likely they discovered each other via a channel we have downlink enabled for + if (p->to == nodeDB->getNodeNum() || (tx && tx->has_user && rx && rx->has_user)) router->enqueueReceivedMessage(p); } else if (router && perhapsDecode(p)) // ignore messages if we don't have the channel key router->enqueueReceivedMessage(p); @@ -527,7 +527,7 @@ void MQTT::onSend(const meshtastic_MeshPacket &mp, const meshtastic_MeshPacket & } if (ch.settings.uplink_enabled || mp.pki_encrypted) { - const char *channelId = channels.getGlobalId(chIndex); // FIXME, for now we just use the human name for the channel + const char *channelId = mp.pki_encrypted ? "PKI" : channels.getGlobalId(chIndex); meshtastic_ServiceEnvelope *env = mqttPool.allocZeroed(); env->channel_id = (char *)channelId; @@ -546,12 +546,7 @@ void MQTT::onSend(const meshtastic_MeshPacket &mp, const meshtastic_MeshPacket & // FIXME - this size calculation is super sloppy, but it will go away once we dynamically alloc meshpackets static uint8_t bytes[meshtastic_MeshPacket_size + 64]; size_t numBytes = pb_encode_to_bytes(bytes, sizeof(bytes), &meshtastic_ServiceEnvelope_msg, env); - std::string topic; - if (mp.pki_encrypted) { - topic = cryptTopic + "PKI/" + owner.id; - } else { - topic = cryptTopic + channelId + "/" + owner.id; - } + std::string topic = cryptTopic + channelId + "/" + owner.id; LOG_DEBUG("MQTT Publish %s, %u bytes\n", topic.c_str(), numBytes); publish(topic.c_str(), bytes, numBytes, false); @@ -561,12 +556,7 @@ void MQTT::onSend(const meshtastic_MeshPacket &mp, const meshtastic_MeshPacket & // handle json topic auto jsonString = MeshPacketSerializer::JsonSerialize((meshtastic_MeshPacket *)&mp_decoded); if (jsonString.length() != 0) { - std::string topicJson; - if (mp.pki_encrypted) { - topicJson = jsonTopic + "PKI/" + owner.id; - } else { - topicJson = jsonTopic + channelId + "/" + owner.id; - } + std::string topicJson = jsonTopic + channelId + "/" + owner.id; LOG_INFO("JSON publish message to %s, %u bytes: %s\n", topicJson.c_str(), jsonString.length(), jsonString.c_str()); publish(topicJson.c_str(), jsonString.c_str(), false);