diff --git a/src/mesh/CryptoEngine.cpp b/src/mesh/CryptoEngine.cpp index cb8989976..d296aa199 100644 --- a/src/mesh/CryptoEngine.cpp +++ b/src/mesh/CryptoEngine.cpp @@ -230,8 +230,6 @@ bool CryptoEngine::encryptCurve25519(uint32_t toNode, uint32_t fromNode, meshtas if (!HardwareRNG::fill((uint8_t *)&extraNonceTmp, sizeof(extraNonceTmp))) CryptRNG.rand((uint8_t *)&extraNonceTmp, sizeof(extraNonceTmp)); auth = bytesOut + numBytes; - memcpy((uint8_t *)(auth + 8), &extraNonceTmp, - sizeof(uint32_t)); // do not use dereference on potential non aligned pointers : *extraNonce = extraNonceTmp; LOG_DEBUG("Random nonce value: %d", extraNonceTmp); if (remotePublic.size == 0) { LOG_DEBUG("Node %d or their public_key not found", toNode); @@ -246,8 +244,7 @@ bool CryptoEngine::encryptCurve25519(uint32_t toNode, uint32_t fromNode, meshtas // Calculate the shared secret with the destination node and encrypt printBytes("Attempt encrypt with nonce: ", nonce, 13); printBytes("Attempt encrypt with shared_key starting with: ", shared_key, 8); - aes_ccm_ae(shared_key, 32, nonce, 8, bytes, numBytes, nullptr, 0, bytesOut, - auth); // this can write up to 15 bytes longer than numbytes past bytesOut + aes_ccm_ae(shared_key, 32, nonce, 8, bytes, numBytes, nullptr, 0, bytesOut, auth); memcpy((uint8_t *)(auth + 8), &extraNonceTmp, sizeof(uint32_t)); // do not use dereference on potential non aligned pointers : *extraNonce = extraNonceTmp; return true; diff --git a/src/mesh/aes-ccm.cpp b/src/mesh/aes-ccm.cpp index 5ed7ff928..29e96cd4b 100644 --- a/src/mesh/aes-ccm.cpp +++ b/src/mesh/aes-ccm.cpp @@ -111,11 +111,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) diff --git a/test/test_crypto/test_main.cpp b/test/test_crypto/test_main.cpp index 30ed9490d..7a93bdac4 100644 --- a/test/test_crypto/test_main.cpp +++ b/test/test_crypto/test_main.cpp @@ -2,6 +2,7 @@ #include "CryptoEngine.h" #include "TestUtil.h" +#include "aes-ccm.h" #include #include @@ -310,6 +311,42 @@ void test_AES_CTR(void) TEST_ASSERT_EQUAL_MEMORY(expected, plain, 16); } +void test_AES_CCM_partial_block_bounds(void) +{ + // aes_ccm_encr() used to write a whole 16-byte AES block at the output before XOR-ing, + // so a trailing partial block scribbled up to 15 bytes past what the caller allocated. + const uint8_t guard = 0xA5; + const size_t guardLen = 16; + const size_t lengths[] = {5, 20}; // pure partial block, and one full block plus a partial one + uint8_t key[32]; + uint8_t nonce[13]; + uint8_t auth[8]; + + HexToBytes(key, "603DEB1015CA71BE2B73AEF0857D77811F352C073B6108D72D9810A30914DFF4"); + HexToBytes(nonce, "000102030405060708090A0B0C"); + + for (size_t n = 0; n < sizeof(lengths) / sizeof(lengths[0]); n++) { + const size_t len = lengths[n]; + uint8_t plain[32]; + uint8_t crypt[32 + guardLen]; + uint8_t decrypted[32 + guardLen]; + + for (size_t i = 0; i < len; i++) + plain[i] = (uint8_t)i; + memset(crypt + len, guard, guardLen); + memset(decrypted + len, guard, guardLen); + + TEST_ASSERT_EQUAL(0, aes_ccm_ae(key, sizeof(key), nonce, sizeof(auth), plain, len, nullptr, 0, crypt, auth)); + for (size_t i = 0; i < guardLen; i++) + TEST_ASSERT_EQUAL_UINT8(guard, crypt[len + i]); + + TEST_ASSERT_TRUE(aes_ccm_ad(key, sizeof(key), nonce, sizeof(auth), crypt, len, nullptr, 0, auth, decrypted)); + for (size_t i = 0; i < guardLen; i++) + TEST_ASSERT_EQUAL_UINT8(guard, decrypted[len + i]); + TEST_ASSERT_EQUAL_MEMORY(plain, decrypted, len); + } +} + void setup() { // NOTE!!! Wait for >2 secs @@ -323,6 +360,7 @@ void setup() RUN_TEST(test_ECB_AES256); RUN_TEST(test_DH25519); RUN_TEST(test_AES_CTR); + RUN_TEST(test_AES_CCM_partial_block_bounds); RUN_TEST(test_PKC); RUN_TEST(test_XEdDSA); RUN_TEST(test_XEdDSA_cross_key_reject);