Merge 917b6d0cd7 into ba296db701

src/mesh/CryptoEngine.cpp

@@ -92,10 +92,9 @@ bool CryptoEngine::encryptCurve25519(uint32_t toNode, uint32_t fromNode, meshtas
         LOG_DEBUG("Node %d or their public_key not found", toNode);
         return false;
     }
-    if (!crypto->setDHPublicKey(remotePublic.bytes)) {
+    if (!setCryptoSharedSecret(remotePublic)) {
         return false;
     }
-    crypto->hash(shared_key, 32);
     initNonce(fromNode, packetNum, extraNonceTmp);
 
     // Calculate the shared secret with the destination node and encrypt
@@ -134,10 +133,9 @@ bool CryptoEngine::decryptCurve25519(uint32_t fromNode, meshtastic_UserLite_publ
     }
 
     // Calculate the shared secret with the sending node and decrypt
-    if (!crypto->setDHPublicKey(remotePublic.bytes)) {
+    if (!setCryptoSharedSecret(remotePublic)) {
         return false;
     }
-    crypto->hash(shared_key, 32);
 
     initNonce(fromNode, packetNum, extraNonce);
     printBytes("Attempt decrypt with nonce: ", nonce, 13);
@@ -266,6 +264,69 @@ void CryptoEngine::initNonce(uint32_t fromNode, uint64_t packetId, uint32_t extr
     if (extraNonce)
         memcpy(nonce + sizeof(uint32_t), &extraNonce, sizeof(uint32_t));
 }
+
+bool CryptoEngine::setCryptoSharedSecret(meshtastic_UserLite_public_key_t pubkey)
+{
+    // The last used timestamp is in units of ~1.165 hours, which gives us
+    // ~12.3 days before the timestamps roll over. This is ok since a periodic
+    // misfire on evicting the oldest secret has very little impact.
+    const uint8_t now = (millis() >> 22) & 0xff;
+
+    // Get a short lookup key from the pubkey
+    uint32_t lookupKey;
+    memcpy(&lookupKey, pubkey.bytes, sizeof(lookupKey));
+
+    uint16_t oldestDelta = 0;
+    size_t oldestIndex = 0;
+    for (size_t i = 0; i < MAX_CACHED_SHARED_SECRETS; i++) {
+        CachedSharedSecret &entry = sharedSecretCache[i];
+        if (entry.lookup_key == lookupKey) {
+            // Cache hit! Copy it into shared_key.
+            memcpy(shared_key, entry.shared_secret, 32);
+            // Update the last used timestamp
+            entry.last_used = now;
+            return true;
+        }
+
+        if (sharedSecretCache[oldestIndex].lookup_key == 0) {
+            // We already have a valid slot to insert into. Keep looking for a cache hit.
+            continue;
+        }
+
+        if (entry.lookup_key == 0) {
+            // This entry is empty. We can insert into it later, if needed.
+            oldestIndex = i;
+            continue;
+        }
+
+        // Track the oldest entry in case the cache is full.
+        uint16_t delta = 0;
+        if (now >= entry.last_used) {
+            delta = now - entry.last_used;
+        } else {
+            // Assume a larger last used timestamp is further in the past
+            delta = uint16_t(0x100) + now - entry.last_used;
+        }
+        if (delta > oldestDelta) {
+            oldestIndex = i;
+            oldestDelta = delta;
+        }
+    }
+
+    // Cache miss. Generate the shared secret.
+    if (!setDHPublicKey(pubkey.bytes)) {
+        return false;
+    }
+    hash(shared_key, 32);
+
+    // Insert the calculated shared secret into the cache, overwriting an old entry if needed.
+    CachedSharedSecret &oldestEntry = sharedSecretCache[oldestIndex];
+    oldestEntry.lookup_key = lookupKey;
+    oldestEntry.last_used = now;
+    memcpy(oldestEntry.shared_secret, shared_key, 32);
+    return true;
+}
+
 #ifndef HAS_CUSTOM_CRYPTO_ENGINE
 CryptoEngine *crypto = new CryptoEngine;
 #endif

src/mesh/CryptoEngine.h

@@ -15,6 +15,12 @@ struct CryptoKey {
     int8_t length;
 };
 
+struct CachedSharedSecret {
+    uint32_t lookup_key;
+    uint8_t shared_secret[32];
+    uint8_t last_used;
+};
+
 /**
  * see docs/software/crypto.md for details.
  *
@@ -23,6 +29,18 @@ struct CryptoKey {
 #define MAX_BLOCKSIZE 256
 #define TEST_CURVE25519_FIELD_OPS // Exposes Curve25519::isWeakPoint() for testing keys
 
+/**
+ * Max number of cached secrets to track. This should be roughly dependent on MAX_NUM_NODES but
+ * cannot be directly because it is not a constant expression.
+ */
+#if defined(ARCH_STM32WL)
+#define MAX_CACHED_SHARED_SECRETS 2
+#elif defined(ARCH_NRF52)
+#define MAX_CACHED_SHARED_SECRETS 8
+#else
+#define MAX_CACHED_SHARED_SECRETS 10
+#endif
+
 class CryptoEngine
 {
   public:
@@ -92,6 +110,19 @@ class CryptoEngine
      * a 32 bit block counter (starts at zero)
      */
     void initNonce(uint32_t fromNode, uint64_t packetId, uint32_t extraNonce = 0);
+
+    /**
+     * Cache mapping peers' public keys -> {shared_secret, last_used}
+     */
+    CachedSharedSecret sharedSecretCache[MAX_CACHED_SHARED_SECRETS] = {0};
+
+    /**
+     * Set cryptographic (hashed) shared_key calculated from the given pubkey
+     */
+    bool setCryptoSharedSecret(meshtastic_UserLite_public_key_t pubkey);
+
+    // Allow unit test harness to peer into private/protected members
+    friend struct TestCryptoEngine;
 };
 
 extern CryptoEngine *crypto;

test/test_crypto/test_main.cpp

@@ -4,6 +4,19 @@
 #include "TestUtil.h"
 #include <unity.h>
 
+struct TestCryptoEngine {
+    static bool getCachedSecret(uint32_t lookupKey, CachedSharedSecret &entry)
+    {
+        for (size_t i = 0; i < MAX_CACHED_SHARED_SECRETS; i++) {
+            entry = crypto->sharedSecretCache[i];
+            if (entry.lookup_key == lookupKey) {
+                return true;
+            }
+        }
+        return false;
+    }
+};
+
 void HexToBytes(uint8_t *result, const std::string hex, size_t len = 0)
 {
     if (len) {
@@ -108,6 +121,33 @@ void test_DH25519(void)
     TEST_ASSERT(crypto->setDHPublicKey(public_key));
     crypto->hash(crypto->shared_key, 32);
     TEST_ASSERT_EQUAL_MEMORY(expected_shared, crypto->shared_key, 32);
+
+    // Caching code path generates the same secret
+    uint8_t now = (millis() >> 22) & 0xff;
+    meshtastic_UserLite_public_key_t userlite_public_key;
+    memcpy(userlite_public_key.bytes, public_key, 32);
+    TEST_ASSERT(crypto->setCryptoSharedSecret(userlite_public_key));
+    TEST_ASSERT_EQUAL_MEMORY(expected_shared, crypto->shared_key, 32);
+
+    // Check it was added to the cache
+    CachedSharedSecret entry;
+    uint32_t lookupKey;
+    memcpy(&lookupKey, public_key, sizeof(lookupKey));
+    TEST_ASSERT(TestCryptoEngine::getCachedSecret(lookupKey, entry));
+    TEST_ASSERT_EQUAL_MEMORY(expected_shared, entry.shared_secret, 32);
+    TEST_ASSERT_TRUE(entry.last_used >= now);
+
+    // Calling again should fetch from the cache. Shared secret is the same.
+    // FIXME If tests could mock the millis() time, it would be ideal to mock the time forward by a
+    // couple hours before hitting the cache.
+    TEST_ASSERT(crypto->setCryptoSharedSecret(userlite_public_key));
+    TEST_ASSERT_EQUAL_MEMORY(expected_shared, crypto->shared_key, 32);
+
+    // Check cache was updated
+    now = (millis() >> 22) & 0xff;
+    TEST_ASSERT(TestCryptoEngine::getCachedSecret(lookupKey, entry));
+    TEST_ASSERT_EQUAL_MEMORY(expected_shared, entry.shared_secret, 32);
+    TEST_ASSERT_TRUE(entry.last_used >= now);
 }
 
 void test_PKC(void)