Skip to content
12 changes: 12 additions & 0 deletions src/mesh/Channels.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,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;
}
}
Expand Down Expand Up @@ -556,6 +562,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
Expand Down
15 changes: 9 additions & 6 deletions src/mesh/Channels.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,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
*
Expand All @@ -138,12 +147,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
Expand Down
40 changes: 37 additions & 3 deletions src/mesh/CryptoEngine.cpp
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
#include "CryptoEngine.h"
// #include "NodeDB.h"
#include "aes-ccm.h"
#include "architecture.h"
#include <memory>

#if !(MESHTASTIC_EXCLUDE_PKI)
#include "HardwareRNG.h"
#include "NodeDB.h"
#include "aes-ccm.h"
#include "meshUtils.h"
#include <Crypto.h>
#include <Curve25519.h>
Expand Down Expand Up @@ -317,8 +317,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<AESSmall256>(new AESSmall256());
if (key_len == 16) {
aes = std::unique_ptr<BlockCipher>(new AESSmall128());
aes->setKey(key_bytes, 16);
} else if (key_len != 0) {
aes = std::unique_ptr<BlockCipher>(new AESSmall256());
aes->setKey(key_bytes, key_len);
}
}
Expand Down Expand Up @@ -369,6 +372,37 @@ bool CryptoEngine::getPendingPublicKey(uint32_t node, meshtastic_NodeInfoLite_pu
}

#endif

bool CryptoEngine::encryptPacketCCM(const CryptoKey &psk, uint32_t fromNode, uint64_t packetId, size_t numBytes,
const uint8_t *plaintext, uint8_t *ciphertextWithTag)
{
// length is int8_t and the aes_ccm_* key length is size_t, so the -1 "invalid key"
// sentinel would widen into a huge unsigned length rather than being rejected.
if (psk.length <= 0) {
LOG_ERROR("AEAD encryption requires a valid, non-empty PSK");
return false;
}
initNonce(fromNode, packetId);
// Output layout: [ciphertext (numBytes)] [auth_tag (AEAD_TAG_SIZE bytes)]
return aes_ccm_ae(psk.bytes, psk.length, 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 (psk.length <= 0) {
LOG_ERROR("AEAD decryption requires a valid, non-empty PSK");
return false;
}
if (totalBytes <= AEAD_TAG_SIZE)
return false;
initNonce(fromNode, packetId);
size_t crypt_len = totalBytes - AEAD_TAG_SIZE;
const uint8_t *auth = ciphertextWithTag + crypt_len;
return aes_ccm_ad(psk.bytes, psk.length, nonce, AEAD_TAG_SIZE, ciphertextWithTag, crypt_len, nullptr, 0, auth, plaintext);
}

Comment thread
coderabbitai[bot] marked this conversation as resolved.
concurrency::Lock *cryptLock;

void CryptoEngine::setKey(const CryptoKey &k)
Expand Down
11 changes: 9 additions & 2 deletions src/mesh/CryptoEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ 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

// Temporary holder for a peer's not-yet-verified public key, learned in-band during an
// in-progress key-verification handshake before it is committed to NodeDB. Lets the Router
Expand All @@ -73,9 +74,15 @@ 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<AESSmall256> aes = nullptr;
std::unique_ptr<BlockCipher> 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.
Expand Down
1 change: 1 addition & 0 deletions src/mesh/RadioInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,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
Expand Down
94 changes: 74 additions & 20 deletions src/mesh/Router.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@
#include "serialization/MeshPacketSerializer.h"
#endif

// The size checks below budget for the tag that encryptPacketCCM actually appends, so the
// two constants must not drift apart.
static_assert(MESHTASTIC_AEAD_OVERHEAD == CryptoEngine::AEAD_TAG_SIZE,
"MESHTASTIC_AEAD_OVERHEAD must match CryptoEngine::AEAD_TAG_SIZE");

#define MAX_RX_FROMRADIO \
4 // max number of packets destined to our queue, we dispatch packets quickly so it doesn't need to be big

Expand Down Expand Up @@ -923,15 +928,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);
Comment on lines +937 to +942

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use hex formatting for the one-byte channel index in log messages.

LOG_WARN("AEAD authentication failed for ch %d", chIndex); and the two LOG_ERROR("AEAD encryption failed for ch %d", chIndex); calls format chIndex, a one-byte value, with %d. The coding guideline requires one-byte values, flags, and reason codes to use 0x%x.

📝 Proposed fix
-                        LOG_WARN("AEAD authentication failed for ch %d", chIndex);
+                        LOG_WARN("AEAD authentication failed for ch 0x%x", chIndex);
-                LOG_ERROR("AEAD encryption failed for ch %d", chIndex);
+                LOG_ERROR("AEAD encryption failed for ch 0x%x", chIndex);

(apply to both LOG_ERROR occurrences)

Based on coding guidelines: "format one-byte values, flags, and reason codes as 0x%x instead."

Also applies to: 1216-1216, 1248-1248

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mesh/Router.cpp` around lines 932 - 937, Update the AEAD authentication
warning and both AEAD encryption error logs around decrypt/encrypt handling to
format the one-byte chIndex with 0x%x instead of %d. Preserve the existing
messages and arguments, changing only the channel-index format specifier in the
identified LOG_WARN and LOG_ERROR calls.

Source: Coding guidelines

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_DEBUG("Invalid protobufs in received mesh packet id=0x%08x (bad psk?)", p->id);
} else if (decodedtmp.portnum == meshtastic_PortNum_UNKNOWN_APP) {
LOG_DEBUG("Invalid portnum (bad psk?)");
Expand Down Expand Up @@ -1184,32 +1206,64 @@ 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);
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
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;

hash = channels.setActiveByIndex(chIndex);
p->channel = hash;
if (hash < 0)
return meshtastic_Routing_Error_NO_CHANNEL;

CryptoKey k = channels.getKey(chIndex);
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
hash = channels.setActiveByIndex(chIndex);
p->channel = hash;
if (hash < 0)
return meshtastic_Routing_Error_NO_CHANNEL;

// 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;
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
Expand Down
4 changes: 1 addition & 3 deletions src/mesh/aes-ccm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
*/
#define AES_BLOCK_SIZE 16
#include "aes-ccm.h"
#if !MESHTASTIC_EXCLUDE_PKI

/**
* Constant-time comparison of two byte arrays
Expand Down Expand Up @@ -177,5 +176,4 @@ bool aes_ccm_ad(const uint8_t *key, size_t key_len, const uint8_t *nonce, size_t
return false;
}
return true;
}
#endif
}
4 changes: 1 addition & 3 deletions src/mesh/aes-ccm.h
Original file line number Diff line number Diff line change
@@ -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
const uint8_t *aad, size_t aad_len, const uint8_t *auth, uint8_t *plain);
18 changes: 13 additions & 5 deletions src/mesh/generated/meshtastic/channel.pb.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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}

Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading