From 15efcdb3e2ebe20c35818447537728c9de2f089f Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Mon, 21 Nov 2016 23:36:36 -0500 Subject: Whack attribute code with a club until it works with PKCS #11. PKCS #11 supports zero-length attributes (eg, CKA_LABEL) so hack of using zero length attribute as NIL value won't work, instead we use a slightly more portable version of the hack PKCS #11 uses (PKCS #11 stuffs -1 into a CK_ULONG, we stuff 0xFFFFFFFF into a uint32_t). ks_attribute.c code was trying too hard and tripping over its own socks. Instead of trying to maintain attributes[] in place during modification, we now perform the minimum necessary change then re-scan the block. This is (very slightly) slower but more robust, both because the scan code has better error checking and because it's the scan code that we want to be sure is happy before committing a change. Rename hal_rpc_pkey_attribute_t to hal_pkey_attribute_t. --- hal.h | 27 ++++++++++++++-------- hal_internal.h | 24 +++++++++---------- ks_attribute.c | 34 ++++++++------------------- ks_flash.c | 22 +++++++++--------- ks_volatile.c | 26 ++++++++++----------- libhal.py | 13 ++++++----- rpc_api.c | 6 ++--- rpc_client.c | 11 +++++---- rpc_pkey.c | 6 ++--- rpc_server.c | 26 +++++++++++++-------- tests/test-rpc_pkey.c | 21 ++++++++--------- unit-tests.py | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++ 12 files changed, 173 insertions(+), 107 deletions(-) diff --git a/hal.h b/hal.h index ee27649..72b1d58 100644 --- a/hal.h +++ b/hal.h @@ -720,6 +720,21 @@ typedef uint32_t hal_key_flags_t; #define HAL_KEY_FLAG_TOKEN (1 << 3) #define HAL_KEY_FLAG_PUBLIC (1 << 4) +/* + * hal_pkey_attribute_t.length would be size_t, except that we also + * need it to transport HAL_PKEY_ATTRIBUTE_NIL safely, which we can + * only do with a known-width type. The RPC code conveys size_t as a + * uint32_t in any case, so we just use that here and have done. + */ + +typedef struct { + uint32_t type; + uint32_t length; + const void *value; +} hal_pkey_attribute_t; + +#define HAL_PKEY_ATTRIBUTE_NIL (0xFFFFFFFF) + extern hal_error_t hal_rpc_pkey_load(const hal_client_handle_t client, const hal_session_handle_t session, hal_pkey_handle_t *pkey, @@ -778,18 +793,12 @@ extern hal_error_t hal_rpc_pkey_verify(const hal_pkey_handle_t pkey, const uint8_t * const input, const size_t input_len, const uint8_t * const signature, const size_t signature_len); -typedef struct { - uint32_t type; - size_t length; - const void *value; -} hal_rpc_pkey_attribute_t; - extern hal_error_t hal_rpc_pkey_match(const hal_client_handle_t client, const hal_session_handle_t session, const hal_key_type_t type, const hal_curve_name_t curve, const hal_key_flags_t flags, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len, hal_uuid_t *result, unsigned *result_len, @@ -797,11 +806,11 @@ extern hal_error_t hal_rpc_pkey_match(const hal_client_handle_t client, const hal_uuid_t * const previous_uuid); extern hal_error_t hal_rpc_pkey_set_attributes(const hal_pkey_handle_t pkey, - const hal_rpc_pkey_attribute_t *const attributes, + const hal_pkey_attribute_t *const attributes, const unsigned attributes_len); extern hal_error_t hal_rpc_pkey_get_attributes(const hal_pkey_handle_t pkey, - hal_rpc_pkey_attribute_t *attributes, + hal_pkey_attribute_t *attributes, const unsigned attributes_len, uint8_t *attributes_buffer, const size_t attributes_buffer_len); diff --git a/hal_internal.h b/hal_internal.h index 88424cf..9aa360b 100644 --- a/hal_internal.h +++ b/hal_internal.h @@ -243,7 +243,7 @@ typedef struct { const hal_key_type_t type, const hal_curve_name_t curve, const hal_key_flags_t flags, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len, hal_uuid_t *result, unsigned *result_len, @@ -251,11 +251,11 @@ typedef struct { const hal_uuid_t * const previous_uuid); hal_error_t (*set_attributes)(const hal_pkey_handle_t pkey, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len); hal_error_t (*get_attributes)(const hal_pkey_handle_t pkey, - hal_rpc_pkey_attribute_t *attributes, + hal_pkey_attribute_t *attributes, const unsigned attributes_len, uint8_t *attributes_buffer, const size_t attributes_buffer_len); @@ -462,7 +462,7 @@ struct hal_ks_driver { const hal_key_type_t type, const hal_curve_name_t curve, const hal_key_flags_t flags, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len, hal_uuid_t *result, unsigned *result_len, @@ -471,12 +471,12 @@ struct hal_ks_driver { hal_error_t (*set_attributes)(hal_ks_t *ks, hal_pkey_slot_t *slot, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len); hal_error_t (*get_attributes)(hal_ks_t *ks, hal_pkey_slot_t *slot, - hal_rpc_pkey_attribute_t *attributes, + hal_pkey_attribute_t *attributes, const unsigned attributes_len, uint8_t *attributes_buffer, const size_t attributes_buffer_len); @@ -591,7 +591,7 @@ static inline hal_error_t hal_ks_match(hal_ks_t *ks, const hal_key_type_t type, const hal_curve_name_t curve, const hal_key_flags_t flags, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len, hal_uuid_t *result, unsigned *result_len, @@ -610,7 +610,7 @@ static inline hal_error_t hal_ks_match(hal_ks_t *ks, static inline hal_error_t hal_ks_set_attributes(hal_ks_t *ks, hal_pkey_slot_t *slot, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len) { if (ks == NULL || ks->driver == NULL || slot == NULL || @@ -625,7 +625,7 @@ static inline hal_error_t hal_ks_set_attributes(hal_ks_t *ks, static inline hal_error_t hal_ks_get_attributes(hal_ks_t *ks, hal_pkey_slot_t *slot, - hal_rpc_pkey_attribute_t *attributes, + hal_pkey_attribute_t *attributes, const unsigned attributes_len, uint8_t *attributes_buffer, const size_t attributes_buffer_len) @@ -780,19 +780,19 @@ extern const size_t hal_ks_attribute_header_size; extern hal_error_t hal_ks_attribute_scan(const uint8_t * const bytes, const size_t bytes_len, - hal_rpc_pkey_attribute_t *attributes, + hal_pkey_attribute_t *attributes, const unsigned attributes_len, size_t *total_len); extern hal_error_t hal_ks_attribute_delete(uint8_t *bytes, const size_t bytes_len, - hal_rpc_pkey_attribute_t *attributes, + hal_pkey_attribute_t *attributes, unsigned *attributes_len, size_t *total_len, const uint32_t type); extern hal_error_t hal_ks_attribute_insert(uint8_t *bytes, const size_t bytes_len, - hal_rpc_pkey_attribute_t *attributes, + hal_pkey_attribute_t *attributes, unsigned *attributes_len, size_t *total_len, const uint32_t type, diff --git a/ks_attribute.c b/ks_attribute.c index 2621ed7..92e450d 100644 --- a/ks_attribute.c +++ b/ks_attribute.c @@ -44,7 +44,7 @@ * issues, and doing it this way just isn't expensive enough to worry about. */ -const size_t hal_ks_attribute_header_size = 2 * sizeof(uint32_t); +const size_t hal_ks_attribute_header_size = 6; static inline hal_error_t read_header(const uint8_t * const bytes, const size_t bytes_len, uint32_t *attribute_type, size_t *attribute_len) @@ -80,7 +80,7 @@ static inline hal_error_t write_header(uint8_t *bytes, const size_t bytes_len, } hal_error_t hal_ks_attribute_scan(const uint8_t * const bytes, const size_t bytes_len, - hal_rpc_pkey_attribute_t *attributes, const unsigned attributes_len, + hal_pkey_attribute_t *attributes, const unsigned attributes_len, size_t *total_len) { if (bytes == NULL) @@ -95,6 +95,8 @@ hal_error_t hal_ks_attribute_scan(const uint8_t * const bytes, const size_t byte hal_error_t err = read_header(b, end - b, &type, &length); if (err != HAL_OK) return err; + if (b + hal_ks_attribute_header_size + length > end) + return HAL_ERROR_BAD_ATTRIBUTE_LENGTH; b += hal_ks_attribute_header_size; if (attributes != NULL) { attributes[i].type = type; @@ -102,8 +104,6 @@ hal_error_t hal_ks_attribute_scan(const uint8_t * const bytes, const size_t byte attributes[i].value = b; } b += length; - if (b > end) - return HAL_ERROR_BAD_ATTRIBUTE_LENGTH; } if (total_len != NULL) @@ -113,7 +113,7 @@ hal_error_t hal_ks_attribute_scan(const uint8_t * const bytes, const size_t byte } hal_error_t hal_ks_attribute_delete(uint8_t *bytes, const size_t bytes_len, - hal_rpc_pkey_attribute_t *attributes, unsigned *attributes_len, + hal_pkey_attribute_t *attributes, unsigned *attributes_len, size_t *total_len, const uint32_t type) { @@ -138,17 +138,11 @@ hal_error_t hal_ks_attribute_delete(uint8_t *bytes, const size_t bytes_len, bytes + delete_offset + delete_length, *total_len - delete_length - delete_offset); - *total_len -= delete_length; - - memmove(&attributes[i], &attributes[i + 1], *attributes_len - i - 1); - - --*attributes_len; - - return HAL_OK; + return hal_ks_attribute_scan(bytes, bytes_len, attributes, --*attributes_len, total_len); } hal_error_t hal_ks_attribute_insert(uint8_t *bytes, const size_t bytes_len, - hal_rpc_pkey_attribute_t *attributes, unsigned *attributes_len, + hal_pkey_attribute_t *attributes, unsigned *attributes_len, size_t *total_len, const uint32_t type, const uint8_t * const value, const size_t value_len) @@ -172,19 +166,9 @@ hal_error_t hal_ks_attribute_insert(uint8_t *bytes, const size_t bytes_len, if ((err = write_header(b, bytes_len - *total_len, type, value_len)) != HAL_OK) return err; - b += hal_ks_attribute_header_size; + memcpy(b + hal_ks_attribute_header_size, value, value_len); - memcpy(b, value, value_len); - - *total_len += hal_ks_attribute_header_size + value_len; - - attributes[*attributes_len].type = type; - attributes[*attributes_len].length = value_len; - attributes[*attributes_len].value = b; - - ++*attributes_len; - - return HAL_OK; + return hal_ks_attribute_scan(bytes, bytes_len, attributes, ++*attributes_len, total_len); } /* diff --git a/ks_flash.c b/ks_flash.c index 9b1bf7c..e983c29 100644 --- a/ks_flash.c +++ b/ks_flash.c @@ -1129,7 +1129,7 @@ static hal_error_t ks_match(hal_ks_t *ks, const hal_key_type_t type, const hal_curve_name_t curve, const hal_key_flags_t flags, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len, hal_uuid_t *result, unsigned *result_len, @@ -1187,7 +1187,7 @@ static hal_error_t ks_match(hal_ks_t *ks, return err; if (*attrs_len > 0) { - hal_rpc_pkey_attribute_t attrs[*attrs_len]; + hal_pkey_attribute_t attrs[*attrs_len]; if ((err = hal_ks_attribute_scan(bytes, bytes_len, attrs, *attrs_len, NULL)) != HAL_OK) return err; @@ -1197,7 +1197,7 @@ static hal_error_t ks_match(hal_ks_t *ks, if (!need_attr[j]) continue; - for (hal_rpc_pkey_attribute_t *a = attrs; a < attrs + *attrs_len; a++) { + for (hal_pkey_attribute_t *a = attrs; a < attrs + *attrs_len; a++) { if (a->type != attributes[j].type) continue; need_attr[j] = 0; @@ -1225,7 +1225,7 @@ static hal_error_t ks_match(hal_ks_t *ks, static hal_error_t ks_set_attributes(hal_ks_t *ks, hal_pkey_slot_t *slot, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len) { #warning This function is much too long @@ -1270,14 +1270,14 @@ static hal_error_t ks_set_attributes(hal_ks_t *ks, updated_attributes_len += *attrs_len; - hal_rpc_pkey_attribute_t attrs[*attrs_len + attributes_len]; + hal_pkey_attribute_t attrs[*attrs_len + attributes_len]; size_t total; if ((err = hal_ks_attribute_scan(bytes, bytes_len, attrs, *attrs_len, &total)) != HAL_OK) return err; for (int i = 0; err == HAL_OK && i < attributes_len; i++) { - if (attributes[i].length == 0) + if (attributes[i].length == HAL_PKEY_ATTRIBUTE_NIL) err = hal_ks_attribute_delete(bytes, bytes_len, attrs, attrs_len, &total, attributes[i].type); else @@ -1324,7 +1324,7 @@ static hal_error_t ks_set_attributes(hal_ks_t *ks, * run faster. */ - hal_rpc_pkey_attribute_t updated_attributes[updated_attributes_len]; + hal_pkey_attribute_t updated_attributes[updated_attributes_len]; const unsigned total_chunks_old = block->header.total_chunks; size_t bytes_available = 0; @@ -1349,7 +1349,7 @@ static hal_error_t ks_set_attributes(hal_ks_t *ks, if ((err = locate_attributes(block, chunk, &bytes, &bytes_len, &attrs_len)) != HAL_OK) return err; - hal_rpc_pkey_attribute_t attrs[*attrs_len]; + hal_pkey_attribute_t attrs[*attrs_len]; size_t total; if ((err = hal_ks_attribute_scan(bytes, bytes_len, attrs, *attrs_len, &total)) != HAL_OK) @@ -1431,7 +1431,7 @@ static hal_error_t ks_set_attributes(hal_ks_t *ks, */ { - hal_rpc_pkey_attribute_t old_attrs[updated_attributes_len], new_attrs[updated_attributes_len]; + hal_pkey_attribute_t old_attrs[updated_attributes_len], new_attrs[updated_attributes_len]; unsigned *old_attrs_len = NULL, *new_attrs_len = NULL; flash_block_t *old_block = NULL, *new_block = NULL; uint8_t *old_bytes = NULL, *new_bytes = NULL; @@ -1573,7 +1573,7 @@ static hal_error_t ks_set_attributes(hal_ks_t *ks, static hal_error_t ks_get_attributes(hal_ks_t *ks, hal_pkey_slot_t *slot, - hal_rpc_pkey_attribute_t *attributes, + hal_pkey_attribute_t *attributes, const unsigned attributes_len, uint8_t *attributes_buffer, const size_t attributes_buffer_len) @@ -1619,7 +1619,7 @@ static hal_error_t ks_get_attributes(hal_ks_t *ks, if (*attrs_len == 0) continue; - hal_rpc_pkey_attribute_t attrs[*attrs_len]; + hal_pkey_attribute_t attrs[*attrs_len]; if ((err = hal_ks_attribute_scan(bytes, bytes_len, attrs, *attrs_len, NULL)) != HAL_OK) return err; diff --git a/ks_volatile.c b/ks_volatile.c index 9588639..99ad68c 100644 --- a/ks_volatile.c +++ b/ks_volatile.c @@ -360,7 +360,7 @@ static hal_error_t ks_match(hal_ks_t *ks, const hal_key_type_t type, const hal_curve_name_t curve, const hal_key_flags_t flags, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len, hal_uuid_t *result, unsigned *result_len, @@ -411,16 +411,16 @@ static hal_error_t ks_match(hal_ks_t *ks, if (k->attributes_len == 0) continue; - hal_rpc_pkey_attribute_t key_attrs[k->attributes_len]; + hal_pkey_attribute_t key_attrs[k->attributes_len]; if ((err = hal_ks_attribute_scan(k->der + k->der_len, sizeof(k->der) - k->der_len, key_attrs, k->attributes_len, NULL)) != HAL_OK) return err; - for (const hal_rpc_pkey_attribute_t *required = attributes; + for (const hal_pkey_attribute_t *required = attributes; ok && required < attributes + attributes_len; required++) { - hal_rpc_pkey_attribute_t *present = key_attrs; + hal_pkey_attribute_t *present = key_attrs; while (ok && present->type != required->type) ok = ++present < key_attrs + k->attributes_len; @@ -442,7 +442,7 @@ static hal_error_t ks_match(hal_ks_t *ks, static hal_error_t ks_set_attributes(hal_ks_t *ks, hal_pkey_slot_t *slot, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len) { if (ks == NULL || slot == NULL || attributes == NULL || attributes_len == 0) @@ -463,7 +463,7 @@ static hal_error_t ks_set_attributes(hal_ks_t *ks, if (!key_visible_to_session(ksv, slot->client_handle, slot->session_handle, k)) return HAL_ERROR_KEY_NOT_FOUND; - hal_rpc_pkey_attribute_t attrs[k->attributes_len + attributes_len]; + hal_pkey_attribute_t attrs[k->attributes_len + attributes_len]; uint8_t *bytes = k->der + k->der_len; size_t bytes_len = sizeof(k->der) - k->der_len; size_t total_len; @@ -471,13 +471,13 @@ static hal_error_t ks_set_attributes(hal_ks_t *ks, if ((err = hal_ks_attribute_scan(bytes, bytes_len, attrs, k->attributes_len, &total_len)) != HAL_OK) return err; - for (const hal_rpc_pkey_attribute_t *a = attributes; a < attributes + attributes_len; a++) { - if (a->length > 0 && a->value != NULL) - err = hal_ks_attribute_insert(bytes, bytes_len, attrs, &k->attributes_len, &total_len, - a->type, a->value, a->length); - else + for (const hal_pkey_attribute_t *a = attributes; a < attributes + attributes_len; a++) { + if (a->length == HAL_PKEY_ATTRIBUTE_NIL) err = hal_ks_attribute_delete(bytes, bytes_len, attrs, &k->attributes_len, &total_len, a->type); + else + err = hal_ks_attribute_insert(bytes, bytes_len, attrs, &k->attributes_len, &total_len, + a->type, a->value, a->length); if (err != HAL_OK) return err; } @@ -487,7 +487,7 @@ static hal_error_t ks_set_attributes(hal_ks_t *ks, static hal_error_t ks_get_attributes(hal_ks_t *ks, hal_pkey_slot_t *slot, - hal_rpc_pkey_attribute_t *attributes, + hal_pkey_attribute_t *attributes, const unsigned attributes_len, uint8_t *attributes_buffer, const size_t attributes_buffer_len) @@ -511,7 +511,7 @@ static hal_error_t ks_get_attributes(hal_ks_t *ks, if (!key_visible_to_session(ksv, slot->client_handle, slot->session_handle, k)) return HAL_ERROR_KEY_NOT_FOUND; - hal_rpc_pkey_attribute_t attrs[k->attributes_len > 0 ? k->attributes_len : 1]; + hal_pkey_attribute_t attrs[k->attributes_len > 0 ? k->attributes_len : 1]; if ((err = hal_ks_attribute_scan(k->der + k->der_len, sizeof(k->der) - k->der_len, attrs, k->attributes_len, NULL)) != HAL_OK) diff --git a/libhal.py b/libhal.py index dc8d07d..369e5e1 100644 --- a/libhal.py +++ b/libhal.py @@ -231,6 +231,8 @@ HAL_KEY_FLAG_USAGE_DATAENCIPHERMENT = (1 << 2) HAL_KEY_FLAG_TOKEN = (1 << 3) HAL_KEY_FLAG_PUBLIC = (1 << 4) +HAL_PKEY_ATTRIBUTE_NIL = (0xFFFFFFFF) + class UUID(uuid.UUID): @@ -383,11 +385,10 @@ class PKey(Handle): self.hsm.pkey_set_attributes(self, attributes) def get_attributes(self, attributes): - lengths = self.hsm.pkey_get_attributes(self, attributes, 0) - attributes = (k for k, v in lengths.iteritems() if v > 0) - buffer_length = sum(lengths.itervalues()) - result = dict((a, None) for a in lengths) - result.update(self.hsm.pkey_get_attributes(self, attributes, buffer_length)) + attrs = self.hsm.pkey_get_attributes(self, attributes, 0) + attrs = dict((k, v) for k, v in attrs.iteritems() if v != HAL_PKEY_ATTRIBUTE_NIL) + result = dict((a, None) for a in attributes) + result.update(self.hsm.pkey_get_attributes(self, attrs.iterkeys(), sum(attrs.itervalues()))) return result @@ -498,7 +499,7 @@ class HSM(object): packer.pack_uint(len(arg)) for name, value in arg.iteritems(): self._pack_arg(packer, name) - self._pack_arg(packer, value) + self._pack_arg(packer, HAL_PKEY_ATTRIBUTE_NIL if value is None else value) @contextlib.contextmanager def rpc(self, code, *args, **kwargs): diff --git a/rpc_api.c b/rpc_api.c index e239008..6ffd7a0 100644 --- a/rpc_api.c +++ b/rpc_api.c @@ -339,7 +339,7 @@ hal_error_t hal_rpc_pkey_match(const hal_client_handle_t client, const hal_key_type_t type, const hal_curve_name_t curve, const hal_key_flags_t flags, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len, hal_uuid_t *result, unsigned *result_len, @@ -361,7 +361,7 @@ hal_error_t hal_rpc_pkey_match(const hal_client_handle_t client, } hal_error_t hal_rpc_pkey_set_attributes(const hal_pkey_handle_t pkey, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len) { if (attributes == NULL || attributes_len == 0) @@ -370,7 +370,7 @@ hal_error_t hal_rpc_pkey_set_attributes(const hal_pkey_handle_t pkey, } hal_error_t hal_rpc_pkey_get_attributes(const hal_pkey_handle_t pkey, - hal_rpc_pkey_attribute_t *attributes, + hal_pkey_attribute_t *attributes, const unsigned attributes_len, uint8_t *attributes_buffer, const size_t attributes_buffer_len) diff --git a/rpc_client.c b/rpc_client.c index 4da0cb9..0183947 100644 --- a/rpc_client.c +++ b/rpc_client.c @@ -777,7 +777,7 @@ static hal_error_t pkey_remote_match(const hal_client_handle_t client, const hal_key_type_t type, const hal_curve_name_t curve, const hal_key_flags_t flags, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len, hal_uuid_t *result, unsigned *result_len, @@ -831,7 +831,7 @@ static hal_error_t pkey_remote_match(const hal_client_handle_t client, } static hal_error_t pkey_remote_set_attributes(const hal_pkey_handle_t pkey, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len) { size_t outbuf_len = nargs(4 + 2 * attributes_len); @@ -850,7 +850,10 @@ static hal_error_t pkey_remote_set_attributes(const hal_pkey_handle_t pkey, check(hal_xdr_encode_int(&optr, olimit, attributes_len)); for (int i = 0; i < attributes_len; i++) { check(hal_xdr_encode_int(&optr, olimit, attributes[i].type)); - check(hal_xdr_encode_buffer(&optr, olimit, attributes[i].value, attributes[i].length)); + if (attributes[i].length == HAL_PKEY_ATTRIBUTE_NIL) + check(hal_xdr_encode_int(&optr, olimit, HAL_PKEY_ATTRIBUTE_NIL)); + else + check(hal_xdr_encode_buffer(&optr, olimit, attributes[i].value, attributes[i].length)); } check(hal_rpc_send(outbuf, optr - outbuf)); @@ -861,7 +864,7 @@ static hal_error_t pkey_remote_set_attributes(const hal_pkey_handle_t pkey, } static hal_error_t pkey_remote_get_attributes(const hal_pkey_handle_t pkey, - hal_rpc_pkey_attribute_t *attributes, + hal_pkey_attribute_t *attributes, const unsigned attributes_len, uint8_t *attributes_buffer, const size_t attributes_buffer_len) diff --git a/rpc_pkey.c b/rpc_pkey.c index 98b0ba1..e2e42c9 100644 --- a/rpc_pkey.c +++ b/rpc_pkey.c @@ -935,7 +935,7 @@ static hal_error_t pkey_local_match(const hal_client_handle_t client, const hal_key_type_t type, const hal_curve_name_t curve, const hal_key_flags_t flags, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len, hal_uuid_t *result, unsigned *result_len, @@ -967,7 +967,7 @@ static hal_error_t pkey_local_match(const hal_client_handle_t client, } static hal_error_t pkey_local_set_attributes(const hal_pkey_handle_t pkey, - const hal_rpc_pkey_attribute_t *attributes, + const hal_pkey_attribute_t *attributes, const unsigned attributes_len) { hal_pkey_slot_t *slot = find_handle(pkey); @@ -991,7 +991,7 @@ static hal_error_t pkey_local_set_attributes(const hal_pkey_handle_t pkey, } static hal_error_t pkey_local_get_attributes(const hal_pkey_handle_t pkey, - hal_rpc_pkey_attribute_t *attributes, + hal_pkey_attribute_t *attributes, const unsigned attributes_len, uint8_t *attributes_buffer, const size_t attributes_buffer_len) diff --git a/rpc_server.c b/rpc_server.c index f4f2a06..a21679a 100644 --- a/rpc_server.c +++ b/rpc_server.c @@ -657,10 +657,10 @@ static hal_error_t pkey_match(const uint8_t **iptr, const uint8_t * const ilimit check(hal_xdr_decode_int(iptr, ilimit, &flags)); check(hal_xdr_decode_int(iptr, ilimit, &attributes_len)); - hal_rpc_pkey_attribute_t attributes[attributes_len > 0 ? attributes_len : 1]; + hal_pkey_attribute_t attributes[attributes_len > 0 ? attributes_len : 1]; for (int i = 0; i < attributes_len; i++) { - hal_rpc_pkey_attribute_t *a = &attributes[i]; + hal_pkey_attribute_t *a = &attributes[i]; const uint8_t *value; uint32_t value_len; check(hal_xdr_decode_int(iptr, ilimit, &a->type)); @@ -710,16 +710,22 @@ static hal_error_t pkey_set_attributes(const uint8_t **iptr, const uint8_t * con check(hal_xdr_decode_int(iptr, ilimit, &pkey.handle)); check(hal_xdr_decode_int(iptr, ilimit, &attributes_len)); - hal_rpc_pkey_attribute_t attributes[attributes_len > 0 ? attributes_len : 1]; + hal_pkey_attribute_t attributes[attributes_len > 0 ? attributes_len : 1]; for (int i = 0; i < attributes_len; i++) { - hal_rpc_pkey_attribute_t *a = &attributes[i]; - const uint8_t *value; - uint32_t value_len; + hal_pkey_attribute_t *a = &attributes[i]; check(hal_xdr_decode_int(iptr, ilimit, &a->type)); - check(hal_xdr_decode_buffer_in_place(iptr, ilimit, &value, &value_len)); - a->value = value; - a->length = value_len; + const uint8_t *iptr_prior_to_decoding_length = *iptr; + check(hal_xdr_decode_int(iptr, ilimit, &a->length)); + if (a->length == HAL_PKEY_ATTRIBUTE_NIL) { + a->value = NULL; + } + else { + *iptr = iptr_prior_to_decoding_length; + const uint8_t *value; + check(hal_xdr_decode_buffer_in_place(iptr, ilimit, &value, &a->length)); + a->value = value; + } } ret = hal_rpc_pkey_set_attributes(pkey, attributes, attributes_len); @@ -740,7 +746,7 @@ static hal_error_t pkey_get_attributes(const uint8_t **iptr, const uint8_t * con check(hal_xdr_decode_int(iptr, ilimit, &pkey.handle)); check(hal_xdr_decode_int(iptr, ilimit, &attributes_len)); - hal_rpc_pkey_attribute_t attributes[attributes_len > 0 ? attributes_len : 1]; + hal_pkey_attribute_t attributes[attributes_len > 0 ? attributes_len : 1]; for (int i = 0; i < attributes_len; i++) check(hal_xdr_decode_int(iptr, ilimit, &attributes[i].type)); diff --git a/tests/test-rpc_pkey.c b/tests/test-rpc_pkey.c index f021827..c07a318 100644 --- a/tests/test-rpc_pkey.c +++ b/tests/test-rpc_pkey.c @@ -69,28 +69,27 @@ static int test_attributes(const hal_pkey_handle_t pkey, uint8_t buf_1[*size], buf_2[*size]; memset(buf_1, 0x55, sizeof(buf_1)); snprintf((char *) buf_1, sizeof(buf_1), format, (unsigned long) *size); - hal_rpc_pkey_attribute_t attr_1[1] = {{ *size, sizeof(buf_1), buf_1 }}; - hal_rpc_pkey_attribute_t attr_2[1] = {{ *size, 0, NULL }}; - hal_rpc_pkey_attribute_t attr_3[1] = {{ *size, 0, NULL }}; + hal_pkey_attribute_t attr_set = { .type = *size, .length = sizeof(buf_1), .value = buf_1 }; + hal_pkey_attribute_t attr_get = { .type = *size }; + hal_pkey_attribute_t attr_del = { .type = *size, .length = HAL_PKEY_ATTRIBUTE_NIL }; - if ((err = hal_rpc_pkey_set_attributes(pkey, attr_1, sizeof(attr_1)/sizeof(*attr_1))) != HAL_OK) + if ((err = hal_rpc_pkey_set_attributes(pkey, &attr_set, 1)) != HAL_OK) lose("Could not set attribute %lu: %s\n", (unsigned long) *size, hal_error_string(err)); - if ((err = hal_rpc_pkey_get_attributes(pkey, attr_2, sizeof(attr_2)/sizeof(*attr_2), - buf_2, sizeof(buf_2))) != HAL_OK) + if ((err = hal_rpc_pkey_get_attributes(pkey, &attr_get, 1, buf_2, sizeof(buf_2))) != HAL_OK) lose("Could not get attribute %lu: %s\n", (unsigned long) *size, hal_error_string(err)); - if (attr_2[0].length != *size) + if (attr_get.length != *size) lose("Unexpected size returned for attribute %lu: %lu\n", - (unsigned long) *size, (unsigned long) attr_2[0].length); + (unsigned long) *size, (unsigned long) attr_get.length); - if ((err = hal_rpc_pkey_set_attributes(pkey, attr_3, sizeof(attr_3)/sizeof(*attr_3))) != HAL_OK) + if ((err = hal_rpc_pkey_set_attributes(pkey, &attr_del, 1)) != HAL_OK) lose("Could not delete attribute %lu: %s\n", (unsigned long) *size, hal_error_string(err)); - if ((err = hal_rpc_pkey_set_attributes(pkey, attr_1, sizeof(attr_1)/sizeof(*attr_1))) != HAL_OK) + if ((err = hal_rpc_pkey_set_attributes(pkey, &attr_set, 1)) != HAL_OK) lose("Could not (re)set attribute %lu: %s\n", (unsigned long) *size, hal_error_string(err)); } @@ -113,7 +112,7 @@ static int test_attributes(const hal_pkey_handle_t pkey, uint8_t buf[*size]; memset(buf, 0x55, sizeof(buf)); snprintf((char *) buf, sizeof(buf), format, (unsigned long) *size); - hal_rpc_pkey_attribute_t attribute[1] = {{ *size, sizeof(buf), buf }}; + hal_pkey_attribute_t attribute[1] = {{ *size, sizeof(buf), buf }}; if ((err = hal_rpc_pkey_match(client, session, HAL_KEY_TYPE_NONE, HAL_CURVE_NONE, flags, attribute, sizeof(attribute)/sizeof(*attribute), diff --git a/unit-tests.py b/unit-tests.py index 8ae9c74..a8779c5 100644 --- a/unit-tests.py +++ b/unit-tests.py @@ -594,6 +594,70 @@ class TestPKeyAttribute(TestCaseLoggedIn): self.load_and_fill(HAL_KEY_FLAG_TOKEN, n_attrs = 4, n_fill = 512) # [16, 1024] +class TestPKeyAttributeP11(TestCaseLoggedIn): + """ + Attribute creation/lookup/deletion tests based on a PKCS #11 trace. + """ + + def setUp(self): + der = PreloadedKey.db[HAL_KEY_TYPE_EC_PRIVATE, HAL_CURVE_P256].der + self.k = hsm.pkey_load(HAL_KEY_TYPE_EC_PRIVATE, HAL_CURVE_P256, der, HAL_KEY_FLAG_TOKEN) + self.addCleanup(self.k.delete) + super(TestPKeyAttributeP11, self).setUp() + + def test_set_many_attributes(self): + self.k.set_attributes({ + 0x001 : "\x01", + 0x108 : "\x01", + 0x105 : "\x00", + 0x002 : "\x01", + 0x107 : "\x00", + 0x102 : "\x45\x43\x2d\x50\x32\x35\x36", + 0x003 : "\x45\x43\x2d\x50\x32\x35\x36", + 0x162 : "\x00", + 0x103 : "\x01", + 0x000 : "\x03\x00\x00\x00", + 0x100 : "\x03\x00\x00\x00", + 0x101 : "", + 0x109 : "\x00", + 0x10c : "\x00", + 0x110 : "", + 0x111 : "", + 0x163 : "\x00", + 0x166 : "\xff\xff\xff\xff", + 0x170 : "\x01", + 0x210 : "\x00", + 0x163 : "\x01", + 0x166 : "\x40\x10\x00\x00", + 0x180 : "\x06\x08\x2a\x86\x48\xce\x3d\x03\x01\x07" }) + + def test_set_many_attributes_with_deletions(self): + self.k.set_attributes({ + 0x001 : "\x01", + 0x108 : "\x01", + 0x105 : "\x00", + 0x002 : "\x01", + 0x107 : "\x00", + 0x102 : "\x45\x43\x2d\x50\x32\x35\x36", + 0x003 : "\x45\x43\x2d\x50\x32\x35\x36", + 0x162 : "\x00", + 0x103 : "\x01", + 0x000 : "\x03\x00\x00\x00", + 0x100 : "\x03\x00\x00\x00", + 0x101 : None, + 0x109 : "\x00", + 0x10c : "\x00", + 0x110 : None, + 0x111 : None, + 0x163 : "\x00", + 0x166 : "\xff\xff\xff\xff", + 0x170 : "\x01", + 0x210 : "\x00", + 0x163 : "\x01", + 0x166 : "\x40\x10\x00\x00", + 0x180 : "\x06\x08\x2a\x86\x48\xce\x3d\x03\x01\x07" }) + + class TestPKeyAttributeWriteSpeedToken(TestCaseLoggedIn): """ Attribute speed tests. -- cgit v1.2.3