From c9fc4a5779db08a6c8a0029b779826a188d8b438 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Sun, 23 Apr 2017 18:30:50 -0400 Subject: Avoid deadlock triggered by low-probability race condition. Static code analysis (Doxygen call graph) detected a low-probability race condition which could have triggered a deadlock on the keystore mutex if the mkmif code returns with an error like HAL_ERROR_CORE_BUSY when we're trying to fetch the KEK. This is a knock-on effect of the awful kludge of backing up the KEK in the keystore flash as an alternative to powering the MKM with a battery as called for in the design. This code path should not exist at all, but, for now, we avoid the deadlock by making it the caller's responsibility to grab the keystore mutex before looking up the KEK. --- hal_internal.h | 1 + 1 file changed, 1 insertion(+) (limited to 'hal_internal.h') diff --git a/hal_internal.h b/hal_internal.h index f17179c..56d0936 100644 --- a/hal_internal.h +++ b/hal_internal.h @@ -421,6 +421,7 @@ extern hal_error_t hal_mkm_volatile_erase(const size_t len); /* #warning MKM flash backup kludge enabled. Do NOT use this in production! */ extern hal_error_t hal_mkm_flash_read(uint8_t *buf, const size_t len); +extern hal_error_t hal_mkm_flash_read_no_lock(uint8_t *buf, const size_t len); extern hal_error_t hal_mkm_flash_write(const uint8_t * const buf, const size_t len); extern hal_error_t hal_mkm_flash_erase(const size_t len); -- cgit v1.2.3 From 9c9f26fa04882cdc1ddb01410688f4d2651782af Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Wed, 26 Apr 2017 20:01:01 -0400 Subject: Lower PBKDF2 password iterations and add delay on bad PIN. Consistent user complaints about HSM login taking too long. Underlying issue has both superficial and fundamental causes. Superficial: Our PBKDF2 implementation is slow. We could almost certainly make it faster by taking advantage of partial pre-calculation (see notes in code) and by reenabling use of FPGA hash cores when when checking passwords (which mgiht require linking the bootloader against a separate libhal build to avoid chicken-and-egg problem of needing FPGA to log into console to configure FPGA). Fundamental: The PBKDF2 iteration counts we used to use (10,000 minimum, 20,000 default) are in line with current NIST recommendations. The new, faster values (1,000 and 2,000, respectively) are not, or, rather, they're in line with what NIST recommended a decade ago. Well, OK, maybe the Coretex M4 is so slow that it's living in the past, but still. The fundamental issue is that anybody who can capture the encoded PIN can mount an offline dictionary attack on it, so we'd like to make that expensive. But the users are unhappy with the current behavior, so this change falls back to the ancient technique of adding a delay (currently five seconds, configurable at compile time) after a bad PIN, which makes it painful to use the login function as an oracle but does nothing about the offline dictionary attack problem. Feh. Note that users can still choose a higher iteration count, by setting the iteration count via the console. It's just not the default out of the box anymore. --- hal_internal.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'hal_internal.h') diff --git a/hal_internal.h b/hal_internal.h index 56d0936..36f24d4 100644 --- a/hal_internal.h +++ b/hal_internal.h @@ -99,9 +99,14 @@ extern void hal_ks_lock(void); extern void hal_ks_unlock(void); /* - * Logging. + * Thread sleep. Currently used only for bad-PIN delays. */ +extern void hal_sleep(const unsigned seconds); + +/* + * Logging. + */ typedef enum { HAL_LOG_DEBUG, HAL_LOG_INFO, HAL_LOG_WARN, HAL_LOG_ERROR, HAL_LOG_SILENT -- cgit v1.2.3