From c3aa56ef30ff4069065c1819d8ab0ac8ca8d7aba Mon Sep 17 00:00:00 2001 From: Jonathan Bennett Date: Sat, 10 Aug 2024 22:38:05 -0500 Subject: [PATCH] Refactor platform cryptography, add tests --- src/mesh/CryptoEngine.cpp | 42 +++++++++- src/mesh/CryptoEngine.h | 6 +- src/mesh/Router.cpp | 2 +- src/platform/esp32/ESP32CryptoEngine.cpp | 41 ++-------- src/platform/esp32/architecture.h | 3 + src/platform/nrf52/NRF52CryptoEngine.cpp | 29 ++----- src/platform/nrf52/architecture.h | 3 + .../portduino/CrossPlatformCryptoEngine.cpp | 78 ------------------- src/platform/rp2040/rp2040CryptoEngine.cpp | 66 ---------------- src/platform/stm32wl/STM32WLCryptoEngine.cpp | 67 ---------------- test/test_crypto/test_main.cpp | 27 +++++++ 11 files changed, 88 insertions(+), 276 deletions(-) delete mode 100644 src/platform/portduino/CrossPlatformCryptoEngine.cpp delete mode 100644 src/platform/rp2040/rp2040CryptoEngine.cpp delete mode 100644 src/platform/stm32wl/STM32WLCryptoEngine.cpp diff --git a/src/mesh/CryptoEngine.cpp b/src/mesh/CryptoEngine.cpp index fd7246fa9..e83236eab 100644 --- a/src/mesh/CryptoEngine.cpp +++ b/src/mesh/CryptoEngine.cpp @@ -1,6 +1,7 @@ #include "CryptoEngine.h" #include "NodeDB.h" #include "RadioInterface.h" +#include "architecture.h" #include "configuration.h" #if !(MESHTASTIC_EXCLUDE_PKI) @@ -188,14 +189,44 @@ void CryptoEngine::setKey(const CryptoKey &k) * * @param bytes is updated in place */ -void CryptoEngine::encrypt(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes) +void CryptoEngine::encryptPacket(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes) { - LOG_WARN("noop encryption!\n"); + if (key.length > 0) { + initNonce(fromNode, packetId); + if (numBytes <= MAX_BLOCKSIZE) { + encryptAESCtr(key, nonce, numBytes, bytes); + } else { + LOG_ERROR("Packet too large for crypto engine: %d. noop encryption!\n", numBytes); + } + } } void CryptoEngine::decrypt(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes) { - LOG_WARN("noop decryption!\n"); + // For CTR, the implementation is the same + encryptPacket(fromNode, packetId, numBytes, bytes); +} + +// Generic implementation of AES-CTR encryption. +void CryptoEngine::encryptAESCtr(CryptoKey _key, uint8_t *_nonce, size_t numBytes, uint8_t *bytes) +{ + if (ctr) { + delete ctr; + ctr = nullptr; + } + if (_key.length == 16) + ctr = new CTR(); + else + ctr = new CTR(); + ctr->setKey(_key.bytes, _key.length); + static uint8_t scratch[MAX_BLOCKSIZE]; + memcpy(scratch, bytes, numBytes); + memset(scratch + numBytes, 0, + sizeof(scratch) - numBytes); // Fill rest of buffer with zero (in case cypher looks at it) + + ctr->setIV(_nonce, 16); + ctr->setCounterSize(4); + ctr->encrypt(bytes, scratch, numBytes); } /** @@ -208,4 +239,7 @@ void CryptoEngine::initNonce(uint32_t fromNode, uint64_t packetId) // use memcpy to avoid breaking strict-aliasing memcpy(nonce, &packetId, sizeof(uint64_t)); memcpy(nonce + sizeof(uint64_t), &fromNode, sizeof(uint32_t)); -} \ No newline at end of file +} +#ifndef HAS_CUSTOM_CRYPTO_ENGINE +CryptoEngine *crypto = new CryptoEngine; +#endif \ No newline at end of file diff --git a/src/mesh/CryptoEngine.h b/src/mesh/CryptoEngine.h index fd607c29e..5ca9db7c1 100644 --- a/src/mesh/CryptoEngine.h +++ b/src/mesh/CryptoEngine.h @@ -1,5 +1,6 @@ #pragma once #include "AES.h" +#include "CTR.h" #include "concurrency/LockGuard.h" #include "configuration.h" #include "mesh-pb-constants.h" @@ -65,15 +66,16 @@ class CryptoEngine * * @param bytes is updated in place */ - virtual void encrypt(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes); + virtual void encryptPacket(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes); virtual void decrypt(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes); + virtual void encryptAESCtr(CryptoKey key, uint8_t *nonce, size_t numBytes, uint8_t *bytes); #ifndef PIO_UNIT_TESTING protected: #endif /** Our per packet nonce */ uint8_t nonce[16] = {0}; - CryptoKey key = {}; + CTRCommon *ctr = NULL; #if !(MESHTASTIC_EXCLUDE_PKI) uint8_t shared_key[32] = {0}; uint8_t private_key[32] = {0}; diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 1fecef6d7..b00b66a47 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -480,7 +480,7 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p) memcpy(p->encrypted.bytes, ScratchEncrypted, numbytes); p->channel = 0; } else { - crypto->encrypt(getFrom(p), p->id, numbytes, bytes); + crypto->encryptPacket(getFrom(p), p->id, numbytes, bytes); memcpy(p->encrypted.bytes, bytes, numbytes); } #else diff --git a/src/platform/esp32/ESP32CryptoEngine.cpp b/src/platform/esp32/ESP32CryptoEngine.cpp index 998419df8..230139036 100644 --- a/src/platform/esp32/ESP32CryptoEngine.cpp +++ b/src/platform/esp32/ESP32CryptoEngine.cpp @@ -13,58 +13,29 @@ class ESP32CryptoEngine : public CryptoEngine ~ESP32CryptoEngine() { mbedtls_aes_free(&aes); } - /** - * Set the key used for encrypt, decrypt. - * - * As a special case: If all bytes are zero, we assume _no encryption_ and send all data in cleartext. - * - * @param numBytes must be 16 (AES128), 32 (AES256) or 0 (no crypt) - * @param bytes a _static_ buffer that will remain valid for the life of this crypto instance (i.e. this class will cache the - * provided pointer) - */ - virtual void setKey(const CryptoKey &k) override - { - CryptoEngine::setKey(k); - - if (key.length != 0) { - auto res = mbedtls_aes_setkey_enc(&aes, key.bytes, key.length * 8); - assert(!res); - } - } - /** * Encrypt a packet * * @param bytes is updated in place + * TODO: return bool, and handle graciously when something fails */ - virtual void encrypt(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes) override + virtual void encryptAESCtr(CryptoKey _key, uint8_t *_nonce, size_t numBytes, uint8_t *bytes) override { - if (key.length > 0) { - LOG_DEBUG("ESP32 crypt fr=%x, num=%x, numBytes=%d!\n", fromNode, (uint32_t)packetId, numBytes); - initNonce(fromNode, packetId); + if (_key.length > 0) { if (numBytes <= MAX_BLOCKSIZE) { + mbedtls_aes_setkey_enc(&aes, _key.bytes, _key.length * 8); static uint8_t scratch[MAX_BLOCKSIZE]; uint8_t stream_block[16]; size_t nc_off = 0; memcpy(scratch, bytes, numBytes); memset(scratch + numBytes, 0, sizeof(scratch) - numBytes); // Fill rest of buffer with zero (in case cypher looks at it) - - auto res = mbedtls_aes_crypt_ctr(&aes, numBytes, &nc_off, nonce, stream_block, scratch, bytes); - assert(!res); + mbedtls_aes_crypt_ctr(&aes, numBytes, &nc_off, _nonce, stream_block, scratch, bytes); } else { LOG_ERROR("Packet too large for crypto engine: %d. noop encryption!\n", numBytes); } } } - - virtual void decrypt(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes) override - { - // For CTR, the implementation is the same - encrypt(fromNode, packetId, numBytes, bytes); - } - - private: }; -CryptoEngine *crypto = new ESP32CryptoEngine(); +CryptoEngine *crypto = new ESP32CryptoEngine(); \ No newline at end of file diff --git a/src/platform/esp32/architecture.h b/src/platform/esp32/architecture.h index fd3f92a9c..b6def5b01 100644 --- a/src/platform/esp32/architecture.h +++ b/src/platform/esp32/architecture.h @@ -42,6 +42,9 @@ #ifndef DEFAULT_VREF #define DEFAULT_VREF 1100 #endif +#ifndef HAS_CUSTOM_CRYPTO_ENGINE +#define HAS_CUSTOM_CRYPTO_ENGINE 1 +#endif #if defined(HAS_AXP192) || defined(HAS_AXP2101) #define HAS_PMU diff --git a/src/platform/nrf52/NRF52CryptoEngine.cpp b/src/platform/nrf52/NRF52CryptoEngine.cpp index a7cf3d5bf..5de13c58b 100644 --- a/src/platform/nrf52/NRF52CryptoEngine.cpp +++ b/src/platform/nrf52/NRF52CryptoEngine.cpp @@ -9,41 +9,24 @@ class NRF52CryptoEngine : public CryptoEngine ~NRF52CryptoEngine() {} - /** - * Encrypt a packet - * - * @param bytes is updated in place - */ - virtual void encrypt(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes) override + virtual void encryptAESCtr(CryptoKey _key, uint8_t *_nonce, size_t numBytes, uint8_t *bytes) override { - if (key.length > 16) { - LOG_DEBUG("Software encrypt fr=%x, num=%x, numBytes=%d!\n", fromNode, (uint32_t)packetId, numBytes); + if (_key.length > 16) { AES_ctx ctx; - initNonce(fromNode, packetId); - AES_init_ctx_iv(&ctx, key.bytes, nonce); + AES_init_ctx_iv(&ctx, _key.bytes, _nonce); AES_CTR_xcrypt_buffer(&ctx, bytes, numBytes); - } else if (key.length > 0) { - LOG_DEBUG("nRF52 encrypt fr=%x, num=%x, numBytes=%d!\n", fromNode, (uint32_t)packetId, numBytes); + } else if (_key.length > 0) { nRFCrypto.begin(); nRFCrypto_AES ctx; uint8_t myLen = ctx.blockLen(numBytes); char encBuf[myLen] = {0}; - initNonce(fromNode, packetId); ctx.begin(); - ctx.Process((char *)bytes, numBytes, nonce, key.bytes, key.length, encBuf, ctx.encryptFlag, ctx.ctrMode); + ctx.Process((char *)bytes, numBytes, _nonce, _key.bytes, _key.length, encBuf, ctx.encryptFlag, ctx.ctrMode); ctx.end(); nRFCrypto.end(); memcpy(bytes, encBuf, numBytes); } } - - virtual void decrypt(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes) override - { - // For CTR, the implementation is the same - encrypt(fromNode, packetId, numBytes, bytes); - } - - private: }; -CryptoEngine *crypto = new NRF52CryptoEngine(); +CryptoEngine *crypto = new NRF52CryptoEngine(); \ No newline at end of file diff --git a/src/platform/nrf52/architecture.h b/src/platform/nrf52/architecture.h index d5685d611..8781347c3 100644 --- a/src/platform/nrf52/architecture.h +++ b/src/platform/nrf52/architecture.h @@ -32,6 +32,9 @@ #ifndef HAS_CPU_SHUTDOWN #define HAS_CPU_SHUTDOWN 1 #endif +#ifndef HAS_CUSTOM_CRYPTO_ENGINE +#define HAS_CUSTOM_CRYPTO_ENGINE 1 +#endif // // set HW_VENDOR diff --git a/src/platform/portduino/CrossPlatformCryptoEngine.cpp b/src/platform/portduino/CrossPlatformCryptoEngine.cpp deleted file mode 100644 index 46ef942f0..000000000 --- a/src/platform/portduino/CrossPlatformCryptoEngine.cpp +++ /dev/null @@ -1,78 +0,0 @@ -#include "AES.h" -#include "CTR.h" -#include "CryptoEngine.h" -#include "configuration.h" - -/** A platform independent AES engine implemented using Tiny-AES - */ -class CrossPlatformCryptoEngine : public CryptoEngine -{ - - CTRCommon *ctr = NULL; - - public: - CrossPlatformCryptoEngine() {} - - ~CrossPlatformCryptoEngine() {} - - /** - * Set the key used for encrypt, decrypt. - * - * As a special case: If all bytes are zero, we assume _no encryption_ and send all data in cleartext. - * - * @param numBytes must be 16 (AES128), 32 (AES256) or 0 (no crypt) - * @param bytes a _static_ buffer that will remain valid for the life of this crypto instance (i.e. this class will cache the - * provided pointer) - */ - virtual void setKey(const CryptoKey &k) override - { - CryptoEngine::setKey(k); - LOG_DEBUG("Installing AES%d key!\n", key.length * 8); - if (ctr) { - delete ctr; - ctr = NULL; - } - if (key.length != 0) { - if (key.length == 16) - ctr = new CTR(); - else - ctr = new CTR(); - - ctr->setKey(key.bytes, key.length); - } - } - - /** - * Encrypt a packet - * - * @param bytes is updated in place - */ - virtual void encrypt(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes) override - { - if (key.length > 0) { - initNonce(fromNode, packetId); - if (numBytes <= MAX_BLOCKSIZE) { - static uint8_t scratch[MAX_BLOCKSIZE]; - memcpy(scratch, bytes, numBytes); - memset(scratch + numBytes, 0, - sizeof(scratch) - numBytes); // Fill rest of buffer with zero (in case cypher looks at it) - - ctr->setIV(nonce, sizeof(nonce)); - ctr->setCounterSize(4); - ctr->encrypt(bytes, scratch, numBytes); - } else { - LOG_ERROR("Packet too large for crypto engine: %d. noop encryption!\n", numBytes); - } - } - } - - virtual void decrypt(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes) override - { - // For CTR, the implementation is the same - encrypt(fromNode, packetId, numBytes, bytes); - } - - private: -}; - -CryptoEngine *crypto = new CrossPlatformCryptoEngine(); diff --git a/src/platform/rp2040/rp2040CryptoEngine.cpp b/src/platform/rp2040/rp2040CryptoEngine.cpp deleted file mode 100644 index 5486e51e5..000000000 --- a/src/platform/rp2040/rp2040CryptoEngine.cpp +++ /dev/null @@ -1,66 +0,0 @@ -#include "AES.h" -#include "CTR.h" -#include "CryptoEngine.h" -#include "configuration.h" - -class RP2040CryptoEngine : public CryptoEngine -{ - - CTRCommon *ctr = NULL; - - public: - RP2040CryptoEngine() {} - - ~RP2040CryptoEngine() {} - - virtual void setKey(const CryptoKey &k) override - { - CryptoEngine::setKey(k); - LOG_DEBUG("Installing AES%d key!\n", key.length * 8); - if (ctr) { - delete ctr; - ctr = NULL; - } - if (key.length != 0) { - if (key.length == 16) - ctr = new CTR(); - else - ctr = new CTR(); - - ctr->setKey(key.bytes, key.length); - } - } - /** - * Encrypt a packet - * - * @param bytes is updated in place - */ - virtual void encrypt(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes) override - { - if (key.length > 0) { - initNonce(fromNode, packetId); - if (numBytes <= MAX_BLOCKSIZE) { - static uint8_t scratch[MAX_BLOCKSIZE]; - memcpy(scratch, bytes, numBytes); - memset(scratch + numBytes, 0, - sizeof(scratch) - numBytes); // Fill rest of buffer with zero (in case cypher looks at it) - - ctr->setIV(nonce, sizeof(nonce)); - ctr->setCounterSize(4); - ctr->encrypt(bytes, scratch, numBytes); - } else { - LOG_ERROR("Packet too large for crypto engine: %d. noop encryption!\n", numBytes); - } - } - } - - virtual void decrypt(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes) override - { - // For CTR, the implementation is the same - encrypt(fromNode, packetId, numBytes, bytes); - } - - private: -}; - -CryptoEngine *crypto = new RP2040CryptoEngine(); diff --git a/src/platform/stm32wl/STM32WLCryptoEngine.cpp b/src/platform/stm32wl/STM32WLCryptoEngine.cpp deleted file mode 100644 index 4debdf78e..000000000 --- a/src/platform/stm32wl/STM32WLCryptoEngine.cpp +++ /dev/null @@ -1,67 +0,0 @@ -#undef RNG -#include "AES.h" -#include "CTR.h" -#include "CryptoEngine.h" -#include "configuration.h" - -class STM32WLCryptoEngine : public CryptoEngine -{ - - CTRCommon *ctr = NULL; - - public: - STM32WLCryptoEngine() {} - - ~STM32WLCryptoEngine() {} - - virtual void setKey(const CryptoKey &k) override - { - CryptoEngine::setKey(k); - LOG_DEBUG("Installing AES%d key!\n", key.length * 8); - if (ctr) { - delete ctr; - ctr = NULL; - } - if (key.length != 0) { - if (key.length == 16) - ctr = new CTR(); - else - ctr = new CTR(); - - ctr->setKey(key.bytes, key.length); - } - } - /** - * Encrypt a packet - * - * @param bytes is updated in place - */ - virtual void encrypt(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes) override - { - if (key.length > 0) { - initNonce(fromNode, packetId); - if (numBytes <= MAX_BLOCKSIZE) { - static uint8_t scratch[MAX_BLOCKSIZE]; - memcpy(scratch, bytes, numBytes); - memset(scratch + numBytes, 0, - sizeof(scratch) - numBytes); // Fill rest of buffer with zero (in case cypher looks at it) - - ctr->setIV(nonce, sizeof(nonce)); - ctr->setCounterSize(4); - ctr->encrypt(bytes, scratch, numBytes); - } else { - LOG_ERROR("Packet too large for crypto engine: %d. noop encryption!\n", numBytes); - } - } - } - - virtual void decrypt(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes) override - { - // For CTR, the implementation is the same - encrypt(fromNode, packetId, numBytes, bytes); - } - - private: -}; - -CryptoEngine *crypto = new STM32WLCryptoEngine(); diff --git a/test/test_crypto/test_main.cpp b/test/test_crypto/test_main.cpp index e9aee928e..129c88283 100644 --- a/test/test_crypto/test_main.cpp +++ b/test/test_crypto/test_main.cpp @@ -99,6 +99,32 @@ void test_DH25519(void) crypto->setDHPrivateKey(private_key); TEST_ASSERT(!crypto->setDHPublicKey(public_key)); // Weak public key results in 0 shared key } +void test_AES_CTR(void) +{ + uint8_t expected[32]; + uint8_t plain[32]; + uint8_t nonce[32]; + CryptoKey k; + + // vectors from https://www.rfc-editor.org/rfc/rfc3686#section-6 + k.length = 32; + HexToBytes(k.bytes, "776BEFF2851DB06F4C8A0542C8696F6C6A81AF1EEC96B4D37FC1D689E6C1C104"); + HexToBytes(nonce, "00000060DB5672C97AA8F0B200000001"); + HexToBytes(expected, "145AD01DBF824EC7560863DC71E3E0C0"); + memcpy(plain, "Single block msg", 16); + + crypto->encryptAESCtr(k, nonce, 16, plain); + TEST_ASSERT_EQUAL_MEMORY(expected, plain, 16); + + k.length = 16; + memcpy(plain, "Single block msg", 16); + HexToBytes(k.bytes, "AE6852F8121067CC4BF7A5765577F39E"); + HexToBytes(nonce, "00000030000000000000000000000001"); + HexToBytes(expected, "E4095D4FB7A7B3792D6175A3261311B8"); + crypto->encryptAESCtr(k, nonce, 16, plain); + TEST_ASSERT_EQUAL_MEMORY(expected, plain, 16); +} + void setup() { // NOTE!!! Wait for >2 secs @@ -109,6 +135,7 @@ void setup() RUN_TEST(test_SHA256); RUN_TEST(test_ECB_AES256); RUN_TEST(test_DH25519); + RUN_TEST(test_AES_CTR); } void loop()