From dcf3c671314b36285277073c0a3d3a09bf4d93e6 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Mon, 24 Oct 2016 15:15:51 -0400 Subject: Make previous_uuid an input-only argument to hal_rpc_pkey_match(). In retrospect it's obvious that this never needed to be an input/output argument, as its value will always be the same as the last value in the returned array. Doh. So simplify the RPC and call sequence slightly by removing the unnecessary output value. --- hal.h | 2 +- hal_internal.h | 6 +++--- ks_flash.c | 4 ++-- ks_volatile.c | 4 ++-- libhal.py | 7 ++----- rpc_api.c | 2 +- rpc_client.c | 17 ++++++++++++----- rpc_pkey.c | 2 +- rpc_server.c | 10 +++------- 9 files changed, 27 insertions(+), 27 deletions(-) diff --git a/hal.h b/hal.h index 21104c3..194948a 100644 --- a/hal.h +++ b/hal.h @@ -779,7 +779,7 @@ extern hal_error_t hal_rpc_pkey_match(const hal_client_handle_t client, hal_uuid_t *result, unsigned *result_len, const unsigned result_max, - hal_uuid_t *previous_uuid); + const hal_uuid_t * const previous_uuid); extern hal_error_t hal_rpc_pkey_set_attribute(const hal_pkey_handle_t pkey, const uint32_t type, diff --git a/hal_internal.h b/hal_internal.h index 3e6cf29..44deaf6 100644 --- a/hal_internal.h +++ b/hal_internal.h @@ -252,7 +252,7 @@ typedef struct { hal_uuid_t *result, unsigned *result_len, const unsigned result_max, - hal_uuid_t *previous_uuid); + const hal_uuid_t * const previous_uuid); hal_error_t (*set_attribute)(const hal_pkey_handle_t pkey, const uint32_t type, @@ -499,7 +499,7 @@ struct hal_ks_driver { hal_uuid_t *result, unsigned *result_len, const unsigned result_max, - hal_uuid_t *previous_uuid); + const hal_uuid_t * const previous_uuid); hal_error_t (*set_attribute)(hal_ks_t *ks, hal_pkey_slot_t *slot, @@ -624,7 +624,7 @@ static inline hal_error_t hal_ks_match(hal_ks_t *ks, hal_uuid_t *result, unsigned *result_len, const unsigned result_max, - hal_uuid_t *previous_uuid) + const hal_uuid_t * const previous_uuid) { if (ks == NULL || ks->driver == NULL || ks->driver->match == NULL) return HAL_ERROR_BAD_ARGUMENTS; diff --git a/ks_flash.c b/ks_flash.c index 7c96f90..10df54b 100644 --- a/ks_flash.c +++ b/ks_flash.c @@ -1156,7 +1156,7 @@ static hal_error_t ks_match(hal_ks_t *ks, hal_uuid_t *result, unsigned *result_len, const unsigned result_max, - hal_uuid_t *previous_uuid) + const hal_uuid_t * const previous_uuid) { if (ks == NULL || attributes == NULL || result == NULL || result_len == NULL || previous_uuid == NULL) @@ -1240,7 +1240,7 @@ static hal_error_t ks_match(hal_ks_t *ks, if (attributes_len > 0 && memchr(need_attr, 1, sizeof(need_attr)) != NULL) continue; - *previous_uuid = result[*result_len] = db.ksi.names[b].name; + result[*result_len] = db.ksi.names[b].name; ++*result_len; possible = 0; } diff --git a/ks_volatile.c b/ks_volatile.c index 2018adc..e88b871 100644 --- a/ks_volatile.c +++ b/ks_volatile.c @@ -380,7 +380,7 @@ static hal_error_t ks_match(hal_ks_t *ks, hal_uuid_t *result, unsigned *result_len, const unsigned result_max, - hal_uuid_t *previous_uuid) + const hal_uuid_t * const previous_uuid) { if (ks == NULL || attributes == NULL || result == NULL || result_len == NULL || previous_uuid == NULL) @@ -451,7 +451,7 @@ static hal_error_t ks_match(hal_ks_t *ks, continue; } - *previous_uuid = result[*result_len] = ksv->db->ksi.names[b].name; + result[*result_len] = ksv->db->ksi.names[b].name; ++*result_len; } diff --git a/libhal.py b/libhal.py index 911ad7c..8334f12 100644 --- a/libhal.py +++ b/libhal.py @@ -571,11 +571,8 @@ class HSM(object): previous_uuid = UUID(int = 0), length = 512, client = 0, session = 0): with self.rpc(RPC_FUNC_PKEY_MATCH, session, type, curve, flags, attributes, length, previous_uuid, client = client) as r: - x = tuple(UUID(bytes = r.unpack_bytes()) - for i in xrange(r.unpack_uint())) - y = UUID(bytes = r.unpack_bytes()) - assert len(x) == 0 or y == x[-1] - return x + return tuple(UUID(bytes = r.unpack_bytes()) + for i in xrange(r.unpack_uint())) def pkey_set_attribute(self, pkey, attr_type, attr_value = None): if attr_value is None and isinstance(attr_type, Attribute): diff --git a/rpc_api.c b/rpc_api.c index 3ff814a..28bc73e 100644 --- a/rpc_api.c +++ b/rpc_api.c @@ -348,7 +348,7 @@ hal_error_t hal_rpc_pkey_match(const hal_client_handle_t client, hal_uuid_t *result, unsigned *result_len, const unsigned result_max, - hal_uuid_t *previous_uuid) + const hal_uuid_t * const previous_uuid) { if ((attributes == NULL && attributes_len > 0) || previous_uuid == NULL || result == NULL || result_len == NULL || result_max == 0) diff --git a/rpc_client.c b/rpc_client.c index e7e5567..959e26a 100644 --- a/rpc_client.c +++ b/rpc_client.c @@ -101,6 +101,16 @@ static hal_error_t read_matching_packet(const rpc_func_num_t expected_func, /* * RPC calls. + * + * In reading these, it helps to know that every call takes a minimum + * of two arguments (function code and client handle, even if the + * latter is just a dummy), and that every call returns a minimum of + * three values (function code, client handle, and return status). + * This may seem a bit redundant, but There Are Reasons: + * read_matching_packet() wants to make sure the result we're getting + * is from the function we thought we called, and having the client + * handle always present in a known place vastly simplifies the task + * of the client-side MUX daemon. */ static hal_error_t get_version(uint32_t *version) @@ -800,7 +810,7 @@ static hal_error_t pkey_remote_match(const hal_client_handle_t client, hal_uuid_t *result, unsigned *result_len, const unsigned result_max, - hal_uuid_t *previous_uuid) + const hal_uuid_t * const previous_uuid) { size_t attributes_buffer_len = 0; if (attributes != NULL) @@ -809,7 +819,7 @@ static hal_error_t pkey_remote_match(const hal_client_handle_t client, uint8_t outbuf[nargs(9 + attributes_len * 2) + pad(attributes_buffer_len) + pad(sizeof(hal_uuid_t))]; uint8_t *optr = outbuf, *olimit = outbuf + sizeof(outbuf); - uint8_t inbuf[nargs(5) + pad(result_max * sizeof(hal_uuid_t)) + pad(sizeof(hal_uuid_t))]; + uint8_t inbuf[nargs(4) + pad(result_max * sizeof(hal_uuid_t))]; const uint8_t *iptr = inbuf, *ilimit = inbuf + sizeof(inbuf); hal_error_t rpc_ret; @@ -842,9 +852,6 @@ static hal_error_t pkey_remote_match(const hal_client_handle_t client, if (uuid_len != sizeof(result[i].uuid)) return HAL_ERROR_KEY_NAME_TOO_LONG; } - check(hal_xdr_decode_buffer(&iptr, ilimit, previous_uuid->uuid, &uuid_len)); - if (uuid_len != sizeof(previous_uuid->uuid)) - return HAL_ERROR_KEY_NAME_TOO_LONG; *result_len = array_len; } return rpc_ret; diff --git a/rpc_pkey.c b/rpc_pkey.c index 0a53221..053c8b4 100644 --- a/rpc_pkey.c +++ b/rpc_pkey.c @@ -878,7 +878,7 @@ static hal_error_t pkey_local_match(const hal_client_handle_t client, hal_uuid_t *result, unsigned *result_len, const unsigned result_max, - hal_uuid_t *previous_uuid) + const hal_uuid_t * const previous_uuid) { hal_ks_t *ks = NULL; hal_error_t err; diff --git a/rpc_server.c b/rpc_server.c index 18f6823..6ed4959 100644 --- a/rpc_server.c +++ b/rpc_server.c @@ -672,7 +672,6 @@ static hal_error_t pkey_match(const uint8_t **iptr, const uint8_t * const ilimit hal_session_handle_t session; uint32_t type, curve, attributes_len, result_max, previous_uuid_len; const uint8_t *previous_uuid_ptr; - hal_uuid_t previous_uuid; hal_key_flags_t flags; hal_error_t ret; @@ -696,10 +695,10 @@ static hal_error_t pkey_match(const uint8_t **iptr, const uint8_t * const ilimit check(hal_xdr_decode_int(iptr, ilimit, &result_max)); check(hal_xdr_decode_buffer_in_place(iptr, ilimit, &previous_uuid_ptr, &previous_uuid_len)); - if (previous_uuid_len != sizeof(previous_uuid.uuid)) + if (previous_uuid_len != sizeof(hal_uuid_t)) return HAL_ERROR_KEY_NAME_TOO_LONG; - memcpy(previous_uuid.uuid, previous_uuid_ptr, sizeof(previous_uuid.uuid)); + const hal_uuid_t * const previous_uuid = (const void *) previous_uuid_ptr; hal_uuid_t result[result_max]; unsigned result_len; @@ -707,7 +706,7 @@ static hal_error_t pkey_match(const uint8_t **iptr, const uint8_t * const ilimit ret = hal_rpc_local_pkey_dispatch.match(client, session, type, curve, flags, attributes, attributes_len, result, &result_len, result_max, - &previous_uuid); + previous_uuid); if (ret == HAL_OK) { uint8_t *optr_orig = *optr; @@ -715,9 +714,6 @@ static hal_error_t pkey_match(const uint8_t **iptr, const uint8_t * const ilimit for (int i = 0; ret == HAL_OK && i < result_len; ++i) ret = hal_xdr_encode_buffer(optr, olimit, result[i].uuid, sizeof(result[i].uuid)); - if (ret == HAL_OK) - ret = hal_xdr_encode_buffer(optr, olimit, previous_uuid.uuid, - sizeof(previous_uuid.uuid)); if (ret != HAL_OK) *optr = optr_orig; } -- cgit v1.2.3