From 117ff869a549aa1fb6554d5e5cf4c69784f865ab Mon Sep 17 00:00:00 2001 From: Matias Denda Date: Wed, 25 Feb 2026 12:27:24 -0300 Subject: [PATCH 1/6] Add AEAD (AES-CCM) authenticated encryption for PSK channels Extend PSK channel encryption with optional AES-CCM authenticated encryption (use_aead flag in ChannelSettings). When enabled, messages include a 12-byte authentication tag that prevents forgery, bit-flipping, and injection attacks by anyone with the channel PSK. Changes: - Add encryptPacketCCM/decryptPacketCCM to CryptoEngine with key promotion (16-byte keys zero-padded to 32 for AESSmall256 compat) - Move AES-CCM primitives (aes-ccm.h/cpp, aesSetKey, aesEncrypt) outside PKI guard so they're available unconditionally - Add isAEADEnabled() to Channels with hash differentiation (XOR 0xAE) - Add AEAD encrypt/decrypt branches in Router perhapsEncode/perhapsDecode with no CTR fallback on AEAD channels - Add use_aead field to channel.pb.h (bool, tag 8) - Add MESHTASTIC_AEAD_OVERHEAD constant to RadioInterface.h - Add comprehensive test suite: round-trip (AES-128/256), tamper detection (ciphertext, tag, sweep), wrong PSK, wrong sender, packet-too-small, deterministic output verification Addresses firmware#4030. --- src/mesh/Channels.cpp | 12 + src/mesh/Channels.h | 15 +- src/mesh/CryptoEngine.cpp | 33 ++- src/mesh/CryptoEngine.h | 11 +- src/mesh/RadioInterface.h | 1 + src/mesh/Router.cpp | 83 +++++-- src/mesh/aes-ccm.cpp | 4 +- src/mesh/aes-ccm.h | 4 +- src/mesh/generated/meshtastic/channel.pb.h | 18 +- test/test_crypto/test_main.cpp | 243 +++++++++++++++++++++ 10 files changed, 385 insertions(+), 39 deletions(-) diff --git a/src/mesh/Channels.cpp b/src/mesh/Channels.cpp index 4dcd94e3b9b..4528f0615e0 100644 --- a/src/mesh/Channels.cpp +++ b/src/mesh/Channels.cpp @@ -45,6 +45,12 @@ int16_t Channels::generateHash(ChannelIndex channelNum) h ^= xorHash(k.bytes, k.length); + // Differentiate AEAD channels in routing so AEAD and non-AEAD + // channels with the same PSK have different hashes + auto &ch = getByIndex(channelNum); + if (ch.has_settings && ch.settings.use_aead) + h ^= 0xAE; + return h; } } @@ -450,6 +456,12 @@ bool Channels::setDefaultPresetCryptoForHash(ChannelHash channelHash) return false; } +bool Channels::isAEADEnabled(ChannelIndex chIndex) +{ + auto &ch = getByIndex(chIndex); + return ch.has_settings && ch.settings.use_aead; +} + /** Given a channel index setup crypto for encoding that channel (or the primary channel if that channel is unsecured) * * This method is called before encoding outbound packets diff --git a/src/mesh/Channels.h b/src/mesh/Channels.h index a3cc7791c7c..7c24dbc814d 100644 --- a/src/mesh/Channels.h +++ b/src/mesh/Channels.h @@ -98,6 +98,15 @@ class Channels int16_t getHash(ChannelIndex i) { return hashes[i]; } + /** Return true if the channel has AEAD (authenticated encryption) enabled */ + bool isAEADEnabled(ChannelIndex chIndex); + + /** + * Return the key used for encrypting this channel (if channel is secondary and no key provided, use the primary channel's + * PSK) + */ + CryptoKey getKey(ChannelIndex chIndex); + private: /** Given a channel index, change to use the crypto key specified by that index * @@ -129,12 +138,6 @@ class Channels * Write default channels defined in UserPrefs */ void initDefaultChannel(ChannelIndex chIndex); - - /** - * Return the key used for encrypting this channel (if channel is secondary and no key provided, use the primary channel's - * PSK) - */ - CryptoKey getKey(ChannelIndex chIndex); }; /// Singleton channel table diff --git a/src/mesh/CryptoEngine.cpp b/src/mesh/CryptoEngine.cpp index 72216a63c8f..4f2947b8aaa 100644 --- a/src/mesh/CryptoEngine.cpp +++ b/src/mesh/CryptoEngine.cpp @@ -1,11 +1,11 @@ #include "CryptoEngine.h" // #include "NodeDB.h" +#include "aes-ccm.h" #include "architecture.h" #include #if !(MESHTASTIC_EXCLUDE_PKI) #include "NodeDB.h" -#include "aes-ccm.h" #include "meshUtils.h" #include #include @@ -197,6 +197,37 @@ bool CryptoEngine::setDHPublicKey(uint8_t *pubKey) } #endif + +bool CryptoEngine::encryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uint64_t packetId, + size_t numBytes, const uint8_t *plaintext, + uint8_t *ciphertextWithTag) +{ + initNonce(fromNode, packetId); + // AESSmall256::setKey() requires exactly 32 bytes. CryptoKey.bytes is always + // 32 bytes (zero-padded by Channels::getKey()), so pass the full buffer. + // This effectively promotes AES-128 PSKs to AES-256 with zero-padded key. + size_t keyLen = (psk.length > 0 && psk.length < 32) ? 32 : psk.length; + // Output layout: [ciphertext (numBytes)] [auth_tag (AEAD_TAG_SIZE bytes)] + return aes_ccm_ae(psk.bytes, keyLen, nonce, AEAD_TAG_SIZE, + plaintext, numBytes, nullptr, 0, + ciphertextWithTag, ciphertextWithTag + numBytes) == 0; +} + +bool CryptoEngine::decryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uint64_t packetId, + size_t totalBytes, const uint8_t *ciphertextWithTag, + uint8_t *plaintext) +{ + if (totalBytes <= AEAD_TAG_SIZE) + return false; + initNonce(fromNode, packetId); + size_t keyLen = (psk.length > 0 && psk.length < 32) ? 32 : psk.length; + size_t crypt_len = totalBytes - AEAD_TAG_SIZE; + const uint8_t *auth = ciphertextWithTag + crypt_len; + return aes_ccm_ad(psk.bytes, keyLen, nonce, AEAD_TAG_SIZE, + ciphertextWithTag, crypt_len, nullptr, 0, + auth, plaintext); +} + concurrency::Lock *cryptLock; void CryptoEngine::setKey(const CryptoKey &k) diff --git a/src/mesh/CryptoEngine.h b/src/mesh/CryptoEngine.h index 19d572355ba..4f736ec0caf 100644 --- a/src/mesh/CryptoEngine.h +++ b/src/mesh/CryptoEngine.h @@ -45,13 +45,22 @@ class CryptoEngine 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); +#endif virtual void aesSetKey(const uint8_t *key, size_t key_len); virtual void aesEncrypt(uint8_t *in, uint8_t *out); std::unique_ptr aes = nullptr; -#endif + static constexpr size_t AEAD_TAG_SIZE = 12; + + bool encryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uint64_t packetId, + size_t numBytes, const uint8_t *plaintext, + uint8_t *ciphertextWithTag); + + bool decryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uint64_t packetId, + size_t totalBytes, const uint8_t *ciphertextWithTag, + uint8_t *plaintext); /** * Set the key used for encrypt, decrypt. diff --git a/src/mesh/RadioInterface.h b/src/mesh/RadioInterface.h index 1fe3dd7b0f5..599d0cc1fa4 100644 --- a/src/mesh/RadioInterface.h +++ b/src/mesh/RadioInterface.h @@ -16,6 +16,7 @@ typedef struct _meshtastic_Config_LoRaConfig meshtastic_Config_LoRaConfig; #define MAX_LORA_PAYLOAD_LEN 255 // max length of 255 per Semtech's datasheets on SX12xx #define MESHTASTIC_HEADER_LENGTH 16 #define MESHTASTIC_PKC_OVERHEAD 12 +#define MESHTASTIC_AEAD_OVERHEAD 12 #define PACKET_FLAGS_HOP_LIMIT_MASK 0x07 #define PACKET_FLAGS_WANT_ACK_MASK 0x08 diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 32544a0515b..401a18a34e6 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -472,15 +472,32 @@ DecodeState perhapsDecode(meshtastic_MeshPacket *p) // 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); + + size_t decryptedSize = rawSize; + + if (channels.isAEADEnabled(chIndex)) { + // AEAD decryption — no CTR fallback + if (rawSize <= MESHTASTIC_AEAD_OVERHEAD) { + LOG_ERROR("Packet too small for AEAD (size=%d)", rawSize); + continue; + } + CryptoKey k = channels.getKey(chIndex); + if (!crypto->decryptPacketCCM(k, p->from, p->id, rawSize, p->encrypted.bytes, bytes)) { + LOG_WARN("AEAD authentication failed for ch %d", chIndex); + continue; // reject — no fallback to CTR + } + decryptedSize = rawSize - MESHTASTIC_AEAD_OVERHEAD; + } else { + // Standard AES-CTR decryption + crypto->decrypt(p->from, p->id, rawSize, bytes); + } // printBytes("plaintext", bytes, p->encrypted.size); // Take those raw bytes and convert them back into a well structured protobuf we can understand meshtastic_Data decodedtmp; memset(&decodedtmp, 0, sizeof(decodedtmp)); - if (!pb_decode_from_bytes(bytes, rawSize, &meshtastic_Data_msg, &decodedtmp)) { + if (!pb_decode_from_bytes(bytes, decryptedSize, &meshtastic_Data_msg, &decodedtmp)) { LOG_ERROR("Invalid protobufs in received mesh packet id=0x%08x (bad psk?)!", p->id); } else if (decodedtmp.portnum == meshtastic_PortNum_UNKNOWN_APP) { LOG_ERROR("Invalid portnum (bad psk?)!"); @@ -648,32 +665,58 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p) // Client specifically requested PKI encryption return meshtastic_Routing_Error_PKI_FAILED; } - hash = channels.setActiveByIndex(chIndex); - - // Now that we are encrypting the packet channel should be the hash (no longer the index) - p->channel = hash; - if (hash < 0) { - // No suitable channel could be found for - return meshtastic_Routing_Error_NO_CHANNEL; + if (channels.isAEADEnabled(chIndex)) { + // AEAD (AES-CCM) authenticated encryption path + if (numbytes + MESHTASTIC_HEADER_LENGTH + MESHTASTIC_AEAD_OVERHEAD > MAX_LORA_PAYLOAD_LEN) + return meshtastic_Routing_Error_TOO_LARGE; + + hash = channels.setActiveByIndex(chIndex); + p->channel = hash; + if (hash < 0) + return meshtastic_Routing_Error_NO_CHANNEL; + + CryptoKey k = channels.getKey(chIndex); + crypto->encryptPacketCCM(k, getFrom(p), p->id, numbytes, bytes, p->encrypted.bytes); + numbytes += MESHTASTIC_AEAD_OVERHEAD; + } else { + // Standard AES-CTR encryption path + hash = channels.setActiveByIndex(chIndex); + p->channel = hash; + if (hash < 0) + return meshtastic_Routing_Error_NO_CHANNEL; + + crypto->encryptPacket(getFrom(p), p->id, numbytes, bytes); + memcpy(p->encrypted.bytes, bytes, numbytes); } - crypto->encryptPacket(getFrom(p), p->id, numbytes, bytes); - memcpy(p->encrypted.bytes, bytes, numbytes); } #else if (p->pki_encrypted == true) { // Client specifically requested PKI encryption return meshtastic_Routing_Error_PKI_FAILED; } - hash = channels.setActiveByIndex(chIndex); + if (channels.isAEADEnabled(chIndex)) { + // AEAD (AES-CCM) authenticated encryption path + if (numbytes + MESHTASTIC_HEADER_LENGTH + MESHTASTIC_AEAD_OVERHEAD > MAX_LORA_PAYLOAD_LEN) + return meshtastic_Routing_Error_TOO_LARGE; - // Now that we are encrypting the packet channel should be the hash (no longer the index) - p->channel = hash; - if (hash < 0) { - // No suitable channel could be found for - return meshtastic_Routing_Error_NO_CHANNEL; + hash = channels.setActiveByIndex(chIndex); + p->channel = hash; + if (hash < 0) + return meshtastic_Routing_Error_NO_CHANNEL; + + CryptoKey k = channels.getKey(chIndex); + crypto->encryptPacketCCM(k, getFrom(p), p->id, numbytes, bytes, p->encrypted.bytes); + numbytes += MESHTASTIC_AEAD_OVERHEAD; + } else { + // Standard AES-CTR encryption path + hash = channels.setActiveByIndex(chIndex); + p->channel = hash; + if (hash < 0) + return meshtastic_Routing_Error_NO_CHANNEL; + + crypto->encryptPacket(getFrom(p), p->id, numbytes, bytes); + memcpy(p->encrypted.bytes, bytes, numbytes); } - crypto->encryptPacket(getFrom(p), p->id, numbytes, bytes); - memcpy(p->encrypted.bytes, bytes, numbytes); #endif // Copy back into the packet and set the variant type diff --git a/src/mesh/aes-ccm.cpp b/src/mesh/aes-ccm.cpp index 5ed7ff92838..c95841b1188 100644 --- a/src/mesh/aes-ccm.cpp +++ b/src/mesh/aes-ccm.cpp @@ -8,7 +8,6 @@ */ #define AES_BLOCK_SIZE 16 #include "aes-ccm.h" -#if !MESHTASTIC_EXCLUDE_PKI /** * Constant-time comparison of two byte arrays @@ -176,5 +175,4 @@ bool aes_ccm_ad(const uint8_t *key, size_t key_len, const uint8_t *nonce, size_t return false; } return true; -} -#endif \ No newline at end of file +} \ No newline at end of file diff --git a/src/mesh/aes-ccm.h b/src/mesh/aes-ccm.h index 6b8edcde490..8d160543f23 100644 --- a/src/mesh/aes-ccm.h +++ b/src/mesh/aes-ccm.h @@ -1,10 +1,8 @@ #pragma once #include "CryptoEngine.h" -#if !MESHTASTIC_EXCLUDE_PKI int aes_ccm_ae(const uint8_t *key, size_t key_len, const uint8_t *nonce, size_t M, const uint8_t *plain, size_t plain_len, const uint8_t *aad, size_t aad_len, uint8_t *crypt, uint8_t *auth); bool aes_ccm_ad(const uint8_t *key, size_t key_len, const uint8_t *nonce, size_t M, const uint8_t *crypt, size_t crypt_len, - const uint8_t *aad, size_t aad_len, const uint8_t *auth, uint8_t *plain); -#endif \ No newline at end of file + const uint8_t *aad, size_t aad_len, const uint8_t *auth, uint8_t *plain); \ No newline at end of file diff --git a/src/mesh/generated/meshtastic/channel.pb.h b/src/mesh/generated/meshtastic/channel.pb.h index 9dc757ab4a5..22c25f79659 100644 --- a/src/mesh/generated/meshtastic/channel.pb.h +++ b/src/mesh/generated/meshtastic/channel.pb.h @@ -97,6 +97,12 @@ typedef struct _meshtastic_ChannelSettings { /* Per-channel module settings. */ bool has_module_settings; meshtastic_ModuleSettings module_settings; + /* Enable authenticated encryption (AES-CCM) for this channel. + When true, messages include a 12-byte authentication tag that prevents + forgery and bit-flipping attacks. All nodes on the channel must have + this enabled — unauthenticated (AES-CTR) packets are rejected. + Experimental. Default: false (standard AES-CTR encryption). */ + bool use_aead; } meshtastic_ChannelSettings; /* A pair of a channel number, mode and the (sharable) settings for that channel */ @@ -128,10 +134,10 @@ extern "C" { /* Initializer values for message structs */ -#define meshtastic_ChannelSettings_init_default {0, {0, {0}}, "", 0, 0, 0, false, meshtastic_ModuleSettings_init_default} +#define meshtastic_ChannelSettings_init_default {0, {0, {0}}, "", 0, 0, 0, false, meshtastic_ModuleSettings_init_default, 0} #define meshtastic_ModuleSettings_init_default {0, 0} #define meshtastic_Channel_init_default {0, false, meshtastic_ChannelSettings_init_default, _meshtastic_Channel_Role_MIN} -#define meshtastic_ChannelSettings_init_zero {0, {0, {0}}, "", 0, 0, 0, false, meshtastic_ModuleSettings_init_zero} +#define meshtastic_ChannelSettings_init_zero {0, {0, {0}}, "", 0, 0, 0, false, meshtastic_ModuleSettings_init_zero, 0} #define meshtastic_ModuleSettings_init_zero {0, 0} #define meshtastic_Channel_init_zero {0, false, meshtastic_ChannelSettings_init_zero, _meshtastic_Channel_Role_MIN} @@ -145,6 +151,7 @@ extern "C" { #define meshtastic_ChannelSettings_uplink_enabled_tag 5 #define meshtastic_ChannelSettings_downlink_enabled_tag 6 #define meshtastic_ChannelSettings_module_settings_tag 7 +#define meshtastic_ChannelSettings_use_aead_tag 8 #define meshtastic_Channel_index_tag 1 #define meshtastic_Channel_settings_tag 2 #define meshtastic_Channel_role_tag 3 @@ -157,7 +164,8 @@ X(a, STATIC, SINGULAR, STRING, name, 3) \ X(a, STATIC, SINGULAR, FIXED32, id, 4) \ X(a, STATIC, SINGULAR, BOOL, uplink_enabled, 5) \ X(a, STATIC, SINGULAR, BOOL, downlink_enabled, 6) \ -X(a, STATIC, OPTIONAL, MESSAGE, module_settings, 7) +X(a, STATIC, OPTIONAL, MESSAGE, module_settings, 7) \ +X(a, STATIC, SINGULAR, BOOL, use_aead, 8) #define meshtastic_ChannelSettings_CALLBACK NULL #define meshtastic_ChannelSettings_DEFAULT NULL #define meshtastic_ChannelSettings_module_settings_MSGTYPE meshtastic_ModuleSettings @@ -187,8 +195,8 @@ extern const pb_msgdesc_t meshtastic_Channel_msg; /* Maximum encoded size of messages (where known) */ #define MESHTASTIC_MESHTASTIC_CHANNEL_PB_H_MAX_SIZE meshtastic_Channel_size -#define meshtastic_ChannelSettings_size 72 -#define meshtastic_Channel_size 87 +#define meshtastic_ChannelSettings_size 74 +#define meshtastic_Channel_size 89 #define meshtastic_ModuleSettings_size 8 #ifdef __cplusplus diff --git a/test/test_crypto/test_main.cpp b/test/test_crypto/test_main.cpp index 36dc37b9dd1..f029b35049f 100644 --- a/test/test_crypto/test_main.cpp +++ b/test/test_crypto/test_main.cpp @@ -1,5 +1,6 @@ // trunk-ignore-all(gitleaks): These are dummy values. Not real secrets. #include "CryptoEngine.h" +#include "aes-ccm.h" #include "TestUtil.h" #include @@ -178,6 +179,247 @@ void test_AES_CTR(void) TEST_ASSERT_EQUAL_MEMORY(expected, plain, 16); } +// Helper to create a zero-initialized CryptoKey (matching Channels::getKey() behavior) +static CryptoKey makePsk(const std::string &hex) +{ + CryptoKey k; + memset(k.bytes, 0, sizeof(k.bytes)); + k.length = hex.length() / 2; + HexToBytes(k.bytes, hex); + return k; +} + +void test_AES_CCM_AEAD(void) +{ + // ========================================================================= + // Test 1: Known-answer AES-CCM encrypt + verify tag + // ========================================================================= + { + CryptoKey psk = makePsk("d4f1bb3a20290759f0bcffabcf4e6901"); + + uint32_t fromNode = 0x12345678; + uint64_t packetId = 0xAABBCCDD; + + uint8_t plaintext[10]; + HexToBytes(plaintext, "08011204746573744800"); + + uint8_t ciphertextWithTag[10 + CryptoEngine::AEAD_TAG_SIZE]; + memset(ciphertextWithTag, 0, sizeof(ciphertextWithTag)); + + TEST_ASSERT_TRUE(crypto->encryptPacketCCM(psk, fromNode, packetId, 10, plaintext, ciphertextWithTag)); + + // Ciphertext should differ from plaintext + TEST_ASSERT_FALSE(memcmp(plaintext, ciphertextWithTag, 10) == 0); + + // Tag bytes (last 12) should not all be zero + bool tagAllZero = true; + for (size_t i = 0; i < CryptoEngine::AEAD_TAG_SIZE; i++) { + if (ciphertextWithTag[10 + i] != 0) { + tagAllZero = false; + break; + } + } + TEST_ASSERT_FALSE(tagAllZero); + } + + // ========================================================================= + // Test 2: Round-trip encrypt → decrypt → compare (AES-256) + // ========================================================================= + { + CryptoKey psk = makePsk("603DEB1015CA71BE2B73AEF0857D77811F352C073B6108D72D9810A30914DFF4"); + + uint32_t fromNode = 0xDEADBEEF; + uint64_t packetId = 0x0102030405060708; + + const char *msg = "Hello Meshtastic AEAD!"; + size_t msgLen = strlen(msg); + + uint8_t ciphertextWithTag[64]; + memset(ciphertextWithTag, 0, sizeof(ciphertextWithTag)); + TEST_ASSERT_TRUE(crypto->encryptPacketCCM(psk, fromNode, packetId, msgLen, (const uint8_t *)msg, ciphertextWithTag)); + + uint8_t decrypted[64]; + memset(decrypted, 0, sizeof(decrypted)); + size_t totalBytes = msgLen + CryptoEngine::AEAD_TAG_SIZE; + TEST_ASSERT_TRUE(crypto->decryptPacketCCM(psk, fromNode, packetId, totalBytes, ciphertextWithTag, decrypted)); + + TEST_ASSERT_EQUAL_MEMORY(msg, decrypted, msgLen); + } + + // ========================================================================= + // Test 3: Tampered ciphertext — flip a bit, verify rejection + // ========================================================================= + { + CryptoKey psk = makePsk("d4f1bb3a20290759f0bcffabcf4e6901"); + + uint32_t fromNode = 0xABCD1234; + uint64_t packetId = 0x11223344; + + uint8_t plaintext[8] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}; + uint8_t ciphertextWithTag[8 + CryptoEngine::AEAD_TAG_SIZE]; + + TEST_ASSERT_TRUE(crypto->encryptPacketCCM(psk, fromNode, packetId, 8, plaintext, ciphertextWithTag)); + + // Flip a bit in the ciphertext portion + ciphertextWithTag[3] ^= 0x01; + + uint8_t decrypted[8]; + TEST_ASSERT_FALSE(crypto->decryptPacketCCM(psk, fromNode, packetId, 8 + CryptoEngine::AEAD_TAG_SIZE, + ciphertextWithTag, decrypted)); + } + + // ========================================================================= + // Test 4: Tampered auth tag — modify tag, verify rejection + // ========================================================================= + { + CryptoKey psk = makePsk("d4f1bb3a20290759f0bcffabcf4e6901"); + + uint32_t fromNode = 0xABCD1234; + uint64_t packetId = 0x55667788; + + uint8_t plaintext[16] = {0}; + for (int i = 0; i < 16; i++) + plaintext[i] = (uint8_t)i; + + uint8_t ciphertextWithTag[16 + CryptoEngine::AEAD_TAG_SIZE]; + TEST_ASSERT_TRUE(crypto->encryptPacketCCM(psk, fromNode, packetId, 16, plaintext, ciphertextWithTag)); + + // Corrupt the auth tag (last byte) + ciphertextWithTag[16 + CryptoEngine::AEAD_TAG_SIZE - 1] ^= 0xFF; + + uint8_t decrypted[16]; + TEST_ASSERT_FALSE(crypto->decryptPacketCCM(psk, fromNode, packetId, 16 + CryptoEngine::AEAD_TAG_SIZE, + ciphertextWithTag, decrypted)); + } + + // ========================================================================= + // Test 5: Packet too small for AEAD — totalBytes <= AEAD_TAG_SIZE + // ========================================================================= + { + CryptoKey psk = makePsk("d4f1bb3a20290759f0bcffabcf4e6901"); + + uint8_t dummy[CryptoEngine::AEAD_TAG_SIZE] = {0}; + uint8_t out[1]; + + TEST_ASSERT_FALSE(crypto->decryptPacketCCM(psk, 0x1234, 0x5678, CryptoEngine::AEAD_TAG_SIZE, dummy, out)); + TEST_ASSERT_FALSE(crypto->decryptPacketCCM(psk, 0x1234, 0x5678, 0, dummy, out)); + } + + // ========================================================================= + // Test 6: Wrong PSK — decrypt with different key, verify rejection + // ========================================================================= + { + CryptoKey pskA = makePsk("d4f1bb3a20290759f0bcffabcf4e6901"); + CryptoKey pskB = makePsk("00112233445566778899aabbccddeeff"); + + uint32_t fromNode = 0x99887766; + uint64_t packetId = 0xDEADFACE; + + uint8_t plaintext[12] = "Hello World"; + uint8_t ciphertextWithTag[12 + CryptoEngine::AEAD_TAG_SIZE]; + + TEST_ASSERT_TRUE(crypto->encryptPacketCCM(pskA, fromNode, packetId, 12, plaintext, ciphertextWithTag)); + + // Attempt decryption with wrong key + uint8_t decrypted[12]; + TEST_ASSERT_FALSE(crypto->decryptPacketCCM(pskB, fromNode, packetId, 12 + CryptoEngine::AEAD_TAG_SIZE, + ciphertextWithTag, decrypted)); + } + + // ========================================================================= + // Test 7: Round-trip with AES-128 PSK (16-byte key, zero-padded to 256) + // Verifies that the key promotion to AES-256 works correctly. + // ========================================================================= + { + CryptoKey psk = makePsk("d4f1bb3a20290759f0bcffabcf4e6901"); + + uint32_t fromNode = 0x42424242; + uint64_t packetId = 0xBEEF1234; + + uint8_t plaintext[20] = "AES128 round trip!"; + uint8_t ciphertextWithTag[20 + CryptoEngine::AEAD_TAG_SIZE]; + + TEST_ASSERT_TRUE(crypto->encryptPacketCCM(psk, fromNode, packetId, 20, plaintext, ciphertextWithTag)); + + uint8_t decrypted[20]; + TEST_ASSERT_TRUE(crypto->decryptPacketCCM(psk, fromNode, packetId, 20 + CryptoEngine::AEAD_TAG_SIZE, + ciphertextWithTag, decrypted)); + TEST_ASSERT_EQUAL_MEMORY(plaintext, decrypted, 20); + } + + // ========================================================================= + // Test 8: AES-256-CCM round-trip + per-byte tamper detection + // ========================================================================= + { + CryptoKey psk = makePsk("603DEB1015CA71BE2B73AEF0857D77811F352C073B6108D72D9810A30914DFF4"); + + uint32_t fromNode = 0x01020304; + uint64_t packetId = 0x0A0B0C0D0E0F1011; + + uint8_t plaintext[32]; + for (int i = 0; i < 32; i++) + plaintext[i] = (uint8_t)(i * 7 + 3); + + uint8_t ciphertextWithTag[32 + CryptoEngine::AEAD_TAG_SIZE]; + TEST_ASSERT_TRUE(crypto->encryptPacketCCM(psk, fromNode, packetId, 32, plaintext, ciphertextWithTag)); + + // Valid decrypt + uint8_t decrypted[32]; + TEST_ASSERT_TRUE(crypto->decryptPacketCCM(psk, fromNode, packetId, 32 + CryptoEngine::AEAD_TAG_SIZE, + ciphertextWithTag, decrypted)); + TEST_ASSERT_EQUAL_MEMORY(plaintext, decrypted, 32); + + // Tamper with each of first 4 ciphertext bytes and verify rejection + for (int i = 0; i < 4; i++) { + uint8_t tampered[32 + CryptoEngine::AEAD_TAG_SIZE]; + memcpy(tampered, ciphertextWithTag, sizeof(tampered)); + tampered[i] ^= 0x80; + TEST_ASSERT_FALSE(crypto->decryptPacketCCM(psk, fromNode, packetId, 32 + CryptoEngine::AEAD_TAG_SIZE, + tampered, decrypted)); + } + } + + // ========================================================================= + // Test 9: Deterministic — same inputs produce same output + // ========================================================================= + { + CryptoKey psk = makePsk("d4f1bb3a20290759f0bcffabcf4e6901"); + + uint32_t fromNode = 0xCAFEBABE; + uint64_t packetId = 0xFEEDFACE; + + uint8_t plaintext[5] = {0xDE, 0xAD, 0xBE, 0xEF, 0x42}; + uint8_t ct1[5 + CryptoEngine::AEAD_TAG_SIZE]; + uint8_t ct2[5 + CryptoEngine::AEAD_TAG_SIZE]; + + TEST_ASSERT_TRUE(crypto->encryptPacketCCM(psk, fromNode, packetId, 5, plaintext, ct1)); + TEST_ASSERT_TRUE(crypto->encryptPacketCCM(psk, fromNode, packetId, 5, plaintext, ct2)); + + TEST_ASSERT_EQUAL_MEMORY(ct1, ct2, 5 + CryptoEngine::AEAD_TAG_SIZE); + } + + // ========================================================================= + // Test 10: Wrong fromNode — nonce mismatch, verify rejection + // ========================================================================= + { + CryptoKey psk = makePsk("d4f1bb3a20290759f0bcffabcf4e6901"); + + uint32_t fromNodeA = 0x11111111; + uint32_t fromNodeB = 0x22222222; + uint64_t packetId = 0xAAAABBBB; + + uint8_t plaintext[6] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06}; + uint8_t ciphertextWithTag[6 + CryptoEngine::AEAD_TAG_SIZE]; + + TEST_ASSERT_TRUE(crypto->encryptPacketCCM(psk, fromNodeA, packetId, 6, plaintext, ciphertextWithTag)); + + // Decrypt with different fromNode (nonce will differ) + uint8_t decrypted[6]; + TEST_ASSERT_FALSE(crypto->decryptPacketCCM(psk, fromNodeB, packetId, 6 + CryptoEngine::AEAD_TAG_SIZE, + ciphertextWithTag, decrypted)); + } +} + void setup() { // NOTE!!! Wait for >2 secs @@ -192,6 +434,7 @@ void setup() RUN_TEST(test_DH25519); RUN_TEST(test_AES_CTR); RUN_TEST(test_PKC); + RUN_TEST(test_AES_CCM_AEAD); exit(UNITY_END()); // stop unit testing } From 4e2dc53cdac96e69bddc7910fac89994bb36977c Mon Sep 17 00:00:00 2001 From: Matias Denda Date: Wed, 25 Feb 2026 12:36:11 -0300 Subject: [PATCH 2/6] Fix -s flag preventing config YAML from being loaded The simradio flag (-s) was the first branch in an if/else-if chain that also handled config file loading (-c). When both flags were used together (meshtasticd -s -c config.yaml), the config YAML was never parsed because -s short-circuited into the first branch. This meant YAML settings like EnableUDP, DisplayMode, StatusMessage, and all other Config section options were silently ignored in sim mode. Fix: load the config YAML independently of the simradio flag, then apply the -s override afterwards. This preserves all non-radio settings from the YAML while still forcing simradio mode as intended. --- src/platform/portduino/PortduinoGlue.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/platform/portduino/PortduinoGlue.cpp b/src/platform/portduino/PortduinoGlue.cpp index 3f0b2147c7a..29c80e9303d 100644 --- a/src/platform/portduino/PortduinoGlue.cpp +++ b/src/platform/portduino/PortduinoGlue.cpp @@ -180,9 +180,10 @@ void portduinoSetup() // Force stdout to be line buffered setvbuf(stdout, stdoutBuffer, _IOLBF, sizeof(stdoutBuffer)); - if (portduino_config.force_simradio == true) { - portduino_config.lora_module = use_simradio; - } else if (configPath != nullptr) { + // Load config YAML first (independent of simradio flag). + // The -s flag forces simradio mode but should NOT prevent config loading, + // since the YAML may contain non-radio settings (EnableUDP, display, etc.). + if (configPath != nullptr) { if (loadConfig(configPath)) { if (!yamlOnly) std::cout << "Using " << configPath << " as config file" << std::endl; @@ -212,6 +213,11 @@ void portduinoSetup() portduino_config.lora_module = use_simradio; } + // Apply -s flag override after config loading so YAML settings are preserved + if (portduino_config.force_simradio) { + portduino_config.lora_module = use_simradio; + } + if (portduino_config.config_directory != "") { std::string filetype = ".yaml"; for (const std::filesystem::directory_entry &entry : From a1594493c91f683c26b9ca23c7b583025b747b2e Mon Sep 17 00:00:00 2001 From: Matias Denda Date: Wed, 25 Feb 2026 13:06:38 -0300 Subject: [PATCH 3/6] Fix stack buffer overflow in aes_ccm_encr for partial blocks aes_ccm_encr() wrote a full 16-byte AES block directly to the output buffer before XOR-ing with input, even when the last block was smaller than AES_BLOCK_SIZE. This caused a stack buffer overflow when the plaintext/ciphertext buffer was exactly sized to the data length (detected by AddressSanitizer in the coverage test environment). Use a temporary buffer for the AES block output on the last partial block, matching the pattern already used in aes_ccm_encr_auth() and aes_ccm_decr_auth(). --- src/mesh/aes-ccm.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/mesh/aes-ccm.cpp b/src/mesh/aes-ccm.cpp index c95841b1188..c534aadbedf 100644 --- a/src/mesh/aes-ccm.cpp +++ b/src/mesh/aes-ccm.cpp @@ -110,11 +110,12 @@ static void aes_ccm_encr(size_t L, const uint8_t *in, size_t len, uint8_t *out, in += AES_BLOCK_SIZE; } if (last) { + uint8_t tmp[AES_BLOCK_SIZE]; WPA_PUT_BE16(&a[AES_BLOCK_SIZE - 2], i); - crypto->aesEncrypt(a, out); + crypto->aesEncrypt(a, tmp); /* XOR zero-padded last block */ for (i = 0; i < last; i++) - *out++ ^= *in++; + out[i] = tmp[i] ^ in[i]; } } static void aes_ccm_encr_auth(size_t M, const uint8_t *x, uint8_t *a, uint8_t *auth) From 4488ec2acb820cce7cf1a78573cd00bbea7284a9 Mon Sep 17 00:00:00 2001 From: Matias Denda Date: Wed, 25 Feb 2026 14:39:38 -0300 Subject: [PATCH 4/6] Apply clang-format to match project style --- src/mesh/CryptoEngine.cpp | 19 +++++++------------ src/mesh/CryptoEngine.h | 8 +++----- test/test_crypto/test_main.cpp | 28 ++++++++++++++-------------- 3 files changed, 24 insertions(+), 31 deletions(-) diff --git a/src/mesh/CryptoEngine.cpp b/src/mesh/CryptoEngine.cpp index 4f2947b8aaa..6dc01ea68ed 100644 --- a/src/mesh/CryptoEngine.cpp +++ b/src/mesh/CryptoEngine.cpp @@ -198,9 +198,8 @@ bool CryptoEngine::setDHPublicKey(uint8_t *pubKey) #endif -bool CryptoEngine::encryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uint64_t packetId, - size_t numBytes, const uint8_t *plaintext, - uint8_t *ciphertextWithTag) +bool CryptoEngine::encryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uint64_t packetId, size_t numBytes, + const uint8_t *plaintext, uint8_t *ciphertextWithTag) { initNonce(fromNode, packetId); // AESSmall256::setKey() requires exactly 32 bytes. CryptoKey.bytes is always @@ -208,14 +207,12 @@ bool CryptoEngine::encryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uin // This effectively promotes AES-128 PSKs to AES-256 with zero-padded key. size_t keyLen = (psk.length > 0 && psk.length < 32) ? 32 : psk.length; // Output layout: [ciphertext (numBytes)] [auth_tag (AEAD_TAG_SIZE bytes)] - return aes_ccm_ae(psk.bytes, keyLen, nonce, AEAD_TAG_SIZE, - plaintext, numBytes, nullptr, 0, - ciphertextWithTag, ciphertextWithTag + numBytes) == 0; + return aes_ccm_ae(psk.bytes, keyLen, nonce, AEAD_TAG_SIZE, plaintext, numBytes, nullptr, 0, ciphertextWithTag, + ciphertextWithTag + numBytes) == 0; } -bool CryptoEngine::decryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uint64_t packetId, - size_t totalBytes, const uint8_t *ciphertextWithTag, - uint8_t *plaintext) +bool CryptoEngine::decryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uint64_t packetId, size_t totalBytes, + const uint8_t *ciphertextWithTag, uint8_t *plaintext) { if (totalBytes <= AEAD_TAG_SIZE) return false; @@ -223,9 +220,7 @@ bool CryptoEngine::decryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uin size_t keyLen = (psk.length > 0 && psk.length < 32) ? 32 : psk.length; size_t crypt_len = totalBytes - AEAD_TAG_SIZE; const uint8_t *auth = ciphertextWithTag + crypt_len; - return aes_ccm_ad(psk.bytes, keyLen, nonce, AEAD_TAG_SIZE, - ciphertextWithTag, crypt_len, nullptr, 0, - auth, plaintext); + return aes_ccm_ad(psk.bytes, keyLen, nonce, AEAD_TAG_SIZE, ciphertextWithTag, crypt_len, nullptr, 0, auth, plaintext); } concurrency::Lock *cryptLock; diff --git a/src/mesh/CryptoEngine.h b/src/mesh/CryptoEngine.h index 4f736ec0caf..869486cfdc8 100644 --- a/src/mesh/CryptoEngine.h +++ b/src/mesh/CryptoEngine.h @@ -54,13 +54,11 @@ class CryptoEngine static constexpr size_t AEAD_TAG_SIZE = 12; - bool encryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uint64_t packetId, - size_t numBytes, const uint8_t *plaintext, + bool encryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uint64_t packetId, size_t numBytes, const uint8_t *plaintext, uint8_t *ciphertextWithTag); - bool decryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uint64_t packetId, - size_t totalBytes, const uint8_t *ciphertextWithTag, - uint8_t *plaintext); + bool decryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uint64_t packetId, size_t totalBytes, + const uint8_t *ciphertextWithTag, uint8_t *plaintext); /** * Set the key used for encrypt, decrypt. diff --git a/test/test_crypto/test_main.cpp b/test/test_crypto/test_main.cpp index f029b35049f..8d3f8852b84 100644 --- a/test/test_crypto/test_main.cpp +++ b/test/test_crypto/test_main.cpp @@ -264,8 +264,8 @@ void test_AES_CCM_AEAD(void) ciphertextWithTag[3] ^= 0x01; uint8_t decrypted[8]; - TEST_ASSERT_FALSE(crypto->decryptPacketCCM(psk, fromNode, packetId, 8 + CryptoEngine::AEAD_TAG_SIZE, - ciphertextWithTag, decrypted)); + TEST_ASSERT_FALSE( + crypto->decryptPacketCCM(psk, fromNode, packetId, 8 + CryptoEngine::AEAD_TAG_SIZE, ciphertextWithTag, decrypted)); } // ========================================================================= @@ -288,8 +288,8 @@ void test_AES_CCM_AEAD(void) ciphertextWithTag[16 + CryptoEngine::AEAD_TAG_SIZE - 1] ^= 0xFF; uint8_t decrypted[16]; - TEST_ASSERT_FALSE(crypto->decryptPacketCCM(psk, fromNode, packetId, 16 + CryptoEngine::AEAD_TAG_SIZE, - ciphertextWithTag, decrypted)); + TEST_ASSERT_FALSE( + crypto->decryptPacketCCM(psk, fromNode, packetId, 16 + CryptoEngine::AEAD_TAG_SIZE, ciphertextWithTag, decrypted)); } // ========================================================================= @@ -322,8 +322,8 @@ void test_AES_CCM_AEAD(void) // Attempt decryption with wrong key uint8_t decrypted[12]; - TEST_ASSERT_FALSE(crypto->decryptPacketCCM(pskB, fromNode, packetId, 12 + CryptoEngine::AEAD_TAG_SIZE, - ciphertextWithTag, decrypted)); + TEST_ASSERT_FALSE( + crypto->decryptPacketCCM(pskB, fromNode, packetId, 12 + CryptoEngine::AEAD_TAG_SIZE, ciphertextWithTag, decrypted)); } // ========================================================================= @@ -342,8 +342,8 @@ void test_AES_CCM_AEAD(void) TEST_ASSERT_TRUE(crypto->encryptPacketCCM(psk, fromNode, packetId, 20, plaintext, ciphertextWithTag)); uint8_t decrypted[20]; - TEST_ASSERT_TRUE(crypto->decryptPacketCCM(psk, fromNode, packetId, 20 + CryptoEngine::AEAD_TAG_SIZE, - ciphertextWithTag, decrypted)); + TEST_ASSERT_TRUE( + crypto->decryptPacketCCM(psk, fromNode, packetId, 20 + CryptoEngine::AEAD_TAG_SIZE, ciphertextWithTag, decrypted)); TEST_ASSERT_EQUAL_MEMORY(plaintext, decrypted, 20); } @@ -365,8 +365,8 @@ void test_AES_CCM_AEAD(void) // Valid decrypt uint8_t decrypted[32]; - TEST_ASSERT_TRUE(crypto->decryptPacketCCM(psk, fromNode, packetId, 32 + CryptoEngine::AEAD_TAG_SIZE, - ciphertextWithTag, decrypted)); + TEST_ASSERT_TRUE( + crypto->decryptPacketCCM(psk, fromNode, packetId, 32 + CryptoEngine::AEAD_TAG_SIZE, ciphertextWithTag, decrypted)); TEST_ASSERT_EQUAL_MEMORY(plaintext, decrypted, 32); // Tamper with each of first 4 ciphertext bytes and verify rejection @@ -374,8 +374,8 @@ void test_AES_CCM_AEAD(void) uint8_t tampered[32 + CryptoEngine::AEAD_TAG_SIZE]; memcpy(tampered, ciphertextWithTag, sizeof(tampered)); tampered[i] ^= 0x80; - TEST_ASSERT_FALSE(crypto->decryptPacketCCM(psk, fromNode, packetId, 32 + CryptoEngine::AEAD_TAG_SIZE, - tampered, decrypted)); + TEST_ASSERT_FALSE( + crypto->decryptPacketCCM(psk, fromNode, packetId, 32 + CryptoEngine::AEAD_TAG_SIZE, tampered, decrypted)); } } @@ -415,8 +415,8 @@ void test_AES_CCM_AEAD(void) // Decrypt with different fromNode (nonce will differ) uint8_t decrypted[6]; - TEST_ASSERT_FALSE(crypto->decryptPacketCCM(psk, fromNodeB, packetId, 6 + CryptoEngine::AEAD_TAG_SIZE, - ciphertextWithTag, decrypted)); + TEST_ASSERT_FALSE( + crypto->decryptPacketCCM(psk, fromNodeB, packetId, 6 + CryptoEngine::AEAD_TAG_SIZE, ciphertextWithTag, decrypted)); } } From 4cab9b3740314505f99c1e58b8d7c57b2a600cd9 Mon Sep 17 00:00:00 2001 From: Matias Denda Date: Thu, 26 Feb 2026 15:43:30 -0300 Subject: [PATCH 5/6] Guard AEAD path against empty PSK and check encrypt return value - Add early return in encryptPacketCCM/decryptPacketCCM when psk.length == 0, preventing null dereference in aesSetKey - Check encryptPacketCCM return value in Router::perhapsEncode (both PKI and non-PKI paths), returning BAD_REQUEST on failure instead of silently transmitting corrupt packets - Add unit test for empty PSK (encrypt and decrypt must return false without crashing) --- src/mesh/CryptoEngine.cpp | 12 ++++++++++-- src/mesh/Router.cpp | 10 ++++++++-- test/test_crypto/test_main.cpp | 25 +++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/mesh/CryptoEngine.cpp b/src/mesh/CryptoEngine.cpp index 6dc01ea68ed..62e4d969c86 100644 --- a/src/mesh/CryptoEngine.cpp +++ b/src/mesh/CryptoEngine.cpp @@ -201,11 +201,15 @@ bool CryptoEngine::setDHPublicKey(uint8_t *pubKey) bool CryptoEngine::encryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uint64_t packetId, size_t numBytes, const uint8_t *plaintext, uint8_t *ciphertextWithTag) { + if (psk.length == 0) { + LOG_ERROR("AEAD encryption requires a non-empty PSK"); + return false; + } initNonce(fromNode, packetId); // AESSmall256::setKey() requires exactly 32 bytes. CryptoKey.bytes is always // 32 bytes (zero-padded by Channels::getKey()), so pass the full buffer. // This effectively promotes AES-128 PSKs to AES-256 with zero-padded key. - size_t keyLen = (psk.length > 0 && psk.length < 32) ? 32 : psk.length; + size_t keyLen = (psk.length < 32) ? 32 : psk.length; // Output layout: [ciphertext (numBytes)] [auth_tag (AEAD_TAG_SIZE bytes)] return aes_ccm_ae(psk.bytes, keyLen, nonce, AEAD_TAG_SIZE, plaintext, numBytes, nullptr, 0, ciphertextWithTag, ciphertextWithTag + numBytes) == 0; @@ -214,10 +218,14 @@ bool CryptoEngine::encryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uin bool CryptoEngine::decryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uint64_t packetId, size_t totalBytes, const uint8_t *ciphertextWithTag, uint8_t *plaintext) { + if (psk.length == 0) { + LOG_ERROR("AEAD decryption requires a non-empty PSK"); + return false; + } if (totalBytes <= AEAD_TAG_SIZE) return false; initNonce(fromNode, packetId); - size_t keyLen = (psk.length > 0 && psk.length < 32) ? 32 : psk.length; + size_t keyLen = (psk.length < 32) ? 32 : psk.length; size_t crypt_len = totalBytes - AEAD_TAG_SIZE; const uint8_t *auth = ciphertextWithTag + crypt_len; return aes_ccm_ad(psk.bytes, keyLen, nonce, AEAD_TAG_SIZE, ciphertextWithTag, crypt_len, nullptr, 0, auth, plaintext); diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 401a18a34e6..a3d70f46c63 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -676,7 +676,10 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p) return meshtastic_Routing_Error_NO_CHANNEL; CryptoKey k = channels.getKey(chIndex); - crypto->encryptPacketCCM(k, getFrom(p), p->id, numbytes, bytes, p->encrypted.bytes); + if (!crypto->encryptPacketCCM(k, getFrom(p), p->id, numbytes, bytes, p->encrypted.bytes)) { + LOG_ERROR("AEAD encryption failed for ch %d", chIndex); + return meshtastic_Routing_Error_BAD_REQUEST; + } numbytes += MESHTASTIC_AEAD_OVERHEAD; } else { // Standard AES-CTR encryption path @@ -705,7 +708,10 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p) return meshtastic_Routing_Error_NO_CHANNEL; CryptoKey k = channels.getKey(chIndex); - crypto->encryptPacketCCM(k, getFrom(p), p->id, numbytes, bytes, p->encrypted.bytes); + if (!crypto->encryptPacketCCM(k, getFrom(p), p->id, numbytes, bytes, p->encrypted.bytes)) { + LOG_ERROR("AEAD encryption failed for ch %d", chIndex); + return meshtastic_Routing_Error_BAD_REQUEST; + } numbytes += MESHTASTIC_AEAD_OVERHEAD; } else { // Standard AES-CTR encryption path diff --git a/test/test_crypto/test_main.cpp b/test/test_crypto/test_main.cpp index 8d3f8852b84..8501cba2c61 100644 --- a/test/test_crypto/test_main.cpp +++ b/test/test_crypto/test_main.cpp @@ -418,6 +418,31 @@ void test_AES_CCM_AEAD(void) TEST_ASSERT_FALSE( crypto->decryptPacketCCM(psk, fromNodeB, packetId, 6 + CryptoEngine::AEAD_TAG_SIZE, ciphertextWithTag, decrypted)); } + + // ========================================================================= + // Test 11: Empty PSK — must return false, not crash + // ========================================================================= + { + CryptoKey emptyPsk; + memset(&emptyPsk, 0, sizeof(emptyPsk)); + emptyPsk.length = 0; + + uint32_t fromNode = 0xDEADBEEF; + uint64_t packetId = 0x12345678; + + uint8_t plaintext[8] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}; + uint8_t ciphertextWithTag[8 + CryptoEngine::AEAD_TAG_SIZE]; + uint8_t decrypted[8]; + + // Encrypt with empty PSK must fail gracefully + TEST_ASSERT_FALSE(crypto->encryptPacketCCM(emptyPsk, fromNode, packetId, 8, plaintext, ciphertextWithTag)); + + // Decrypt with empty PSK must fail gracefully + // (use dummy ciphertext since encrypt failed) + memset(ciphertextWithTag, 0xAA, sizeof(ciphertextWithTag)); + TEST_ASSERT_FALSE(crypto->decryptPacketCCM(emptyPsk, fromNode, packetId, 8 + CryptoEngine::AEAD_TAG_SIZE, + ciphertextWithTag, decrypted)); + } } void setup() From aa66aba6f2b9b3554c89cf5f3ecd32ff6c65368f Mon Sep 17 00:00:00 2001 From: Matias Denda Date: Fri, 27 Feb 2026 11:03:20 -0300 Subject: [PATCH 6/6] Use true AES-128 for 16-byte PSKs instead of promoting to AES-256 aesSetKey now dispatches based on key length: 16 bytes creates AESSmall128, 32 bytes creates AESSmall256. The aes member type changes from AESSmall256 to BlockCipher (polymorphic base class). This removes the unnecessary key promotion that added two extra AES rounds (14 vs 12) with no security benefit since the entropy stays at 128 bits for 16-byte keys. encryptPacketCCM/decryptPacketCCM now pass psk.length directly to aes_ccm_ae/aes_ccm_ad instead of promoting to 32. New tests: ECB AES-128 with NIST vectors, AEAD test verifying AES-128 and AES-256 produce different ciphertexts with same key material and cross-key decryption fails. --- src/mesh/CryptoEngine.cpp | 16 ++++----- src/mesh/CryptoEngine.h | 2 +- test/test_crypto/test_main.cpp | 65 ++++++++++++++++++++++++++++++++-- 3 files changed, 71 insertions(+), 12 deletions(-) diff --git a/src/mesh/CryptoEngine.cpp b/src/mesh/CryptoEngine.cpp index 62e4d969c86..c4b0cd99d15 100644 --- a/src/mesh/CryptoEngine.cpp +++ b/src/mesh/CryptoEngine.cpp @@ -171,8 +171,11 @@ void CryptoEngine::hash(uint8_t *bytes, size_t numBytes) void CryptoEngine::aesSetKey(const uint8_t *key_bytes, size_t key_len) { aes = nullptr; - if (key_len != 0) { - aes = std::unique_ptr(new AESSmall256()); + if (key_len == 16) { + aes = std::unique_ptr(new AESSmall128()); + aes->setKey(key_bytes, 16); + } else if (key_len != 0) { + aes = std::unique_ptr(new AESSmall256()); aes->setKey(key_bytes, key_len); } } @@ -206,12 +209,8 @@ bool CryptoEngine::encryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uin return false; } initNonce(fromNode, packetId); - // AESSmall256::setKey() requires exactly 32 bytes. CryptoKey.bytes is always - // 32 bytes (zero-padded by Channels::getKey()), so pass the full buffer. - // This effectively promotes AES-128 PSKs to AES-256 with zero-padded key. - size_t keyLen = (psk.length < 32) ? 32 : psk.length; // Output layout: [ciphertext (numBytes)] [auth_tag (AEAD_TAG_SIZE bytes)] - return aes_ccm_ae(psk.bytes, keyLen, nonce, AEAD_TAG_SIZE, plaintext, numBytes, nullptr, 0, ciphertextWithTag, + return aes_ccm_ae(psk.bytes, psk.length, nonce, AEAD_TAG_SIZE, plaintext, numBytes, nullptr, 0, ciphertextWithTag, ciphertextWithTag + numBytes) == 0; } @@ -225,10 +224,9 @@ bool CryptoEngine::decryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uin if (totalBytes <= AEAD_TAG_SIZE) return false; initNonce(fromNode, packetId); - size_t keyLen = (psk.length < 32) ? 32 : psk.length; size_t crypt_len = totalBytes - AEAD_TAG_SIZE; const uint8_t *auth = ciphertextWithTag + crypt_len; - return aes_ccm_ad(psk.bytes, keyLen, nonce, AEAD_TAG_SIZE, ciphertextWithTag, crypt_len, nullptr, 0, auth, plaintext); + return aes_ccm_ad(psk.bytes, psk.length, nonce, AEAD_TAG_SIZE, ciphertextWithTag, crypt_len, nullptr, 0, auth, plaintext); } concurrency::Lock *cryptLock; diff --git a/src/mesh/CryptoEngine.h b/src/mesh/CryptoEngine.h index 869486cfdc8..96def328f6c 100644 --- a/src/mesh/CryptoEngine.h +++ b/src/mesh/CryptoEngine.h @@ -50,7 +50,7 @@ class CryptoEngine virtual void aesSetKey(const uint8_t *key, size_t key_len); virtual void aesEncrypt(uint8_t *in, uint8_t *out); - std::unique_ptr aes = nullptr; + std::unique_ptr aes = nullptr; static constexpr size_t AEAD_TAG_SIZE = 12; diff --git a/test/test_crypto/test_main.cpp b/test/test_crypto/test_main.cpp index 8501cba2c61..e495aa0dc7e 100644 --- a/test/test_crypto/test_main.cpp +++ b/test/test_crypto/test_main.cpp @@ -75,6 +75,29 @@ void test_ECB_AES256(void) crypto->aesEncrypt(plain, result); // Does 16 bytes at a time TEST_ASSERT_EQUAL_MEMORY(expected, result, 16); } +void test_ECB_AES128(void) +{ + // https://csrc.nist.gov/CSRC/media/Projects/Cryptographic-Standards-and-Guidelines/documents/examples/AES_ECB.pdf + uint8_t key[16] = {0}; + uint8_t plain[16] = {0}; + uint8_t result[16] = {0}; + uint8_t expected[16] = {0}; + + HexToBytes(key, "2B7E151628AED2A6ABF7158809CF4F3C"); + + HexToBytes(plain, "6BC1BEE22E409F96E93D7E117393172A"); + HexToBytes(expected, "3AD77BB40D7A3660A89ECAF32466EF97"); + crypto->aesSetKey(key, 16); + crypto->aesEncrypt(plain, result); + TEST_ASSERT_EQUAL_MEMORY(expected, result, 16); + + HexToBytes(plain, "AE2D8A571E03AC9C9EB76FAC45AF8E51"); + HexToBytes(expected, "F5D3D58503B9699DE785895A96FDBAAF"); + crypto->aesSetKey(key, 16); + crypto->aesEncrypt(plain, result); + TEST_ASSERT_EQUAL_MEMORY(expected, result, 16); +} + void test_DH25519(void) { // test vectors from wycheproof x25519 @@ -327,8 +350,7 @@ void test_AES_CCM_AEAD(void) } // ========================================================================= - // Test 7: Round-trip with AES-128 PSK (16-byte key, zero-padded to 256) - // Verifies that the key promotion to AES-256 works correctly. + // Test 7: Round-trip with AES-128 PSK (16-byte key, true AES-128-CCM) // ========================================================================= { CryptoKey psk = makePsk("d4f1bb3a20290759f0bcffabcf4e6901"); @@ -443,6 +465,44 @@ void test_AES_CCM_AEAD(void) TEST_ASSERT_FALSE(crypto->decryptPacketCCM(emptyPsk, fromNode, packetId, 8 + CryptoEngine::AEAD_TAG_SIZE, ciphertextWithTag, decrypted)); } + + // ========================================================================= + // Test 12: AES-128 vs AES-256 produce different ciphertexts + // Verifies that 16-byte keys use true AES-128, not AES-256 with padding. + // ========================================================================= + { + // Same 16 bytes of key material, but one is AES-128 (16 bytes) + // and the other is AES-256 (32 bytes, zero-padded). + CryptoKey psk128 = makePsk("d4f1bb3a20290759f0bcffabcf4e6901"); + CryptoKey psk256; + memset(psk256.bytes, 0, sizeof(psk256.bytes)); + HexToBytes(psk256.bytes, "d4f1bb3a20290759f0bcffabcf4e6901"); + psk256.length = 32; // same first 16 bytes, but treated as AES-256 + + uint32_t fromNode = 0x55AA55AA; + uint64_t packetId = 0x1234ABCD; + + uint8_t plaintext[8] = {0x10, 0x20, 0x30, 0x40, 0x50, 0x60, 0x70, 0x80}; + uint8_t ct128[8 + CryptoEngine::AEAD_TAG_SIZE]; + uint8_t ct256[8 + CryptoEngine::AEAD_TAG_SIZE]; + + TEST_ASSERT_TRUE(crypto->encryptPacketCCM(psk128, fromNode, packetId, 8, plaintext, ct128)); + TEST_ASSERT_TRUE(crypto->encryptPacketCCM(psk256, fromNode, packetId, 8, plaintext, ct256)); + + // AES-128 and AES-256 with the same key material must produce different output + TEST_ASSERT_FALSE(memcmp(ct128, ct256, 8 + CryptoEngine::AEAD_TAG_SIZE) == 0); + + // Both must still round-trip correctly + uint8_t dec128[8], dec256[8]; + TEST_ASSERT_TRUE(crypto->decryptPacketCCM(psk128, fromNode, packetId, 8 + CryptoEngine::AEAD_TAG_SIZE, ct128, dec128)); + TEST_ASSERT_EQUAL_MEMORY(plaintext, dec128, 8); + TEST_ASSERT_TRUE(crypto->decryptPacketCCM(psk256, fromNode, packetId, 8 + CryptoEngine::AEAD_TAG_SIZE, ct256, dec256)); + TEST_ASSERT_EQUAL_MEMORY(plaintext, dec256, 8); + + // Cross-key decryption must fail + TEST_ASSERT_FALSE(crypto->decryptPacketCCM(psk256, fromNode, packetId, 8 + CryptoEngine::AEAD_TAG_SIZE, ct128, dec128)); + TEST_ASSERT_FALSE(crypto->decryptPacketCCM(psk128, fromNode, packetId, 8 + CryptoEngine::AEAD_TAG_SIZE, ct256, dec256)); + } } void setup() @@ -455,6 +515,7 @@ void setup() initializeTestEnvironment(); UNITY_BEGIN(); // IMPORTANT LINE! RUN_TEST(test_SHA256); + RUN_TEST(test_ECB_AES128); RUN_TEST(test_ECB_AES256); RUN_TEST(test_DH25519); RUN_TEST(test_AES_CTR);