Browse Source

use constant-time comparison for MAC verification

The HMAC check in MACThenDecrypt used standard memcmp(), which
short-circuits on the first mismatched byte. This makes the comparison
time dependent on how many bytes of the MAC are correct, leaking
information through a timing side-channel.

With a 2-byte MAC (65,536 possible values), an attacker on a local
interface (serial, BLE, or WiFi) can measure response latency to
distinguish "first byte wrong" from "first byte correct, second wrong".
This reduces a brute-force from 65,536 attempts down to roughly 384
(256 + 128 on average), making MAC forgery practical. An attacker could
use this to forge packets that pass MAC verification without knowing the
shared secret, allowing them to inject arbitrary messages that appear to
come from a trusted peer.

Replace memcmp with a constant-time XOR-accumulate loop so the
comparison always takes the same time regardless of which bytes match.
pull/1656/head
Wessel Nieboer 4 months ago
parent
commit
f248aa6cd9
No known key found for this signature in database GPG Key ID: 929C8E45E33B5FD2
  1. 5
      src/Utils.cpp

5
src/Utils.cpp

@ -81,7 +81,10 @@ int Utils::MACThenDecrypt(const uint8_t* shared_secret, uint8_t* dest, const uin
sha.update(src + CIPHER_MAC_SIZE, src_len - CIPHER_MAC_SIZE);
sha.finalizeHMAC(shared_secret, PUB_KEY_SIZE, hmac, CIPHER_MAC_SIZE);
}
if (memcmp(hmac, src, CIPHER_MAC_SIZE) == 0) {
// constant-time comparison to prevent timing side-channel attacks
uint8_t diff = 0;
for (int i = 0; i < CIPHER_MAC_SIZE; i++) diff |= hmac[i] ^ src[i];
if (diff == 0) {
return decrypt(shared_secret, dest, src + CIPHER_MAC_SIZE, src_len - CIPHER_MAC_SIZE);
}
return 0; // invalid HMAC

Loading…
Cancel
Save