Skip to content

Commit

Permalink
sys/fido2: improve & simplify flash handling
Browse files Browse the repository at this point in the history
  • Loading branch information
@Ollrogge
Ollrogge committed Dec 25, 2024
1 parent 91d587f commit 771229a
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 146 deletions.
4 changes: 0 additions & 4 deletions sys/fido2/Makefile.dep
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 11 additions & 10 deletions sys/fido2/ctap/ctap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* @}
*/

#include "fido2/ctap/ctap.h"
#include <string.h>
#include <stdlib.h>
#include <assert.h>
Expand All @@ -35,7 +36,7 @@
#include "fido2/ctap/transport/hid/ctap_hid.h"
#endif

#define ENABLE_DEBUG (0)
#define ENABLE_DEBUG (0)
#include "debug.h"

/**

Check warning on line 42 in sys/fido2/ctap/ctap.c

View workflow job for this annotation

GitHub Actions / static-tests 

Coccinelle proposes the following patch:

--- sys/fido2/ctap/ctap.c
+++ sys/fido2/ctap/ctap.c
@@ -36,7 +36,7 @@
 #include "fido2/ctap/transport/hid/ctap_hid.h"
 #endif
 
-#define ENABLE_DEBUG (0)
+#define ENABLE_DEBUG 0
 #include "debug.h"
 
 /**
Expand Down Expand Up @@ -424,10 +425,12 @@ static uint32_t get_id(void)

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;
Expand Down Expand Up @@ -535,8 +538,7 @@ static int _make_credential(ctap_req_t *req_raw)
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();
}
Expand Down Expand Up @@ -659,8 +661,7 @@ static int _get_assertion(ctap_req_t *req_raw)
_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();
}
Expand Down Expand Up @@ -1414,9 +1415,9 @@ static int _find_matching_rks(ctap_resident_key_t *rks, size_t rks_len,
}

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++;
Expand Down
158 changes: 63 additions & 95 deletions sys/fido2/ctap/ctap_mem.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Check warning on line 25 in sys/fido2/ctap/ctap_mem.c

View workflow job for this annotation

GitHub Actions / static-tests 

Coccinelle proposes the following patch:

--- sys/fido2/ctap/ctap_mem.c
+++ sys/fido2/ctap/ctap_mem.c
@@ -19,7 +19,7 @@
 #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);
Expand All @@ -94,59 +66,32 @@ ctap_status_code_t fido2_ctap_mem_read(void *buf, uint32_t page, uint32_t offset
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;
}
Expand All @@ -157,9 +102,7 @@ ctap_status_code_t fido2_ctap_mem_erase_flash(void)
*/
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;
}
Expand All @@ -174,16 +117,20 @@ ctap_status_code_t fido2_ctap_mem_read_state_from_flash(ctap_state_t *state)
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;
}
Expand All @@ -193,7 +140,9 @@ ctap_status_code_t fido2_ctap_mem_write_rk_to_flash(ctap_resident_key_t *rk)
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) {
Expand All @@ -210,27 +159,43 @@ ctap_status_code_t fido2_ctap_mem_write_rk_to_flash(ctap_resident_key_t *rk)
}
}

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;
Expand All @@ -240,15 +205,18 @@ ctap_status_code_t fido2_ctap_mem_read_rk_from_flash(ctap_resident_key_t *key, u
}

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;
Expand Down
1 change: 1 addition & 0 deletions sys/include/fido2/ctap.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#define FIDO2_CTAP_H

#include <stdint.h>
#include <stddef.h>

#ifdef __cplusplus
extern "C" {
Expand Down
Loading

0 comments on commit 771229a

Please sign in to comment.