sys/fido2: improve & simplify flash handling

sys/fido2/Makefile.dep

@@ -9,10 +9,6 @@ ifneq (,$(filter fido2_ctap_%,$(USEMODULE)))
 endif
 
 ifneq (,$(filter fido2_ctap,$(USEMODULE)))
-  FEATURES_REQUIRED += periph_flashpage
-ifeq (,$(filter native,$(CPU)))
-  FEATURES_REQUIRED += periph_flashpage_in_address_space
-endif
   FEATURES_REQUIRED += periph_gpio_irq
 
   USEPKG += tiny-asn1

sys/fido2/ctap/ctap.c

@@ -15,6 +15,7 @@
  * @}
  */
 
+#include "fido2/ctap/ctap.h"
 #include <string.h>
 #include <stdlib.h>
 #include <assert.h>
@@ -35,20 +36,20 @@
 #include "fido2/ctap/transport/hid/ctap_hid.h"
 #endif
 
-#define ENABLE_DEBUG    (0)
+#define ENABLE_DEBUG (0)
 #include "debug.h"
 
 /**
  * @brief CTAP get_assertion state
  */
 typedef struct {
     ctap_resident_key_t rks[CTAP_MAX_EXCLUDE_LIST_SIZE];    /**< eligible resident keys found */
     uint8_t count;                                          /**< number of rks found  */
     uint8_t cred_counter;                                   /**< amount of creds sent to host */
     uint32_t timer;                                         /**< time gap between get_next_assertion calls in milliseconds  */
     bool uv;                                                /**< indicate if user verified */
     bool up;                                                /**< indicate if user present */
     uint8_t client_data_hash[SHA256_DIGEST_LENGTH];         /**< SHA-256 hash of JSON serialized client data */
 } ctap_get_assertion_state_t;
 
 /*** CTAP methods ***/
