Skip to content

Commit

Permalink
Frame counters for nw keys are now stored to NVM only after send key …
Browse files Browse the repository at this point in the history
…is set (ARMmbed#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.
  • Loading branch information
Mika Leppänen authored Jun 16, 2021
1 parent 3b3010a commit 95c506a
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 25 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
46 changes: 21 additions & 25 deletions source/6LoWPAN/ws/ws_pae_controller.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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;

}
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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;
Expand All @@ -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)
Expand Down Expand Up @@ -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;
Expand All @@ -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);

Expand All @@ -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);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions source/Security/protocols/sec_prot_keys.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 95c506a

Please sign in to comment.