From 3306dffe23f3782bbc56e801e1f6fee8bcde290d Mon Sep 17 00:00:00 2001 From: Ollrogge Date: Mon, 11 Apr 2022 13:03:41 +0200 Subject: [PATCH] sys/fido2: update flash handling && bug fixes --- sys/Makefile.dep | 1 + sys/fido2/ctap/Kconfig | 19 ++++++--- sys/fido2/ctap/ctap.c | 60 +++++++++++++-------------- sys/fido2/ctap/ctap_mem.c | 54 +++++++++++++++---------- sys/include/fido2/ctap/ctap_mem.h | 67 +++++++++++++++++++------------ 5 files changed, 115 insertions(+), 86 deletions(-) diff --git a/sys/Makefile.dep b/sys/Makefile.dep index a234393941..3cf3a8ce9c 100644 --- a/sys/Makefile.dep +++ b/sys/Makefile.dep @@ -892,6 +892,7 @@ endif ifneq (,$(filter fido2_ctap,$(USEMODULE))) FEATURES_REQUIRED += periph_flashpage + FEATURES_REQUIRED += periph_flashpage_in_address_space FEATURES_REQUIRED += periph_gpio_irq USEPKG += tiny-asn1 diff --git a/sys/fido2/ctap/Kconfig b/sys/fido2/ctap/Kconfig index 3116f39bec..942b832aca 100644 --- a/sys/fido2/ctap/Kconfig +++ b/sys/fido2/ctap/Kconfig @@ -7,6 +7,7 @@ menuconfig MODULE_FIDO2_CTAP bool "FIDO2 CTAP" depends on HAS_PERIPH_FLASHPAGE + depends on HAS_PERIPH_FLASHPAGE_IN_ADDRESS_SPACE depends on HAS_PERIPH_GPIO depends on HAS_PERIPH_GPIO_IRQ depends on MODULE_FIDO2 @@ -27,6 +28,7 @@ menuconfig MODULE_FIDO2_CTAP select MODULE_CRYPTO_AES_256 select MODULE_CIPHER_MODES select MODULE_HASHES + select MODULE_PERIPH_FLASHPAGE_IN_ADDRESS_SPACE help Y to enable CTAP protocol support. The Client-to-Authenticator Protocol (CTAP) is an application layer protocol for the communication @@ -108,12 +110,19 @@ config FIDO2_CTAP_UP_BUTTON_FLANK_RISING endchoice -config FIDO2_CTAP_FLASH_START_PAGE - int "First flash page to store data in" - default -1 +config FIDO2_CTAP_NUM_FLASHPAGES + int "Amount of flashpages to use" + range 2 256 + default 4 help - Configuring this incorrectly can lead to firmware corruption so make sure - the flash page is located after the firmware. + Configure how many flashpages are used to store FIDO2 CTAP data. + + To save a credential (rk) in flash memory, roughly 156 bytes are needed. This + number might change slightly depending on the flash block size. + Therefore, if one wants to e.g. save 40 credentials and the flashpage + size is 4096 bytes roughly 156 * 40 / 4096 (2) flashpages are needed. + To save authenticator state data one additional flashpage is needed. + So in total one has to configure 3 to save 40 credentials. rsource "transport/Kconfig" diff --git a/sys/fido2/ctap/ctap.c b/sys/fido2/ctap/ctap.c index 61473dc22a..4013c71fbb 100644 --- a/sys/fido2/ctap/ctap.c +++ b/sys/fido2/ctap/ctap.c @@ -180,7 +180,7 @@ static int _ctap_decrypt_rk(ctap_resident_key_t *rk, ctap_cred_id_t *id); /** * @brief Write credential to flash */ -static int _save_rk(ctap_resident_key_t *rk); +static int _write_rk_to_flash(ctap_resident_key_t *rk); /** * @brief Save PIN to authenticator state and write the updated state to flash @@ -192,16 +192,6 @@ static int _save_pin(uint8_t *pin, size_t len); */ static int _write_state_to_flash(const ctap_state_t *state); -/** - * @brief Read authenticator state from flash - */ -static int _read_state_from_flash(ctap_state_t *state); - -/** - * @brief Write resident key to flash - */ -static int _write_rk_to_flash(const ctap_resident_key_t *rk, int page, int offset); - /** * @brief Check if PIN protocol version is supported */ @@ -282,7 +272,12 @@ int fido2_ctap_init(void) return -EPROTO; } - ret = _read_state_from_flash(&_state); + /** + * CTAP state information is stored at flashpage 0 of the memory area + * dedicated for storing CTAP data + */ + ret = fido2_ctap_mem_read(&_state, fido2_ctap_mem_flash_page(), 0, + sizeof(_state)); if (ret != CTAP2_OK) { return -EPROTO; @@ -432,6 +427,8 @@ size_t fido2_ctap_reset(ctap_resp_t *resp) static int _reset(void) { + fido2_ctap_mem_erase_flash(); + _state.initialized_marker = CTAP_INITIALIZED_MARKER; _state.rem_pin_att = CTAP_PIN_MAX_ATTS; _state.pin_is_set = false; @@ -582,7 +579,7 @@ static int _make_credential(ctap_req_t *req_raw) /* if created credential is a resident credential, save it to flash */ if (rk) { - ret = _save_rk(&k); + ret = _write_rk_to_flash(&k); if (ret != CTAP2_OK) { goto done; } @@ -735,7 +732,7 @@ static int _get_assertion(ctap_req_t *req_raw) * therefore needs to be saved on the device. */ if (!rk->cred_desc.has_nonce) { - ret = _save_rk(rk); + ret = _write_rk_to_flash(rk); if (ret != CTAP2_OK) { goto done; @@ -821,7 +818,7 @@ static int _get_next_assertion(void) * therefore needs to be saved on the device. */ if (!rk->cred_desc.has_nonce) { - ret = _save_rk(rk); + ret = _write_rk_to_flash(rk); if (ret != CTAP2_OK) { goto done; @@ -1527,17 +1524,14 @@ static int _find_matching_rks(ctap_resident_key_t *rks, size_t rks_len, * so rk's can't be deleted, only overwritten => we can be sure that there are * no holes when reading keys from flash memory */ -static int _save_rk(ctap_resident_key_t *rk) +static int _write_rk_to_flash(ctap_resident_key_t *rk) { int ret; - int page_num = CTAP_FLASH_RK_START_PAGE, offset_into_page = 0; + int page_num = fido2_ctap_mem_flash_page() + CTAP_FLASH_RK_OFF; + int offset_into_page = 0; bool equal = false; ctap_resident_key_t rk_tmp = { 0 }; - if (_state.rk_amount_stored >= fido2_ctap_mem_get_max_rk_amount()) { - return CTAP2_ERR_KEY_STORE_FULL; - } - if (_state.rk_amount_stored > 0) { for (uint16_t i = 0; i <= _state.rk_amount_stored; i++) { page_num = fido2_ctap_mem_get_flashpage_number_of_rk(i); @@ -1571,6 +1565,10 @@ static int _save_rk(ctap_resident_key_t *rk) } if (!equal) { + if (_state.rk_amount_stored >= CTAP_FLASH_MAX_NUM_RKS) { + return CTAP2_ERR_KEY_STORE_FULL; + } + _state.rk_amount_stored++; ret = _write_state_to_flash(&_state); @@ -1579,7 +1577,7 @@ static int _save_rk(ctap_resident_key_t *rk) } } - return _write_rk_to_flash(rk, page_num, offset_into_page); + return fido2_ctap_mem_write(rk, page_num, offset_into_page, CTAP_FLASH_RK_SZ); } static int _make_auth_data_assert(uint8_t *rp_id, size_t rp_id_len, @@ -1792,17 +1790,13 @@ static int _ctap_decrypt_rk(ctap_resident_key_t *rk, ctap_cred_id_t *id) static int _write_state_to_flash(const ctap_state_t *state) { - return fido2_ctap_mem_write(state, CTAP_FLASH_STATE_PAGE, 0, CTAP_FLASH_STATE_SZ); -} - -static int _read_state_from_flash(ctap_state_t *state) -{ - return fido2_ctap_mem_read(state, CTAP_FLASH_STATE_PAGE, 0, sizeof(*state)); -} - -static int _write_rk_to_flash(const ctap_resident_key_t *rk, int page, int offset) -{ - return fido2_ctap_mem_write(rk, page, offset, CTAP_FLASH_RK_SZ); + /** + * CTAP state information is stored at flashpage 0 of the memory area + * dedicated for storing CTAP data + */ + return fido2_ctap_mem_write(state, + fido2_ctap_mem_flash_page(), 0, + CTAP_FLASH_STATE_SZ); } int fido2_ctap_get_sig(const uint8_t *auth_data, size_t auth_data_len, diff --git a/sys/fido2/ctap/ctap_mem.c b/sys/fido2/ctap/ctap_mem.c index 525dc6abcd..fda9cbda1d 100644 --- a/sys/fido2/ctap/ctap_mem.c +++ b/sys/fido2/ctap/ctap_mem.c @@ -25,45 +25,39 @@ #define ENABLE_DEBUG (0) #include "debug.h" +/** + * @brief Reserve flash memory to store CTAP data + */ +FLASH_WRITABLE_INIT(_backing_memory, CONFIG_FIDO2_CTAP_NUM_FLASHPAGES); + /** * @brief MTD device descriptor initialized with flash-page driver */ static mtd_flashpage_t _mtd_flash_dev = MTD_FLASHPAGE_INIT_VAL(CTAP_FLASH_PAGES_PER_SECTOR); static mtd_dev_t *_mtd_dev = &_mtd_flash_dev.base; -/** - * @brief Max amount of resident keys that can be stored - */ -static uint16_t _max_rk_amnt; - /** * @brief Check if flash region is erased */ static bool _flash_is_erased(int page, int offset, size_t len); /** - * @brief Get amount of flashpages + * @brief Get available amount of flashpages to store resident keys */ -static unsigned _amount_of_flashpages(void); +static unsigned _amount_flashpages_rk(void); int fido2_ctap_mem_init(void) { - int ret; - - ret = mtd_init(_mtd_dev); + int ret = mtd_init(_mtd_dev); if (ret < 0) { return ret; } - for (unsigned i = CTAP_FLASH_RK_START_PAGE; i < _amount_of_flashpages(); i++) { - _max_rk_amnt += flashpage_size(i) / CTAP_FLASH_RK_SZ; - } - return CTAP2_OK; } -static unsigned _amount_of_flashpages(void) +static unsigned _amount_flashpages_rk(void) { return _mtd_dev->sector_count * _mtd_dev->pages_per_sector; } @@ -120,16 +114,13 @@ static bool _flash_is_erased(int page, int offset, size_t len) return true; } -uint16_t fido2_ctap_mem_get_max_rk_amount(void) -{ - return _max_rk_amnt; -} - int fido2_ctap_mem_get_flashpage_number_of_rk(uint16_t rk_idx) { uint16_t idx = 0; + unsigned start = fido2_ctap_mem_flash_page() + CTAP_FLASH_RK_OFF; + unsigned amount = _amount_flashpages_rk(); - for (unsigned i = CTAP_FLASH_RK_START_PAGE; i < _amount_of_flashpages(); i++) { + for (unsigned i = start; i < start + amount; i++) { idx += flashpage_size(i) / CTAP_FLASH_RK_SZ; if (idx >= rk_idx) { @@ -143,8 +134,10 @@ int fido2_ctap_mem_get_flashpage_number_of_rk(uint16_t rk_idx) int fido2_ctap_mem_get_offset_of_rk_into_flashpage(uint16_t rk_idx) { uint16_t idx = 0; + unsigned start = fido2_ctap_mem_flash_page() + CTAP_FLASH_RK_OFF; + unsigned amount = _amount_flashpages_rk(); - for (unsigned i = CTAP_FLASH_RK_START_PAGE; i < _amount_of_flashpages(); i++) { + for (unsigned i = start; i < start + amount; i++) { uint16_t old_idx = idx; idx += flashpage_size(i) / CTAP_FLASH_RK_SZ; @@ -155,3 +148,20 @@ int fido2_ctap_mem_get_offset_of_rk_into_flashpage(uint16_t rk_idx) return -1; } + +unsigned fido2_ctap_mem_flash_page(void) +{ + return flashpage_page((void *)_backing_memory); +} + +int fido2_ctap_mem_erase_flash(void) +{ + unsigned start = fido2_ctap_mem_flash_page(); + unsigned end = start + CONFIG_FIDO2_CTAP_NUM_FLASHPAGES; + + for (unsigned page = start; page < end; page++) { + flashpage_erase(page); + } + + return CTAP2_OK; +} diff --git a/sys/include/fido2/ctap/ctap_mem.h b/sys/include/fido2/ctap/ctap_mem.h index f9b2bf4828..d96ff56608 100644 --- a/sys/include/fido2/ctap/ctap_mem.h +++ b/sys/include/fido2/ctap/ctap_mem.h @@ -41,28 +41,15 @@ extern "C" { /** @} */ /** - * @brief First flash page to store data in - * - * @note This can corrupt firmware if CTAP_FLASH_START_PAGE is set to a - * flash page containing firmware. Therefore make sure that CTAP_FLASH_START_PAGE - * is located after the firmware. + * @brief Default amount of flashpages to use */ -#if defined(CONFIG_FIDO2_CTAP_FLASH_START_PAGE) && \ - (CONFIG_FIDO2_CTAP_FLASH_START_PAGE >= 0) -#define CTAP_FLASH_START_PAGE CONFIG_FIDO2_CTAP_FLASH_START_PAGE -#else -#define CTAP_FLASH_START_PAGE (FLASHPAGE_NUMOF - 4) +#ifndef CONFIG_FIDO2_CTAP_NUM_FLASHPAGES +#define CONFIG_FIDO2_CTAP_NUM_FLASHPAGES 4 #endif -/** - * @brief Start page for storing resident keys - */ -#define CTAP_FLASH_RK_START_PAGE CTAP_FLASH_START_PAGE - -/** - * @brief Page for storing authenticator state information - */ -#define CTAP_FLASH_STATE_PAGE CTAP_FLASH_RK_START_PAGE - 1 +#if CONFIG_FIDO2_CTAP_NUM_FLASHPAGES < 2 +#error "ctap_mem.h: Configured number of flashpages is invalid" +#endif /** * @brief Calculate padding needed to align struct size for saving to flash @@ -84,6 +71,12 @@ extern "C" { #define CTAP_FLASH_STATE_SZ (sizeof(ctap_state_t) + \ CTAP_FLASH_ALIGN_PAD(ctap_state_t)) +/** + * @brief Max amount of resident keys that can be stored on device + */ +#define CTAP_FLASH_MAX_NUM_RKS ((CONFIG_FIDO2_CTAP_NUM_FLASHPAGES - 1) * \ + FLASHPAGE_SIZE / CTAP_FLASH_RK_SZ) + /** * @brief Minimum flash sector size needed to hold CTAP related data * @@ -96,6 +89,14 @@ extern "C" { */ #define CTAP_FLASH_PAGES_PER_SECTOR ((CTAP_FLASH_MIN_SECTOR_SZ / FLASHPAGE_SIZE) + 1) +/** + * Offset of flashpage for storing resident keys + * + * The offset is in units of flashpages from the beginning of the flash memory + * area dedicated for storing CTAP data. + */ +#define CTAP_FLASH_RK_OFF 0x1 + /** * @brief Initialize memory helper * @@ -127,13 +128,6 @@ int fido2_ctap_mem_write(const void *buf, uint32_t page, uint32_t offset, uint32 */ int fido2_ctap_mem_read(void *buf, uint32_t page, uint32_t offset, uint32_t len); -/** - * @brief Get maximum amount of resident credentials that can be stored - * - * @return maximum amount that can be stored - */ -uint16_t fido2_ctap_mem_get_max_rk_amount(void); - /** * @brief Get flashpage number resident key with index @p rk_idx. * @@ -155,6 +149,27 @@ int fido2_ctap_mem_get_flashpage_number_of_rk(uint16_t rk_idx); */ int fido2_ctap_mem_get_offset_of_rk_into_flashpage(uint16_t rk_idx); +/** + * @brief Get page number for storing authenticator state information + * + * @return page number + */ +unsigned fido2_ctap_mem_flash_page(void); + +/** + * @brief Get start page for storing resident keys + * + * @return page number + */ +unsigned fido2_ctap_mem_get_rk_start_page(void); + +/** + * @brief Erase all flashpages containing CTAP data + * + * @return @ref ctap_status_codes_t + */ +int fido2_ctap_mem_erase_flash(void); + #ifdef __cplusplus } #endif