From d008267960337e9e44b270b26555a7a894808746 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Mon, 24 Apr 2017 08:33:11 -0400 Subject: Clean up pkey slots and volatile keys on client logout. --- hal_internal.h | 21 ++++++++++++++++++ ks_volatile.c | 30 ++++++++++++++++++++++++- rpc_misc.c | 69 +++++++++++++++++++++++++++++++++++++++++++++------------- rpc_pkey.c | 28 ++++++++++++++++++++++++ 4 files changed, 132 insertions(+), 16 deletions(-) diff --git a/hal_internal.h b/hal_internal.h index 56d0936..b20bff2 100644 --- a/hal_internal.h +++ b/hal_internal.h @@ -427,6 +427,12 @@ extern hal_error_t hal_mkm_flash_erase(const size_t len); #endif +/* + * Clean up pkey stuff that's tied to a particular client. + */ + +extern hal_error_t hal_pkey_client_cleanup(const hal_client_handle_t client); + /* * Keystore API for use by the pkey implementation. * @@ -520,6 +526,9 @@ struct hal_ks_driver { uint8_t *attributes_buffer, const size_t attributes_buffer_len); + hal_error_t (*client_cleanup)(hal_ks_t *ks, + const hal_client_handle_t client); + }; @@ -681,6 +690,18 @@ static inline hal_error_t hal_ks_get_attributes(hal_ks_t *ks, attributes_buffer, attributes_buffer_len); } +static inline hal_error_t hal_ks_client_cleanup(hal_ks_t *ks, + const hal_client_handle_t client) +{ + if (ks == NULL) + return HAL_ERROR_BAD_ARGUMENTS; + + if (ks->client_cleanup == NULL || client.handle == HAL_HANDLE_NONE) + return HAL_OK; + + return ks->driver->client_cleanup(ks, client); +} + /* * Keystore index. This is intended to be usable by both memory-based * (in-memory, mmap(), ...) keystores and keystores based on raw flash. diff --git a/ks_volatile.c b/ks_volatile.c index d565c60..363441a 100644 --- a/ks_volatile.c +++ b/ks_volatile.c @@ -614,6 +614,33 @@ static hal_error_t ks_get_attributes(hal_ks_t *ks, return err; } +static void ks_client_cleanup(hal_ks_t *ks, + hal_client_handle_t client) +{ + if (ks == NULL || client.handle = HAL_HANDLE_NONE) + return HAL_ERROR_BAD_ARGUMENTS; + + ks_t *ksv = ks_to_ksv(ks); + hal_error_t err = HAL_OK; + + hal_ks_lock(); + + for (int i = 0; i < ksv->db->ksi.used; i++) { + unsigned b = ksv->db->ksi.index[i]; + if (ksv->db->keys[b].client.handle == client.handle) { + int hint = i; + if ((err = hal_ks_index_delete(&ksv->db->ksi, &ksv->db->ksi.names[b].name, 0, NULL, &hint)) != HAL_OK) + goto done; + memset(&ksv->db->keys[b], 0, sizeof(ksv->db->keys[b])); + i--; + } + } + + done: + hal_ks_unlock(); + return err; +} + const hal_ks_driver_t hal_ks_volatile_driver[1] = {{ .init = ks_volatile_init, .shutdown = ks_volatile_shutdown, @@ -624,7 +651,8 @@ const hal_ks_driver_t hal_ks_volatile_driver[1] = {{ .delete = ks_delete, .match = ks_match, .set_attributes = ks_set_attributes, - .get_attributes = ks_get_attributes + .get_attributes = ks_get_attributes, + .client_cleanup = ks_client_cleanup }}; #endif /* STATIC_KS_VOLATILE_SLOTS > 0 */ diff --git a/rpc_misc.c b/rpc_misc.c index cf5e4a0..2fd743a 100644 --- a/rpc_misc.c +++ b/rpc_misc.c @@ -101,21 +101,47 @@ static client_slot_t client_handle[HAL_STATIC_CLIENT_STATE_BLOCKS]; * them. HAL_USER_NONE indicates an empty slot in the table. */ -static inline client_slot_t *alloc_slot(void) +static inline client_slot_t *alloc_slot(const hal_client_handle_t client, + const hal_user_t user) { client_slot_t *slot = NULL; hal_critical_section_start(); #if HAL_STATIC_CLIENT_STATE_BLOCKS > 0 + + for (int i = 0; slot == NULL && i < sizeof(client_handle)/sizeof(*client_handle); i++) + if (client_handle[i].logged_in != HAL_USER_NONE && client_handle[i].handle.handle == handle.handle) + slot = &client_handle[i]; + for (int i = 0; slot == NULL && i < sizeof(client_handle)/sizeof(*client_handle); i++) if (client_handle[i].logged_in == HAL_USER_NONE) slot = &client_handle[i]; + #endif + if (slot != NULL) { + slot->handle = client; + slot->logged_in = user; + } + hal_critical_section_end(); return slot; } +static inline void clear_slot(client_slot_t *slot) +{ + if (slot == NULL) + return; + + hal_pkey_client_cleanup(slot->handle); + + hal_critical_section_start(); + + memset(slot, 0, sizeof(*slot)); + + hal_critical_section_end(); +} + static inline client_slot_t *find_handle(const hal_client_handle_t handle) { client_slot_t *slot = NULL; @@ -158,14 +184,9 @@ static hal_error_t login(const hal_client_handle_t client, if (diff != 0) return HAL_ERROR_PIN_INCORRECT; - client_slot_t *slot = find_handle(client); - - if (slot == NULL && (slot = alloc_slot()) == NULL) + if (alloc_slot(client, user) == NULL) return HAL_ERROR_NO_CLIENT_SLOTS_AVAILABLE; - slot->handle = client; - slot->logged_in = user; - return HAL_OK; } @@ -184,21 +205,39 @@ static hal_error_t is_logged_in(const hal_client_handle_t client, static hal_error_t logout(const hal_client_handle_t client) { - client_slot_t *slot = find_handle(client); - - if (slot != NULL) - slot->logged_in = HAL_USER_NONE; - - return HAL_OK; + return clear_slot(find_handle(client)); } static hal_error_t logout_all(void) { + /* + * This is a bit inefficient, but it lets us keep the control + * structure simple. + */ + + client_slot_t *slot; + hal_error_t err; + + do { + slot = NULL; + #if HAL_STATIC_CLIENT_STATE_BLOCKS > 0 - for (int i = 0; i < sizeof(client_handle)/sizeof(*client_handle); i++) - client_handle[i].logged_in = HAL_USER_NONE; + + hal_critical_section_start(); + + for (int i = 0; slot == NULL && i < sizeof(client_handle)/sizeof(*client_handle); i++) + if (client_handle[i].logged_in != HAL_USER_NONE) + slot = &client_handle[i]; + + hal_critical_section_end(); + #endif + if ((err = clear_slot(slot)) != HAL_OK) + return err; + + } while (slot != NULL); + return HAL_OK; } diff --git a/rpc_pkey.c b/rpc_pkey.c index bdf8a7e..0da2410 100644 --- a/rpc_pkey.c +++ b/rpc_pkey.c @@ -128,6 +128,34 @@ static inline hal_pkey_slot_t *find_handle(const hal_pkey_handle_t handle) return slot; } +/* + * Clean up key state associated with a client. + */ + +hal_error_t hal_pkey_client_cleanup(const hal_client_handle_t client) +{ + if (client.handle == HAL_HANDLE_NONE) + return HAL_OK; + + hal_error_t err; + + if ((err = hal_ks_client_cleanup(hal_ks_volatile_driver, client)) != HAL_OK) + return err; + + if ((err = hal_ks_client_cleanup(hal_ks_flash_driver, client)) != HAL_OK) + return err; + + hal_critical_section_start(); + + for (int i = 0; i < sizeof(pkey_slot)/sizeof(*pkey_slot); i++) + if (pkey_slot[i].pkey_handle.handle == client.handle) + memset(&pkey_slot[i], 0, sizeof(pkey_slot[i])); + + hal_critical_section_end(); + + return HAL_OK; +} + /* * Access rules are a bit complicated, mostly due to PKCS #11. * -- cgit v1.2.3 From 4ee44177c6da04e210a52528763b2c96a8f3d824 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Mon, 24 Apr 2017 17:23:17 -0400 Subject: Call a portable entrenching tool a portable entrenching tool. --- hal_internal.h | 16 ++++++++-------- ks_volatile.c | 6 +++--- rpc_misc.c | 2 +- rpc_pkey.c | 10 ++++------ 4 files changed, 16 insertions(+), 18 deletions(-) diff --git a/hal_internal.h b/hal_internal.h index b20bff2..f6c31fe 100644 --- a/hal_internal.h +++ b/hal_internal.h @@ -428,10 +428,10 @@ extern hal_error_t hal_mkm_flash_erase(const size_t len); #endif /* - * Clean up pkey stuff that's tied to a particular client. + * Clean up pkey stuff that's tied to a particular client on logout. */ -extern hal_error_t hal_pkey_client_cleanup(const hal_client_handle_t client); +extern hal_error_t hal_pkey_logout(const hal_client_handle_t client); /* * Keystore API for use by the pkey implementation. @@ -526,8 +526,8 @@ struct hal_ks_driver { uint8_t *attributes_buffer, const size_t attributes_buffer_len); - hal_error_t (*client_cleanup)(hal_ks_t *ks, - const hal_client_handle_t client); + hal_error_t (*logout)(hal_ks_t *ks, + const hal_client_handle_t client); }; @@ -690,16 +690,16 @@ static inline hal_error_t hal_ks_get_attributes(hal_ks_t *ks, attributes_buffer, attributes_buffer_len); } -static inline hal_error_t hal_ks_client_cleanup(hal_ks_t *ks, - const hal_client_handle_t client) +static inline hal_error_t hal_ks_logout(hal_ks_t *ks, + const hal_client_handle_t client) { if (ks == NULL) return HAL_ERROR_BAD_ARGUMENTS; - if (ks->client_cleanup == NULL || client.handle == HAL_HANDLE_NONE) + if (ks->logout == NULL || client.handle == HAL_HANDLE_NONE) return HAL_OK; - return ks->driver->client_cleanup(ks, client); + return ks->driver->logout(ks, client); } /* diff --git a/ks_volatile.c b/ks_volatile.c index 363441a..29c3576 100644 --- a/ks_volatile.c +++ b/ks_volatile.c @@ -614,8 +614,8 @@ static hal_error_t ks_get_attributes(hal_ks_t *ks, return err; } -static void ks_client_cleanup(hal_ks_t *ks, - hal_client_handle_t client) +static void ks_logout(hal_ks_t *ks, + hal_client_handle_t client) { if (ks == NULL || client.handle = HAL_HANDLE_NONE) return HAL_ERROR_BAD_ARGUMENTS; @@ -652,7 +652,7 @@ const hal_ks_driver_t hal_ks_volatile_driver[1] = {{ .match = ks_match, .set_attributes = ks_set_attributes, .get_attributes = ks_get_attributes, - .client_cleanup = ks_client_cleanup + .logout = ks_logout }}; #endif /* STATIC_KS_VOLATILE_SLOTS > 0 */ diff --git a/rpc_misc.c b/rpc_misc.c index 2fd743a..4db6ed3 100644 --- a/rpc_misc.c +++ b/rpc_misc.c @@ -133,7 +133,7 @@ static inline void clear_slot(client_slot_t *slot) if (slot == NULL) return; - hal_pkey_client_cleanup(slot->handle); + hal_pkey_logout(slot->handle); hal_critical_section_start(); diff --git a/rpc_pkey.c b/rpc_pkey.c index 0da2410..165419e 100644 --- a/rpc_pkey.c +++ b/rpc_pkey.c @@ -129,20 +129,18 @@ static inline hal_pkey_slot_t *find_handle(const hal_pkey_handle_t handle) } /* - * Clean up key state associated with a client. + * Clean up key state associated with a client when logging out. */ -hal_error_t hal_pkey_client_cleanup(const hal_client_handle_t client) +hal_error_t hal_pkey_logout(const hal_client_handle_t client) { if (client.handle == HAL_HANDLE_NONE) return HAL_OK; hal_error_t err; - if ((err = hal_ks_client_cleanup(hal_ks_volatile_driver, client)) != HAL_OK) - return err; - - if ((err = hal_ks_client_cleanup(hal_ks_flash_driver, client)) != HAL_OK) + if ((err = hal_ks_logout(hal_ks_volatile_driver, client)) != HAL_OK || + (err = hal_ks_logout(hal_ks_flash_driver, client)) != HAL_OK) return err; hal_critical_section_start(); -- cgit v1.2.3 From 358b3803cdedad607cf649221d0b7e3ce66b45f2 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Tue, 25 Apr 2017 17:14:40 -0400 Subject: Shake dumb compile-time bugs out of new logout code. What I get for writing code while build and test environment is tied up with a multi-day run testing something else. --- hal_internal.h | 2 +- ks_volatile.c | 6 +++--- rpc_misc.c | 14 ++++++++++---- rpc_pkey.c | 19 +++++++++++++++++-- 4 files changed, 31 insertions(+), 10 deletions(-) diff --git a/hal_internal.h b/hal_internal.h index f6c31fe..13c79e9 100644 --- a/hal_internal.h +++ b/hal_internal.h @@ -696,7 +696,7 @@ static inline hal_error_t hal_ks_logout(hal_ks_t *ks, if (ks == NULL) return HAL_ERROR_BAD_ARGUMENTS; - if (ks->logout == NULL || client.handle == HAL_HANDLE_NONE) + if (ks->driver->logout == NULL || client.handle == HAL_HANDLE_NONE) return HAL_OK; return ks->driver->logout(ks, client); diff --git a/ks_volatile.c b/ks_volatile.c index 29c3576..6a17e45 100644 --- a/ks_volatile.c +++ b/ks_volatile.c @@ -614,10 +614,10 @@ static hal_error_t ks_get_attributes(hal_ks_t *ks, return err; } -static void ks_logout(hal_ks_t *ks, - hal_client_handle_t client) +static hal_error_t ks_logout(hal_ks_t *ks, + hal_client_handle_t client) { - if (ks == NULL || client.handle = HAL_HANDLE_NONE) + if (ks == NULL || client.handle == HAL_HANDLE_NONE) return HAL_ERROR_BAD_ARGUMENTS; ks_t *ksv = ks_to_ksv(ks); diff --git a/rpc_misc.c b/rpc_misc.c index 4db6ed3..e9ff4c6 100644 --- a/rpc_misc.c +++ b/rpc_misc.c @@ -110,7 +110,8 @@ static inline client_slot_t *alloc_slot(const hal_client_handle_t client, #if HAL_STATIC_CLIENT_STATE_BLOCKS > 0 for (int i = 0; slot == NULL && i < sizeof(client_handle)/sizeof(*client_handle); i++) - if (client_handle[i].logged_in != HAL_USER_NONE && client_handle[i].handle.handle == handle.handle) + if (client_handle[i].logged_in != HAL_USER_NONE && + client_handle[i].handle.handle == client.handle) slot = &client_handle[i]; for (int i = 0; slot == NULL && i < sizeof(client_handle)/sizeof(*client_handle); i++) @@ -128,18 +129,23 @@ static inline client_slot_t *alloc_slot(const hal_client_handle_t client, return slot; } -static inline void clear_slot(client_slot_t *slot) +static inline hal_error_t clear_slot(client_slot_t *slot) { if (slot == NULL) - return; + return HAL_OK; + + hal_error_t err; - hal_pkey_logout(slot->handle); + if ((err = hal_pkey_logout(slot->handle)) != HAL_OK) + return err; hal_critical_section_start(); memset(slot, 0, sizeof(*slot)); hal_critical_section_end(); + + return HAL_OK; } static inline client_slot_t *find_handle(const hal_client_handle_t handle) diff --git a/rpc_pkey.c b/rpc_pkey.c index 165419e..5af6c0e 100644 --- a/rpc_pkey.c +++ b/rpc_pkey.c @@ -138,9 +138,24 @@ hal_error_t hal_pkey_logout(const hal_client_handle_t client) return HAL_OK; hal_error_t err; + hal_ks_t *ks; - if ((err = hal_ks_logout(hal_ks_volatile_driver, client)) != HAL_OK || - (err = hal_ks_logout(hal_ks_flash_driver, client)) != HAL_OK) + if ((err = hal_ks_open(hal_ks_volatile_driver, &ks)) != HAL_OK) + return err; + if ((err = hal_ks_logout(ks, client)) == HAL_OK) + err = hal_ks_close(ks); + else + (void) hal_ks_close(ks); + if (err != HAL_OK) + return err; + + if ((err = hal_ks_open(hal_ks_token_driver, &ks)) != HAL_OK) + return err; + if ((err = hal_ks_logout(ks, client)) == HAL_OK) + err = hal_ks_close(ks); + else + (void) hal_ks_close(ks); + if (err != HAL_OK) return err; hal_critical_section_start(); -- cgit v1.2.3