fix(crypto): constant-time MAC compare

[swaits]

Summary

Utils::MACThenDecrypt in src/Utils.cpp compares the computed HMAC to the packet's MAC bytes with memcmp, which is the textbook timing-oracle pattern — it returns at the first mismatched byte. This change replaces it with an XOR-OR constant-time loop.

Background

if (memcmp(hmac, src, CIPHER_MAC_SIZE) == 0) {  // before

memcmp is allowed to return as soon as it finds a mismatch. Across many MAC-validation attempts, an attacker can in principle observe the byte-of-first-mismatch through the (very small) timing difference and reduce a 16-bit forgery problem toward 2×8-bit problems.

The practical exploitability here is low — CIPHER_MAC_SIZE = 2 so there are only ~2 bytes of timing signal to leak, and the LoRa receive-path noise dwarfs sub-microsecond timing differences. But:

1. The fix is six lines and adds no overhead worth measuring.
2. It removes the pattern from any future code reviewer's "is this safe?" pile.
3. As CIPHER_MAC_SIZE widens in any future protocol revision, having constant-time compare already in place is one less migration step.

Change

src/Utils.cpp — replace the memcmp MAC check with an XOR-OR accumulator:

uint8_t diff = 0;
for (int k = 0; k < CIPHER_MAC_SIZE; k++) diff |= hmac[k] ^ src[k];
if (diff == 0) { ... }

Every byte is always touched; loop length depends only on the public CIPHER_MAC_SIZE, never on the input data.

Why this is the minimal fix

This is the standard constant-time-equality pattern (e.g. NaCl crypto_verify, OpenSSL CRYPTO_memcmp, Go crypto/subtle.ConstantTimeCompare). Adding a dependency on a vendor library for two bytes would be over-engineering on an embedded target.

Risk / compatibility

• Wire format: unchanged.
• Behaviour: bit-for-bit identical accept/reject decisions; only the timing of the rejection path changes.
• Performance: ~2 byte-XORs per inbound encrypted packet. Below measurement noise.

References

• CWE-208 — Observable Timing Discrepancy.
• D. J. Bernstein, "Cache-timing attacks on AES" (2005) — canonical motivation for constant-time crypto primitives.
• Go stdlib crypto/subtle documentation, "Functions in this package … all execute in constant time" — reference pattern.

[nextgens]

This is #1656 but worse.

[swaits]

You're right on both counts — apologies, and thanks for the pointer.

secure_compare() from <Crypto.h> (already a dep via AES128/SHA256) is the right primitive; my hand-rolled XOR loop reinvents it and, with CIPHER_MAC_SIZE = 2 as a compile-time constant, is more vulnerable to compiler unrolling than the library version. #1656 is the cleaner fix.

Closing in favor of #1656.