@@ -424,10 +425,12 @@
 
 static int _reset(void)
 {
-    int ret = fido2_ctap_mem_erase_flash();
+    if (_state.initialized_marker == CTAP_INITIALIZED_MARKER) {
+        int ret = fido2_ctap_mem_erase_flash(&_state);
 
-    if (ret != CTAP2_OK) {
-        return ret;
+        if (ret != CTAP2_OK) {
+            return ret;
+        }
     }
 
     _state.initialized_marker = CTAP_INITIALIZED_MARKER;
@@ -535,8 +538,7 @@
         uv = true;
     }
     /* CTAP specification (version 20190130) section 5.5.8.1 */
-    else if (!fido2_ctap_pin_is_set() && req.pin_auth_present
-             && req.pin_auth_len == 0) {
+    else if (!fido2_ctap_pin_is_set() && req.pin_auth_present && req.pin_auth_len == 0) {
         if (!IS_ACTIVE(CONFIG_FIDO2_CTAP_DISABLE_UP)) {
             fido2_ctap_utils_user_presence_test();
         }
@@ -659,8 +661,7 @@
         _assert_state.uv = true;
     }
     /* CTAP specification (version 20190130) section 5.5.8.2 */
-    else if (!fido2_ctap_pin_is_set() && req.pin_auth_present
-             && req.pin_auth_len == 0) {
+    else if (!fido2_ctap_pin_is_set() && req.pin_auth_present && req.pin_auth_len == 0) {
         if (!IS_ACTIVE(CONFIG_FIDO2_CTAP_DISABLE_UP)) {
             fido2_ctap_utils_user_presence_test();
         }
@@ -1414,9 +1415,9 @@
     }
 
     ctap_resident_key_t rk = { 0 };
-    uint32_t addr = 0x0;
+    uint32_t off = 0x0;
 
-    while (fido2_ctap_mem_read_rk_from_flash(&rk, rp_id_hash, &addr) == CTAP2_OK) {
+    while (fido2_ctap_mem_read_rk_from_flash(&rk, rp_id_hash, &off) == CTAP2_OK) {
         if (allow_list_len == 0) {
             memcpy(&rks[index], &rk, sizeof(rk));
             index++;

sys/fido2/ctap/ctap_mem.c

@@ -11,75 +11,47 @@
  * @{
  * @file
  *
- * @author      Nils Ollrogge <nils.ollrogge@fu-berlin.de>
+ * @author      Nils Ollrogge <nils-ollrogge@outlook.de>
  * @}
  */
-
-#include <string.h>
-
-#include "bitarithm.h"
-
-#include "mtd.h"
 #include "mtd_flashpage.h"
 
 #include "fido2/ctap/ctap_mem.h"
 #include "fido2/ctap/ctap_utils.h"
 
-#define ENABLE_DEBUG    (0)
+#define ENABLE_DEBUG (0)
 #include "debug.h"
 
+static mtd_dev_t *_mtd_dev;
+
 #ifdef BOARD_NATIVE
 #include "mtd_default.h"
-/* native mtd is file backed => Start address of flash is 0. */
-char *_backing_memory = NULL;
-static mtd_dev_t *_mtd_dev = NULL;
-#else
-/**
- * @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;
 #endif
 
-/**
- * @brief   Check if flash region is erased
- */
-static bool _flash_is_erased(uint32_t addr, size_t len);
-
-/**
- * @brief   Get available amount of flashpages to store resident keys
- */
-static unsigned _amount_flashpages_rk(void);
-
 /**
  * @brief Write to flash memory
  */
-static ctap_status_code_t _flash_write(const void *buf, uint32_t addr, size_t len);
+static ctap_status_code_t _flash_write(const void *buf, uint32_t page, uint32_t off, size_t len);
 
 ctap_status_code_t fido2_ctap_mem_init(void)
 {
 #ifdef BOARD_NATIVE
     _mtd_dev = mtd_default_get_dev(0);
+#else
+    _mtd_dev = mtd_aux;
 #endif
 
     int ret = mtd_init(_mtd_dev);
 
     if (ret < 0) {
+        DEBUG("%s, %d: mtd_init failed\n", RIOT_FILE_RELATIVE,
+              __LINE__);
         return ret;
     }
 
     return CTAP2_OK;
 }
 
-static unsigned _amount_flashpages_rk(void)
-{
-    return _mtd_dev->sector_count * _mtd_dev->pages_per_sector;
-}
-
 ctap_status_code_t fido2_ctap_mem_read(void *buf, uint32_t page, uint32_t offset, uint32_t len)
 {
     assert(buf);
@@ -94,59 +66,32 @@
     return CTAP2_OK;
 }
 
-static ctap_status_code_t _flash_write(const void *buf, uint32_t addr, size_t len)
+static ctap_status_code_t _flash_write(const void *buf, uint32_t page, uint32_t off, size_t len)
 {
     assert(buf);
     int ret;
 
-    if (!_flash_is_erased(addr, len)) {
-        /* page size is always a power of two */
-        const uint32_t page_shift = bitarithm_msb(_mtd_dev->page_size);
-        const uint32_t page_mask = _mtd_dev->page_size - 1;
-
-        ret = mtd_write_page(_mtd_dev, buf, addr >> page_shift, addr & page_mask, len);
-
-        if (ret < 0) {
-            return CTAP1_ERR_OTHER;
-        }
-    }
-    else {
-        ret = mtd_write(_mtd_dev, buf, addr, len);
+    ret = mtd_write_page(_mtd_dev, buf, page, off, len);
 
-        if (ret < 0) {
-            return CTAP1_ERR_OTHER;
-        }
+    if (ret < 0) {
+        DEBUG("%s, %d: mtd_write_page failed\n", RIOT_FILE_RELATIVE,
+              __LINE__);
+        return CTAP1_ERR_OTHER;
     }
 
     return CTAP2_OK;
 }
 
-static bool _flash_is_erased(uint32_t addr, size_t len)
-{
-#ifdef BOARD_NATIVE
-    return true;
-#else
-    for (size_t i = 0; i < len; i++) {
-        if (*(uint32_t *)(addr + i) != FLASHPAGE_ERASE_STATE) {
-            return false;
-        }
-    }
-
-    return true;
-#endif
-}
-
-static uint32_t _flash_start_addr(void)
-{
-    return (uint32_t)_backing_memory;
-}
-
-ctap_status_code_t fido2_ctap_mem_erase_flash(void)
+ctap_status_code_t fido2_ctap_mem_erase_flash(ctap_state_t *state)
 {
-    unsigned addr = _flash_start_addr();
-    unsigned sector_size = _mtd_dev->pages_per_sector * _mtd_dev->page_size;
+    /* Calculate total pages needed, rounding up */
+    uint32_t used_pages = (CTAP_FLASH_STATE_SZ + state->rk_amount_stored * CTAP_FLASH_RK_SZ +
+                           _mtd_dev->page_size - 1) / _mtd_dev->page_size;
+    /* Calculate the number of sectors needed, rounding up */
+    uint32_t sector_cnt = (used_pages + _mtd_dev->pages_per_sector - 1) /
+                          _mtd_dev->pages_per_sector;
 
-    int ret = mtd_erase(_mtd_dev, addr, sector_size * CONFIG_FIDO2_CTAP_NUM_FLASHPAGES);
+    int ret = mtd_erase_sector(_mtd_dev, 0, sector_cnt);
 
     return ret == 0 ? CTAP2_OK : CTAP1_ERR_OTHER;
 }
@@ -157,9 +102,7 @@
  */
 ctap_status_code_t fido2_ctap_mem_read_state_from_flash(ctap_state_t *state)
 {
-    uint32_t addr = _flash_start_addr();
-
-    int ret = mtd_read(_mtd_dev, state, addr, sizeof(ctap_state_t));
+    int ret = mtd_read_page(_mtd_dev, state, 0, 0, sizeof(ctap_state_t));
 
     return ret == 0 ? CTAP2_OK : CTAP1_ERR_OTHER;
 }
@@ -174,16 +117,20 @@
 ctap_status_code_t fido2_ctap_mem_write_rk_to_flash(ctap_resident_key_t *rk)
 {
     int ret;
-    uint32_t addr = _flash_start_addr() + FLASHPAGE_SIZE;
     uint16_t amt_stored = fido2_ctap_get_state()->rk_amount_stored;
     ctap_resident_key_t tmp = { 0 };
     bool equal = false;
 
+    /* skip ctap_state_t struct */
+    uint32_t off = CTAP_FLASH_STATE_SZ;
+    uint32_t page = off / _mtd_dev->page_size;
+    uint32_t page_off = off % _mtd_dev->page_size;
+
     for (uint16_t i = 0; i < amt_stored; i++) {
-        ret = mtd_read(_mtd_dev, &tmp, addr, sizeof(ctap_resident_key_t));
+        ret = mtd_read_page(_mtd_dev, &tmp, page, page_off, sizeof(ctap_resident_key_t));
 
         if (ret < 0) {
-            DEBUG("%s, %d: mtd_read failed", RIOT_FILE_RELATIVE,
+            DEBUG("%s, %d: mtd_read failed\n", RIOT_FILE_RELATIVE,
                   __LINE__);
             return false;
         }
@@ -193,7 +140,9 @@
             break;
         }
 
-        addr += CTAP_FLASH_RK_SZ;
+        off += CTAP_FLASH_RK_SZ;
+        page = off / _mtd_dev->page_size;
+        page_off = off % _mtd_dev->page_size;
     }
 
     if (!equal) {
@@ -210,27 +159,43 @@
         }
     }
 
-    return _flash_write(rk, addr, CTAP_FLASH_RK_SZ);
+    return _flash_write(rk, page, page_off, CTAP_FLASH_RK_SZ);
 }
 
 ctap_status_code_t fido2_ctap_mem_write_state_to_flash(ctap_state_t *state)
 {
-    return _flash_write(state, _flash_start_addr(), CTAP_FLASH_STATE_SZ);
+    return _flash_write(state, 0, 0, CTAP_FLASH_STATE_SZ);
 }
 
-ctap_status_code_t fido2_ctap_mem_read_rk_from_flash(ctap_resident_key_t *key, uint8_t *rp_id_hash,
-                                                     uint32_t *addr)
+ctap_status_code_t fido2_ctap_mem_read_rk_from_flash(ctap_resident_key_t *key,
+                                                     const uint8_t *rp_id_hash,
+                                                     uint32_t *off)
 {
     uint16_t end;
     uint16_t amt_stored = fido2_ctap_get_state()->rk_amount_stored;
 
-    if (*addr == 0x0) {
+    /* some error checks for weird offsets that indicate something went wrong */
+    if (*off != 0x0 && *off < CTAP_FLASH_STATE_SZ) {
+        DEBUG("%s, %d: Incorrect offset detected\n", RIOT_FILE_RELATIVE,
+              __LINE__);
+
+        return CTAP1_ERR_OTHER;
+    }
+
+    if (*off > CTAP_FLASH_STATE_SZ && (*off - CTAP_FLASH_STATE_SZ) % CTAP_FLASH_RK_SZ != 0) {
+        DEBUG("%s, %d: Incorrect offset detected\n", RIOT_FILE_RELATIVE,
+              __LINE__);
+
+        return CTAP1_ERR_OTHER;
+    }
+
+    if (*off == 0x0) {
         end = amt_stored;
-        *addr = _flash_start_addr() + FLASHPAGE_SIZE;
+        /* skip ctap_state_t struct */
+        *off = CTAP_FLASH_STATE_SZ;
     }
     else {
-        uint32_t start_addr = _flash_start_addr() + FLASHPAGE_SIZE;
-        uint16_t rks_read = (*addr - start_addr) / CTAP_FLASH_RK_SZ;
+        uint16_t rks_read = (*off - CTAP_FLASH_STATE_SZ) / CTAP_FLASH_RK_SZ;
 
         if (rks_read > amt_stored) {
             return CTAP1_ERR_OTHER;
@@ -240,15 +205,18 @@
     }
 
     for (uint16_t i = 0; i < end; i++) {
-        int ret = mtd_read(_mtd_dev, key, *addr, sizeof(ctap_resident_key_t));
+        uint32_t page = *off / _mtd_dev->page_size;
+        uint32_t page_off = *off % _mtd_dev->page_size;
+
+        int ret = mtd_read_page(_mtd_dev, key, page, page_off, sizeof(ctap_resident_key_t));
 
         if (ret < 0) {
             DEBUG("%s, %d: mtd_read failed", RIOT_FILE_RELATIVE,
                   __LINE__);
             return CTAP1_ERR_OTHER;
         }
 
-        *addr += CTAP_FLASH_RK_SZ;
+        *off += CTAP_FLASH_RK_SZ;
 
         if (memcmp(key->rp_id_hash, rp_id_hash, SHA256_DIGEST_LENGTH) == 0) {
             return CTAP2_OK;

sys/include/fido2/ctap.h

@@ -26,6 +26,7 @@
 #define FIDO2_CTAP_H
 
 #include <stdint.h>
+#include <stddef.h>
 
 #ifdef __cplusplus
 extern "C" {