From e1de439a7f7c132e469ac2ed32e9b90ad527c3e3 Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Thu, 19 Dec 2024 17:14:27 -0800 Subject: [PATCH] Remove unnecessary memcpy for PKI crypto (#5608) * Remove unnecessary memcpy for PKI crypto * Update comment s/packet_id/id/ * Create a copy of bytes for each channel decrypt --------- Co-authored-by: Jonathan Bennett --- src/mesh/CryptoEngine.cpp | 24 +++++++++++++++++------- src/mesh/CryptoEngine.h | 4 ++-- src/mesh/Router.cpp | 14 +++++--------- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/mesh/CryptoEngine.cpp b/src/mesh/CryptoEngine.cpp index 94b9b6543..1624ab0d5 100644 --- a/src/mesh/CryptoEngine.cpp +++ b/src/mesh/CryptoEngine.cpp @@ -58,10 +58,16 @@ void CryptoEngine::clearKeys() * Encrypt a packet's payload using a key generated with Curve25519 and SHA256 * for a specific node. * - * @param bytes is updated in place + * @param toNode The MeshPacket `to` field. + * @param fromNode The MeshPacket `from` field. + * @param remotePublic The remote node's Curve25519 public key. + * @param packetId The MeshPacket `id` field. + * @param numBytes Number of bytes of plaintext in the bytes buffer. + * @param bytes Buffer containing plaintext input. + * @param bytesOut Output buffer to be populated with encrypted ciphertext. */ bool CryptoEngine::encryptCurve25519(uint32_t toNode, uint32_t fromNode, meshtastic_UserLite_public_key_t remotePublic, - uint64_t packetNum, size_t numBytes, uint8_t *bytes, uint8_t *bytesOut) + uint64_t packetNum, size_t numBytes, const uint8_t *bytes, uint8_t *bytesOut) { uint8_t *auth; long extraNonceTmp = random(); @@ -93,14 +99,18 @@ bool CryptoEngine::encryptCurve25519(uint32_t toNode, uint32_t fromNode, meshtas * Decrypt a packet's payload using a key generated with Curve25519 and SHA256 * for a specific node. * - * @param bytes is updated in place + * @param fromNode The MeshPacket `from` field. + * @param remotePublic The remote node's Curve25519 public key. + * @param packetId The MeshPacket `id` field. + * @param numBytes Number of bytes of ciphertext in the bytes buffer. + * @param bytes Buffer containing ciphertext input. + * @param bytesOut Output buffer to be populated with decrypted plaintext. */ bool CryptoEngine::decryptCurve25519(uint32_t fromNode, meshtastic_UserLite_public_key_t remotePublic, uint64_t packetNum, - size_t numBytes, uint8_t *bytes, uint8_t *bytesOut) + size_t numBytes, const uint8_t *bytes, uint8_t *bytesOut) { - uint8_t *auth; // set to last 8 bytes of text? - uint32_t extraNonce; // pointer was not really used - auth = bytes + numBytes - 12; + const uint8_t *auth = bytes + numBytes - 12; // set to last 8 bytes of text? + uint32_t extraNonce; // pointer was not really used memcpy(&extraNonce, auth + 8, sizeof(uint32_t)); // do not use dereference on potential non aligned pointers : (uint32_t *)(auth + 8); LOG_INFO("Random nonce value: %d", extraNonce); diff --git a/src/mesh/CryptoEngine.h b/src/mesh/CryptoEngine.h index 32862d95c..6bbcb3b8a 100644 --- a/src/mesh/CryptoEngine.h +++ b/src/mesh/CryptoEngine.h @@ -40,9 +40,9 @@ class CryptoEngine void clearKeys(); void setDHPrivateKey(uint8_t *_private_key); virtual bool encryptCurve25519(uint32_t toNode, uint32_t fromNode, meshtastic_UserLite_public_key_t remotePublic, - uint64_t packetNum, size_t numBytes, uint8_t *bytes, uint8_t *bytesOut); + uint64_t packetNum, size_t numBytes, const uint8_t *bytes, uint8_t *bytesOut); virtual bool decryptCurve25519(uint32_t fromNode, meshtastic_UserLite_public_key_t remotePublic, uint64_t packetNum, - size_t numBytes, uint8_t *bytes, uint8_t *bytesOut); + size_t numBytes, const uint8_t *bytes, uint8_t *bytesOut); virtual bool setDHPublicKey(uint8_t *publicKey); virtual void hash(uint8_t *bytes, size_t numBytes); diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index e714ef215..f55e7cc5a 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -37,7 +37,6 @@ static MemoryDynamic staticPool; Allocator &packetPool = staticPool; static uint8_t bytes[MAX_LORA_PAYLOAD_LEN + 1] __attribute__((__aligned__)); -static uint8_t ScratchEncrypted[MAX_LORA_PAYLOAD_LEN + 1] __attribute__((__aligned__)); /** * Constructor @@ -327,9 +326,6 @@ bool perhapsDecode(meshtastic_MeshPacket *p) } bool decrypted = false; ChannelIndex chIndex = 0; - memcpy(bytes, p->encrypted.bytes, - rawSize); // we have to copy into a scratch buffer, because these bytes are a union with the decoded protobuf - memcpy(ScratchEncrypted, p->encrypted.bytes, rawSize); #if !(MESHTASTIC_EXCLUDE_PKI) // Attempt PKI decryption first if (p->channel == 0 && isToUs(p) && p->to > 0 && !isBroadcast(p->to) && nodeDB->getMeshNode(p->from) != nullptr && @@ -337,7 +333,7 @@ bool perhapsDecode(meshtastic_MeshPacket *p) rawSize > MESHTASTIC_PKC_OVERHEAD) { LOG_DEBUG("Attempt PKI decryption"); - if (crypto->decryptCurve25519(p->from, nodeDB->getMeshNode(p->from)->user.public_key, p->id, rawSize, ScratchEncrypted, + if (crypto->decryptCurve25519(p->from, nodeDB->getMeshNode(p->from)->user.public_key, p->id, rawSize, p->encrypted.bytes, bytes)) { LOG_INFO("PKI Decryption worked!"); memset(&p->decoded, 0, sizeof(p->decoded)); @@ -349,8 +345,6 @@ bool perhapsDecode(meshtastic_MeshPacket *p) p->pki_encrypted = true; memcpy(&p->public_key.bytes, nodeDB->getMeshNode(p->from)->user.public_key.bytes, 32); p->public_key.size = 32; - // memcpy(bytes, ScratchEncrypted, rawSize); // TODO: Rename the bytes buffers - // chIndex = 8; } else { LOG_ERROR("PKC Decrypted, but pb_decode failed!"); return false; @@ -367,6 +361,9 @@ bool perhapsDecode(meshtastic_MeshPacket *p) for (chIndex = 0; chIndex < channels.getNumChannels(); chIndex++) { // Try to use this hash/channel pair if (channels.decryptForHash(chIndex, p->channel)) { + // we have to copy into a scratch buffer, because these bytes are a union with the decoded protobuf. Create a + // fresh copy for each decrypt attempt. + memcpy(bytes, p->encrypted.bytes, rawSize); // Try to decrypt the packet if we can crypto->decrypt(p->from, p->id, rawSize, bytes); @@ -515,9 +512,8 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p) *node->user.public_key.bytes); return meshtastic_Routing_Error_PKI_FAILED; } - crypto->encryptCurve25519(p->to, getFrom(p), node->user.public_key, p->id, numbytes, bytes, ScratchEncrypted); + crypto->encryptCurve25519(p->to, getFrom(p), node->user.public_key, p->id, numbytes, bytes, p->encrypted.bytes); numbytes += MESHTASTIC_PKC_OVERHEAD; - memcpy(p->encrypted.bytes, ScratchEncrypted, numbytes); p->channel = 0; p->pki_encrypted = true; } else {