From 88dccf2e247b29ab3edf9f6cd4f72cf2104ade88 Mon Sep 17 00:00:00 2001
From: liquidraver <504870+liquidraver@users.noreply.github.com>
Date: Thu, 28 May 2026 13:08:18 +0200
Subject: [PATCH] crypto: switch login password compare to volatile-based
 ct_memeq
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

mbedtls_ct_memcmp is declared in the tf-psa-crypto header but its
implementation isn't compiled into the current Zephyr mbedtls build
(would require enabling additional TLS features). Use a local
ct_memeq() with `volatile uint8_t` accumulator instead — pattern
matches rweather/arduinolibs Crypto.cpp secure_compare().

Disassembly verified on rak3401_1watt (Thumb-2): loop branches on
the iterator pointer not the accumulator, result is load-modify-
stored to stack every iteration (volatile preserved), final return
uses clz+shift instead of a conditional branch on the value.

Spotted by nextgens during review of meshcore-dev/MeshCore#2556
mitigations.
---
 zephcore/app/RepeaterMesh.cpp | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/zephcore/app/RepeaterMesh.cpp b/zephcore/app/RepeaterMesh.cpp
index 6196f02..6298d67 100644
--- a/zephcore/app/RepeaterMesh.cpp
+++ b/zephcore/app/RepeaterMesh.cpp
@@ -122,18 +122,22 @@ void RepeaterMesh::putNeighbour(const mesh::Identity& id, uint32_t timestamp, fl
  * byte of `a` matches `b`. No early exit — timing is independent of input,
  * defeating timing-leak attacks against password comparison.
  *
+ * `volatile` on the accumulator prevents the compiler from short-circuiting
+ * the XOR-OR loop back into a branching memcmp under aggressive LTO. Pattern
+ * matches rweather/arduinolibs Crypto.cpp secure_compare(). mbedtls offers
+ * mbedtls_ct_memcmp() but its implementation lives in a .c file that isn't
+ * pulled into the Zephyr mbedtls build under current Kconfig — would require
+ * enabling additional TLS features just for this one function.
+ *
  * Mitigation for the password-handling weakness tracked upstream as
- * meshcore-dev/MeshCore#2556. The protocol still sends plaintext passwords
- * (encrypted only at the mesh layer to whatever pubkey the user selected),
- * but this fix removes the timing oracle that would otherwise let an
- * already-MITM attacker reveal the stored password byte-by-byte. */
+ * meshcore-dev/MeshCore#2556. See CRYPTO_AUDIT_INDEX.md P4.F1. */
 static bool ct_memeq(const uint8_t *a, const uint8_t *b, size_t n)
 {
-    uint8_t diff = 0;
+    volatile uint8_t result = 0;
     for (size_t i = 0; i < n; i++) {
-        diff |= (uint8_t)(a[i] ^ b[i]);
+        result |= (uint8_t)(a[i] ^ b[i]);
     }
-    return diff == 0;
+    return result == 0;
 }
 
 uint8_t RepeaterMesh::handleLoginReq(const mesh::Identity& sender, const uint8_t* secret, uint32_t sender_timestamp, const uint8_t* data, bool is_flood) {