Merge pull request #16 from jhoenicke/master

PIN handling - constant time.

firmware/protect.c

@@ -156,16 +156,16 @@ bool protectPin(bool use_cached)
 			delay(10000000);
 		}
 	}
-	storage_increasePinFails();
-	bool increase_failed = (fails >= storage_getPinFails());
 	const char *pin;
 	pin = requestPin(PinMatrixRequestType_PinMatrixRequestType_Current, "Please enter current PIN:");
 	if (!pin) {
 		fsm_sendFailure(FailureType_Failure_PinCancelled, "PIN Cancelled");
 		return false;
 	}
+	storage_increasePinFails();
+	bool increase_failed = (fails >= storage_getPinFails());
 	if (storage_isPinCorrect(pin) && !increase_failed) {
-		session_cachePin(pin);
+		session_cachePin();
 		storage_resetPinFails();
 		return true;
 	} else {

firmware/storage.c

@@ -48,7 +48,6 @@ static bool   sessionRootNodeCached;
 static HDNode sessionRootNode;
 
 static bool sessionPinCached;
-static char sessionPin[17];
 
 static bool sessionPassphraseCached;
 static char sessionPassphrase[51];
@@ -122,7 +121,7 @@ void session_clear(void)
 {
 	sessionRootNodeCached = false;   memset(&sessionRootNode, 0, sizeof(sessionRootNode));
 	sessionPassphraseCached = false; memset(&sessionPassphrase, 0, sizeof(sessionPassphrase));
-	sessionPinCached = false;        memset(&sessionPin, 0, sizeof(sessionPin));
+	sessionPinCached = false;
 }
 
 static uint8_t meta_backup[FLASH_META_LEN];
@@ -306,14 +305,27 @@ const uint8_t *storage_getHomescreen(void)
 	return (storage.has_homescreen && storage.homescreen.size == 1024) ? storage.homescreen.bytes : 0;
 }
 
+/* Check whether pin matches storage.  The pin must be a null-terminated
+ * string with at most 9 characters.
+ */
 bool storage_isPinCorrect(const char *pin)
 {
-	return strcmp(storage.pin, pin) == 0;
+	/* The execution time of the following code only depends on the
+	 * (public) input.  This avoids timing attacks.
+	 */
+	char diff = 0;
+	uint32_t i = 0;
+	while (pin[i]) {
+		diff |= storage.pin[i] - pin[i];
+		i++;
+	}
+	diff |= storage.pin[i];
+	return diff == 0;
 }
 
 bool storage_hasPin(void)
 {
-	return storage.has_pin && strlen(storage.pin) > 0;
+	return storage.has_pin && storage.pin[0] != 0;
 }
 
 void storage_setPin(const char *pin)
@@ -340,15 +352,14 @@ bool session_isPassphraseCached(void)
 	return sessionPassphraseCached;
 }
 
-void session_cachePin(const char *pin)
+void session_cachePin(void)
 {
-	strlcpy(sessionPin, pin, sizeof(sessionPin));
 	sessionPinCached = true;
 }
 
 bool session_isPinCached(void)
 {
-	return sessionPinCached && strcmp(sessionPin, storage.pin) == 0;
+	return sessionPinCached;
 }
 
 void storage_resetPinFails(void)

firmware/storage.h

@@ -52,7 +52,7 @@ bool session_isPassphraseCached(void);
 bool storage_isPinCorrect(const char *pin);
 bool storage_hasPin(void);
 void storage_setPin(const char *pin);
-void session_cachePin(const char *pin);
+void session_cachePin(void);
 bool session_isPinCached(void);
 void storage_resetPinFails(void);
 void storage_increasePinFails(void);