From 585150c46d797e3758874d2876c85bb461b8a17f Mon Sep 17 00:00:00 2001
From: Patrick Fallberg <patrick@fallberg.net>
Date: Sun, 22 May 2016 18:34:00 +0200
Subject: [PATCH] Add and use timing neutral memcmp for signing

Fixes #443
---
 libraries/MySensors/core/MySigning.cpp        | 29 +++++++++++++++++++
 libraries/MySensors/core/MySigning.h          | 14 +++++++++
 .../MySensors/core/MySigningAtsha204.cpp      |  2 +-
 .../MySensors/core/MySigningAtsha204Soft.cpp  |  2 +-
 4 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/libraries/MySensors/core/MySigning.cpp b/libraries/MySensors/core/MySigning.cpp
index ef2d207..61ad1c6 100644
--- 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
index 21eea0b..bf41cdf 100644
--- 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
 /** @}*/
\ No newline at end of file
diff --git a/libraries/MySensors/core/MySigningAtsha204.cpp b/libraries/MySensors/core/MySigningAtsha204.cpp
index 61a1293..c920539 100644
--- 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
index 037905a..a326b0d 100644
--- 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);