From 9811b424b5b56da8c670430236ce62291194e4bc Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Mon, 26 Sep 2016 18:38:25 -0400 Subject: Rewrite block_erase_maybe() to run the "maybe" check in constant time. Running this check in constant time probably isn't necessary, but it plugs a (somewhat far-fetched) timing leak and is easy enough. While we're at this, we also skip the CRC check, which is irrelevant here. --- ks_flash.c | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) (limited to 'ks_flash.c') diff --git a/ks_flash.c b/ks_flash.c index 00f9786..368a8ed 100644 --- a/ks_flash.c +++ b/ks_flash.c @@ -461,30 +461,26 @@ static hal_error_t block_erase(const unsigned blockno) * Erase a flash block if it hasn't already been erased. * We have to disable fast read for this to work properly. * May not be necessary, trying to avoid unnecessary wear. + * + * Unclear whether there's any sane reason why this needs to be + * constant time, given how slow erasure is. But side channel attacks + * can be tricky things, and it's theoretically possible that we could + * leak information about, eg, key length, so we do constant time. */ static hal_error_t block_erase_maybe(const unsigned blockno) { - flash_block_t *block = cache_pick_lru(); - hal_error_t err; - - if (block == NULL) - return HAL_ERROR_IMPOSSIBLE; - - err = block_read(blockno, block, 0); - - if (err == HAL_ERROR_KEYSTORE_BAD_BLOCK_TYPE || - err == HAL_ERROR_KEYSTORE_BAD_CRC) - return block_erase(blockno); - - if (err != HAL_OK) - return err; - - for (int i = 0; i < sizeof(*block); i++) - if (block->bytes[i] != 0xFF) - return block_erase(blockno); + uint8_t mask = 0xFF; + + for (uint32_t a = block_offset(blockno); a < block_offset(blockno + 1); a += KEYSTORE_PAGE_SIZE) { + uint8_t page[KEYSTORE_PAGE_SIZE]; + if (keystore_read_data(a, page, sizeof(page)) != 1) + return HAL_ERROR_KEYSTORE_ACCESS; + for (int i = 0; i < KEYSTORE_PAGE_SIZE; i++) + mask &= page[i]; + } - return HAL_OK; + return mask == 0xFF ? HAL_OK : block_erase(blockno); } /* -- cgit v1.2.3