Refactor platform cryptography, add tests

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<AES128>();
+    else
+        ctr = new CTR<AES256>();
+    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));
-}
+}
+#ifndef HAS_CUSTOM_CRYPTO_ENGINE
+CryptoEngine *crypto = new CryptoEngine;
+#endif

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};

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

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) {
+        if (_key.length > 0) {
-            LOG_DEBUG("ESP32 crypt fr=%x, num=%x, numBytes=%d!\n", fromNode, (uint32_t)packetId, numBytes);
-            initNonce(fromNode, packetId);
             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)
-
+                mbedtls_aes_crypt_ctr(&aes, numBytes, &nc_off, _nonce, stream_block, scratch, bytes);
-                auto res = mbedtls_aes_crypt_ctr(&aes, numBytes, &nc_off, nonce, stream_block, scratch, bytes);
-                assert(!res);
             } 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();

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

src/platform/nrf52/NRF52CryptoEngine.cpp

@@ -9,41 +9,24 @@ class NRF52CryptoEngine : public CryptoEngine
 
     ~NRF52CryptoEngine() {}
 
-    /**
+    virtual void encryptAESCtr(CryptoKey _key, uint8_t *_nonce, size_t numBytes, uint8_t *bytes) override
-     * 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 > 16) {
+        if (_key.length > 16) {
-            LOG_DEBUG("Software encrypt fr=%x, num=%x, numBytes=%d!\n", fromNode, (uint32_t)packetId, numBytes);
             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) {
+        } else if (_key.length > 0) {
-            LOG_DEBUG("nRF52 encrypt fr=%x, num=%x, numBytes=%d!\n", fromNode, (uint32_t)packetId, numBytes);
             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();

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

src/platform/portduino/CrossPlatformCryptoEngine.cpp

@@ -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<AES128>();
-            else
-                ctr = new CTR<AES256>();
-
-            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();

src/platform/rp2040/rp2040CryptoEngine.cpp

@@ -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<AES128>();
-            else
-                ctr = new CTR<AES256>();
-
-            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();

src/platform/stm32wl/STM32WLCryptoEngine.cpp

@@ -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<AES128>();
-            else
-                ctr = new CTR<AES256>();
-
-            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();

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()