Signing infrastructure refactored

Signing logic has now been centralized to MySigning.cpp in order to simplify
maintenance and debugging.
Debug prints have been extended and "demoted" to verbose level.
To get signing related debug info, MY_DEBUG_VERBOSE will need to be defined.
This fixes #278

libraries/MySensors/MySensor.h

@@ -95,9 +95,11 @@
 
 // SIGNING
 #if defined(MY_SIGNING_ATSHA204) || defined(MY_SIGNING_SOFT)
-	// SIGNING COMMON FUNCTIONS
-	#include "core/MySigning.cpp"
 	#define MY_SIGNING_FEATURE
+#endif
+#include "core/MySigning.cpp"
+#if defined(MY_SIGNING_FEATURE)
+	// SIGNING COMMON FUNCTIONS
 	#if defined(MY_SIGNING_ATSHA204) && defined(MY_SIGNING_SOFT)
 		#error Only one signing engine can be activated
 	#endif

libraries/MySensors/core/MyMessage.h

@@ -163,9 +163,9 @@ typedef enum {
 	I_INCLUSION_MODE, I_CONFIG, I_FIND_PARENT, I_FIND_PARENT_RESPONSE,
 	I_LOG_MESSAGE, I_CHILDREN, I_SKETCH_NAME, I_SKETCH_VERSION,
 	I_REBOOT, I_GATEWAY_READY,
-	I_REQUEST_SIGNING,    /*!< Indicates sender require signed messages (bool param) */
-	I_GET_NONCE,          /*!< Request for a nonce */
-	I_GET_NONCE_RESPONSE, /*!< Payload is nonce data */
+	I_REQUEST_SIGNING, /*!< Indicates sender require signed messages (bool param) */
+	I_NONCE_REQUEST,   /*!< Request for a nonce */
+	I_NONCE_RESPONSE,  /*!< Payload is nonce data */
 	I_HEARTBEAT, I_PRESENTATION, I_DISCOVER, I_DISCOVER_RESPONSE, I_HEARTBEAT_RESPONSE
 } mysensor_internal;
 

libraries/MySensors/core/MySensorCore.cpp

