Clarify the nonce values/sizes

src/mesh/CryptoEngine.cpp

@@ -221,7 +221,7 @@ bool CryptoEngine::ensurePkiKeys(meshtastic_Config_SecurityConfig &security, mes
  * @param bytesOut Output buffer to be populated with encrypted ciphertext.
  */
 bool CryptoEngine::encryptCurve25519(uint32_t toNode, uint32_t fromNode, meshtastic_NodeInfoLite_public_key_t remotePublic,
-                                     uint64_t packetNum, size_t numBytes, const uint8_t *bytes, uint8_t *bytesOut)
+                                     uint32_t packetId, size_t numBytes, const uint8_t *bytes, uint8_t *bytesOut)
 {
     uint8_t *auth;
     // The extra nonce must be unpredictable: use the hardware RNG, falling back to the
@@ -239,7 +239,7 @@ bool CryptoEngine::encryptCurve25519(uint32_t toNode, uint32_t fromNode, meshtas
         return false;
     }
     hash(shared_key, 32);
-    initNonce(fromNode, packetNum, extraNonceTmp);
+    initNonce(fromNode, packetId, extraNonceTmp);
 
     // Calculate the shared secret with the destination node and encrypt
     printBytes("Attempt encrypt with nonce: ", nonce, 13);
@@ -261,7 +261,7 @@ bool CryptoEngine::encryptCurve25519(uint32_t toNode, uint32_t fromNode, meshtas
  * @param bytes Buffer containing ciphertext input.
  * @param bytesOut Output buffer to be populated with decrypted plaintext.
  */
-bool CryptoEngine::decryptCurve25519(uint32_t fromNode, meshtastic_NodeInfoLite_public_key_t remotePublic, uint64_t packetNum,
+bool CryptoEngine::decryptCurve25519(uint32_t fromNode, meshtastic_NodeInfoLite_public_key_t remotePublic, uint32_t packetId,
                                      size_t numBytes, const uint8_t *bytes, uint8_t *bytesOut)
 {
     const uint8_t *auth = bytes + numBytes - 12; // set to last 8 bytes of text?
@@ -281,7 +281,7 @@ bool CryptoEngine::decryptCurve25519(uint32_t fromNode, meshtastic_NodeInfoLite_
     }
     hash(shared_key, 32);
 
-    initNonce(fromNode, packetNum, extraNonce);
+    initNonce(fromNode, packetId, extraNonce);
     printBytes("Attempt decrypt with nonce: ", nonce, 13);
     printBytes("Attempt decrypt with shared_key starting with: ", shared_key, 8);
     return aes_ccm_ad(shared_key, 32, nonce, 8, bytes, numBytes - 12, nullptr, 0, auth, bytesOut);
@@ -382,7 +382,7 @@ void CryptoEngine::setKey(const CryptoKey &k)
  *
  * @param bytes is updated in place
  */
-void CryptoEngine::encryptPacket(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes)
+void CryptoEngine::encryptPacket(uint32_t fromNode, uint32_t packetId, size_t numBytes, uint8_t *bytes)
 {
     if (key.length > 0) {
         initNonce(fromNode, packetId);
@@ -394,7 +394,7 @@ void CryptoEngine::encryptPacket(uint32_t fromNode, uint64_t packetId, size_t nu
     }
 }
 
-void CryptoEngine::decrypt(uint32_t fromNode, uint64_t packetId, size_t numBytes, uint8_t *bytes)
+void CryptoEngine::decrypt(uint32_t fromNode, uint32_t packetId, size_t numBytes, uint8_t *bytes)
 {
     // For CTR, the implementation is the same
     encryptPacket(fromNode, packetId, numBytes, bytes);
@@ -421,16 +421,28 @@ void CryptoEngine::encryptAESCtr(CryptoKey _key, uint8_t *_nonce, size_t numByte
 
 /**
  * Init our 128 bit nonce for a new packet
+ *
+ * 0            4            8            12           16
+ * +------------+------------+------------+------------+
+ * |  packetId  | extraNonce |  fromNode  |  counter   |
+ * +------------+------------+------------+------------+
+ *
+ * |<--- 13 bytes used for AES-CCM nonce ---->|
+ * |<-------- 16 bytes used for AES-CTR nonce -------->|
+ *
+ * extraNonce is always 0 for AES-CTR (Channel/classic). counter is the AES-CTR
+ * block counter: it starts at 0 and encryptAESCtr's setCounterSize(4) makes it
+ * the only part CTR increments.
  */
-void CryptoEngine::initNonce(uint32_t fromNode, uint64_t packetId, uint32_t extraNonce)
+void CryptoEngine::initNonce(uint32_t fromNode, uint32_t packetId, uint32_t extraNonce)
 {
     memset(nonce, 0, sizeof(nonce));
 
     // use memcpy to avoid breaking strict-aliasing
-    memcpy(nonce, &packetId, sizeof(uint64_t));
-    memcpy(nonce + sizeof(uint64_t), &fromNode, sizeof(uint32_t));
+    memcpy(nonce, &packetId, sizeof(uint32_t));
     if (extraNonce)
         memcpy(nonce + sizeof(uint32_t), &extraNonce, sizeof(uint32_t));
+    memcpy(nonce + sizeof(uint32_t) * 2, &fromNode, sizeof(uint32_t));
 }
 #ifndef HAS_CUSTOM_CRYPTO_ENGINE
 CryptoEngine *crypto = new CryptoEngine;

src/mesh/CryptoEngine.h

@@ -53,8 +53,8 @@ class CryptoEngine
     // a stored node header. NodeInfoLite is the on-device storage type since
     // the slim refactor flattened UserLite into it.
     virtual bool encryptCurve25519(uint32_t toNode, uint32_t fromNode, meshtastic_NodeInfoLite_public_key_t remotePublic,
-                                   uint64_t packetNum, size_t numBytes, const uint8_t *bytes, uint8_t *bytesOut);
-    virtual bool decryptCurve25519(uint32_t fromNode, meshtastic_NodeInfoLite_public_key_t remotePublic, uint64_t packetNum,
+                                   uint32_t packetId, size_t numBytes, const uint8_t *bytes, uint8_t *bytesOut);
+    virtual bool decryptCurve25519(uint32_t fromNode, meshtastic_NodeInfoLite_public_key_t remotePublic, uint32_t packetId,
                                    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);
@@ -93,8 +93,8 @@ class CryptoEngine
      *
      * @param bytes is updated in place
      */
-    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 encryptPacket(uint32_t fromNode, uint32_t packetId, size_t numBytes, uint8_t *bytes);
+    virtual void decrypt(uint32_t fromNode, uint32_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:
@@ -121,12 +121,10 @@ class CryptoEngine
     /**
      * Init our 128 bit nonce for a new packet
      *
-     * The NONCE is constructed by concatenating (from MSB to LSB):
-     * a 64 bit packet number (stored in little endian order)
-     * a 32 bit sending node number (stored in little endian order)
-     * a 32 bit block counter (starts at zero)
+     * Little endian throughout: packetId | extraNonce | fromNode | AES-CTR block counter.
+     * See initNonce() in CryptoEngine.cpp for the byte offsets.
      */
-    void initNonce(uint32_t fromNode, uint64_t packetId, uint32_t extraNonce = 0);
+    void initNonce(uint32_t fromNode, uint32_t packetId, uint32_t extraNonce = 0);
 };
 
 extern CryptoEngine *crypto;

test/test_crypto/test_main.cpp

@@ -134,7 +134,7 @@ void test_PKC(void)
     uint8_t expected_nonce[16];
 
     uint32_t fromNode = 0x0929;
-    uint64_t packetNum = 0x13b2d662;
+    uint32_t packetNum = 0x13b2d662;
     HexToBytes(public_key.bytes, "db18fc50eea47f00251cb784819a3cf5fc361882597f589f0d7ff820e8064457");
     public_key.size = 32;
     HexToBytes(private_key, "a00330633e63522f8a4d81ec6d9d1e6617f6c8ffd3a4c698229537d44e522277");