Merge pull request mysensors#444 from fallberg/memcmp

Add and use timing neutral memcmp for signing
JosephEverhart · May 22, 2016 · 347bbab · 347bbab
2 parents 1a11c54 + 585150c
commit 347bbab
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 2 deletions.
diff --git a/libraries/MySensors/core/MySigning.cpp b/libraries/MySensors/core/MySigning.cpp
@@ -407,3 +407,32 @@ uint8_t* signerSha256Final(void) {
 	memcpy(sha256_hash, _soft_sha256.result(), 32);
 	return sha256_hash;
 }
+
+int signerMemcmp(const void* a, const void* b, size_t sz) {
+	int retVal;
+	size_t i;
+	int done = 0;
+	const uint8_t* ptrA = (const uint8_t*)a;
+	const uint8_t* ptrB = (const uint8_t*)b;
+	for (i=0; i < sz; i++) {
+		if (ptrA[i] == ptrB[i]) {
+			if (done > 0) {
+				done = 1;
+			} else {
+				done = 0;
+			}
+		}	else {
+			if (done > 0) {
+				done = 2;
+			} else {
+				done = 3;
+			}
+		}
+	}
+	if (done > 0) {
+		retVal = -1;
+	} else {
+		retVal = 0;
+	}
+	return retVal;
+}
diff --git a/libraries/MySensors/core/MySigning.h b/libraries/MySensors/core/MySigning.h
@@ -622,5 +622,19 @@ void signerSha256Update(const uint8_t* data, size_t sz);
  */
 uint8_t* signerSha256Final(void);
 
+/**
+ * @brief Do a timing neutral memory comparison.
+ *
+ * The function behaves similar to memcmp with the difference that it will
+ * always use the same number of instructions for a given number of bytes,
+ * no matter how the two buffers differ and the response is either 0 or -1.
+ *
+ * @param a First buffer for comparison.
+ * @param b Second buffer for comparison.
+ * @param sz The number of bytes to compare.
+ * @returns 0 if buffers match, -1 if they do not.
+ */
+int signerMemcmp(const void* a, const void* b, size_t sz);
+
 #endif
 /** @}*/
diff --git a/libraries/MySensors/core/MySigningAtsha204.cpp b/libraries/MySensors/core/MySigningAtsha204.cpp
@@ -212,7 +212,7 @@ bool signerAtsha204VerifyMsg(MyMessage &msg) {
 		_singning_rx_buffer[SHA204_BUFFER_POS_DATA] = SIGNING_IDENTIFIER;
 
 		// Compare the caluclated signature with the provided signature
-		if (memcmp(&msg.data[mGetLength(msg)], &_singning_rx_buffer[SHA204_BUFFER_POS_DATA], MAX_PAYLOAD-mGetLength(msg))) {
+		if (signerMemcmp(&msg.data[mGetLength(msg)], &_singning_rx_buffer[SHA204_BUFFER_POS_DATA], MAX_PAYLOAD-mGetLength(msg))) {
 			DEBUG_SIGNING_PRINTBUF(F("Signature bad: "), &_singning_rx_buffer[SHA204_BUFFER_POS_DATA], MAX_PAYLOAD-mGetLength(msg));
 #ifdef MY_SIGNING_NODE_WHITELISTING
 			DEBUG_SIGNING_PRINTBUF(F("Is the sender whitelisted and serial correct?"), NULL, 0);

diff --git a/libraries/MySensors/core/MySigningAtsha204Soft.cpp b/libraries/MySensors/core/MySigningAtsha204Soft.cpp
@@ -222,7 +222,7 @@ bool signerAtsha204SoftVerifyMsg(MyMessage &msg) {
 		_signing_hmac[0] = SIGNING_IDENTIFIER;
 
 		// Compare the caluclated signature with the provided signature
-		if (memcmp(&msg.data[mGetLength(msg)], _signing_hmac, MAX_PAYLOAD-mGetLength(msg))) {
+		if (signerMemcmp(&msg.data[mGetLength(msg)], _signing_hmac, MAX_PAYLOAD-mGetLength(msg))) {
 			DEBUG_SIGNING_PRINTBUF(F("Signature bad: "), _signing_hmac, MAX_PAYLOAD-mGetLength(msg));
 #ifdef MY_SIGNING_NODE_WHITELISTING
 			DEBUG_SIGNING_PRINTBUF(F("Is the sender whitelisted and serial correct?"), NULL, 0);