@@ -62,15 +62,12 @@ static inline bool isValidParent( const uint8_t parent ) {
 void _begin() {
 	#if !defined(MY_DISABLED_SERIAL)
 	    hwInit();
-    #endif
-	
-	#if defined(MY_SIGNING_SOFT)
-		// initialize pseudo-RNG
-		randomSeed(analogRead(MY_SIGNING_SOFT_RANDOMSEED_PIN));
 	#endif
 
 	debug(PSTR("Starting " MY_NODE_TYPE " (" MY_CAPABILIIES ", " LIBRARY_VERSION ")\n"));
 
+	signerInit();
+
 	#if defined(MY_RADIO_FEATURE)
 		_failedTransmissions = 0;
 
@@ -107,11 +104,6 @@ void _begin() {
 	#endif
 
 
-	#if defined(MY_SIGNING_FEATURE)
-		// Read out the signing requirements from EEPROM
-		hwReadConfigBlock((void*)_doSign, (void*)EEPROM_SIGNING_REQUIREMENT_TABLE_ADDRESS, sizeof(_doSign));
-	#endif
-
 	#if defined(MY_LEDS_BLINKING_FEATURE)
 		ledsInit();
 	#endif

libraries/MySensors/core/MySigning.cpp

@@ -19,7 +19,292 @@
 
 #include "MySigning.h"
 
+#if defined(MY_DEBUG_VERBOSE)
+#define SIGN_DEBUG(x,...) debug(x, ##__VA_ARGS__)
+#else
+#define SIGN_DEBUG(x,...)
+#endif
 
+#if defined(MY_SIGNING_REQUEST_SIGNATURES) && (!defined(MY_SIGNING_ATSHA204) && !defined(MY_SIGNING_SOFT))
+#error You have to pick either MY_SIGNING_ATSHA204 or MY_SIGNING_SOFT in order to require signatures!
+#endif
+#ifdef MY_SIGNING_FEATURE
 uint8_t _doSign[32]; // Bitfield indicating which sensors require signed communication
 MyMessage _msgSign;  // Buffer for message to sign.
+uint8_t _signingNonceStatus;
 
+// Status when waiting for signing nonce in signerProcessInternal
+enum { SIGN_WAITING_FOR_NONCE = 0, SIGN_OK = 1 };
+
+// Macros for manipulating signing requirement table
+#define DO_SIGN(node) (~_doSign[node>>3]&(1<<node%8))
+#define SET_SIGN(node) (_doSign[node>>3]&=~(1<<node%8))
+#define CLEAR_SIGN(node) (_doSign[node>>3]|=(1<<node%8))
+
+/*
+ * Stores signing identifier and a new nonce in provided message for signing operations.
+ *
+ * All space in message payload buffer is used for signing identifier and nonce.
+ * Returns false if subsystem is busy processing an ongoing signing operation.
+ * If successful, this marks the start of a signing operation at the receiving side so
+ * implementation is expected to do any necessary initializations within this call.
+ * This function is typically called as action when receiving a I_NONCE_REQUEST message.
+ */
+#if defined(MY_SIGNING_SOFT)
+extern bool signerAtsha204SoftCheckTimer(void);
+extern bool signerAtsha204SoftGetNonce(MyMessage &msg);
+extern bool signerAtsha204SoftPutNonce(MyMessage &msg);
+extern bool signerAtsha204SoftVerifyMsg(MyMessage &msg);
+extern bool signerAtsha204SoftSignMsg(MyMessage &msg);
+#endif
+#if defined(MY_SIGNING_ATSHA204)
+extern bool signerAtsha204CheckTimer(void);
+extern bool signerAtsha204GetNonce(MyMessage &msg);
+extern bool signerAtsha204PutNonce(MyMessage &msg);
+extern bool signerAtsha204VerifyMsg(MyMessage &msg);
+extern bool signerAtsha204SignMsg(MyMessage &msg);
+#endif
+
+// Helper function to centralize signing/verification exceptions
+static bool skipSign(MyMessage &msg) {
+	if mGetAck(msg) {
+		SIGN_DEBUG(PSTR("Skipping security for ACK on command %d type %d\n"), mGetCommand(msg), msg.type);
+		return true;
+	}	else if (mGetCommand(msg) == C_INTERNAL &&
+		(msg.type == I_NONCE_REQUEST || msg.type == I_NONCE_RESPONSE       || msg.type == I_REQUEST_SIGNING ||
+	   msg.type == I_ID_REQUEST    || msg.type == I_ID_RESPONSE          ||
+	   msg.type == I_FIND_PARENT   || msg.type == I_FIND_PARENT_RESPONSE ||
+	   msg.type == I_HEARTBEAT     || msg.type == I_HEARTBEAT_RESPONSE)) {
+		SIGN_DEBUG(PSTR("Skipping security for command %d type %d\n"), mGetCommand(msg), msg.type);
+		return true;
+	} else if (mGetCommand(msg) == C_STREAM &&
+		(msg.type == ST_FIRMWARE_REQUEST || msg.type == ST_FIRMWARE_RESPONSE ||
+		 msg.type == ST_SOUND || msg.type == ST_IMAGE)) {
+		SIGN_DEBUG(PSTR("Skipping security for command %d type %d\n"), mGetCommand(msg), msg.type);
+		return true;
+	} else {
+		return false;
+	}
+}
+#endif // MY_SIGNING_FEATURE
+
+void signerInit(void) {
+#if defined(MY_SIGNING_FEATURE)
+	// Read out the signing requirements from EEPROM
+	hwReadConfigBlock((void*)_doSign, (void*)EEPROM_SIGNING_REQUIREMENT_TABLE_ADDRESS, sizeof(_doSign));
+
+#if defined(MY_SIGNING_SOFT)
+	// initialize pseudo-RNG
+	randomSeed(analogRead(MY_SIGNING_SOFT_RANDOMSEED_PIN));
+#endif
+#endif
+}
+
+void signerPresentation(MyMessage &msg) {
+#ifdef MY_SIGNING_FEATURE
+#if defined(MY_SIGNING_REQUEST_SIGNATURES)
+	_sendRoute(build(msg, _nc.nodeId, GATEWAY_ADDRESS, NODE_SENSOR_ID,
+		C_INTERNAL, I_REQUEST_SIGNING, false).set(true));
+	// If we do require signing, wait for the gateway to tell us how it prefer us to transmit our messages
+	SIGN_DEBUG(PSTR("Request for signing sent, waiting for GW response...\n"));
+	wait(2000);
+#else
+	_sendRoute(build(msg, _nc.nodeId, GATEWAY_ADDRESS, NODE_SENSOR_ID,
+		C_INTERNAL, I_REQUEST_SIGNING, false).set(false));
+#endif
+#else
+	// Ww do not support signing, make sure gateway knows this
+	_sendRoute(build(msg, _nc.nodeId, GATEWAY_ADDRESS, NODE_SENSOR_ID,
+		C_INTERNAL, I_REQUEST_SIGNING, false).set(false));
+#endif
+}
+
+bool signerProcessInternal(MyMessage &msg) {
+	if (mGetCommand(msg) == C_INTERNAL) {
+#if !defined(MY_SIGNING_FEATURE) && defined(MY_GATEWAY_FEATURE)
+		// If we act as gateway and do not have the signing feature and receive a signing request we still
+		// need to do make sure the requester does not believe the gateway still require signatures
+		if (msg.type == I_REQUEST_SIGNING) {
+			_sendRoute(build(msg, _nc.nodeId, msg.sender, NODE_SENSOR_ID,
+				C_INTERNAL, I_REQUEST_SIGNING, false).set((uint8_t)false));
+			SIGN_DEBUG(PSTR("Informing node %d that we do not require signatures because we do not support it\n"),
+				msg.sender);
+			return true; // No need to further process I_REQUEST_SIGNING in this case
+		}
+#elif defined(MY_SIGNING_FEATURE)
+		if (msg.type == I_NONCE_REQUEST) {
+#if defined(MY_SIGNING_SOFT)
+			if (signerAtsha204SoftGetNonce(msg)) {
+#endif
+#if defined(MY_SIGNING_ATSHA204)
+			if (signerAtsha204GetNonce(msg)) {
+#endif
+				SIGN_DEBUG(PSTR("Transmittng nonce\n"));
+				_sendRoute(build(msg, _nc.nodeId, msg.sender, NODE_SENSOR_ID,
+					C_INTERNAL, I_NONCE_RESPONSE, false));
+			} else {
+				SIGN_DEBUG(PSTR("Failed to generate nonce!\n"));
+			}
+			return true; // No need to further process I_NONCE_REQUEST
+		} else if (msg.type == I_REQUEST_SIGNING) {
+			if (msg.getBool()) {
+				// We received an indicator that the sender require us to sign all messages we send to it
+				SIGN_DEBUG(PSTR("Mark node %d as one that require signed messages\n"), msg.sender);
+				SET_SIGN(msg.sender);
+			} else {
+				// We received an indicator that the sender does not require us to sign all messages we send to it
+				SIGN_DEBUG(PSTR("Mark node %d as one that do not require signed messages\n"), msg.sender);
+				CLEAR_SIGN(msg.sender);
+			}
+			// Save updated table
+			hwWriteConfigBlock((void*)_doSign, (void*)EEPROM_SIGNING_REQUIREMENT_TABLE_ADDRESS, sizeof(_doSign));
+
+			// Inform sender about our preference if we are a gateway, but only require signing if the sender
+			// required signing
+			// We do not want a gateway to require signing from all nodes in a network just because it wants one node
+			// to sign it's messages
+#if defined (MY_GATEWAY_FEATURE)
+#ifdef MY_SIGNING_REQUEST_SIGNATURES
+			if (DO_SIGN(msg.sender)) {
+				_sendRoute(build(msg, _nc.nodeId, msg.sender, NODE_SENSOR_ID,
+					C_INTERNAL, I_REQUEST_SIGNING, false).set((uint8_t)true));
+				SIGN_DEBUG(PSTR("Informing node %d that we require signatures\n"), msg.sender);
+			}
+			else {
+				_sendRoute(build(msg, _nc.nodeId, msg.sender, NODE_SENSOR_ID,
+					C_INTERNAL, I_REQUEST_SIGNING, false).set((uint8_t)false));
+				SIGN_DEBUG(PSTR("Informing node %d that we do not require signatures\n"), msg.sender);
+			}
+#else // no MY_SIGNING_REQUEST_SIGNATURES
+			_sendRoute(build(msg, _nc.nodeId, msg.sender, NODE_SENSOR_ID,
+				C_INTERNAL, I_REQUEST_SIGNING, false).set((uint8_t)false));
+			SIGN_DEBUG(PSTR("Informing node %d that we do not require signatures\n"), msg.sender);
+#endif
+#endif // MY_GATEWAY_FEATURE
+			return true; // No need to further process I_REQUEST_SIGNING
+		} else if (msg.type == I_NONCE_RESPONSE) {
+			// Proceed with signing if nonce has been received
+			SIGN_DEBUG(PSTR("Nonce received from %d. Proceeding with signing...\n"), msg.sender);
+			if (msg.sender != _msgSign.destination) {
+				SIGN_DEBUG(PSTR("Nonce did not come from the destination (%d) of the message to be signed! "
+					"It came from %d.\n"), _msgSign.destination, msg.sender);
+				SIGN_DEBUG(PSTR("Silently discarding this nonce\n"));
+				return true; // No need to further process I_NONCE_RESPONSE
+			}
+#if defined(MY_SIGNING_SOFT)
+			if (!signerAtsha204SoftPutNonce(msg)) {
+#endif
+#if defined(MY_SIGNING_ATSHA204)
+			if (!signerAtsha204PutNonce(msg)) {
+#endif
+				SIGN_DEBUG(PSTR("Failed to store nonce to signing backend!\n"));
+#if defined(MY_SIGNING_SOFT)
+			} else if (!signerAtsha204SoftSignMsg(_msgSign)) {
+#endif
+#if defined(MY_SIGNING_ATSHA204)
+			} else if (!signerAtsha204SignMsg(_msgSign)) {
+#endif
+				SIGN_DEBUG(PSTR("Failed to sign message!\n"));
+			} else {
+				SIGN_DEBUG(PSTR("Message signed\n"));
+				_signingNonceStatus = SIGN_OK; // _msgSign now contains the signed message pending transmission
+			}
+			return true; // No need to further process I_NONCE_RESPONSE
+		}
+#endif // MY_SIGNING_FEATURE
+	}
+	return false;
+}
+
+bool signerCheckTimer(void) {
+#if defined(MY_SIGNING_SOFT)
+	return signerAtsha204SoftCheckTimer();
+#elif defined(MY_SIGNING_ATSHA204)
+	return signerAtsha204CheckTimer();
+#else
+	return true; // Without a configured backend, we always give "positive" results
+#endif
+}
+
+bool signerSignMsg(MyMessage &msg) {
+#if defined(MY_SIGNING_FEATURE)
+	// If destination is known to require signed messages and we are the sender,
+	// sign this message unless it is a handshake message
+	if (DO_SIGN(msg.destination) && msg.sender == _nc.nodeId) {
+		if (skipSign(msg)) {
+			return true;
+		} else {
+			// Send nonce-request
+			_signingNonceStatus=SIGN_WAITING_FOR_NONCE;
+			if (!_sendRoute(build(_msgTmp, _nc.nodeId, msg.destination, msg.sensor,
+				C_INTERNAL, I_NONCE_REQUEST, false).set(""))) {
+				SIGN_DEBUG(PSTR("Failed to transmit nonce request!\n"));
+				return false;
+			}
+			SIGN_DEBUG(PSTR("Nonce requested from %d. Waiting...\n"), msg.destination);
+			// We have to wait for the nonce to arrive before we can sign our original message
+			// Other messages could come in-between. We trust _process() takes care of them
+			unsigned long enter = hwMillis();
+			_msgSign = msg; // Copy the message to sign since message buffer might be touched in _process()
+			while (hwMillis() - enter < MY_VERIFICATION_TIMEOUT_MS && _signingNonceStatus==SIGN_WAITING_FOR_NONCE) {
+				_process();
+			}
+			if (hwMillis() - enter > MY_VERIFICATION_TIMEOUT_MS) {
+				SIGN_DEBUG(PSTR("Timeout waiting for nonce!\n"));
+				return false;
+			}
+			if (_signingNonceStatus == SIGN_OK) {
+				// process() received a nonce and signerProcessInternal successfully signed the message
+				msg = _msgSign; // Write the signed message back
+				SIGN_DEBUG(PSTR("Message to send has been signed\n"));
+			} else {
+				SIGN_DEBUG(PSTR("Message to send could not be signed!\n"));
+				return false;
+			}
+			// After this point, only the 'last' member of the message structure is allowed to be altered if the
+			// message has been signed, or signature will become invalid and the message rejected by the receiver
+		}
+	} else if (_nc.nodeId == msg.sender) {
+		mSetSigned(msg, 0); // Message is not supposed to be signed, make sure it is marked unsigned
+	}
+#else
+	(void)msg;
+#endif // MY_SIGNING_FEATURE
+	return true;
+}
+
+bool signerVerifyMsg(MyMessage &msg) {
+	bool verificationResult = true;
+	// Before processing message, reject unsigned messages if signing is required and check signature
+	// (if it is signed and addressed to us)
+	// Note that we do not care at all about any signature found if we do not require signing
+#if defined(MY_SIGNING_FEATURE) && defined(MY_SIGNING_REQUEST_SIGNATURES)
+	// If we are a node, or we are a gateway and the sender require signatures
+	// and we are the destination...
+	if ((!MY_IS_GATEWAY || DO_SIGN(msg.sender)) && msg.destination == _nc.nodeId) {
+		// Internal messages of certain types are not verified
+		if (skipSign(msg)) {
+			verificationResult = true;
+		}
+		else if (!mGetSigned(msg)) {
+			// Got unsigned message that should have been signed
+			SIGN_DEBUG(PSTR("Message is not signed, but it should have been!\n"));
+			verificationResult = false;
+		} else {
+#if defined(MY_SIGNING_SOFT)
+			verificationResult = signerAtsha204SoftVerifyMsg(msg);
+#endif
+#if defined(MY_SIGNING_ATSHA204)
+			verificationResult = signerAtsha204VerifyMsg(msg);
+#endif
+			if (!verificationResult) {
+				SIGN_DEBUG(PSTR("Signature verification failed!\n"));
+			}
+			mSetSigned(msg,0); // Clear the sign-flag now as verification is completed
+		}
+	}
+#else
+	(void)msg;
+#endif // MY_SIGNING_REQUEST_SIGNATURES
+	return verificationResult;
+}

libraries/MySensors/core/MySigning.h

@@ -364,8 +364,8 @@
  * <a href="http://www.atmel.com/Images/Atmel-8885-CryptoAuth-ATSHA204A-Datasheet.pdf">datasheet</a>.
  * In the MySensors protocol, the following internal messagetypes handles signature requirements and nonce requests:<br>
  * @ref I_REQUEST_SIGNING <br>
- * @ref I_GET_NONCE <br>
- * @ref I_GET_NONCE_RESPONSE <br>
+ * @ref I_NONCE_REQUEST <br>
+ * @ref I_NONCE_RESPONSE <br>
  *
  * Also, the version field in the header has been reduced from 3 to 2 bits in order to fit a single bit to indicate that a message is signed.
  *
@@ -484,29 +484,42 @@ typedef struct {
 } whitelist_entry_t;
 #endif
 
-// Macros for manipulating signing requirement table
-/** @brief Return 'true' if provided node ID is requiring signed messages */
-#define DO_SIGN(node) (~_doSign[node>>3]&(1<<(node%8)))
-/** @brief Mark provided node ID to require signed messages in table */
-#define SET_SIGN(node) (_doSign[node>>3]&=~(1<<(node%8)))
-/** @brief Mark provided node ID to not require signed messages in table */
-#define CLEAR_SIGN(node) (_doSign[node>>3]|=(1<<(node%8)))
 /** @brief Helper macro to determine the number of elements in a array */
 #define NUM_OF(x) (sizeof(x)/sizeof(x[0]))
 
 /**
- * @brief Stores signing identifier and a new nonce in provided message for signing operations.
+ * @brief Initializes signing infrastructure and associated backend.
  *
- * All space in message payload buffer is used for signing identifier and nonce.<br>
- * Returns @c false if subsystem is busy processing an ongoing signing operation.<br>
- * If successful, this marks the start of a signing operation at the receiving side so
- * implementation is expected to do any necessary initializations within this call.
- * \n@b Usage: This function is typically called as action when receiving a @ref I_GET_NONCE message.
- *
- * @param msg The message to put the nonce in.
- * @returns @c true if nonce is successfully put in provided message.
+ * This function makes sure that the internal states of the signing infrastructure
+ * is set to a known initial state.
+ * \n@b Usage: This fuction should be called before any signing related operations take place.
  */
-bool signerGetNonce(MyMessage &msg);
+void signerInit(void);
+
+/**
+ * @brief Does signing specific presentation for a node.
+ *
+ * This function makes sure any signing related presentation info is shared with the gateway.
+ * The presentation of the gateways signing preferences is done in @ref signerProcessInternal().
+ * \n@b Usage: This function should be called by the presentation routine of the mysensors library.
+ * There is no need to call this directly from a sketch.
+ *
+ * @param msg Message buffer to use.
+ */
+void signerPresentation(MyMessage &msg);
+ 
+/**
+ * @brief Manages internal signing message handshaking.
+ *
+ * This function takes care of signing related message handshaknig such as nonce exchange.
+ * \n@b Usage: This function should be called by the incoming message handler before any further message
+ * processing is performed on internal messages. This function should only be called for @ref C_INTERNAL class
+ * messages.
+ *
+ * @param msg Message buffer to process.
+ * @returns @c true if caller should stop further message processing.
+ */
+bool signerProcessInternal(MyMessage &msg);
 
 /**
  * @brief Check timeout of verification session.
@@ -525,7 +538,7 @@ bool signerCheckTimer(void);
  * Returns @c false if signing identifier found in message is not supported by the used backend.<br>
  * If successful, this marks the start of a signing operation at the sending side so
  * implementation is expected to do any necessary initializations within this call.
- * \n@b Usage: This function is typically called as action when receiving a @ref I_GET_NONCE_RESPONSE
+ * \n@b Usage: This function is typically called as action when receiving a @ref I_NONCE_RESPONSE
  * message.
  *
  * @param msg The message to get the nonce from.
@@ -537,13 +550,13 @@ bool signerPutNonce(MyMessage &msg);
  * @brief Signs provided message. All remaining space in message payload buffer is used for
  * signing identifier and signature.
  *
- * Nonce used for signature calculation is the one generated previously using @ref signerGetNonce().<br>
+ * Nonce used for signature calculation is the one generated previously within @ref signerProcessInternal().<br>
  * Nonce will be cleared when this function is called to prevent re-use of nonce.<br>
  * Returns @c false if subsystem is busy processing an ongoing signing operation.<br>
  * Returns @c false if not two bytes or more of free payload space is left in provided message.<br>
  * This ends a signing operation at the sending side so implementation is expected to do any
  * deinitializations and enter a power saving state within this call.
- * \n@b Usage: This function is typically called as action when receiving a @ref I_GET_NONCE_RESPONSE
+ * \n@b Usage: This function is typically called as action when receiving a @ref I_NONCE_RESPONSE
  * message and after @ref signerPutNonce() has successfully been executed.
  * 
  * @param msg The message to sign.

libraries/MySensors/core/MySigningAtsha204.cpp

@@ -29,7 +29,7 @@
 
 #define SIGNING_IDENTIFIER (1)
 
-// Define MY_DEBUG in your sketch to enable signing backend debugprints
+// Define MY_DEBUG_VERBOSE in your sketch to enable signing backend debugprints
 
 ATSHA204Class atsha204(MY_SIGNING_ATSHA204_PIN);
 unsigned long _signing_timestamp;
@@ -43,11 +43,11 @@ uint8_t _singning_tx_buffer[SHA204_CMD_SIZE_MAX];
 	const whitelist_entry_t _signing_whitelist[] = MY_SIGNING_NODE_WHITELISTING;
 #endif
 
-void signerCalculateSignature(MyMessage &msg);
-uint8_t* signerSha256(const uint8_t* data, size_t sz);
+static void signerCalculateSignature(MyMessage &msg);
+static uint8_t* signerSha256(const uint8_t* data, size_t sz);
 
 
-#ifdef MY_DEBUG
+#ifdef MY_DEBUG_VERBOSE
 static char i2h(uint8_t i)
 {
 	uint8_t k = i & 0x0F;
@@ -57,8 +57,7 @@ static char i2h(uint8_t i)
 		return 'A' + k - 10;
 }
 
-static void DEBUG_SIGNING_PRINTBUF(const __FlashStringHelper* str, uint8_t* buf, uint8_t sz)
-{
+static void DEBUG_SIGNING_PRINTBUF(const __FlashStringHelper* str, uint8_t* buf, uint8_t sz) {
 	static char printBuffer[300];
 #ifdef MY_GATEWAY_FEATURE
 	// prepend debug message to be handled correctly by controller (C_INTERNAL, I_LOG_MESSAGE)
@@ -86,7 +85,20 @@ static void DEBUG_SIGNING_PRINTBUF(const __FlashStringHelper* str, uint8_t* buf,
 #define DEBUG_SIGNING_PRINTBUF(str, buf, sz)
 #endif
 
-bool signerGetNonce(MyMessage &msg) {
+bool signerAtsha204CheckTimer(void) {
+	if (_signing_verification_ongoing) {
+		if (millis() < _signing_timestamp || millis() > _signing_timestamp + MY_VERIFICATION_TIMEOUT_MS) {
+			DEBUG_SIGNING_PRINTBUF(F("Verification timeout"), NULL, 0);
+			// Purge nonce
+			memset(_signing_current_nonce, 0x00, NONCE_NUMIN_SIZE_PASSTHROUGH);
+			_signing_verification_ongoing = false;
+			return false; 
+		}
+	}
+	return true;
+}
+
+bool signerAtsha204GetNonce(MyMessage &msg) {
 	DEBUG_SIGNING_PRINTBUF(F("Signing backend: ATSHA204"), NULL, 0);
 	// Generate random number for use as nonce
 	// We used a basic whitening technique that takes the first byte of a new random value and builds up a 32-byte random value
@@ -118,20 +130,7 @@ bool signerGetNonce(MyMessage &msg) {
 	return true;
 }
 
-bool signerCheckTimer() {
-	if (_signing_verification_ongoing) {
-		if (millis() < _signing_timestamp || millis() > _signing_timestamp + MY_VERIFICATION_TIMEOUT_MS) {
-			DEBUG_SIGNING_PRINTBUF(F("Verification timeout"), NULL, 0);
-			// Purge nonce
-			memset(_signing_current_nonce, 0x00, NONCE_NUMIN_SIZE_PASSTHROUGH);
-			_signing_verification_ongoing = false;
-			return false; 
-		}
-	}
-	return true;
-}
-
-bool signerPutNonce(MyMessage &msg) {
+bool signerAtsha204PutNonce(MyMessage &msg) {
 	DEBUG_SIGNING_PRINTBUF(F("Signing backend: ATSHA204"), NULL, 0);
 	if (((uint8_t*)msg.getCustom())[0] != SIGNING_IDENTIFIER) {
 		DEBUG_SIGNING_PRINTBUF(F("Incorrect signing identifier"), NULL, 0);
@@ -144,7 +143,7 @@ bool signerPutNonce(MyMessage &msg) {
 	return true;
 }
 
-bool signerSignMsg(MyMessage &msg) {
+bool signerAtsha204SignMsg(MyMessage &msg) {
 	// If we cannot fit any signature in the message, refuse to sign it
 	if (mGetLength(msg) > MAX_PAYLOAD-2) {
 		DEBUG_SIGNING_PRINTBUF(F("Message too large"), NULL, 0);
@@ -174,7 +173,7 @@ bool signerSignMsg(MyMessage &msg) {
 	return true;
 }
 
-bool signerVerifyMsg(MyMessage &msg) {
+bool signerAtsha204VerifyMsg(MyMessage &msg) {
 	if (!_signing_verification_ongoing) {
 		DEBUG_SIGNING_PRINTBUF(F("No active verification session"), NULL, 0);
 		return false; 
@@ -226,7 +225,7 @@ bool signerVerifyMsg(MyMessage &msg) {
 }
 
 // Helper to calculate signature of msg (returned in _singning_rx_buffer[SHA204_BUFFER_POS_DATA])
-void signerCalculateSignature(MyMessage &msg) {
+static void signerCalculateSignature(MyMessage &msg) {
 	memset(_signing_temp_message, 0, 32);
 	memcpy(_signing_temp_message, (uint8_t*)&msg.data[1-HEADER_SIZE], MAX_MESSAGE_LENGTH-1-(MAX_PAYLOAD-mGetLength(msg)));
 
@@ -259,7 +258,7 @@ void signerCalculateSignature(MyMessage &msg) {
 
 // Helper to calculate a generic SHA256 digest of provided buffer (only supports one block)
 // The pointer to the hash is returned, but the hash is also stored in _singning_rx_buffer[SHA204_BUFFER_POS_DATA])
-uint8_t* signerSha256(const uint8_t* data, size_t sz) {
+static uint8_t* signerSha256(const uint8_t* data, size_t sz) {
 	// Initiate SHA256 calculator
 	(void)atsha204.sha204m_execute(SHA204_SHA, SHA_INIT, 0, 0, NULL,
 									SHA_COUNT_SHORT, _singning_tx_buffer, SHA_RSP_SIZE_SHORT, _singning_rx_buffer);

libraries/MySensors/core/MySigningAtsha204Soft.cpp

@@ -33,7 +33,7 @@
 
 #define SIGNING_IDENTIFIER (1)
 
-// Define MY_DEBUG in your sketch to enable signing backend debugprints
+// Define MY_DEBUG_VERBOSE in your sketch to enable signing backend debugprints
 
 Sha256Class _signing_sha256;
 unsigned long _signing_timestamp;
@@ -48,9 +48,9 @@ uint8_t _signing_hmac[32];
 	const whitelist_entry_t _signing_whitelist[] = MY_SIGNING_NODE_WHITELISTING;
 #endif
 
-void signerCalculateSignature(MyMessage &msg);
+static void signerCalculateSignature(MyMessage &msg);
 
-#ifdef MY_DEBUG
+#ifdef MY_DEBUG_VERBOSE
 static char i2h(uint8_t i)
  {
 	uint8_t k = i & 0x0F;
@@ -60,8 +60,7 @@ static char i2h(uint8_t i)
 		return 'A' + k - 10;
 }
 
-static void DEBUG_SIGNING_PRINTBUF(const __FlashStringHelper* str, uint8_t* buf, uint8_t sz)
-{
+static void DEBUG_SIGNING_PRINTBUF(const __FlashStringHelper* str, uint8_t* buf, uint8_t sz) {
 	static char printBuffer[300];
 #ifdef MY_GATEWAY_FEATURE
 	// prepend debug message to be handled correctly by controller (C_INTERNAL, I_LOG_MESSAGE)
@@ -89,7 +88,20 @@ static void DEBUG_SIGNING_PRINTBUF(const __FlashStringHelper* str, uint8_t* buf,
 #define DEBUG_SIGNING_PRINTBUF(str, buf, sz)
 #endif
 
-bool signerGetNonce(MyMessage &msg) {
+bool signerAtsha204SoftCheckTimer(void) {
+	if (_signing_verification_ongoing) {
+		if (millis() < _signing_timestamp || millis() > _signing_timestamp + MY_VERIFICATION_TIMEOUT_MS) {
+			DEBUG_SIGNING_PRINTBUF(F("Verification timeout"), NULL, 0);
+			// Purge nonce
+			memset(_signing_current_nonce, 0xAA, 32);
+			_signing_verification_ongoing = false;
+			return false; 
+		}
+	}
+	return true;
+}
+
+bool signerAtsha204SoftGetNonce(MyMessage &msg) {
 	DEBUG_SIGNING_PRINTBUF(F("Signing backend: ATSHA204Soft"), NULL, 0);
 
 	// We used a basic whitening technique that takes the first byte of a new random value and builds up a 32-byte random value
@@ -118,20 +130,7 @@ bool signerGetNonce(MyMessage &msg) {
 	return true;
 }
 
-bool signerCheckTimer() {
-	if (_signing_verification_ongoing) {
-		if (millis() < _signing_timestamp || millis() > _signing_timestamp + MY_VERIFICATION_TIMEOUT_MS) {
-			DEBUG_SIGNING_PRINTBUF(F("Verification timeout"), NULL, 0);
-			// Purge nonce
-			memset(_signing_current_nonce, 0xAA, 32);
-			_signing_verification_ongoing = false;
-			return false; 
-		}
-	}
-	return true;
-}
-
-bool signerPutNonce(MyMessage &msg) {
+bool signerAtsha204SoftPutNonce(MyMessage &msg) {
 	DEBUG_SIGNING_PRINTBUF(F("Signing backend: ATSHA204Soft"), NULL, 0);
 	if (((uint8_t*)msg.getCustom())[0] != SIGNING_IDENTIFIER) {
 		DEBUG_SIGNING_PRINTBUF(F("Incorrect signing identifier"), NULL, 0);
@@ -144,7 +143,7 @@ bool signerPutNonce(MyMessage &msg) {
 	return true;
 }
 
-bool signerSignMsg(MyMessage &msg) {
+bool signerAtsha204SoftSignMsg(MyMessage &msg) {
 	// If we cannot fit any signature in the message, refuse to sign it
 	if (mGetLength(msg) > MAX_PAYLOAD-2) {
 		DEBUG_SIGNING_PRINTBUF(F("Message too large"), NULL, 0);
@@ -176,7 +175,7 @@ bool signerSignMsg(MyMessage &msg) {
 	return true;
 }
 
-bool signerVerifyMsg(MyMessage &msg) {
+bool signerAtsha204SoftVerifyMsg(MyMessage &msg) {
 	if (!_signing_verification_ongoing) {
 		DEBUG_SIGNING_PRINTBUF(F("No active verification session"), NULL, 0);
 		return false; 
@@ -231,7 +230,7 @@ bool signerVerifyMsg(MyMessage &msg) {
 }
 
 // Helper to calculate signature of msg (returned in hmac)
-void signerCalculateSignature(MyMessage &msg) {
+static void signerCalculateSignature(MyMessage &msg) {
 	memset(_signing_temp_message, 0, 32);
 	memcpy(_signing_temp_message, (uint8_t*)&msg.data[1-HEADER_SIZE], MAX_MESSAGE_LENGTH-1-(MAX_PAYLOAD-mGetLength(msg)));
 	DEBUG_SIGNING_PRINTBUF(F("Message to process: "), (uint8_t*)&msg.data[1-HEADER_SIZE], MAX_MESSAGE_LENGTH-1-(MAX_PAYLOAD-mGetLength(msg)));

libraries/MySensors/core/MyTransport.cpp

@@ -23,10 +23,6 @@
 bool _autoFindParent;
 uint8_t _failedTransmissions;
 
-#ifdef MY_SIGNING_FEATURE
-	uint8_t _signingNonceStatus;
-#endif
-
 #ifdef MY_OTA_FIRMWARE_FEATURE
 	SPIFlash _flash(MY_OTA_FLASH_SS, MY_OTA_FLASH_JDECID);
 	NodeFirmwareConfig _fc;
@@ -70,9 +66,7 @@ inline void transportProcess() {
 		return;
 	}
 
-	#ifdef MY_SIGNING_FEATURE
 	(void)signerCheckTimer(); // Manage signing timeout
-	#endif
 
 	uint8_t payloadLength = transportReceive((uint8_t *)&_msg);
 	(void)payloadLength; //until somebody makes use of it
@@ -86,34 +80,12 @@ inline void transportProcess() {
 	uint8_t destination = _msg.destination;
 	
 
-	#ifdef MY_SIGNING_FEATURE
-		// Before processing message, reject unsigned messages if signing is required and check signature (if it is signed and addressed to us)
-		// Note that we do not care at all about any signature found if we do not require signing, nor do we care about ACKs (they are never signed)
-		#ifdef MY_SIGNING_REQUEST_SIGNATURES
-
-			if ((!MY_IS_GATEWAY || DO_SIGN(sender)) &&
-				destination == _nc.nodeId &&
-				!mGetAck(_msg) &&
-				(mGetCommand(_msg) != C_INTERNAL ||
-				 (type != I_GET_NONCE_RESPONSE && type != I_GET_NONCE && type != I_REQUEST_SIGNING &&
-				  type != I_ID_REQUEST && type != I_ID_RESPONSE && 
-				  type != I_FIND_PARENT && type != I_FIND_PARENT_RESPONSE &&
-				  type != I_HEARTBEAT && type != I_HEARTBEAT_RESPONSE))) {
-				if (!mGetSigned(_msg)) {
-					// Got unsigned message that should have been signed
-					debug(PSTR("no sign\n"));
-					ledBlinkErr(1);
-					return;
-				}
-				else if (!signerVerifyMsg(_msg)) {
-					debug(PSTR("verify fail\n"));
-					ledBlinkErr(1);
-					return; // This signed message has been tampered with!
-				}
-			}
-		#endif
-	#endif
-
+	// Reject massages that do not pass verification
+	if (!signerVerifyMsg(_msg)) {
+		debug(PSTR("verify fail\n"));
+		ledBlinkErr(1);
+		return;	
+	}
 
 	if (destination == _nc.nodeId) {
 		debug(PSTR("read: %d-%d-%d s=%d,c=%d,t=%d,pt=%d,l=%d,sg=%d:%s\n"),
@@ -138,7 +110,6 @@ inline void transportProcess() {
 
 	if (destination == _nc.nodeId) {
 		// This message is addressed to this node
-		mSetSigned(_msg,0); // Clear the sign-flag now as verification is completed
 		// prevent buffer overflow by limiting max. possible message length (5 bits=31 bytes max) to MAX_PAYLOAD (25 bytes)
 		mSetLength(_msg, min(mGetLength(_msg),MAX_PAYLOAD));
 		// null terminate data
@@ -163,6 +134,10 @@ inline void transportProcess() {
 		}
 
 		if (command == C_INTERNAL) {
+			// Process signing related internal messages
+			if (signerProcessInternal(_msg)) {
+				return; // Signer processing indicated no further action needed
+			}
 			if (type == I_FIND_PARENT_RESPONSE) {
 				if (_autoFindParent) {
 					// We've received a reply to a FIND_PARENT message. Check if the distance is
@@ -183,44 +158,6 @@ inline void transportProcess() {
 					}
 				}
 				return;
-			#ifdef MY_SIGNING_FEATURE
-			} else if (type == I_GET_NONCE) {
-				if (signerGetNonce(_msg)) {
-					_sendRoute(build(_msg, _nc.nodeId, sender, NODE_SENSOR_ID, C_INTERNAL, I_GET_NONCE_RESPONSE, false));
-				}
-				return; // Nonce exchange is an internal MySensor protocol message, no need to inform caller about this
-			} else if (type == I_REQUEST_SIGNING) {
-				if (_msg.getBool()) {
-					// We received an indicator that the sender require us to sign all messages we send to it
-					SET_SIGN(sender);
-				} else {
-					// We received an indicator that the sender does not require us to sign all messages we send to it
-					CLEAR_SIGN(sender);
-				}
-				// Save updated table
-				hwWriteConfigBlock((void*)_doSign, (void*)EEPROM_SIGNING_REQUIREMENT_TABLE_ADDRESS, sizeof(_doSign));
-
-				// Inform sender about our preference if we are a gateway, but only require signing if the sender required signing
-				// We do not currently want a gateway to require signing from all nodes in a network just because it wants one node
-				// to sign it's messages
-				#if defined (MY_GATEWAY_FEATURE)
-					#ifdef MY_SIGNING_REQUEST_SIGNATURES
-						if (DO_SIGN(sender))
-							_sendRoute(build(_msg, _nc.nodeId, sender, NODE_SENSOR_ID, C_INTERNAL, I_REQUEST_SIGNING, false).set((uint8_t)true));
-						else
-							_sendRoute(build(_msg, _nc.nodeId, sender, NODE_SENSOR_ID, C_INTERNAL, I_REQUEST_SIGNING, false).set((uint8_t)false));
-					#else
-						_sendRoute(build(_msg, _nc.nodeId, sender, NODE_SENSOR_ID, C_INTERNAL, I_REQUEST_SIGNING, false).set((uint8_t)false));
-					#endif
-				#endif
-				return; // Signing request is an internal MySensor protocol message, no need to inform caller about this
-			} else if (type == I_GET_NONCE_RESPONSE) {
-				// Proceed with signing if nonce has been received
-				if (signerPutNonce(_msg) && signerSignMsg(_msgSign)) {
-					_signingNonceStatus = SIGN_OK;
-				}
-				return; // Just pass along nonce silently (no need to call callback for these)
-			#endif
 			} else if (sender == GATEWAY_ADDRESS) {
 				if (type == I_ID_RESPONSE && _nc.nodeId == AUTO) {
 					_nc.nodeId = _msg.getByte();
@@ -414,47 +351,10 @@ boolean transportSendRoute(MyMessage &message) {
 
 	mSetVersion(message, PROTOCOL_VERSION);
 
-	#ifdef MY_SIGNING_FEATURE
-		uint8_t type = message.type;
-		// If destination is known to require signed messages and we are the sender, sign this message unless it is an ACK or a handshake message
-		if (DO_SIGN(message.destination) && message.sender == _nc.nodeId && !mGetAck(message) &&
-			(mGetCommand(message) != C_INTERNAL ||
-			 (type != I_GET_NONCE && type != I_GET_NONCE_RESPONSE && type != I_REQUEST_SIGNING &&
-			  type != I_ID_REQUEST && type != I_ID_RESPONSE &&
-			  type != I_FIND_PARENT && type != I_FIND_PARENT_RESPONSE &&
-			  type != I_HEARTBEAT && type != I_HEARTBEAT_RESPONSE))) {
-			// Send nonce-request
-			_signingNonceStatus=SIGN_WAITING_FOR_NONCE;
-			if (!_sendRoute(build(_msgTmp, _nc.nodeId, message.destination, message.sensor, C_INTERNAL, I_GET_NONCE, false).set(""))) {
-				debug(PSTR("nonce tr err\n"));
-				return false;
-			}
-			// We have to wait for the nonce to arrive before we can sign our original message
-			// Other messages could come in-between. We trust process() takes care of them
-			unsigned long enter = hwMillis();
-			_msgSign = message; // Copy the message to sign since message buffer might be touched in process()
-
-			while (hwMillis() - enter < MY_VERIFICATION_TIMEOUT_MS && _signingNonceStatus==SIGN_WAITING_FOR_NONCE) {
-				_process();
-			}
-			if (hwMillis() - enter > MY_VERIFICATION_TIMEOUT_MS) {
-				debug(PSTR("nonce tmo\n"));
-				ledBlinkErr(1);
-				return false;
-			}
-			if (_signingNonceStatus == SIGN_OK) {
-				message = _msgSign; // Write the signed message back
-			} else {
-				debug(PSTR("sign fail\n"));
-				ledBlinkErr(1);
-				return false;
-			}
-			// After this point, only the 'last' member of the message structure is allowed to be altered if the message has been signed,
-			// or signature will become invalid and the message rejected by the receiver
-		} else if (_nc.nodeId == message.sender) {
-			mSetSigned(message, 0); // Message is not supposed to be signed, make sure it is marked unsigned
-		}
-	#endif
+	if (!signerSignMsg(message)) {
+		debug(PSTR("sign fail\n"));
+		ledBlinkErr(1);
+	}
 
 	#if !defined(MY_REPEATER_FEATURE)
 
@@ -552,19 +452,8 @@ void transportPresentNode() {
 	// Present node and request config
 	#ifndef MY_GATEWAY_FEATURE
 		if (_nc.nodeId != AUTO) {
-			#ifdef MY_SIGNING_FEATURE
-				// Notify gateway (and possibly controller) about the signing preferences of this node
-				#if defined(MY_SIGNING_REQUEST_SIGNATURES)
-					_sendRoute(build(_msg, _nc.nodeId, GATEWAY_ADDRESS, NODE_SENSOR_ID, C_INTERNAL, I_REQUEST_SIGNING, false).set(true));
-					// If we do require signing, wait for the gateway to tell us how it prefer us to transmit our messages
-					wait(2000);
-				#else
-					_sendRoute(build(_msg, _nc.nodeId, GATEWAY_ADDRESS, NODE_SENSOR_ID, C_INTERNAL, I_REQUEST_SIGNING, false).set(false));
-				#endif
-			#else
-				// We do not support signing, make sure gateway knows this
-				_sendRoute(build(_msg, _nc.nodeId, GATEWAY_ADDRESS, NODE_SENSOR_ID, C_INTERNAL, I_REQUEST_SIGNING, false).set(false));
-			#endif
+			// Send signing preferences for this node
+			signerPresentation(_msg);
 
 			// Send presentation for this radio node
 			#ifdef MY_REPEATER_FEATURE

libraries/MySensors/core/MyTransport.h

@@ -41,9 +41,6 @@
 // Search for a new parent node after this many transmission failures
 #define SEARCH_FAILURES  5
 
-// Status when waiting for signing nonce in process
-enum { SIGN_WAITING_FOR_NONCE = 0, SIGN_OK = 1 };
-
 
 // FW config structure, stored in eeprom
 typedef struct {

libraries/MySensors/drivers/ATSHA204/ATSHA204.h

@@ -199,7 +199,9 @@
 
 #define SHA204_COMMAND_EXEC_MAX      ((uint8_t) (69.0 * CPU_CLOCK_DEVIATION_POSITIVE + 0.5))  //! maximum command delay
 #define SHA204_CMD_SIZE_MIN          ((uint8_t)  7)  //! minimum number of bytes in command (from count byte to second CRC byte)
+#ifndef SHA204_CMD_SIZE_MAX
 #define SHA204_CMD_SIZE_MAX          ((uint8_t) SHA_COUNT_LONG)  //! maximum size of command packet (SHA)
+#endif
 #define SHA204_CRC_SIZE              ((uint8_t)  2)  //! number of CRC bytes
 #define SHA204_BUFFER_POS_STATUS     (1)  //! buffer index of status byte in status response
 #define SHA204_BUFFER_POS_DATA       (1)  //! buffer index of first data byte in data response

libraries/sha204/sha204_library.h

@@ -259,7 +259,9 @@
 
 #define SHA204_COMMAND_EXEC_MAX      ((uint8_t) (69.0 * CPU_CLOCK_DEVIATION_POSITIVE + 0.5))  //! maximum command delay
 #define SHA204_CMD_SIZE_MIN          ((uint8_t)  7)  //! minimum number of bytes in command (from count byte to second CRC byte)
+#ifndef SHA204_CMD_SIZE_MAX
 #define SHA204_CMD_SIZE_MAX          ((uint8_t) 84)  //! maximum size of command packet (CheckMac)
+#endif
 #define SHA204_CRC_SIZE              ((uint8_t)  2)  //! number of CRC bytes
 #define SHA204_BUFFER_POS_STATUS     (1)  //! buffer index of status byte in status response
 #define SHA204_BUFFER_POS_DATA       (1)  //! buffer index of first data byte in data response