From 776c4e8cfed92bc2d894f002cb7d222abc65bb50 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Mon, 29 May 2017 13:16:14 -0400 Subject: Simplify per-session keys. Cosmetic cleanup of pkey_slot along the way. --- hal_internal.h | 6 +++--- ks.c | 68 +++++++++++++--------------------------------------------- ks.h | 7 +++--- ks_volatile.c | 27 +++++++++++++++++------ rpc_pkey.c | 16 +++++++------- 5 files changed, 51 insertions(+), 73 deletions(-) diff --git a/hal_internal.h b/hal_internal.h index e998ae3..add7890 100644 --- a/hal_internal.h +++ b/hal_internal.h @@ -449,9 +449,9 @@ extern hal_error_t hal_mkm_flash_erase(const size_t len); */ typedef struct { - hal_client_handle_t client_handle; - hal_session_handle_t session_handle; - hal_pkey_handle_t pkey_handle; + hal_client_handle_t client; + hal_session_handle_t session; + hal_pkey_handle_t pkey; hal_key_type_t type; hal_curve_name_t curve; hal_key_flags_t flags; diff --git a/ks.c b/ks.c index 2b33771..e966b94 100644 --- a/ks.c +++ b/ks.c @@ -477,45 +477,6 @@ static inline int acceptable_key_type(const hal_key_type_t type) } } -/* - * Test whether the current session can see a particular key. One - * might expect this to be based on whether the session matches, and - * indeed it would be in a sane world, but in the world of PKCS #11, - * keys belong to sessions, are visible to other sessions, and may - * even be modifiable by other sessions, but softly and silently - * vanish away when the original creating session is destroyed. - * - * In our terms, this means that visibility of session objects is - * determined only by the client handle, so taking the session handle - * as an argument here isn't really necessary, but we've flipflopped - * on that enough times that at least for now I'd prefer to leave the - * session handle here and not have to revise all the RPC calls again. - * Remove it at some later date and redo the RPC calls if we manage to - * avoid revising this yet again. - */ - -static inline hal_error_t key_visible(hal_ks_t * const ks, - const hal_client_handle_t client, - const hal_session_handle_t session, - const unsigned blockno) -{ - hal_error_t err; - - if (ks == NULL) - return HAL_ERROR_IMPOSSIBLE; - - if (!ks->per_session) - return HAL_OK; - - if ((err = hal_ks_block_test_owner(ks, blockno, client, session)) != HAL_ERROR_KEY_NOT_FOUND) - return err; - - if ((err = hal_rpc_is_logged_in(client, HAL_USER_WHEEL)) != HAL_ERROR_FORBIDDEN) - return err; - - return HAL_ERROR_KEY_NOT_FOUND; -} - hal_error_t hal_ks_store(hal_ks_t *ks, hal_pkey_slot_t *slot, const uint8_t * const der, const size_t der_len) @@ -571,7 +532,7 @@ hal_error_t hal_ks_store(hal_ks_t *ks, err = hal_ks_block_write(ks, b, block); if (err == HAL_OK) - err = hal_ks_block_set_owner(ks, b, slot->client_handle, slot->session_handle); + err = hal_ks_block_set_owner(ks, b, slot->client, slot->session); if (err == HAL_OK) goto done; @@ -598,9 +559,9 @@ hal_error_t hal_ks_fetch(hal_ks_t *ks, hal_ks_lock(); - if ((err = hal_ks_index_find(ks, &slot->name, &b, &slot->hint)) != HAL_OK || - (err = key_visible(ks, slot->client_handle, slot->session_handle, b)) != HAL_OK || - (err = hal_ks_block_read_cached(ks, b, &block)) != HAL_OK) + if ((err = hal_ks_index_find(ks, &slot->name, &b, &slot->hint)) != HAL_OK || + (err = hal_ks_block_test_owner(ks, b, slot->client, slot->session)) != HAL_OK || + (err = hal_ks_block_read_cached(ks, b, &block)) != HAL_OK) goto done; if (hal_ks_block_get_type(block) != HAL_KS_BLOCK_TYPE_KEY) { @@ -652,8 +613,8 @@ hal_error_t hal_ks_delete(hal_ks_t *ks, hal_ks_lock(); - if ((err = hal_ks_index_delete(ks, &slot->name, &b, &slot->hint)) != HAL_OK || - (err = key_visible(ks, slot->client_handle, slot->session_handle, b)) != HAL_OK) + if ((err = hal_ks_index_delete(ks, &slot->name, &b, &slot->hint)) != HAL_OK || + (err = hal_ks_block_test_owner(ks, b, slot->client, slot->session)) != HAL_OK) goto done; hal_ks_cache_release(ks, hal_ks_cache_find_block(ks, b)); @@ -725,9 +686,10 @@ hal_error_t hal_ks_match(hal_ks_t *ks, if ((err = hal_ks_block_read_cached(ks, b, &block)) != HAL_OK) goto done; - if ((err = key_visible(ks, client, session, b)) == HAL_ERROR_KEY_NOT_FOUND) + if ((err = hal_ks_block_test_owner(ks, b, client, session)) == HAL_ERROR_KEY_NOT_FOUND) continue; - else if (err != HAL_OK) + + if (err != HAL_OK) goto done; if ((type != HAL_KEY_TYPE_NONE && type != block->key.type) || @@ -799,9 +761,9 @@ hal_error_t hal_ks_set_attributes(hal_ks_t *ks, hal_ks_lock(); { - if ((err = hal_ks_index_find(ks, &slot->name, &b, &slot->hint)) != HAL_OK || - (err = key_visible(ks, slot->client_handle, slot->session_handle, b)) != HAL_OK || - (err = hal_ks_block_read_cached(ks, b, &block)) != HAL_OK) + if ((err = hal_ks_index_find(ks, &slot->name, &b, &slot->hint)) != HAL_OK || + (err = hal_ks_block_test_owner(ks, b, slot->client, slot->session)) != HAL_OK || + (err = hal_ks_block_read_cached(ks, b, &block)) != HAL_OK) goto done; hal_ks_cache_mark_used(ks, block, b); @@ -865,9 +827,9 @@ hal_error_t hal_ks_get_attributes(hal_ks_t *ks, hal_ks_lock(); { - if ((err = hal_ks_index_find(ks, &slot->name, &b, &slot->hint)) != HAL_OK || - (err = key_visible(ks, slot->client_handle, slot->session_handle, b)) != HAL_OK || - (err = hal_ks_block_read_cached(ks, b, &block)) != HAL_OK) + if ((err = hal_ks_index_find(ks, &slot->name, &b, &slot->hint)) != HAL_OK || + (err = hal_ks_block_test_owner(ks, b, slot->client, slot->session)) != HAL_OK || + (err = hal_ks_block_read_cached(ks, b, &block)) != HAL_OK) goto done; hal_ks_cache_mark_used(ks, block, b); diff --git a/ks.h b/ks.h index 44581a2..240d3e6 100644 --- a/ks.h +++ b/ks.h @@ -204,7 +204,6 @@ struct hal_ks { unsigned cache_lru; /* Cache LRU counter */ unsigned cache_size; /* Size (how many blocks) in cache */ hal_ks_cache_block_t *cache; /* Cache */ - int per_session; /* Whether objects have per-session semantics (PKCS #11, sigh) */ }; /* @@ -280,7 +279,8 @@ static inline hal_error_t hal_ks_block_erase_maybe(hal_ks_t *ks, const unsigned ks->driver->erase_maybe(ks, blockno); } -static inline hal_error_t hal_ks_block_set_owner(hal_ks_t *ks, const unsigned blockno, +static inline hal_error_t hal_ks_block_set_owner(hal_ks_t *ks, + const unsigned blockno, const hal_client_handle_t client, const hal_session_handle_t session) { @@ -290,7 +290,8 @@ static inline hal_error_t hal_ks_block_set_owner(hal_ks_t *ks, const unsigned bl ks->driver->set_owner(ks, blockno, client, session); } -static inline hal_error_t hal_ks_block_test_owner(hal_ks_t *ks, const unsigned blockno, +static inline hal_error_t hal_ks_block_test_owner(hal_ks_t *ks, + const unsigned blockno, const hal_client_handle_t client, const hal_session_handle_t session) { diff --git a/ks_volatile.c b/ks_volatile.c index 0b39133..02054ff 100644 --- a/ks_volatile.c +++ b/ks_volatile.c @@ -167,6 +167,20 @@ static hal_error_t ks_volatile_set_owner(hal_ks_t *ks, /* * Test key ownership. + * + * One might expect this to be based on whether the session matches, + * and indeed it would be in a sane world, but in the world of PKCS + * #11, keys belong to sessions, are visible to other sessions, and + * may even be modifiable by other sessions, but softly and silently + * vanish away when the original creating session is destroyed. + * + * In our terms, this means that visibility of session objects is + * determined only by the client handle, so taking the session handle + * as an argument here isn't really necessary, but we've flipflopped + * on that enough times that at least for now I'd prefer to leave the + * session handle here and not have to revise all the RPC calls again. + * Remove it at some later date and redo the RPC calls if we manage to + * avoid revising this yet again. */ static hal_error_t ks_volatile_test_owner(hal_ks_t *ks, @@ -177,11 +191,14 @@ static hal_error_t ks_volatile_test_owner(hal_ks_t *ks, if (ks != hal_ks_volatile || db->keys == NULL || blockno >= ks->size) return HAL_ERROR_IMPOSSIBLE; - if (db->keys[blockno].client.handle == client.handle && - db->keys[blockno].session.handle == session.handle) + if (db->keys[blockno].client.handle == HAL_HANDLE_NONE || + db->keys[blockno].client.handle == client.handle) + return HAL_OK; + + if (hal_rpc_is_logged_in(client, HAL_USER_WHEEL) == HAL_OK) return HAL_OK; - else - return HAL_ERROR_KEY_NOT_FOUND; + + return HAL_ERROR_KEY_NOT_FOUND; } /* @@ -233,8 +250,6 @@ static hal_error_t ks_volatile_init(hal_ks_t *ks, const int alloc) if ((err = hal_ks_init_common(ks)) != HAL_OK) goto done; - ks->per_session = 1; - err = HAL_OK; done: diff --git a/rpc_pkey.c b/rpc_pkey.c index d280c54..63bc8bd 100644 --- a/rpc_pkey.c +++ b/rpc_pkey.c @@ -79,10 +79,10 @@ static inline hal_pkey_slot_t *alloc_slot(const hal_key_flags_t flags) glop |= HAL_PKEY_HANDLE_TOKEN_FLAG; for (int i = 0; slot == NULL && i < sizeof(pkey_slot)/sizeof(*pkey_slot); i++) { - if (pkey_slot[i].pkey_handle.handle != HAL_HANDLE_NONE) + if (pkey_slot[i].pkey.handle != HAL_HANDLE_NONE) continue; memset(&pkey_slot[i], 0, sizeof(pkey_slot[i])); - pkey_slot[i].pkey_handle.handle = i | glop; + pkey_slot[i].pkey.handle = i | glop; pkey_slot[i].hint = -1; slot = &pkey_slot[i]; } @@ -120,7 +120,7 @@ static inline hal_pkey_slot_t *find_handle(const hal_pkey_handle_t handle) #if HAL_STATIC_PKEY_STATE_BLOCKS > 0 const int i = (int) (handle.handle & 0xFFFF); - if (i < sizeof(pkey_slot)/sizeof(*pkey_slot) && pkey_slot[i].pkey_handle.handle == handle.handle) + if (i < sizeof(pkey_slot)/sizeof(*pkey_slot) && pkey_slot[i].pkey.handle == handle.handle) slot = &pkey_slot[i]; #endif @@ -334,7 +334,7 @@ static hal_error_t pkey_local_load(const hal_client_handle_t client, return err; } - *pkey = slot->pkey_handle; + *pkey = slot->pkey; *name = slot->name; return HAL_OK; } @@ -364,7 +364,7 @@ static hal_error_t pkey_local_open(const hal_client_handle_t client, slot->session_handle = session; if ((err = hal_ks_fetch(hal_ks_token, slot, NULL, NULL, 0)) == HAL_OK) - slot->pkey_handle.handle |= HAL_PKEY_HANDLE_TOKEN_FLAG; + slot->pkey.handle |= HAL_PKEY_HANDLE_TOKEN_FLAG; else if (err == HAL_ERROR_KEY_NOT_FOUND) err = hal_ks_fetch(hal_ks_volatile, slot, NULL, NULL, 0); @@ -372,7 +372,7 @@ static hal_error_t pkey_local_open(const hal_client_handle_t client, if (err != HAL_OK) goto fail; - *pkey = slot->pkey_handle; + *pkey = slot->pkey; return HAL_OK; fail: @@ -434,7 +434,7 @@ static hal_error_t pkey_local_generate_rsa(const hal_client_handle_t client, return err; } - *pkey = slot->pkey_handle; + *pkey = slot->pkey; *name = slot->name; return HAL_OK; } @@ -492,7 +492,7 @@ static hal_error_t pkey_local_generate_ec(const hal_client_handle_t client, return err; } - *pkey = slot->pkey_handle; + *pkey = slot->pkey; *name = slot->name; return HAL_OK; } -- cgit v1.2.3