From 95c506a276b6d1230bb1824beb5adba7955cd3e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mika=20Lepp=C3=A4nen?= Date: Wed, 16 Jun 2021 14:42:07 +0300 Subject: [PATCH] Frame counters for nw keys are now stored to NVM only after send key is set (#2641) * Frame counters for nw keys are now stored to NVM only after send key is set This way if the device resets right after startup, frame counters are not incremented to NVM, which could lead to frame counter are exhaustion. --- CHANGELOG.md | 1 + source/6LoWPAN/ws/ws_pae_controller.c | 46 +++++++++++------------ source/Security/protocols/sec_prot_keys.h | 1 + 3 files changed, 23 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90c4645e0bfa..5e3fc9c7fd91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * Statistics for MAC data request latencies. Separated to adaptation layer and MAC queueing delays. * Changed initial EAPOL-key retries from trickle to exponential backup. * Adjusted stagger random from [min,max] to [min,min+max] and for small networks set the stagger value to 10 seconds. +* Frame counter values for network keys are now stored to NVM only after network key for sending is set. ### Bug fixes * Added ignoring of retry messages from RADIUS server when waiting EAP-TLS diff --git a/source/6LoWPAN/ws/ws_pae_controller.c b/source/6LoWPAN/ws/ws_pae_controller.c index f57934fcadee..805cb07d8bc2 100644 --- a/source/6LoWPAN/ws/ws_pae_controller.c +++ b/source/6LoWPAN/ws/ws_pae_controller.c @@ -122,7 +122,6 @@ static bool ws_pae_controller_auth_congestion_get(protocol_interface_info_entry_ #endif static pae_controller_t *ws_pae_controller_get(protocol_interface_info_entry_t *interface_ptr); static void ws_pae_controller_frame_counter_timer(uint16_t seconds, pae_controller_t *entry); -static void ws_pae_controller_frame_counter_timer_trigger(uint16_t seconds, pae_controller_t *entry); static void ws_pae_controller_frame_counter_store(pae_controller_t *entry, bool use_threshold); static void ws_pae_controller_nvm_frame_counter_write(frame_cnt_nvm_tlv_t *tlv_entry); static int8_t ws_pae_controller_nvm_frame_counter_read(uint32_t *restart_cnt, uint64_t *stored_time, uint16_t *pan_version, frame_counters_t *counters); @@ -516,10 +515,6 @@ static int8_t ws_pae_controller_nw_key_check_and_insert(protocol_interface_info_ controller->nw_frame_counter_set(controller->interface_ptr, curr_frame_counter, i); } } - /* Trigger storing of frame counters; there is 5 seconds delay to give time for the - other keys to be inserted, so that frame counters for several keys are updated on - a same time. */ - ws_pae_controller_frame_counter_timer_trigger(FRAME_COUNTER_STORE_TRIGGER, controller); } } @@ -614,6 +609,8 @@ static void ws_pae_controller_frame_counter_store_and_nw_keys_remove(protocol_in tr_info("NW keys remove"); + controller->gtk_index = -1; + nw_key_t *nw_key = controller->nw_key; for (uint8_t i = 0; i < GTK_NUM; i++) { // Deletes the key if it is set @@ -637,9 +634,12 @@ static void ws_pae_controller_nw_key_index_check_and_set(protocol_interface_info } if (controller->nw_send_key_index_set) { + controller->gtk_index = index; + /* Checks if frame counters needs to be stored for the new GTK that is taken into + use; this is the last check that stored counters are in sync before activating key */ + ws_pae_controller_frame_counter_store(controller, true); tr_info("NW send key index set: %i", index + 1); controller->nw_send_key_index_set(interface_ptr, index); - controller->gtk_index = index; } // Do not update PAN version for initial key index set @@ -661,13 +661,14 @@ static void ws_pae_controller_active_nw_key_set(protocol_interface_info_entry_t } if (controller->nw_send_key_index_set) { + controller->gtk_index = index; /* Checks if frame counters needs to be stored for the new GTK that is taken into use; this is the last check that stored counters are in sync before activating key */ ws_pae_controller_frame_counter_store(controller, true); // Activates key on MAC controller->nw_send_key_index_set(controller->interface_ptr, index); tr_info("NW send key index set: %i", index + 1); - controller->gtk_index = index; + } } @@ -801,7 +802,6 @@ static int8_t ws_pae_controller_frame_counter_read(pae_controller_t *controller) // Increments PAN version to ensure that it is fresh controller->sec_keys_nw_info.pan_version += PAN_VERSION_STORAGE_READ_INCREMENT; - bool updated = false; // Checks frame counters for (uint8_t index = 0; index < GTK_NUM; index++) { if (controller->frame_counters.counter[index].set) { @@ -817,15 +817,8 @@ static int8_t ws_pae_controller_frame_counter_read(pae_controller_t *controller) controller->frame_counters.counter[index].frame_counter; tr_info("Read frame counter: index %i value %"PRIu32"", index, controller->frame_counters.counter[index].frame_counter); - - updated = true; } } - if (updated) { - // Writes incremented frame counters - ws_pae_nvm_store_frame_counter_tlv_create((frame_cnt_nvm_tlv_t *) &controller->pae_nvm_buffer, controller->restart_cnt, controller->sec_keys_nw_info.pan_version, &controller->frame_counters); - ws_pae_controller_nvm_frame_counter_write((frame_cnt_nvm_tlv_t *) &controller->pae_nvm_buffer); - } } return ret_value; @@ -836,6 +829,7 @@ static void ws_pae_controller_frame_counter_reset(frame_counters_t *frame_counte for (uint8_t index = 0; index < GTK_NUM; index++) { ws_pae_controller_frame_counter_index_reset(frame_counters, index); } + frame_counters->active_gtk_index = -1; } static void ws_pae_controller_frame_counter_index_reset(frame_counters_t *frame_counters, uint8_t index) @@ -1712,13 +1706,6 @@ static void ws_pae_controller_frame_counter_timer(uint16_t seconds, pae_controll } } -static void ws_pae_controller_frame_counter_timer_trigger(uint16_t seconds, pae_controller_t *entry) -{ - if (entry->frame_cnt_store_timer > seconds) { - entry->frame_cnt_store_timer = seconds; - } -} - static void ws_pae_controller_frame_counter_store(pae_controller_t *entry, bool use_threshold) { bool update_needed = false; @@ -1731,7 +1718,7 @@ static void ws_pae_controller_frame_counter_store(pae_controller_t *entry, bool * frame counters will be still valid. */ if (entry->nw_key[i].installed) { - // Reads frame counter for the key + // Reads MAC frame counter for the key uint32_t curr_frame_counter; entry->nw_frame_counter_read(entry->interface_ptr, &curr_frame_counter, i); @@ -1752,13 +1739,22 @@ static void ws_pae_controller_frame_counter_store(pae_controller_t *entry, bool tr_debug("Stored updated frame counter: index %i value %"PRIu32"", i, frame_counter); } } else { - // For new or modified network keys, stores the frame counter value + // New or modified network key entry->frame_counters.counter[i].set = true; memcpy(entry->frame_counters.counter[i].gtk, entry->nw_key[i].gtk, GTK_LEN); entry->frame_counters.counter[i].frame_counter = curr_frame_counter; entry->frame_counters.counter[i].stored_frame_counter = curr_frame_counter; + tr_debug("Pending to store new frame counter: index %i value %"PRIu32"", i, curr_frame_counter); + } + + /* If currently active key is changed or active key is set for the first time, + stores the frame counter value */ + if (entry->gtk_index == i && entry->frame_counters.active_gtk_index != i) { + entry->frame_counters.active_gtk_index = entry->gtk_index; update_needed = true; - tr_debug("Stored new frame counter: index %i value %"PRIu32"", i, curr_frame_counter); + // Updates MAC frame counter for the key + entry->nw_frame_counter_set(entry->interface_ptr, entry->frame_counters.counter[i].frame_counter, i); + tr_debug("Stored frame counters, active key set: index %i value %"PRIu32"", i, entry->frame_counters.counter[i].frame_counter); } } } diff --git a/source/Security/protocols/sec_prot_keys.h b/source/Security/protocols/sec_prot_keys.h index bd114d5c8aea..6215bdc759f5 100644 --- a/source/Security/protocols/sec_prot_keys.h +++ b/source/Security/protocols/sec_prot_keys.h @@ -114,6 +114,7 @@ typedef struct { typedef struct { frame_counter_t counter[GTK_NUM]; /**< Frame counter for each GTK key */ + int8_t active_gtk_index; /**< Active GTK index */ } frame_counters_t; // Authenticator supplicant security key data