Rewrite security personalization data validation (#885)

* Data is not validated on Linux systems
* If encryption is enabled, data is also validated even if signing
  is disabled.

MySensors.h

@@ -1,4 +1,4 @@
-/*
+/*
  * The MySensors Arduino library handles the wireless radio link and protocol
  * between your home built sensors/actuators and HA controller of choice.
  * The sensors forms a self healing radio network with optional repeaters. Each
@@ -147,6 +147,9 @@ MY_DEFAULT_RX_LED_PIN in your sketch instead to enable LEDs
 #if defined(MY_SIGNING_ATSHA204) || defined(MY_SIGNING_SOFT)
 #define MY_SIGNING_FEATURE //!< MY_SIGNING_FEATURE
 #endif
+#if defined(MY_RF24_ENABLE_ENCRYPTION) || defined(MY_RFM69_ENABLE_ENCRYPTION) || defined(MY_NRF5_ESB_ENABLE_ENCRYPTION)
+#define MY_ENCRYPTION_FEATURE //!< MY_ENCRYPTION_FEATURE
+#endif
 #include "core/MySigning.cpp"
 #include "drivers/ATSHA204/sha256.cpp"
 #if defined(MY_SIGNING_FEATURE)

core/MySigning.cpp

@@ -38,10 +38,11 @@
 #error You have to pick one and only one signing backend
 #endif
 #ifdef MY_SIGNING_FEATURE
-uint8_t _doSign[32];      // Bitfield indicating which sensors require signed communication
+static uint8_t _doSign[32];      // Bitfield indicating which sensors require signed communication
-uint8_t _doWhitelist[32]; // Bitfield indicating which sensors require serial salted signatures
+static uint8_t _doWhitelist[32]; // Bitfield indicating which sensors require salted signatures
-MyMessage _msgSign;       // Buffer for message to sign.
+static MyMessage _msgSign;       // Buffer for message to sign.
-uint8_t _signingNonceStatus;
+static uint8_t _signingNonceStatus;
+static bool stateValid = false;
 
 #ifdef MY_NODE_LOCK_FEATURE
 static uint8_t nof_nonce_requests = 0;
@@ -98,9 +99,25 @@ static void prepareSigningPresentation(MyMessage &msg, uint8_t destination);
 static bool signerInternalProcessPresentation(MyMessage &msg);
 static bool signerInternalProcessNonceRequest(MyMessage &msg);
 static bool signerInternalProcessNonceResponse(MyMessage &msg);
+#if defined(MY_ENCRYPTION_FEATURE) || defined(MY_SIGNING_FEATURE)
+static bool signerInternalValidatePersonalization(void);
+#endif
 
 void signerInit(void)
 {
+#if defined(MY_SIGNING_FEATURE)
+	stateValid = true;
+#endif
+#if defined (MY_ENCRYPTION_FEATURE) || defined (MY_SIGNING_FEATURE)
+	if (!signerInternalValidatePersonalization()) {
+		SIGN_DEBUG(PSTR("!SGN:PER:TAMPERED\n"));
+#if defined(MY_SIGNING_FEATURE)
+		stateValid = false;
+#endif
+	} else {
+		SIGN_DEBUG(PSTR("SGN:PER:OK\n"));
+	}
+#endif
 #if defined(MY_SIGNING_FEATURE)
 	// Read out the signing requirements from EEPROM
 	hwReadConfigBlock((void*)_doSign, (void*)EEPROM_SIGNING_REQUIREMENT_TABLE_ADDRESS,
@@ -181,6 +198,11 @@ bool signerSignMsg(MyMessage &msg)
 	if (DO_SIGN(msg.destination) && msg.sender == getNodeId()) {
 		if (skipSign(msg)) {
 			ret = true;
+		} else {
+			// Before starting, validate that our state is good, or signing will fail
+			if (!stateValid) {
+				SIGN_DEBUG(PSTR("!SGN:SGN:STATE\n")); // Signing system is not in a valid state
+				ret = false;
 			} else {
 				// Send nonce-request
 				_signingNonceStatus=SIGN_WAITING_FOR_NONCE;
@@ -218,6 +240,7 @@ bool signerSignMsg(MyMessage &msg)
 					}
 				}
 			}
+		}
 	} else if (getNodeId() == msg.sender) {
 		mSetSigned(msg, 0); // Message is not supposed to be signed, make sure it is marked unsigned
 		SIGN_DEBUG(PSTR("SGN:SGN:NREQ=%d\n"), msg.destination); // Do not sign message as it is not req
@@ -255,6 +278,11 @@ bool signerVerifyMsg(MyMessage &msg)
 			// Got unsigned message that should have been signed
 			SIGN_DEBUG(PSTR("!SGN:VER:NSG\n")); // Message is not signed, but it should have been!
 			verificationResult = false;
+		} else {
+			// Before starting, validate that our state is good, or signing will fail
+			if (!stateValid) {
+				SIGN_DEBUG(PSTR("!SGN:VER:STATE\n")); // Signing system is not in a valid state
+				verificationResult = false;
 			} else {
 				if (!signerBackendVerifyMsg(msg)) {
 					SIGN_DEBUG(PSTR("!SGN:VER:FAIL\n")); // Signature verification failed!
@@ -262,6 +290,7 @@ bool signerVerifyMsg(MyMessage &msg)
 				} else {
 					SIGN_DEBUG(PSTR("SGN:VER:OK\n"));
 				}
+			}
 #if defined(MY_NODE_LOCK_FEATURE)
 			if (verificationResult) {
 				// On successful verification, clear lock counters
@@ -533,3 +562,32 @@ static bool signerInternalProcessNonceResponse(MyMessage &msg)
 #endif
 	return true; // No need to further process I_NONCE_RESPONSE
 }
+
+#if defined(MY_ENCRYPTION_FEATURE) || defined(MY_SIGNING_FEATURE)
+static bool signerInternalValidatePersonalization(void)
+{
+#ifdef __linux__
+	// Personalization on linux is a bit more crude
+	return true;
+#else
+	uint8_t buffer[32];
+	uint8_t* hash;
+	uint8_t checksum;
+
+	signerSha256Init();
+	hwReadConfigBlock((void*)buffer, (void*)EEPROM_SIGNING_SOFT_HMAC_KEY_ADDRESS, 32);
+	signerSha256Update(buffer, 32);
+	hwReadConfigBlock((void*)buffer, (void*)EEPROM_RF_ENCRYPTION_AES_KEY_ADDRESS, 16);
+	signerSha256Update(buffer, 16);
+	hwReadConfigBlock((void*)buffer, (void*)EEPROM_SIGNING_SOFT_SERIAL_ADDRESS, 9);
+	signerSha256Update(buffer, 9);
+	hash = signerSha256Final();
+	hwReadConfigBlock((void*)&checksum, (void*)EEPROM_PERSONALIZATION_CHECKSUM_ADDRESS, 1);
+	if (checksum != hash[0]) {
+		return false;
+	} else {
+		return true;
+	}
+#endif
+}
+#endif

core/MySigning.h

@@ -790,10 +790,12 @@ int signerMemcmp(const void* a, const void* b, size_t sz);
  * |!| SGN | SGN | SGN FAIL									| Message failed to be signed
  * | | SGN | SGN | NREQ='node'							| 'node' does not require signed messages
  * | | SGN | SGN | 'sender'!='us' NUS				| Will not sign because 'sender' is not 'us' (repeater)
+ * |!| SGN | SGN | STATE  									| Security system in a invalid state (personalization data tampered)
  * |!| SGN | VER | NSG											| Message was not signed, but it should have been
  * |!| SGN | VER | FAIL											| Verification failed
  * | | SGN | VER | OK												| Verification succeeded
  * | | SGN | VER | LEFT='number'						| 'number' of failed verifications left in a row before node is locked
+ * |!| SGN | VER | STATE  									| Security system in a invalid state (personalization data tampered)
  * | | SGN | SKP | MSG CMD='cmd',TYPE='type'| Message with command 'cmd' and type 'type' does not need to be signed
  * | | SGN | SKP | ACK CMD='cmd',TYPE='type'| ACK messages does not need to be signed
  * | | SGN | NCE | LEFT='number'						| 'number' of nonce requests between successful verifications left before node is locked

core/MySigningAtsha204Soft.cpp

@@ -103,33 +103,6 @@ bool signerAtsha204SoftInit(void)
 		_signing_node_serial_info[8] = getNodeId();
 	}
 #else
-	uint8_t buffer[32];
-	uint8_t* hash;
-	uint8_t checksum;
-
-	_signing_sha256.init();
-	hwReadConfigBlock((void*)buffer, (void*)EEPROM_SIGNING_SOFT_HMAC_KEY_ADDRESS, 32);
-	for (int i = 0; i < 32; i++) {
-		_signing_sha256.write(buffer[i]);
-	}
-	hwReadConfigBlock((void*)buffer, (void*)EEPROM_RF_ENCRYPTION_AES_KEY_ADDRESS, 16);
-	for (int i = 0; i < 16; i++) {
-		_signing_sha256.write(buffer[i]);
-	}
-	hwReadConfigBlock((void*)buffer, (void*)EEPROM_SIGNING_SOFT_SERIAL_ADDRESS, 9);
-	for (int i = 0; i < 9; i++) {
-		_signing_sha256.write(buffer[i]);
-	}
-	hash = _signing_sha256.result();
-	hwReadConfigBlock((void*)&checksum, (void*)EEPROM_PERSONALIZATION_CHECKSUM_ADDRESS, 1);
-	if (checksum != hash[0]) {
-		SIGN_DEBUG(PSTR("!SGN:PER:TAMPERED\n"));
-		init_ok = false;
-	} else {
-		SIGN_DEBUG(PSTR("SGN:PER:OK\n"));
-		init_ok = true;
-	}
-
 	if (init_ok) {
 		hwReadConfigBlock((void*)_signing_hmac_key, (void*)EEPROM_SIGNING_SOFT_HMAC_KEY_ADDRESS, 32);
 		hwReadConfigBlock((void*)_signing_node_serial_info, (void*)EEPROM_SIGNING_SOFT_SERIAL_ADDRESS, 9);