From 9ec81408c25207149d8b3bfcfc80faf1c8ff0811 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Tue, 31 Jan 2017 20:12:37 -0500 Subject: Enable low-level debugging support in libhal.py. --- libhal.py | 4 ++-- unit-tests.py | 8 ++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/libhal.py b/libhal.py index 93746e1..7e8cc6f 100644 --- a/libhal.py +++ b/libhal.py @@ -417,7 +417,7 @@ class HSM(object): def _send(self, msg): # Expects an xdrlib.Packer msg = slip_encode(msg.get_buffer()) - #logger.debug("send: %s", ":".join("{:02x}".format(ord(c)) for c in msg)) + logger.debug("send: %s", ":".join("{:02x}".format(ord(c)) for c in msg)) self.socket.sendall(msg) def _recv(self, code): # Returns an xdrlib.Unpacker @@ -428,7 +428,7 @@ class HSM(object): if msg[-1] == "": raise HAL_ERROR_RPC_TRANSPORT() msg.append(self.sockfile.read(1)) - #logger.debug("recv: %s", ":".join("{:02x}".format(ord(c)) for c in msg)) + logger.debug("recv: %s", ":".join("{:02x}".format(ord(c)) for c in msg)) msg = slip_decode("".join(msg)) if not msg: continue diff --git a/unit-tests.py b/unit-tests.py index a8779c5..895bdb5 100644 --- a/unit-tests.py +++ b/unit-tests.py @@ -66,23 +66,19 @@ except ImportError: ecdsa_loaded = False -def log(msg): - if not args.quiet: - sys.stderr.write(msg) - sys.stderr.write("\n") - - def main(): from sys import argv global args args = parse_arguments(argv[1:]) argv = argv[:1] + args.only_test + logging.basicConfig(level = logging.DEBUG if args.debug else logging.INFO) unittest.main(verbosity = 1 if args.quiet else 2, argv = argv, catchbreak = True, testRunner = TextTestRunner) def parse_arguments(argv = ()): from argparse import ArgumentParser, ArgumentDefaultsHelpFormatter parser = ArgumentParser(description = __doc__, formatter_class = ArgumentDefaultsHelpFormatter) parser.add_argument("--quiet", action = "store_true", help = "suppress chatter") + parser.add_argument("--debug", action = "store_true", help = "debug-level logging") parser.add_argument("--wheel-pin", default = "fnord", help = "PIN for wheel user") parser.add_argument("--so-pin", default = "fnord", help = "PIN for security officer") parser.add_argument("--user-pin", default = "fnord", help = "PIN for normal user") -- cgit v1.2.3 From 6a6cc04dda8f613134ae5b30b702de3f1a4dff95 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Thu, 2 Feb 2017 14:03:18 -0500 Subject: Add locking around keystore operations. --- Makefile | 2 +- core.c | 10 - hal_internal.h | 9 + ks_flash.c | 886 ++++++++++++++++++++++++++++++++------------------------- ks_volatile.c | 240 ++++++++++------ locks.c | 108 +++++++ rpc_misc.c | 20 +- rpc_pkey.c | 18 +- 8 files changed, 797 insertions(+), 496 deletions(-) create mode 100644 locks.c diff --git a/Makefile b/Makefile index 1e7316a..6c01ab4 100644 --- a/Makefile +++ b/Makefile @@ -75,7 +75,7 @@ endif # just "building this is harmless even if we don't use it." OBJ += errorstrings.o hash.o asn1.o ecdsa.o rsa.o xdr.o slip.o -OBJ += rpc_api.o rpc_hash.o uuid.o rpc_pkcs1.o crc32.o +OBJ += rpc_api.o rpc_hash.o uuid.o rpc_pkcs1.o crc32.o locks.o # Object files to build when we're on a platform with direct access # to our hardware (Verilog) cores. diff --git a/core.c b/core.c index a17422a..cbc3bc2 100644 --- a/core.c +++ b/core.c @@ -201,16 +201,6 @@ hal_core_t *hal_core_find(const char *name, hal_core_t *core) return NULL; } -__attribute__((weak)) void hal_critical_section_start(void) -{ - return; -} - -__attribute__((weak)) void hal_critical_section_end(void) -{ - return; -} - hal_error_t hal_core_alloc(const char *name, hal_core_t **pcore) { hal_core_t *core; diff --git a/hal_internal.h b/hal_internal.h index 69d9e67..40a600c 100644 --- a/hal_internal.h +++ b/hal_internal.h @@ -89,6 +89,15 @@ extern void *hal_allocate_static_memory(const size_t size); #define HAL_MAX_HASH_BLOCK_LENGTH SHA512_BLOCK_LEN #define HAL_MAX_HASH_DIGEST_LENGTH SHA512_DIGEST_LEN +/* + * Locks and critical sections. + */ + +extern void hal_critical_section_start(void); +extern void hal_critical_section_end(void); +extern void hal_ks_lock(void); +extern void hal_ks_unlock(void); + /* * Dispatch structures for RPC implementation. * diff --git a/ks_flash.c b/ks_flash.c index 82bc59a..c21d668 100644 --- a/ks_flash.c +++ b/ks_flash.c @@ -565,6 +565,10 @@ static inline void *gnaw(uint8_t **mem, size_t *len, const size_t size) static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc) { + hal_error_t err = HAL_OK; + + hal_ks_lock(); + /* * Initialize the in-memory database. */ @@ -577,8 +581,10 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc uint8_t *mem = hal_allocate_static_memory(len); - if (mem == NULL) - return HAL_ERROR_ALLOCATION_FAILURE; + if (mem == NULL) { + err = HAL_ERROR_ALLOCATION_FAILURE; + goto done; + } memset(&db, 0, sizeof(db)); memset(mem, 0, len); @@ -597,8 +603,10 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc db.ksi.used = 0; - if (db.ksi.index == NULL || db.ksi.names == NULL || db.cache == NULL) - return HAL_ERROR_IMPOSSIBLE; + if (db.ksi.index == NULL || db.ksi.names == NULL || db.cache == NULL) { + err = HAL_ERROR_IMPOSSIBLE; + goto done; + } for (int i = 0; i < KS_FLASH_CACHE_SIZE; i++) db.cache[i].blockno = ~0; @@ -613,11 +621,12 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc flash_block_status_t block_status[NUM_FLASH_BLOCKS]; flash_block_t *block = cache_pick_lru(); int first_erased = -1; - hal_error_t err; uint16_t n = 0; - if (block == NULL) - return HAL_ERROR_IMPOSSIBLE; + if (block == NULL) { + err = HAL_ERROR_IMPOSSIBLE; + goto done; + } for (int i = 0; i < NUM_FLASH_BLOCKS; i++) { @@ -637,7 +646,7 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc block_types[i] = block_get_type(block); else - return err; + goto done; switch (block_types[i]) { case BLOCK_TYPE_KEY: @@ -718,7 +727,7 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc */ if ((err = hal_ks_index_setup(&db.ksi)) != HAL_OK) - return err; + goto done; /* * We might want to call hal_ks_index_fsck() here, if we can figure @@ -764,7 +773,7 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc int where = -1; if ((err = hal_ks_index_find_range(&db.ksi, &name, 0, &n_blocks, NULL, &where, 0)) != HAL_OK) - return err; + goto done; while (where > 0 && !hal_uuid_cmp(&name, &db.ksi.names[db.ksi.index[where - 1]].name)) { where--; @@ -778,9 +787,9 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc for (int j = 0; j < n_blocks; j++) { unsigned b = db.ksi.index[where + j]; switch (block_status[b]) { - case BLOCK_STATUS_LIVE: n_live++; break; - case BLOCK_STATUS_TOMBSTONE: n_tomb++; break; - default: return HAL_ERROR_IMPOSSIBLE; + case BLOCK_STATUS_LIVE: n_live++; break; + case BLOCK_STATUS_TOMBSTONE: n_tomb++; break; + default: err = HAL_ERROR_IMPOSSIBLE; goto done; } } @@ -790,7 +799,7 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc unsigned b = db.ksi.index[where + j]; if ((err = block_read(b, block)) != HAL_OK) - return err; + goto done; join_ok &= block->header.this_chunk == j && block->header.total_chunks == n_blocks; @@ -804,18 +813,21 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc tomb_ok &= block->header.this_chunk == i_tomb++ && block->header.total_chunks == n_tomb; break; default: - return HAL_ERROR_IMPOSSIBLE; + err = HAL_ERROR_IMPOSSIBLE; + goto done; } } - if (!live_ok && !tomb_ok && !join_ok) - return HAL_ERROR_KEYSTORE_LOST_DATA; + if (!live_ok && !tomb_ok && !join_ok) { + err = HAL_ERROR_KEYSTORE_LOST_DATA; + goto done; + } if (live_ok) { for (int j = 0; j < n_tomb; j++) { const unsigned b = tomb_blocks[j]; if ((err = block_zero(b)) != HAL_OK) - return err; + goto done; block_types[b] = BLOCK_TYPE_ZEROED; block_status[b] = BLOCK_STATUS_UNKNOWN; } @@ -825,7 +837,7 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc for (int j = 0; j < n_live; j++) { const unsigned b = live_blocks[j]; if ((err = block_zero(b)) != HAL_OK) - return err; + goto done; block_types[b] = BLOCK_TYPE_ZEROED; block_status[b] = BLOCK_STATUS_UNKNOWN; } @@ -855,11 +867,11 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc if (block_status[b1] != BLOCK_STATUS_TOMBSTONE) continue; if ((err = block_read(b1, block)) != HAL_OK) - return err; + goto done; block->header.block_status = BLOCK_STATUS_LIVE; if ((err = hal_ks_index_replace(&db.ksi, &name, j, &b2, &hint)) != HAL_OK || (err = block_write(b2, block)) != HAL_OK) - return err; + goto done; block_types[b1] = BLOCK_TYPE_ZEROED; block_status[b1] = BLOCK_STATUS_UNKNOWN; block_status[b2] = BLOCK_STATUS_LIVE; @@ -875,7 +887,7 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc } else if (err != HAL_ERROR_KEY_NOT_FOUND) - return err; + goto done; else { /* @@ -900,7 +912,7 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc block->pin.user_pin = db.user_pin; if ((err = hal_ks_index_add(&db.ksi, &pin_uuid, 0, &b, NULL)) != HAL_OK) - return err; + goto done; cache_mark_used(block, b); @@ -909,7 +921,7 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc cache_release(block); if (err != HAL_OK) - return err; + goto done; } /* @@ -918,7 +930,7 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc if (db.ksi.used < db.ksi.size && (err = block_erase_maybe(db.ksi.index[db.ksi.used])) != HAL_OK) - return err; + goto done; /* * And we're finally done. @@ -926,7 +938,11 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc db.ks.driver = driver; - return HAL_OK; + err = HAL_OK; + + done: + hal_ks_unlock(); + return err; } static hal_error_t ks_shutdown(const hal_ks_driver_t * const driver) @@ -974,18 +990,24 @@ static hal_error_t ks_store(hal_ks_t *ks, if (ks != &db.ks || slot == NULL || der == NULL || der_len == 0 || !acceptable_key_type(slot->type)) return HAL_ERROR_BAD_ARGUMENTS; - flash_block_t *block = cache_pick_lru(); - flash_key_block_t *k = &block->key; + hal_error_t err = HAL_OK; + flash_block_t *block; + flash_key_block_t *k; uint8_t kek[KEK_LENGTH]; size_t kek_len; - hal_error_t err; unsigned b; - if (block == NULL) - return HAL_ERROR_IMPOSSIBLE; + hal_ks_lock(); + + if ((block = cache_pick_lru()) == NULL) { + err = HAL_ERROR_IMPOSSIBLE; + goto done; + } + + k = &block->key; if ((err = hal_ks_index_add(&db.ksi, &slot->name, 0, &b, &slot->hint)) != HAL_OK) - return err; + goto done; cache_mark_used(block, b); @@ -1008,13 +1030,18 @@ static hal_error_t ks_store(hal_ks_t *ks, memset(kek, 0, sizeof(kek)); - if (err == HAL_OK && - (err = block_write(b, block)) == HAL_OK) - return HAL_OK; + if (err == HAL_OK) + err = block_write(b, block); + + if (err == HAL_OK) + goto done; memset(block, 0, sizeof(*block)); cache_release(block); (void) hal_ks_index_delete(&db.ksi, &slot->name, 0, NULL, &slot->hint); + + done: + hal_ks_unlock(); return err; } @@ -1025,16 +1052,20 @@ static hal_error_t ks_fetch(hal_ks_t *ks, if (ks != &db.ks || slot == NULL) return HAL_ERROR_BAD_ARGUMENTS; + hal_error_t err = HAL_OK; flash_block_t *block; - hal_error_t err; unsigned b; + hal_ks_lock(); + if ((err = hal_ks_index_find(&db.ksi, &slot->name, 0, &b, &slot->hint)) != HAL_OK || (err = block_read_cached(b, &block)) != HAL_OK) - return err; + goto done; - if (block_get_type(block) != BLOCK_TYPE_KEY) - return HAL_ERROR_KEYSTORE_WRONG_BLOCK_TYPE; /* HAL_ERROR_KEY_NOT_FOUND */ + if (block_get_type(block) != BLOCK_TYPE_KEY) { + err = HAL_ERROR_KEYSTORE_WRONG_BLOCK_TYPE; /* HAL_ERROR_KEY_NOT_FOUND */ + goto done; + } cache_mark_used(block, b); @@ -1062,12 +1093,11 @@ static hal_error_t ks_fetch(hal_ks_t *ks, err = hal_aes_keyunwrap(NULL, kek, kek_len, k->der, k->der_len, der, der_len); memset(kek, 0, sizeof(kek)); - - if (err != HAL_OK) - return err; } - return HAL_OK; + done: + hal_ks_unlock(); + return err; } static hal_error_t ks_delete(hal_ks_t *ks, @@ -1076,25 +1106,33 @@ static hal_error_t ks_delete(hal_ks_t *ks, if (ks != &db.ks || slot == NULL) return HAL_ERROR_BAD_ARGUMENTS; - hal_error_t err; + hal_error_t err = HAL_OK; unsigned n; - if ((err = hal_ks_index_delete_range(&db.ksi, &slot->name, 0, &n, NULL, &slot->hint)) != HAL_OK) - return err; + hal_ks_lock(); - unsigned b[n]; + { + if ((err = hal_ks_index_delete_range(&db.ksi, &slot->name, 0, &n, NULL, &slot->hint)) != HAL_OK) + goto done; - if ((err = hal_ks_index_delete_range(&db.ksi, &slot->name, n, NULL, b, &slot->hint)) != HAL_OK) - return err; + unsigned b[n]; - for (int i = 0; i < n; i++) - cache_release(cache_find_block(b[i])); + if ((err = hal_ks_index_delete_range(&db.ksi, &slot->name, n, NULL, b, &slot->hint)) != HAL_OK) + goto done; - for (int i = 0; i < n; i++) - if ((err = block_zero(b[i])) != HAL_OK) - return err; + for (int i = 0; i < n; i++) + cache_release(cache_find_block(b[i])); - return block_erase_maybe(db.ksi.index[db.ksi.used]); + for (int i = 0; i < n; i++) + if ((err = block_zero(b[i])) != HAL_OK) + goto done; + + err = block_erase_maybe(db.ksi.index[db.ksi.used]); + } + + done: + hal_ks_unlock(); + return err; } static inline hal_error_t locate_attributes(flash_block_t *block, const unsigned chunk, @@ -1141,11 +1179,13 @@ static hal_error_t ks_match(hal_ks_t *ks, return HAL_ERROR_BAD_ARGUMENTS; uint8_t need_attr[attributes_len > 0 ? attributes_len : 1]; + hal_error_t err = HAL_OK; flash_block_t *block; int possible = 0; - hal_error_t err; int i = -1; + hal_ks_lock(); + *result_len = 0; err = hal_ks_index_find(&db.ksi, previous_uuid, 0, NULL, &i); @@ -1153,7 +1193,7 @@ static hal_error_t ks_match(hal_ks_t *ks, if (err == HAL_ERROR_KEY_NOT_FOUND) i--; else if (err != HAL_OK) - return err; + goto done; while (*result_len < result_max && ++i < db.ksi.used) { @@ -1166,7 +1206,7 @@ static hal_error_t ks_match(hal_ks_t *ks, continue; if ((err = block_read_cached(b, &block)) != HAL_OK) - return err; + goto done; if (db.ksi.names[b].chunk == 0) { memset(need_attr, 1, sizeof(need_attr)); @@ -1184,13 +1224,13 @@ static hal_error_t ks_match(hal_ks_t *ks, if ((err = locate_attributes(block, db.ksi.names[b].chunk, &bytes, &bytes_len, &attrs_len)) != HAL_OK) - return err; + goto done; if (*attrs_len > 0) { hal_pkey_attribute_t attrs[*attrs_len]; if ((err = hal_ks_attribute_scan(bytes, bytes_len, attrs, *attrs_len, NULL)) != HAL_OK) - return err; + goto done; for (int j = 0; possible && j < attributes_len; j++) { @@ -1220,7 +1260,11 @@ static hal_error_t ks_match(hal_ks_t *ks, possible = 0; } - return HAL_OK; + err = HAL_OK; + + done: + hal_ks_unlock(); + return err; } /* @@ -1259,419 +1303,451 @@ static hal_error_t ks_set_attributes(hal_ks_t *ks, */ unsigned updated_attributes_len = attributes_len; + hal_error_t err = HAL_OK; flash_block_t *block; unsigned chunk = 0; - hal_error_t err; unsigned b; - do { - int hint = slot->hint + chunk; + hal_ks_lock(); - if ((err = hal_ks_index_find(&db.ksi, &slot->name, chunk, &b, &hint)) != HAL_OK || - (err = block_read_cached(b, &block)) != HAL_OK) - return err; + { - if (block->header.this_chunk != chunk) - return HAL_ERROR_IMPOSSIBLE; + do { + int hint = slot->hint + chunk; - cache_mark_used(block, b); + if ((err = hal_ks_index_find(&db.ksi, &slot->name, chunk, &b, &hint)) != HAL_OK || + (err = block_read_cached(b, &block)) != HAL_OK) + goto done; - if (chunk == 0) - slot->hint = hint; + if (block->header.this_chunk != chunk) { + err = HAL_ERROR_IMPOSSIBLE; + goto done; + } - uint8_t *bytes = NULL; - size_t bytes_len = 0; - unsigned *attrs_len; + cache_mark_used(block, b); - if ((err = locate_attributes(block, chunk, &bytes, &bytes_len, &attrs_len)) != HAL_OK) - return err; + if (chunk == 0) + slot->hint = hint; + + uint8_t *bytes = NULL; + size_t bytes_len = 0; + unsigned *attrs_len; + + if ((err = locate_attributes(block, chunk, &bytes, &bytes_len, &attrs_len)) != HAL_OK) + goto done; - updated_attributes_len += *attrs_len; + updated_attributes_len += *attrs_len; #if KS_SET_ATTRIBUTES_SINGLE_BLOCK_UPDATE_FAST_PATH - hal_pkey_attribute_t attrs[*attrs_len + attributes_len]; - size_t total; + 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; + if ((err = hal_ks_attribute_scan(bytes, bytes_len, attrs, *attrs_len, &total)) != HAL_OK) + goto done; - for (int i = 0; err == HAL_OK && i < attributes_len; i++) - if (attributes[i].length == HAL_PKEY_ATTRIBUTE_NIL) - err = hal_ks_attribute_delete(bytes, bytes_len, attrs, attrs_len, &total, - attributes[i].type); - else - err = hal_ks_attribute_insert(bytes, bytes_len, attrs, attrs_len, &total, - attributes[i].type, - attributes[i].value, - attributes[i].length); + for (int i = 0; err == HAL_OK && i < attributes_len; i++) + if (attributes[i].length == HAL_PKEY_ATTRIBUTE_NIL) + err = hal_ks_attribute_delete(bytes, bytes_len, attrs, attrs_len, &total, + attributes[i].type); + else + err = hal_ks_attribute_insert(bytes, bytes_len, attrs, attrs_len, &total, + attributes[i].type, + attributes[i].value, + attributes[i].length); - if (err != HAL_OK) - cache_release(block); + if (err != HAL_OK) + cache_release(block); - if (err == HAL_ERROR_RESULT_TOO_LONG) - continue; + if (err == HAL_ERROR_RESULT_TOO_LONG) + continue; - if (err != HAL_OK) - return err; + if (err == HAL_OK) + err = block_update(b, block, &slot->name, chunk, &hint); - return block_update(b, block, &slot->name, chunk, &hint); + goto done; #endif /* KS_SET_ATTRIBUTES_SINGLE_BLOCK_UPDATE_FAST_PATH */ - } while (++chunk < block->header.total_chunks); + } while (++chunk < block->header.total_chunks); - /* - * If we get here, we're on the slow path, which requires rewriting - * all the chunks in this object but which can also add or remove - * chunks from this object. We need to keep track of all the old - * chunks so we can zero them at the end, and because we can't zero - * them until we've written out the new chunks, we need enough free - * blocks to hold all the new chunks. - * - * Calculating all of this is extremely tedious, but flash writes - * are so much more expensive than anything else we do here that - * it's almost certainly worth it. - * - * We don't need the attribute values to compute the sizes, just the - * attribute sizes, so we scan all the existing blocks, build up a - * structure with the current attribute types and sizes, modify that - * according to our arguments, and compute the needed size. Once we - * have that, we can start rewriting existing blocks. We put all - * the new stuff at the end, which simplifies this slightly. - * - * In theory, this process never requires us to have more than two - * blocks in memory at the same time (source and destination when - * copying across chunk boundaries), but having enough cache buffers - * to keep the whole set in memory will almost certainly make this - * run faster. - */ + /* + * If we get here, we're on the slow path, which requires rewriting + * all the chunks in this object but which can also add or remove + * chunks from this object. We need to keep track of all the old + * chunks so we can zero them at the end, and because we can't zero + * them until we've written out the new chunks, we need enough free + * blocks to hold all the new chunks. + * + * Calculating all of this is extremely tedious, but flash writes + * are so much more expensive than anything else we do here that + * it's almost certainly worth it. + * + * We don't need the attribute values to compute the sizes, just the + * attribute sizes, so we scan all the existing blocks, build up a + * structure with the current attribute types and sizes, modify that + * according to our arguments, and compute the needed size. Once we + * have that, we can start rewriting existing blocks. We put all + * the new stuff at the end, which simplifies this slightly. + * + * In theory, this process never requires us to have more than two + * blocks in memory at the same time (source and destination when + * copying across chunk boundaries), but having enough cache buffers + * to keep the whole set in memory will almost certainly make this + * run faster. + */ - hal_pkey_attribute_t updated_attributes[updated_attributes_len]; - const unsigned total_chunks_old = block->header.total_chunks; - size_t bytes_available = 0; + hal_pkey_attribute_t updated_attributes[updated_attributes_len]; + const unsigned total_chunks_old = block->header.total_chunks; + size_t bytes_available = 0; - updated_attributes_len = 0; + updated_attributes_len = 0; - /* - * Phase 0.1: Walk the old chunks to populate updated_attributes[]. - * This also initializes bytes_available, since we can only get that - * by reading old chunk zero. - */ + /* + * Phase 0.1: Walk the old chunks to populate updated_attributes[]. + * This also initializes bytes_available, since we can only get that + * by reading old chunk zero. + */ - for (chunk = 0; chunk < total_chunks_old; chunk++) { - int hint = slot->hint + chunk; + for (chunk = 0; chunk < total_chunks_old; chunk++) { + int hint = slot->hint + chunk; - if ((err = hal_ks_index_find(&db.ksi, &slot->name, chunk, &b, &hint)) != HAL_OK || - (err = block_read_cached(b, &block)) != HAL_OK) - return err; + if ((err = hal_ks_index_find(&db.ksi, &slot->name, chunk, &b, &hint)) != HAL_OK || + (err = block_read_cached(b, &block)) != HAL_OK) + goto done; - if (block->header.this_chunk != chunk) - return HAL_ERROR_IMPOSSIBLE; + if (block->header.this_chunk != chunk) { + err = HAL_ERROR_IMPOSSIBLE; + goto done; + } - cache_mark_used(block, b); + cache_mark_used(block, b); - uint8_t *bytes = NULL; - size_t bytes_len = 0; - unsigned *attrs_len; + uint8_t *bytes = NULL; + size_t bytes_len = 0; + unsigned *attrs_len; - if ((err = locate_attributes(block, chunk, &bytes, &bytes_len, &attrs_len)) != HAL_OK) - return err; + if ((err = locate_attributes(block, chunk, &bytes, &bytes_len, &attrs_len)) != HAL_OK) + goto done; - hal_pkey_attribute_t attrs[*attrs_len]; - size_t total; + 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) - return err; + if ((err = hal_ks_attribute_scan(bytes, bytes_len, attrs, *attrs_len, &total)) != HAL_OK) + goto done; - if (chunk == 0) - bytes_available = bytes_len; + if (chunk == 0) + bytes_available = bytes_len; - for (int i = 0; i < *attrs_len; i++) { + for (int i = 0; i < *attrs_len; i++) { - if (updated_attributes_len >= sizeof(updated_attributes)/sizeof(*updated_attributes)) - return HAL_ERROR_IMPOSSIBLE; + if (updated_attributes_len >= sizeof(updated_attributes)/sizeof(*updated_attributes)) { + err = HAL_ERROR_IMPOSSIBLE; + goto done; + } - updated_attributes[updated_attributes_len].type = attrs[i].type; - updated_attributes[updated_attributes_len].length = attrs[i].length; - updated_attributes[updated_attributes_len].value = NULL; - updated_attributes_len++; + updated_attributes[updated_attributes_len].type = attrs[i].type; + updated_attributes[updated_attributes_len].length = attrs[i].length; + updated_attributes[updated_attributes_len].value = NULL; + updated_attributes_len++; + } } - } - /* - * Phase 0.2: Merge new attributes into updated_attributes[]. - */ + /* + * Phase 0.2: Merge new attributes into updated_attributes[]. + */ - for (int i = 0; i < attributes_len; i++) { + for (int i = 0; i < attributes_len; i++) { - for (int j = 0; j < updated_attributes_len; j++) - if (updated_attributes[j].type == attributes[i].type) - updated_attributes[j].length = HAL_PKEY_ATTRIBUTE_NIL; + for (int j = 0; j < updated_attributes_len; j++) + if (updated_attributes[j].type == attributes[i].type) + updated_attributes[j].length = HAL_PKEY_ATTRIBUTE_NIL; - if (updated_attributes_len >= sizeof(updated_attributes)/sizeof(*updated_attributes)) - return HAL_ERROR_IMPOSSIBLE; + if (updated_attributes_len >= sizeof(updated_attributes)/sizeof(*updated_attributes)) { + err = HAL_ERROR_IMPOSSIBLE; + goto done; + } - updated_attributes[updated_attributes_len].type = attributes[i].type; - updated_attributes[updated_attributes_len].length = attributes[i].length; - updated_attributes[updated_attributes_len].value = attributes[i].value; - updated_attributes_len++; - } + updated_attributes[updated_attributes_len].type = attributes[i].type; + updated_attributes[updated_attributes_len].length = attributes[i].length; + updated_attributes[updated_attributes_len].value = attributes[i].value; + updated_attributes_len++; + } - /* - * Phase 0.3: Prune trailing deletion actions: we don't need them to - * maintain synchronization with existing attributes, and doing so - * simplifies logic for updating the final new chunk. - */ + /* + * Phase 0.3: Prune trailing deletion actions: we don't need them to + * maintain synchronization with existing attributes, and doing so + * simplifies logic for updating the final new chunk. + */ - while (updated_attributes_len > 0 && - updated_attributes[updated_attributes_len - 1].length == HAL_PKEY_ATTRIBUTE_NIL) - --updated_attributes_len; + while (updated_attributes_len > 0 && + updated_attributes[updated_attributes_len - 1].length == HAL_PKEY_ATTRIBUTE_NIL) + --updated_attributes_len; - /* - * Phase 0.4: Figure out how many chunks all this will occupy. - */ + /* + * Phase 0.4: Figure out how many chunks all this will occupy. + */ - chunk = 0; + chunk = 0; - for (int i = 0; i < updated_attributes_len; i++) { + for (int i = 0; i < updated_attributes_len; i++) { - if (updated_attributes[i].length == HAL_PKEY_ATTRIBUTE_NIL) - continue; + if (updated_attributes[i].length == HAL_PKEY_ATTRIBUTE_NIL) + continue; - const size_t needed = hal_ks_attribute_header_size + updated_attributes[i].length; + const size_t needed = hal_ks_attribute_header_size + updated_attributes[i].length; - if (needed > bytes_available) { - bytes_available = SIZEOF_FLASH_ATTRIBUTE_BLOCK_ATTRIBUTES; - chunk++; + if (needed > bytes_available) { + bytes_available = SIZEOF_FLASH_ATTRIBUTE_BLOCK_ATTRIBUTES; + chunk++; + } + + if (needed > bytes_available) { + err = HAL_ERROR_RESULT_TOO_LONG; + goto done; + } + + bytes_available -= needed; } - if (needed > bytes_available) - return HAL_ERROR_RESULT_TOO_LONG; + const unsigned total_chunks_new = chunk + 1; - bytes_available -= needed; - } + /* + * If there aren't enough free blocks, give up now, before changing anything. + */ - const unsigned total_chunks_new = chunk + 1; + if (db.ksi.used + total_chunks_new > db.ksi.size) { + err = HAL_ERROR_NO_KEY_INDEX_SLOTS; + goto done; + } - /* - * If there aren't enough free blocks, give up now, before changing anything. - */ + /* + * Phase 1: Deprecate all the old chunks, remember where they were. + */ - if (db.ksi.used + total_chunks_new > db.ksi.size) - return HAL_ERROR_NO_KEY_INDEX_SLOTS; + unsigned old_blocks[total_chunks_old]; - /* - * Phase 1: Deprecate all the old chunks, remember where they were. - */ + for (chunk = 0; chunk < total_chunks_old; chunk++) { + int hint = slot->hint + chunk; + if ((err = hal_ks_index_find(&db.ksi, &slot->name, chunk, &b, &hint)) != HAL_OK || + (err = block_deprecate(b)) != HAL_OK) + goto done; + old_blocks[chunk] = b; + } - unsigned old_blocks[total_chunks_old]; + /* + * Phase 2: Write new chunks, copying attributes from old chunks or + * from attributes[], as needed. + */ - for (chunk = 0; chunk < total_chunks_old; chunk++) { - int hint = slot->hint + chunk; - if ((err = hal_ks_index_find(&db.ksi, &slot->name, chunk, &b, &hint)) != HAL_OK || - (err = block_deprecate(b)) != HAL_OK) - return err; - old_blocks[chunk] = b; - } + { + 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; + size_t old_bytes_len = 0, new_bytes_len = 0; + unsigned old_chunk = 0, new_chunk = 0; + size_t old_total = 0, new_total = 0; - /* - * Phase 2: Write new chunks, copying attributes from old chunks or - * from attributes[], as needed. - */ + int updated_attributes_i = 0, old_attrs_i = 0; - { - 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; - size_t old_bytes_len = 0, new_bytes_len = 0; - unsigned old_chunk = 0, new_chunk = 0; - size_t old_total = 0, new_total = 0; + uint32_t new_attr_type; + size_t new_attr_length; + const uint8_t *new_attr_value; - int updated_attributes_i = 0, old_attrs_i = 0; + while (updated_attributes_i < updated_attributes_len) { - uint32_t new_attr_type; - size_t new_attr_length; - const uint8_t *new_attr_value; + if (old_chunk >= total_chunks_old || new_chunk >= total_chunks_new) { + err = HAL_ERROR_IMPOSSIBLE; + goto done; + } - while (updated_attributes_i < updated_attributes_len) { + /* + * If we've gotten as far as new data that comes from + * attributes[], we have it in hand and can just copy it. + */ - if (old_chunk >= total_chunks_old || new_chunk >= total_chunks_new) - return HAL_ERROR_IMPOSSIBLE; + if (updated_attributes_len - updated_attributes_i <= attributes_len) { + new_attr_type = updated_attributes[updated_attributes_i].type; + new_attr_length = updated_attributes[updated_attributes_i].length; + new_attr_value = updated_attributes[updated_attributes_i].value; + } - /* - * If we've gotten as far as new data that comes from - * attributes[], we have it in hand and can just copy it. - */ + /* + * Otherwise, we have to read it from an old block, which may in + * turn require reading in the next old block. + */ - if (updated_attributes_len - updated_attributes_i <= attributes_len) { - new_attr_type = updated_attributes[updated_attributes_i].type; - new_attr_length = updated_attributes[updated_attributes_i].length; - new_attr_value = updated_attributes[updated_attributes_i].value; - } + else { - /* - * Otherwise, we have to read it from an old block, which may in - * turn require reading in the next old block. - */ + if (old_block == NULL) { - else { + if ((err = block_read_cached(old_blocks[old_chunk], &old_block)) != HAL_OK) + goto done; - if (old_block == NULL) { + if (old_block->header.this_chunk != old_chunk) { + err = HAL_ERROR_IMPOSSIBLE; + goto done; + } + + if ((err = locate_attributes(old_block, old_chunk, + &old_bytes, &old_bytes_len, &old_attrs_len)) != HAL_OK || + (err = hal_ks_attribute_scan(old_bytes, old_bytes_len, + old_attrs, *old_attrs_len, &old_total)) != HAL_OK) + goto done; + + old_attrs_i = 0; + } - if ((err = block_read_cached(old_blocks[old_chunk], &old_block)) != HAL_OK) - return err; + if (old_attrs_i >= *old_attrs_len) { + old_chunk++; + old_block = NULL; + continue; + } - if (old_block->header.this_chunk != old_chunk) - return HAL_ERROR_IMPOSSIBLE; + new_attr_type = old_attrs[old_attrs_i].type; + new_attr_length = old_attrs[old_attrs_i].length; + new_attr_value = old_attrs[old_attrs_i].value; - if ((err = locate_attributes(old_block, old_chunk, - &old_bytes, &old_bytes_len, &old_attrs_len)) != HAL_OK || - (err = hal_ks_attribute_scan(old_bytes, old_bytes_len, - old_attrs, *old_attrs_len, &old_total)) != HAL_OK) - return err; + if (new_attr_type != updated_attributes[updated_attributes_i].type) { + err = HAL_ERROR_IMPOSSIBLE; + goto done; + } - old_attrs_i = 0; + old_attrs_i++; } - if (old_attrs_i >= *old_attrs_len) { - old_chunk++; - old_block = NULL; - continue; + /* + * Unless this is a deletion, we should have something to write. + */ + + if (new_attr_length != HAL_PKEY_ATTRIBUTE_NIL && new_attr_value == NULL) { + err = HAL_ERROR_IMPOSSIBLE; + goto done; } - new_attr_type = old_attrs[old_attrs_i].type; - new_attr_length = old_attrs[old_attrs_i].length; - new_attr_value = old_attrs[old_attrs_i].value; + /* + * Initialize the new block if necessary. If it's the new chunk + * zero, we need to copy all the non-attribute data from the old + * chunk zero; otherwise, it's a new empty attribute block. + */ + + if (new_block == NULL) { + + new_block = cache_pick_lru(); + memset(new_block, 0xFF, sizeof(*new_block)); + + if (new_chunk == 0) { + flash_block_t *tmp_block; + if ((err = block_read_cached(old_blocks[0], &tmp_block)) != HAL_OK) + goto done; + if (tmp_block->header.this_chunk != 0) { + err = HAL_ERROR_IMPOSSIBLE; + goto done; + } + new_block->header.block_type = BLOCK_TYPE_KEY; + new_block->key.name = slot->name; + new_block->key.type = tmp_block->key.type; + new_block->key.curve = tmp_block->key.curve; + new_block->key.flags = tmp_block->key.flags; + new_block->key.der_len = tmp_block->key.der_len; + new_block->key.attributes_len = 0; + memcpy(new_block->key.der, tmp_block->key.der, tmp_block->key.der_len); + } + else { + new_block->header.block_type = BLOCK_TYPE_ATTR; + new_block->attr.name = slot->name; + new_block->attr.attributes_len = 0; + } - if (new_attr_type != updated_attributes[updated_attributes_i].type) - return HAL_ERROR_IMPOSSIBLE; + new_block->header.block_status = BLOCK_STATUS_LIVE; + new_block->header.total_chunks = total_chunks_new; + new_block->header.this_chunk = new_chunk; - old_attrs_i++; - } + if ((err = locate_attributes(new_block, new_chunk, + &new_bytes, &new_bytes_len, &new_attrs_len)) != HAL_OK) + goto done; - /* - * Unless this is a deletion, we should have something to write. - */ + new_total = 0; + } - if (new_attr_length != HAL_PKEY_ATTRIBUTE_NIL && new_attr_value == NULL) - return HAL_ERROR_IMPOSSIBLE; + /* + * After all that setup, we finally get to write the frelling attribute. + */ - /* - * Initialize the new block if necessary. If it's the new chunk - * zero, we need to copy all the non-attribute data from the old - * chunk zero; otherwise, it's a new empty attribute block. - */ + if (new_attr_length != HAL_PKEY_ATTRIBUTE_NIL) + err = hal_ks_attribute_insert(new_bytes, new_bytes_len, new_attrs, new_attrs_len, &new_total, + new_attr_type, new_attr_value, new_attr_length); - if (new_block == NULL) { - - new_block = cache_pick_lru(); - memset(new_block, 0xFF, sizeof(*new_block)); - - if (new_chunk == 0) { - flash_block_t *tmp_block; - if ((err = block_read_cached(old_blocks[0], &tmp_block)) != HAL_OK) - return err; - if (tmp_block->header.this_chunk != 0) - return HAL_ERROR_IMPOSSIBLE; - new_block->header.block_type = BLOCK_TYPE_KEY; - new_block->key.name = slot->name; - new_block->key.type = tmp_block->key.type; - new_block->key.curve = tmp_block->key.curve; - new_block->key.flags = tmp_block->key.flags; - new_block->key.der_len = tmp_block->key.der_len; - new_block->key.attributes_len = 0; - memcpy(new_block->key.der, tmp_block->key.der, tmp_block->key.der_len); - } - else { - new_block->header.block_type = BLOCK_TYPE_ATTR; - new_block->attr.name = slot->name; - new_block->attr.attributes_len = 0; + /* + * Figure out what to do next: immediately loop for next + * attribute, write current block, or bail out. + */ + + switch (err) { + case HAL_OK: + if (++updated_attributes_i < updated_attributes_len) + continue; + break; + case HAL_ERROR_RESULT_TOO_LONG: + if (new_chunk > 0 && new_attrs_len == 0) + goto done; + break; + default: + goto done; } - new_block->header.block_status = BLOCK_STATUS_LIVE; - new_block->header.total_chunks = total_chunks_new; - new_block->header.this_chunk = new_chunk; + /* + * If we get here, either the current new block is full or we + * finished the last block, so we need to write it out. + */ - if ((err = locate_attributes(new_block, new_chunk, - &new_bytes, &new_bytes_len, &new_attrs_len)) != HAL_OK) - return err; + int hint = slot->hint + new_chunk; - new_total = 0; - } + if (new_chunk < total_chunks_old) + err = hal_ks_index_replace(&db.ksi, &slot->name, new_chunk, &b, &hint); + else + err = hal_ks_index_add( &db.ksi, &slot->name, new_chunk, &b, &hint); - /* - * After all that setup, we finally get to write the frelling attribute. - */ - - if (new_attr_length != HAL_PKEY_ATTRIBUTE_NIL) - err = hal_ks_attribute_insert(new_bytes, new_bytes_len, new_attrs, new_attrs_len, &new_total, - new_attr_type, new_attr_value, new_attr_length); + if (err != HAL_OK || (err = block_write(b, new_block)) != HAL_OK) + goto done; - /* - * Figure out what to do next: immediately loop for next - * attribute, write current block, or bail out. - */ + cache_mark_used(new_block, b); - switch (err) { - case HAL_OK: - if (++updated_attributes_i < updated_attributes_len) - continue; - break; - case HAL_ERROR_RESULT_TOO_LONG: - if (new_chunk > 0 && new_attrs_len == 0) - return err; - break; - default: - return err; + new_block = NULL; + new_chunk++; } /* - * If we get here, either the current new block is full or we - * finished the last block, so we need to write it out. + * If number of blocks shrank, we need to clear trailing entries from the index. */ - int hint = slot->hint + new_chunk; + for (old_chunk = total_chunks_new; old_chunk < total_chunks_old; old_chunk++) { + int hint = slot->hint + old_chunk; - if (new_chunk < total_chunks_old) - err = hal_ks_index_replace(&db.ksi, &slot->name, new_chunk, &b, &hint); - else - err = hal_ks_index_add( &db.ksi, &slot->name, new_chunk, &b, &hint); + err = hal_ks_index_delete(&db.ksi, &slot->name, old_chunk, NULL, &hint); - if (err != HAL_OK || (err = block_write(b, new_block)) != HAL_OK) - return err; - - cache_mark_used(new_block, b); + if (err != HAL_OK) + goto done; + } - new_block = NULL; - new_chunk++; } /* - * If number of blocks shrank, we need to clear trailing entries from the index. + * Phase 3: Zero the old chunks we deprecated in phase 1. */ - for (old_chunk = total_chunks_new; old_chunk < total_chunks_old; old_chunk++) { - int hint = slot->hint + old_chunk; - - err = hal_ks_index_delete(&db.ksi, &slot->name, old_chunk, NULL, &hint); + for (chunk = 0; chunk < total_chunks_old; chunk++) + if ((err = block_zero(old_blocks[chunk])) != HAL_OK) + goto done; - if (err != HAL_OK) - return err; - } + err = HAL_OK; } - /* - * Phase 3: Zero the old chunks we deprecated in phase 1. - */ - - for (chunk = 0; chunk < total_chunks_old; chunk++) - if ((err = block_zero(old_blocks[chunk])) != HAL_OK) - return err; - - return HAL_OK; + done: + hal_ks_unlock(); + return err; #warning What happens if something goes wrong partway through this awful mess? // We're left in a state with all the old blocks deprecated and @@ -1700,18 +1776,22 @@ static hal_error_t ks_get_attributes(hal_ks_t *ks, flash_block_t *block = NULL; unsigned chunk = 0; unsigned found = 0; - hal_error_t err; + hal_error_t err = HAL_OK; unsigned b; + hal_ks_lock(); + do { int hint = slot->hint + chunk; if ((err = hal_ks_index_find(&db.ksi, &slot->name, chunk, &b, &hint)) != HAL_OK || (err = block_read_cached(b, &block)) != HAL_OK) - return err; + goto done; - if (block->header.this_chunk != chunk) - return HAL_ERROR_IMPOSSIBLE; + if (block->header.this_chunk != chunk) { + err = HAL_ERROR_IMPOSSIBLE; + goto done; + } if (chunk == 0) slot->hint = hint; @@ -1723,7 +1803,7 @@ static hal_error_t ks_get_attributes(hal_ks_t *ks, unsigned *attrs_len; if ((err = locate_attributes(block, chunk, &bytes, &bytes_len, &attrs_len)) != HAL_OK) - return err; + goto done; if (*attrs_len == 0) continue; @@ -1731,7 +1811,7 @@ static hal_error_t ks_get_attributes(hal_ks_t *ks, hal_pkey_attribute_t attrs[*attrs_len]; if ((err = hal_ks_attribute_scan(bytes, bytes_len, attrs, *attrs_len, NULL)) != HAL_OK) - return err; + goto done; for (int i = 0; i < attributes_len; i++) { @@ -1750,8 +1830,10 @@ static hal_error_t ks_get_attributes(hal_ks_t *ks, if (attributes_buffer_len == 0) continue; - if (attrs[j].length > attributes_buffer + attributes_buffer_len - abuf) - return HAL_ERROR_RESULT_TOO_LONG; + if (attrs[j].length > attributes_buffer + attributes_buffer_len - abuf) { + err = HAL_ERROR_RESULT_TOO_LONG; + goto done; + } memcpy(abuf, attrs[j].value, attrs[j].length); attributes[i].value = abuf; @@ -1761,9 +1843,13 @@ static hal_error_t ks_get_attributes(hal_ks_t *ks, } while (found < attributes_len && ++chunk < block->header.total_chunks); if (found < attributes_len && attributes_buffer_len > 0) - return HAL_ERROR_ATTRIBUTE_NOT_FOUND; + err = HAL_ERROR_ATTRIBUTE_NOT_FOUND; + else + err = HAL_OK; - return HAL_OK; + done: + hal_ks_unlock(); + return err; } const hal_ks_driver_t hal_ks_token_driver[1] = {{ @@ -1799,6 +1885,8 @@ void hal_ks_init_read_only_pins_only(void) unsigned b, best_seen = ~0; flash_block_t block[1]; + hal_ks_lock(); + for (b = 0; b < NUM_FLASH_BLOCKS; b++) { if (block_read(b, block) != HAL_OK || block_get_type(block) != BLOCK_TYPE_PIN) continue; @@ -1818,6 +1906,8 @@ void hal_ks_init_read_only_pins_only(void) db.wheel_pin = block->pin.wheel_pin; db.so_pin = block->pin.so_pin; db.user_pin = block->pin.user_pin; + + hal_ks_unlock(); } /* @@ -1830,14 +1920,20 @@ hal_error_t hal_get_pin(const hal_user_t user, if (pin == NULL) return HAL_ERROR_BAD_ARGUMENTS; + hal_error_t err = HAL_OK; + + hal_ks_lock(); + switch (user) { case HAL_USER_WHEEL: *pin = &db.wheel_pin; break; case HAL_USER_SO: *pin = &db.so_pin; break; case HAL_USER_NORMAL: *pin = &db.user_pin; break; - default: return HAL_ERROR_BAD_ARGUMENTS; + default: err = HAL_ERROR_BAD_ARGUMENTS; } - return HAL_OK; + hal_ks_unlock(); + + return err; } /* @@ -1904,8 +2000,10 @@ hal_error_t hal_set_pin(const hal_user_t user, hal_error_t err; unsigned b; + hal_ks_lock(); + if ((err = fetch_pin_block(&b, &block)) != HAL_OK) - return err; + goto done; flash_pin_block_t new_data = block->pin; hal_ks_pin_t *dp, *bp; @@ -1914,7 +2012,7 @@ hal_error_t hal_set_pin(const hal_user_t user, case HAL_USER_WHEEL: bp = &new_data.wheel_pin; dp = &db.wheel_pin; break; case HAL_USER_SO: bp = &new_data.so_pin; dp = &db.so_pin; break; case HAL_USER_NORMAL: bp = &new_data.user_pin; dp = &db.user_pin; break; - default: return HAL_ERROR_BAD_ARGUMENTS; + default: err = HAL_ERROR_BAD_ARGUMENTS; goto done; } const hal_ks_pin_t old_pin = *dp; @@ -1923,6 +2021,8 @@ hal_error_t hal_set_pin(const hal_user_t user, if ((err = update_pin_block(b, block, &new_data)) != HAL_OK) *dp = old_pin; + done: + hal_ks_unlock(); return err; } @@ -1951,16 +2051,20 @@ hal_error_t hal_mkm_flash_read(uint8_t *buf, const size_t len) hal_error_t err; unsigned b; + hal_ks_lock(); + if ((err = fetch_pin_block(&b, &block)) != HAL_OK) - return err; + goto done; if (block->pin.kek_set != FLASH_KEK_SET) - return HAL_ERROR_MASTERKEY_NOT_SET; + err = HAL_ERROR_MASTERKEY_NOT_SET; - if (buf != NULL) + else if (buf != NULL) memcpy(buf, block->pin.kek, len); - return HAL_OK; + done: + hal_ks_unlock(); + return err; } hal_error_t hal_mkm_flash_write(const uint8_t * const buf, const size_t len) @@ -1975,15 +2079,21 @@ hal_error_t hal_mkm_flash_write(const uint8_t * const buf, const size_t len) hal_error_t err; unsigned b; + hal_ks_lock(); + if ((err = fetch_pin_block(&b, &block)) != HAL_OK) - return err; + goto done; flash_pin_block_t new_data = block->pin; new_data.kek_set = FLASH_KEK_SET; memcpy(new_data.kek, buf, len); - return update_pin_block(b, block, &new_data); + err = update_pin_block(b, block, &new_data); + + done: + hal_ks_unlock(); + return err; } hal_error_t hal_mkm_flash_erase(const size_t len) @@ -1995,15 +2105,21 @@ hal_error_t hal_mkm_flash_erase(const size_t len) hal_error_t err; unsigned b; + hal_ks_lock(); + if ((err = fetch_pin_block(&b, &block)) != HAL_OK) - return err; + goto done; flash_pin_block_t new_data = block->pin; new_data.kek_set = FLASH_KEK_SET; memset(new_data.kek, 0, len); - return update_pin_block(b, block, &new_data); + err = update_pin_block(b, block, &new_data); + + done: + hal_ks_unlock(); + return err; } #endif /* HAL_MKM_FLASH_BACKUP_KLUDGE */ diff --git a/ks_volatile.c b/ks_volatile.c index 99ad68c..9762da3 100644 --- a/ks_volatile.c +++ b/ks_volatile.c @@ -187,6 +187,10 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, static hal_error_t ks_volatile_init(const hal_ks_driver_t * const driver, const int alloc) { + hal_error_t err = HAL_OK; + + hal_ks_lock(); + const size_t len = (sizeof(*volatile_ks.db) + sizeof(*volatile_ks.db->ksi.index) * STATIC_KS_VOLATILE_SLOTS + sizeof(*volatile_ks.db->ksi.names) * STATIC_KS_VOLATILE_SLOTS + @@ -195,9 +199,12 @@ static hal_error_t ks_volatile_init(const hal_ks_driver_t * const driver, const uint8_t *mem = NULL; if (alloc && (mem = hal_allocate_static_memory(len)) == NULL) - return HAL_ERROR_ALLOCATION_FAILURE; + err = HAL_ERROR_ALLOCATION_FAILURE; + else + err = ks_init(driver, 1, &volatile_ks, mem, len); - return ks_init(driver, 1, &volatile_ks, mem, len); + hal_ks_unlock(); + return err; } static hal_error_t ks_volatile_shutdown(const hal_ks_driver_t * const driver) @@ -241,14 +248,18 @@ static hal_error_t ks_store(hal_ks_t *ks, return HAL_ERROR_BAD_ARGUMENTS; ks_t *ksv = ks_to_ksv(ks); - hal_error_t err; + hal_error_t err = HAL_OK; unsigned b; - if (ksv->db == NULL) - return HAL_ERROR_KEYSTORE_ACCESS; + hal_ks_lock(); + + if (ksv->db == NULL) { + err = HAL_ERROR_KEYSTORE_ACCESS; + goto done; + } if ((err = hal_ks_index_add(&ksv->db->ksi, &slot->name, 0, &b, &slot->hint)) != HAL_OK) - return err; + goto done; uint8_t kek[KEK_LENGTH]; size_t kek_len; @@ -272,6 +283,8 @@ static hal_error_t ks_store(hal_ks_t *ks, else (void) hal_ks_index_delete(&ksv->db->ksi, &slot->name, 0, NULL, &slot->hint); + done: + hal_ks_unlock(); return err; } @@ -283,19 +296,25 @@ static hal_error_t ks_fetch(hal_ks_t *ks, return HAL_ERROR_BAD_ARGUMENTS; ks_t *ksv = ks_to_ksv(ks); - hal_error_t err; + hal_error_t err = HAL_OK; unsigned b; - if (ksv->db == NULL) - return HAL_ERROR_KEYSTORE_ACCESS; + hal_ks_lock(); + + if (ksv->db == NULL) { + err = HAL_ERROR_KEYSTORE_ACCESS; + goto done; + } if ((err = hal_ks_index_find(&ksv->db->ksi, &slot->name, 0, &b, &slot->hint)) != HAL_OK) - return err; + goto done; const ks_key_t * const k = &ksv->db->keys[b]; - if (!key_visible_to_session(ksv, slot->client_handle, slot->session_handle, k)) - return HAL_ERROR_KEY_NOT_FOUND; + if (!key_visible_to_session(ksv, slot->client_handle, slot->session_handle, k)) { + err = HAL_ERROR_KEY_NOT_FOUND; + goto done; + } slot->type = k->type; slot->curve = k->curve; @@ -319,12 +338,11 @@ static hal_error_t ks_fetch(hal_ks_t *ks, err = hal_aes_keyunwrap(NULL, kek, kek_len, k->der, k->der_len, der, der_len); memset(kek, 0, sizeof(kek)); - - if (err != HAL_OK) - return err; } - return HAL_OK; + done: + hal_ks_unlock(); + return err; } static hal_error_t ks_delete(hal_ks_t *ks, @@ -334,24 +352,32 @@ static hal_error_t ks_delete(hal_ks_t *ks, return HAL_ERROR_BAD_ARGUMENTS; ks_t *ksv = ks_to_ksv(ks); - hal_error_t err; + hal_error_t err = HAL_OK; unsigned b; - if (ksv->db == NULL) - return HAL_ERROR_KEYSTORE_ACCESS; + hal_ks_lock(); + + if (ksv->db == NULL) { + err = HAL_ERROR_KEYSTORE_ACCESS; + goto done; + } if ((err = hal_ks_index_find(&ksv->db->ksi, &slot->name, 0, &b, &slot->hint)) != HAL_OK) - return err; + goto done; - if (!key_visible_to_session(ksv, slot->client_handle, slot->session_handle, &ksv->db->keys[b])) - return HAL_ERROR_KEY_NOT_FOUND; + if (!key_visible_to_session(ksv, slot->client_handle, slot->session_handle, &ksv->db->keys[b])) { + err = HAL_ERROR_KEY_NOT_FOUND; + goto done; + } if ((err = hal_ks_index_delete(&ksv->db->ksi, &slot->name, 0, &b, &slot->hint)) != HAL_OK) - return err; + goto done; memset(&ksv->db->keys[b], 0, sizeof(ksv->db->keys[b])); - return HAL_OK; + done: + hal_ks_unlock(); + return err; } static hal_error_t ks_match(hal_ks_t *ks, @@ -376,9 +402,11 @@ static hal_error_t ks_match(hal_ks_t *ks, if (ksv->db == NULL) return HAL_ERROR_KEYSTORE_ACCESS; - hal_error_t err; + hal_error_t err = HAL_OK; int i = -1; + hal_ks_lock(); + *result_len = 0; err = hal_ks_index_find(&ksv->db->ksi, previous_uuid, 0, NULL, &i); @@ -386,7 +414,7 @@ static hal_error_t ks_match(hal_ks_t *ks, if (err == HAL_ERROR_KEY_NOT_FOUND) i--; else if (err != HAL_OK) - return err; + goto done; while (*result_len < result_max && ++i < ksv->db->ksi.used) { @@ -415,7 +443,7 @@ static hal_error_t ks_match(hal_ks_t *ks, 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; + goto done; for (const hal_pkey_attribute_t *required = attributes; ok && required < attributes + attributes_len; required++) { @@ -437,7 +465,11 @@ static hal_error_t ks_match(hal_ks_t *ks, ++*result_len; } - return HAL_OK; + err = HAL_OK; + + done: + hal_ks_unlock(); + return err; } static hal_error_t ks_set_attributes(hal_ks_t *ks, @@ -449,40 +481,53 @@ static hal_error_t ks_set_attributes(hal_ks_t *ks, return HAL_ERROR_BAD_ARGUMENTS; ks_t *ksv = ks_to_ksv(ks); - hal_error_t err; + hal_error_t err = HAL_OK; unsigned b; - if (ksv->db == NULL) - return HAL_ERROR_KEYSTORE_ACCESS; + hal_ks_lock(); + + { + if (ksv->db == NULL) { + err = HAL_ERROR_KEYSTORE_ACCESS; + goto done; + } + + if ((err = hal_ks_index_find(&ksv->db->ksi, &slot->name, 0, &b, &slot->hint)) != HAL_OK) + goto done; + + ks_key_t * const k = &ksv->db->keys[b]; + + if (!key_visible_to_session(ksv, slot->client_handle, slot->session_handle, k)) { + err = HAL_ERROR_KEY_NOT_FOUND; + goto done; + } + + 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; + + if ((err = hal_ks_attribute_scan(bytes, bytes_len, attrs, k->attributes_len, &total_len)) != HAL_OK) + goto done; + + 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) + goto done; + } + + err = HAL_OK; - if ((err = hal_ks_index_find(&ksv->db->ksi, &slot->name, 0, &b, &slot->hint)) != HAL_OK) - return err; - - ks_key_t * const k = &ksv->db->keys[b]; - - if (!key_visible_to_session(ksv, slot->client_handle, slot->session_handle, k)) - return HAL_ERROR_KEY_NOT_FOUND; - - 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; - - if ((err = hal_ks_attribute_scan(bytes, bytes_len, attrs, k->attributes_len, &total_len)) != HAL_OK) - return err; - - 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; } - return HAL_OK; + done: + hal_ks_unlock(); + return err; } static hal_error_t ks_get_attributes(hal_ks_t *ks, @@ -497,53 +542,70 @@ static hal_error_t ks_get_attributes(hal_ks_t *ks, return HAL_ERROR_BAD_ARGUMENTS; ks_t *ksv = ks_to_ksv(ks); - hal_error_t err; + hal_error_t err = HAL_OK; unsigned b; - if (ksv->db == NULL) - return HAL_ERROR_KEYSTORE_ACCESS; + hal_ks_lock(); - if ((err = hal_ks_index_find(&ksv->db->ksi, &slot->name, 0, &b, &slot->hint)) != HAL_OK) - return err; + { + if (ksv->db == NULL) { + err = HAL_ERROR_KEYSTORE_ACCESS; + goto done; + } - const ks_key_t * const k = &ksv->db->keys[b]; + if ((err = hal_ks_index_find(&ksv->db->ksi, &slot->name, 0, &b, &slot->hint)) != HAL_OK) + goto done; - if (!key_visible_to_session(ksv, slot->client_handle, slot->session_handle, k)) - return HAL_ERROR_KEY_NOT_FOUND; + const ks_key_t * const k = &ksv->db->keys[b]; - hal_pkey_attribute_t attrs[k->attributes_len > 0 ? k->attributes_len : 1]; + if (!key_visible_to_session(ksv, slot->client_handle, slot->session_handle, k)) { + err = HAL_ERROR_KEY_NOT_FOUND; + goto done; + } - if ((err = hal_ks_attribute_scan(k->der + k->der_len, sizeof(k->der) - k->der_len, - attrs, k->attributes_len, NULL)) != HAL_OK) - return err; + hal_pkey_attribute_t attrs[k->attributes_len > 0 ? k->attributes_len : 1]; - uint8_t *abuf = attributes_buffer; + if ((err = hal_ks_attribute_scan(k->der + k->der_len, sizeof(k->der) - k->der_len, + attrs, k->attributes_len, NULL)) != HAL_OK) + goto done; - for (int i = 0; i < attributes_len; i++) { - int j = 0; - while (j < k->attributes_len && attrs[j].type != attributes[i].type) - j++; - const int found = j < k->attributes_len; + uint8_t *abuf = attributes_buffer; - if (attributes_buffer_len == 0) { - attributes[i].value = NULL; - attributes[i].length = found ? attrs[j].length : 0; - continue; - } + for (int i = 0; i < attributes_len; i++) { + int j = 0; + while (j < k->attributes_len && attrs[j].type != attributes[i].type) + j++; + const int found = j < k->attributes_len; + + if (attributes_buffer_len == 0) { + attributes[i].value = NULL; + attributes[i].length = found ? attrs[j].length : 0; + continue; + } - if (!found) - return HAL_ERROR_ATTRIBUTE_NOT_FOUND; + if (!found) { + err = HAL_ERROR_ATTRIBUTE_NOT_FOUND; + goto done; + } + + if (attrs[j].length > attributes_buffer + attributes_buffer_len - abuf) { + err = HAL_ERROR_RESULT_TOO_LONG; + goto done; + } - if (attrs[j].length > attributes_buffer + attributes_buffer_len - abuf) - return HAL_ERROR_RESULT_TOO_LONG; + memcpy(abuf, attrs[j].value, attrs[j].length); + attributes[i].value = abuf; + attributes[i].length = attrs[j].length; + abuf += attrs[j].length; + } + + err = HAL_OK; - memcpy(abuf, attrs[j].value, attrs[j].length); - attributes[i].value = abuf; - attributes[i].length = attrs[j].length; - abuf += attrs[j].length; } - return HAL_OK; + done: + hal_ks_unlock(); + return err; } const hal_ks_driver_t hal_ks_volatile_driver[1] = {{ diff --git a/locks.c b/locks.c new file mode 100644 index 0000000..2165753 --- /dev/null +++ b/locks.c @@ -0,0 +1,108 @@ +/* + * locks.c + * ------- + * Dummy lock code for libhal. + * + * Copyright (c) 2017, NORDUnet A/S All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * - Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * - Neither the name of the NORDUnet nor the names of its contributors may + * be used to endorse or promote products derived from this software + * without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS + * IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A + * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED + * TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR + * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING + * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include +#include +#include +#include + +#include "hal.h" +#include "hal_internal.h" + +/* + * There are three slightly peculiar things about this module. + * + * 1) We want to include optional support for GNU weak functions, + * because they're convenient, but we don't want to require support + * for them. So we wrap this in a compilation time conditional + * which defaults to something compatible with C99, but allow this + * to be overriden via an external definition. + * + * 2) The functions in this module are all no-ops, just here so that + * things will link correctly on platforms that don't define them. + * Real definitions for these functions have to come from the port + * to a specific environment, eg, from sw/stm32/projects/hsm.c. + * + * 3) Because we want to expose as little as possible of the + * underlying mechanisms, some of the functions here are closures + * encapsulating objects things which would otherwise be arguments. + * So, for example, we have functions to lock and unlock the HSM + * keystore, rather than general lock and unlock functions which + * they HSM keystore lock as an argument. Since the versions in + * this file are the no-ops, the lock itself goes away here. + */ + +#ifndef ENABLE_WEAK_FUNCTIONS +#define ENABLE_WEAK_FUNCTIONS 0 +#endif + +#if ENABLE_WEAK_FUNCTIONS +#define WEAK_FUNCTION __attribute__((weak)) +#else +#define WEAK_FUNCTION +#endif + +/* + * Critical sections -- disable preemption BRIEFLY. + */ + +WEAK_FUNCTION void hal_critical_section_start(void) +{ + return; +} + +WEAK_FUNCTION void hal_critical_section_end(void) +{ + return; +} + +/* + * Keystore lock -- lock call blocks indefinitely. + */ + +WEAK_FUNCTION void hal_ks_lock(void) +{ + return; +} + +WEAK_FUNCTION void hal_ks_unlock(void) +{ + return; +} + +/* + * Local variables: + * indent-tabs-mode: nil + * End: + */ diff --git a/rpc_misc.c b/rpc_misc.c index d6fc71d..cf5e4a0 100644 --- a/rpc_misc.c +++ b/rpc_misc.c @@ -103,24 +103,32 @@ static client_slot_t client_handle[HAL_STATIC_CLIENT_STATE_BLOCKS]; static inline client_slot_t *alloc_slot(void) { + client_slot_t *slot = NULL; + hal_critical_section_start(); + #if HAL_STATIC_CLIENT_STATE_BLOCKS > 0 - for (int i = 0; i < sizeof(client_handle)/sizeof(*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) - return &client_handle[i]; + slot = &client_handle[i]; #endif - return NULL; + hal_critical_section_end(); + return slot; } static inline client_slot_t *find_handle(const hal_client_handle_t handle) { + client_slot_t *slot = NULL; + hal_critical_section_start(); + #if HAL_STATIC_CLIENT_STATE_BLOCKS > 0 - for (int i = 0; i < sizeof(client_handle)/sizeof(*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 && client_handle[i].handle.handle == handle.handle) - return &client_handle[i]; + slot = &client_handle[i]; #endif - return NULL; + hal_critical_section_end(); + return slot; } static hal_error_t login(const hal_client_handle_t client, diff --git a/rpc_pkey.c b/rpc_pkey.c index e2e42c9..c0a6479 100644 --- a/rpc_pkey.c +++ b/rpc_pkey.c @@ -61,6 +61,9 @@ static hal_pkey_slot_t pkey_handle[HAL_STATIC_PKEY_STATE_BLOCKS]; static inline hal_pkey_slot_t *alloc_slot(const hal_key_flags_t flags) { + hal_pkey_slot_t *slot = NULL; + hal_critical_section_start(); + #if HAL_STATIC_PKEY_STATE_BLOCKS > 0 static uint16_t next_glop = 0; uint32_t glop = ++next_glop << 16; @@ -71,17 +74,18 @@ static inline hal_pkey_slot_t *alloc_slot(const hal_key_flags_t flags) if ((flags & HAL_KEY_FLAG_TOKEN) != 0) glop |= HAL_PKEY_HANDLE_TOKEN_FLAG; - for (int i = 0; i < sizeof(pkey_handle)/sizeof(*pkey_handle); i++) { + for (int i = 0; slot == NULL && i < sizeof(pkey_handle)/sizeof(*pkey_handle); i++) { if (pkey_handle[i].type != HAL_KEY_TYPE_NONE) continue; memset(&pkey_handle[i], 0, sizeof(pkey_handle[i])); pkey_handle[i].pkey_handle.handle = i | glop; pkey_handle[i].hint = -1; - return &pkey_handle[i]; + slot = &pkey_handle[i]; } #endif - return NULL; + hal_critical_section_end(); + return slot; } /* @@ -91,14 +95,18 @@ static inline hal_pkey_slot_t *alloc_slot(const hal_key_flags_t flags) static inline hal_pkey_slot_t *find_handle(const hal_pkey_handle_t handle) { + hal_pkey_slot_t *slot = NULL; + hal_critical_section_start(); + #if HAL_STATIC_PKEY_STATE_BLOCKS > 0 const int i = (int) (handle.handle & 0xFFFF); if (i < sizeof(pkey_handle)/sizeof(*pkey_handle) && pkey_handle[i].pkey_handle.handle == handle.handle) - return &pkey_handle[i]; + slot = &pkey_handle[i]; #endif - return NULL; + hal_critical_section_end(); + return slot; } /* -- cgit v1.2.3 From a118a9b503537894fd9dc4ff021ef3b0635166eb Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Fri, 3 Feb 2017 00:21:02 -0500 Subject: Clean up csprng driver. --- csprng.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/csprng.c b/csprng.c index 7ff6c69..8ba4fa5 100644 --- a/csprng.c +++ b/csprng.c @@ -45,35 +45,34 @@ hal_error_t hal_get_random(hal_core_t *core, void *buffer, const size_t length) { - uint8_t temp[4], *buf = buffer; + uint8_t temp[4], ior = 0, * const buf = buffer; hal_error_t err; - size_t i; if ((err = hal_core_alloc(CSPRNG_NAME, &core)) != HAL_OK) return err; - for (i = 0; i < length; i += 4) { + for (size_t i = 0; i < length; i += 4) { const int last = (length - i) < 4; if (WAIT_FOR_CSPRNG_VALID && (err = hal_io_wait_valid(core)) != HAL_OK) - goto out; + break; if ((err = hal_io_read(core, CSPRNG_ADDR_RANDOM, (last ? temp : &buf[i]), 4)) != HAL_OK) - goto out; + break; if (last) for (; i < length; i++) buf[i] = temp[i&3]; } - for (i = 0, buf = buffer; i < length; i++, buf++) - if (*buf != 0) { - err = HAL_OK; - goto out; - } - err = HAL_ERROR_CSPRNG_BROKEN; + if (err == HAL_OK) { + for (size_t i = 0; i < length; i++) + ior |= buf[i]; + + if (ior == 0 && length > 0) + err = HAL_ERROR_CSPRNG_BROKEN; + } -out: hal_core_free(core); return err; } -- cgit v1.2.3 From 70cad014ccd74b29d8fb797fefd68ea99a29ba37 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Fri, 3 Feb 2017 00:21:58 -0500 Subject: Debug logging for pkey open/close/delete events. --- libhal.py | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/libhal.py b/libhal.py index 7e8cc6f..3063398 100644 --- a/libhal.py +++ b/libhal.py @@ -405,6 +405,7 @@ class PKey(Handle): class HSM(object): mixed_mode = False + debug_io = False def _raise_if_error(self, status): if status != 0: @@ -417,7 +418,8 @@ class HSM(object): def _send(self, msg): # Expects an xdrlib.Packer msg = slip_encode(msg.get_buffer()) - logger.debug("send: %s", ":".join("{:02x}".format(ord(c)) for c in msg)) + if self.debug_io: + logger.debug("send: %s", ":".join("{:02x}".format(ord(c)) for c in msg)) self.socket.sendall(msg) def _recv(self, code): # Returns an xdrlib.Unpacker @@ -428,7 +430,8 @@ class HSM(object): if msg[-1] == "": raise HAL_ERROR_RPC_TRANSPORT() msg.append(self.sockfile.read(1)) - logger.debug("recv: %s", ":".join("{:02x}".format(ord(c)) for c in msg)) + if self.debug_io: + logger.debug("recv: %s", ":".join("{:02x}".format(ord(c)) for c in msg)) msg = slip_decode("".join(msg)) if not msg: continue @@ -545,25 +548,41 @@ class HSM(object): def pkey_load(self, type, curve, der, flags = 0, client = 0, session = 0): with self.rpc(RPC_FUNC_PKEY_LOAD, session, type, curve, der, flags, client = client) as r: - return PKey(self, r.unpack_uint(), UUID(bytes = r.unpack_bytes())) + pkey = PKey(self, r.unpack_uint(), UUID(bytes = r.unpack_bytes())) + logger.debug("Loaded pkey %s", pkey.uuid) + return pkey def pkey_open(self, uuid, flags = 0, client = 0, session = 0): with self.rpc(RPC_FUNC_PKEY_OPEN, session, uuid, flags, client = client) as r: - return PKey(self, r.unpack_uint(), uuid) + pkey = PKey(self, r.unpack_uint(), uuid) + logger.debug("Opened pkey %s", pkey.uuid) + return pkey def pkey_generate_rsa(self, keylen, exponent = "\x01\x00\x01", flags = 0, client = 0, session = 0): with self.rpc(RPC_FUNC_PKEY_GENERATE_RSA, session, keylen, exponent, flags, client = client) as r: - return PKey(self, r.unpack_uint(), UUID(bytes = r.unpack_bytes())) + pkey = PKey(self, r.unpack_uint(), UUID(bytes = r.unpack_bytes())) + logger.debug("Generated RSA pkey %s", pkey.uuid) + return pkey def pkey_generate_ec(self, curve, flags = 0, client = 0, session = 0): with self.rpc(RPC_FUNC_PKEY_GENERATE_EC, session, curve, flags, client = client) as r: - return PKey(self, r.unpack_uint(), UUID(bytes = r.unpack_bytes())) + pkey = PKey(self, r.unpack_uint(), UUID(bytes = r.unpack_bytes())) + logger.debug("Generated EC pkey %s", pkey.uuid) + return pkey def pkey_close(self, pkey): + try: + logger.debug("Closing pkey %s", pkey.uuid) + except AttributeError: + pass with self.rpc(RPC_FUNC_PKEY_CLOSE, pkey): return def pkey_delete(self, pkey): + try: + logger.debug("Deleting pkey %s", pkey.uuid) + except AttributeError: + pass with self.rpc(RPC_FUNC_PKEY_DELETE, pkey): return -- cgit v1.2.3 From f205126b79558e73bbd856d55bc8bb2ee98f31a8 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Fri, 3 Feb 2017 00:22:41 -0500 Subject: Unit test cleanup. Log exceptions immediately when failing a test; doesn't replace backtrace at end of test run, but since a full test run can take a while it's useful to know what failed closer to when it happened. More conditionals to skip tests which require external Python crypto packages when those packages aren't installed. --- unit-tests.py | 47 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/unit-tests.py b/unit-tests.py index 895bdb5..c1d0d44 100644 --- a/unit-tests.py +++ b/unit-tests.py @@ -39,6 +39,7 @@ LibHAL unit tests, using libhal.py and the Python unit_test framework. import unittest import datetime +import logging import sys from libhal import * @@ -66,6 +67,9 @@ except ImportError: ecdsa_loaded = False +logger = logging.getLogger("unit-tests") + + def main(): from sys import argv global args @@ -121,6 +125,12 @@ class TextTestResult(unittest.TextTestResult): self.stream.flush() super(TextTestResult, self).addSuccess(test) + def addError(self, test, err): + if self.showAll: + self.stream.write("exception {!s} ".format(err[0].__name__)) # err[1] + self.stream.flush() + super(TextTestResult, self).addError(test, err) + class TextTestRunner(unittest.TextTestRunner): resultclass = TextTestResult @@ -331,93 +341,123 @@ class TestPKeyHashing(TestCaseLoggedIn): k1.verify(signature = sig, hash = self.h(alg, mixed_mode = True)) k2.verify(signature = sig, hash = self.h(alg, mixed_mode = True)) + @unittest.skipUnless(pycrypto_loaded, "Requires Python Crypto package") def test_load_sign_verify_rsa_1024_sha256_data(self): self.load_sign_verify_rsa(HAL_DIGEST_ALGORITHM_SHA256, 1024, self.sign_verify_data) + @unittest.skipUnless(pycrypto_loaded, "Requires Python Crypto package") def test_load_sign_verify_rsa_2048_sha384_data(self): self.load_sign_verify_rsa(HAL_DIGEST_ALGORITHM_SHA384, 2048, self.sign_verify_data) + @unittest.skipUnless(pycrypto_loaded, "Requires Python Crypto package") def test_load_sign_verify_rsa_4096_sha512_data(self): self.load_sign_verify_rsa(HAL_DIGEST_ALGORITHM_SHA512, 4096, self.sign_verify_data) + @unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") def test_load_sign_verify_ecdsa_p256_sha256_data(self): self.load_sign_verify_ecdsa(HAL_DIGEST_ALGORITHM_SHA256, HAL_CURVE_P256, self.sign_verify_data) + @unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") def test_load_sign_verify_ecdsa_p384_sha384_data(self): self.load_sign_verify_ecdsa(HAL_DIGEST_ALGORITHM_SHA384, HAL_CURVE_P384, self.sign_verify_data) + @unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") def test_load_sign_verify_ecdsa_p521_sha512_data(self): self.load_sign_verify_ecdsa(HAL_DIGEST_ALGORITHM_SHA512, HAL_CURVE_P521, self.sign_verify_data) + @unittest.skipUnless(pycrypto_loaded, "Requires Python Crypto package") def test_load_sign_verify_rsa_1024_sha256_remote_remote(self): self.load_sign_verify_rsa(HAL_DIGEST_ALGORITHM_SHA256, 1024, self.sign_verify_remote_remote) + @unittest.skipUnless(pycrypto_loaded, "Requires Python Crypto package") def test_load_sign_verify_rsa_2048_sha384_remote_remote(self): self.load_sign_verify_rsa(HAL_DIGEST_ALGORITHM_SHA384, 2048, self.sign_verify_remote_remote) + @unittest.skipUnless(pycrypto_loaded, "Requires Python Crypto package") def test_load_sign_verify_rsa_4096_sha512_remote_remote(self): self.load_sign_verify_rsa(HAL_DIGEST_ALGORITHM_SHA512, 4096, self.sign_verify_remote_remote) + @unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") def test_load_sign_verify_ecdsa_p256_sha256_remote_remote(self): self.load_sign_verify_ecdsa(HAL_DIGEST_ALGORITHM_SHA256, HAL_CURVE_P256, self.sign_verify_remote_remote) + @unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") def test_load_sign_verify_ecdsa_p384_sha384_remote_remote(self): self.load_sign_verify_ecdsa(HAL_DIGEST_ALGORITHM_SHA384, HAL_CURVE_P384, self.sign_verify_remote_remote) + @unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") def test_load_sign_verify_ecdsa_p521_sha512_remote_remote(self): self.load_sign_verify_ecdsa(HAL_DIGEST_ALGORITHM_SHA512, HAL_CURVE_P521, self.sign_verify_remote_remote) + @unittest.skipUnless(pycrypto_loaded, "Requires Python Crypto package") def test_load_sign_verify_rsa_1024_sha256_remote_local(self): self.load_sign_verify_rsa(HAL_DIGEST_ALGORITHM_SHA256, 1024, self.sign_verify_remote_local) + @unittest.skipUnless(pycrypto_loaded, "Requires Python Crypto package") def test_load_sign_verify_rsa_2048_sha384_remote_local(self): self.load_sign_verify_rsa(HAL_DIGEST_ALGORITHM_SHA384, 2048, self.sign_verify_remote_local) + @unittest.skipUnless(pycrypto_loaded, "Requires Python Crypto package") def test_load_sign_verify_rsa_4096_sha512_remote_local(self): self.load_sign_verify_rsa(HAL_DIGEST_ALGORITHM_SHA512, 4096, self.sign_verify_remote_local) + @unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") def test_load_sign_verify_ecdsa_p256_sha256_remote_local(self): self.load_sign_verify_ecdsa(HAL_DIGEST_ALGORITHM_SHA256, HAL_CURVE_P256, self.sign_verify_remote_local) + @unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") def test_load_sign_verify_ecdsa_p384_sha384_remote_local(self): self.load_sign_verify_ecdsa(HAL_DIGEST_ALGORITHM_SHA384, HAL_CURVE_P384, self.sign_verify_remote_local) + @unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") def test_load_sign_verify_ecdsa_p521_sha512_remote_local(self): self.load_sign_verify_ecdsa(HAL_DIGEST_ALGORITHM_SHA512, HAL_CURVE_P521, self.sign_verify_remote_local) + @unittest.skipUnless(pycrypto_loaded, "Requires Python Crypto package") def test_load_sign_verify_rsa_1024_sha256_local_remote(self): self.load_sign_verify_rsa(HAL_DIGEST_ALGORITHM_SHA256, 1024, self.sign_verify_local_remote) + @unittest.skipUnless(pycrypto_loaded, "Requires Python Crypto package") def test_load_sign_verify_rsa_2048_sha384_local_remote(self): self.load_sign_verify_rsa(HAL_DIGEST_ALGORITHM_SHA384, 2048, self.sign_verify_local_remote) + @unittest.skipUnless(pycrypto_loaded, "Requires Python Crypto package") def test_load_sign_verify_rsa_4096_sha512_local_remote(self): self.load_sign_verify_rsa(HAL_DIGEST_ALGORITHM_SHA512, 4096, self.sign_verify_local_remote) + @unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") def test_load_sign_verify_ecdsa_p256_sha256_local_remote(self): self.load_sign_verify_ecdsa(HAL_DIGEST_ALGORITHM_SHA256, HAL_CURVE_P256, self.sign_verify_local_remote) + @unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") def test_load_sign_verify_ecdsa_p384_sha384_local_remote(self): self.load_sign_verify_ecdsa(HAL_DIGEST_ALGORITHM_SHA384, HAL_CURVE_P384, self.sign_verify_local_remote) + @unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") def test_load_sign_verify_ecdsa_p521_sha512_local_remote(self): self.load_sign_verify_ecdsa(HAL_DIGEST_ALGORITHM_SHA512, HAL_CURVE_P521, self.sign_verify_local_remote) + @unittest.skipUnless(pycrypto_loaded, "Requires Python Crypto package") def test_load_sign_verify_rsa_1024_sha256_local_local(self): self.load_sign_verify_rsa(HAL_DIGEST_ALGORITHM_SHA256, 1024, self.sign_verify_local_local) + @unittest.skipUnless(pycrypto_loaded, "Requires Python Crypto package") def test_load_sign_verify_rsa_2048_sha384_local_local(self): self.load_sign_verify_rsa(HAL_DIGEST_ALGORITHM_SHA384, 2048, self.sign_verify_local_local) + @unittest.skipUnless(pycrypto_loaded, "Requires Python Crypto package") def test_load_sign_verify_rsa_4096_sha512_local_local(self): self.load_sign_verify_rsa(HAL_DIGEST_ALGORITHM_SHA512, 4096, self.sign_verify_local_local) + @unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") def test_load_sign_verify_ecdsa_p256_sha256_local_local(self): self.load_sign_verify_ecdsa(HAL_DIGEST_ALGORITHM_SHA256, HAL_CURVE_P256, self.sign_verify_local_local) + @unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") def test_load_sign_verify_ecdsa_p384_sha384_local_local(self): self.load_sign_verify_ecdsa(HAL_DIGEST_ALGORITHM_SHA384, HAL_CURVE_P384, self.sign_verify_local_local) + @unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") def test_load_sign_verify_ecdsa_p521_sha512_local_local(self): self.load_sign_verify_ecdsa(HAL_DIGEST_ALGORITHM_SHA512, HAL_CURVE_P521, self.sign_verify_local_local) @@ -490,7 +530,7 @@ class TestPKeyECDSAInterop(TestCaseLoggedIn): self.load_sign_verify_ecdsa(HAL_DIGEST_ALGORITHM_SHA512, SHA512, HAL_CURVE_P521) -class TestPKeyList(TestCaseLoggedIn): +class TestPKeyMatch(TestCaseLoggedIn): """ Tests involving PKey list and match functions. """ @@ -590,6 +630,7 @@ class TestPKeyAttribute(TestCaseLoggedIn): self.load_and_fill(HAL_KEY_FLAG_TOKEN, n_attrs = 4, n_fill = 512) # [16, 1024] +@unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") class TestPKeyAttributeP11(TestCaseLoggedIn): """ Attribute creation/lookup/deletion tests based on a PKCS #11 trace. @@ -654,6 +695,7 @@ class TestPKeyAttributeP11(TestCaseLoggedIn): 0x180 : "\x06\x08\x2a\x86\x48\xce\x3d\x03\x01\x07" }) +@unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") class TestPKeyAttributeWriteSpeedToken(TestCaseLoggedIn): """ Attribute speed tests. @@ -678,6 +720,7 @@ class TestPKeyAttributeWriteSpeedToken(TestCaseLoggedIn): def test_set_12_attributes(self): self.set_attributes(12) +@unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") class TestPKeyAttributeWriteSpeedVolatile(TestCaseLoggedIn): """ Attribute speed tests. @@ -702,6 +745,7 @@ class TestPKeyAttributeWriteSpeedVolatile(TestCaseLoggedIn): def test_set_12_attributes(self): self.set_attributes(12) +@unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") class TestPKeyAttributeReadSpeedToken(TestCaseLoggedIn): """ Attribute speed tests. @@ -733,6 +777,7 @@ class TestPKeyAttributeReadSpeedToken(TestCaseLoggedIn): def test_get_12_attributes(self): self.get_attributes(12) +@unittest.skipUnless(ecdsa_loaded, "Requires Python ECDSA package") class TestPKeyAttributeReadSpeedVolatile(TestCaseLoggedIn): """ Attribute speed tests. -- cgit v1.2.3 From 33c843ad457f8341b8a277e6d9481937d3925951 Mon Sep 17 00:00:00 2001 From: Paul Selkirk Date: Mon, 13 Feb 2017 23:16:21 -0500 Subject: Add some comments for things I figured out while reviewing code. --- ks_attribute.c | 9 +++++++ ks_flash.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++----- ks_index.c | 42 ++++++++++++++++++++++++----- rpc_pkey.c | 3 ++- 4 files changed, 123 insertions(+), 14 deletions(-) diff --git a/ks_attribute.c b/ks_attribute.c index 92e450d..ec674f5 100644 --- a/ks_attribute.c +++ b/ks_attribute.c @@ -120,11 +120,18 @@ hal_error_t hal_ks_attribute_delete(uint8_t *bytes, const size_t bytes_len, if (bytes == NULL || attributes == NULL || attributes_len == NULL || total_len == NULL) return HAL_ERROR_BAD_ARGUMENTS; + /* + * Search for attribute by type. Note that there can be only one + * attribute of any given type. + */ + int i = 0; while (i < *attributes_len && attributes[i].type != type) i++; + /* If not found, great, it's already deleted from the key. */ + if (i == *attributes_len) return HAL_OK; @@ -152,6 +159,8 @@ hal_error_t hal_ks_attribute_insert(uint8_t *bytes, const size_t bytes_len, total_len == NULL || value == NULL) return HAL_ERROR_BAD_ARGUMENTS; + /* Delete the existing attribute value (if present), then write the new value. */ + hal_error_t err = hal_ks_attribute_delete(bytes, bytes_len, attributes, attributes_len, total_len, type); diff --git a/ks_flash.c b/ks_flash.c index c21d668..b9397f8 100644 --- a/ks_flash.c +++ b/ks_flash.c @@ -33,6 +33,15 @@ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +/* + * This keystore driver operates over bare flash, versus over a flash file + * system or flash translation layer. The block size is large enough to + * hold an AES-keywrapped 4096-bit RSA key. Any remaining space in the key + * block may be used to store attributes (opaque TLV blobs). If the + * attributes overflow the key block, additional blocks may be added, but + * no attribute may exceed the block size. + */ + #include #include #include @@ -312,7 +321,7 @@ static hal_crc32_t calculate_block_crc(const flash_block_t * const block) } /* - * Calculate block offset. + * Calculate offset of the block in the flash address space. */ static inline uint32_t block_offset(const unsigned blockno) @@ -537,6 +546,11 @@ static hal_error_t block_update(const unsigned b1, flash_block_t *block, cache_mark_used(block, b2); + /* + * Erase the first block in the free list. In case of restart, this + * puts the block back at the head of the free list. + */ + return block_erase_maybe(db.ksi.index[db.ksi.used]); } @@ -579,6 +593,12 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc sizeof(*db.ksi.names) * NUM_FLASH_BLOCKS + sizeof(*db.cache) * KS_FLASH_CACHE_SIZE); + /* + * This is done as a single large allocation, rather than 3 smaller + * allocations, to make it atomic - we need all 3, so either all + * succeed or all fail. + */ + uint8_t *mem = hal_allocate_static_memory(len); if (mem == NULL) { @@ -634,7 +654,7 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc * Read one block. If the CRC is bad or the block type is * unknown, it's old data we don't understand, something we were * writing when we crashed, or bad flash; in any of these cases, - * we want the block to ends up near the end of the free list. + * we want the block to end up near the end of the free list. */ err = block_read(i, block); @@ -742,20 +762,22 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc * * For any tombstone we find, we start by looking for all the blocks * with a matching UUID, then see what valid sequences we can - * construct from what we found. + * construct from what we found. This basically works in reverse of + * the update sequence in ks_set_attributes(). * * If we can construct a valid sequence of live blocks, the complete - * update was written out, and we just need to zero the tombstones. + * update was written out, and we just need to finish zeroing the + * tombstones. * * Otherwise, if we can construct a complete sequence of tombstone * blocks, the update failed before it was completely written, so we * have to zero the incomplete sequence of live blocks then restore - * from the tombstones. + * the tombstones. * * Otherwise, if the live and tombstone blocks taken together form a * valid sequence, the update failed while deprecating the old live - * blocks, and the update itself was not written, so we need to - * restore the tombstones and leave the live blocks alone. + * blocks, and none of the new data was written, so we need to restore + * the tombstones and leave the live blocks alone. * * If none of the above applies, we don't understand what happened, * which is a symptom of either a bug or a hardware failure more @@ -775,11 +797,25 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc if ((err = hal_ks_index_find_range(&db.ksi, &name, 0, &n_blocks, NULL, &where, 0)) != HAL_OK) goto done; + /* + * hal_ks_index_find_range does a binary search, not a linear search, + * so it may not return the first instance of a block with the given + * name and chunk=0. Search backwards to make sure we have all chunks. + */ + while (where > 0 && !hal_uuid_cmp(&name, &db.ksi.names[db.ksi.index[where - 1]].name)) { where--; n_blocks++; } + /* + * Rather than calling hal_ks_index_find_range with an array pointer + * to get the list of matching blocks (because of the binary search + * issue), we're going to fondle the index directly. This is really + * not something to do in regular code, but this is error-recovery + * code. + */ + int live_ok = 1, tomb_ok = 1, join_ok = 1; unsigned n_live = 0, n_tomb = 0; unsigned i_live = 0, i_tomb = 0; @@ -823,6 +859,12 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc goto done; } + /* + * If live_ok or tomb_ok, we have to zero out some blocks, and adjust + * the index. Again, don't fondle the index directly, outside of error + * recovery. + */ + if (live_ok) { for (int j = 0; j < n_tomb; j++) { const unsigned b = tomb_blocks[j]; @@ -861,6 +903,10 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc n_blocks = n_tomb; } + /* + * Restore tombstone blocks (tomb_ok or join_ok). + */ + for (int j = 0; j < n_blocks; j++) { int hint = where + j; unsigned b1 = db.ksi.index[hint], b2; @@ -878,6 +924,10 @@ static hal_error_t ks_init(const hal_ks_driver_t * const driver, const int alloc } } + /* + * Fetch or create the PIN block. + */ + err = fetch_pin_block(NULL, &block); if (err == HAL_OK) { @@ -1112,9 +1162,17 @@ static hal_error_t ks_delete(hal_ks_t *ks, hal_ks_lock(); { + /* + * Get the count of blocks to delete. + */ + if ((err = hal_ks_index_delete_range(&db.ksi, &slot->name, 0, &n, NULL, &slot->hint)) != HAL_OK) goto done; + /* + * Then delete them. + */ + unsigned b[n]; if ((err = hal_ks_index_delete_range(&db.ksi, &slot->name, n, NULL, b, &slot->hint)) != HAL_OK) @@ -1123,10 +1181,19 @@ static hal_error_t ks_delete(hal_ks_t *ks, for (int i = 0; i < n; i++) cache_release(cache_find_block(b[i])); + /* + * Zero the blocks, to mark them as recently used. + */ + for (int i = 0; i < n; i++) if ((err = block_zero(b[i])) != HAL_OK) goto done; + /* + * Erase the first block in the free list. In case of restart, this + * puts the block back at the head of the free list. + */ + err = block_erase_maybe(db.ksi.index[db.ksi.used]); } @@ -1455,6 +1522,8 @@ static hal_error_t ks_set_attributes(hal_ks_t *ks, /* * Phase 0.2: Merge new attributes into updated_attributes[]. + * For each new attribute type, mark any existing attributes of that + * type for deletion. Append new attributes to updated_attributes[]. */ for (int i = 0; i < attributes_len; i++) { diff --git a/ks_index.c b/ks_index.c index 0c12fcc..c47451c 100644 --- a/ks_index.c +++ b/ks_index.c @@ -55,8 +55,8 @@ static inline int ks_name_cmp(const hal_ks_name_t * const name1, const hal_ks_na } /* - * Return value indicates whether the name is present in the index. - * "where" indicates the name's position whether present or not. + * Find a block in the index, return true (found) or false (not found). + * "where" indicates the name's position, or the position of the first free block. * * NB: This does NOT return a block number, it returns an index into * ksi->index[]. @@ -145,6 +145,10 @@ static inline void ks_heapsort(hal_ks_index_t *ksi) } } +/* + * Perform a consistency check on the index. + */ + #define fsck(_ksi) \ do { hal_error_t _err = hal_ks_index_fsck(_ksi); if (_err != HAL_OK) return _err; } while (0) @@ -179,16 +183,16 @@ hal_error_t hal_ks_index_fsck(hal_ks_index_t *ksi) return HAL_OK; } +/* + * Set up the index. Only setup task we have at the moment is sorting the index. + */ + hal_error_t hal_ks_index_setup(hal_ks_index_t *ksi) { if (ksi == NULL || ksi->index == NULL || ksi->names == NULL || ksi->size == 0 || ksi->used > ksi->size) return HAL_ERROR_BAD_ARGUMENTS; - /* - * Only setup task we have at the moment is sorting the index. - */ - ks_heapsort(ksi); /* @@ -200,6 +204,10 @@ hal_error_t hal_ks_index_setup(hal_ks_index_t *ksi) return HAL_OK; } +/* + * Find a single block by name and chunk number. + */ + hal_error_t hal_ks_index_find(hal_ks_index_t *ksi, const hal_uuid_t * const name, const unsigned chunk, @@ -225,6 +233,11 @@ hal_error_t hal_ks_index_find(hal_ks_index_t *ksi, return ok ? HAL_OK : HAL_ERROR_KEY_NOT_FOUND; } +/* + * Find all blocks with the given name. + * If 'strict' is set, expect it to be a well-ordered set of chunks. + */ + hal_error_t hal_ks_index_find_range(hal_ks_index_t *ksi, const hal_uuid_t * const name, const unsigned max_blocks, @@ -266,6 +279,10 @@ hal_error_t hal_ks_index_find_range(hal_ks_index_t *ksi, return HAL_OK; } +/* + * Add a single block to the index. + */ + hal_error_t hal_ks_index_add(hal_ks_index_t *ksi, const hal_uuid_t * const name, const unsigned chunk, @@ -309,6 +326,10 @@ hal_error_t hal_ks_index_add(hal_ks_index_t *ksi, return HAL_OK; } +/* + * Delete a single block from the index. + */ + hal_error_t hal_ks_index_delete(hal_ks_index_t *ksi, const hal_uuid_t * const name, const unsigned chunk, @@ -348,6 +369,11 @@ hal_error_t hal_ks_index_delete(hal_ks_index_t *ksi, return HAL_OK; } +/* + * Delete all blocks with the given name. If blocknos is NULL, return a + * count of the matching blocks without deleting anything. + */ + hal_error_t hal_ks_index_delete_range(hal_ks_index_t *ksi, const hal_uuid_t * const name, const unsigned max_blocks, @@ -404,6 +430,10 @@ hal_error_t hal_ks_index_delete_range(hal_ks_index_t *ksi, return HAL_OK; } +/* + * Replace a single block in the index. + */ + hal_error_t hal_ks_index_replace(hal_ks_index_t *ksi, const hal_uuid_t * const name, const unsigned chunk, diff --git a/rpc_pkey.c b/rpc_pkey.c index c0a6479..ba75f7e 100644 --- a/rpc_pkey.c +++ b/rpc_pkey.c @@ -227,7 +227,8 @@ static inline hal_error_t ks_open_from_flags(hal_ks_t **ks, const hal_key_flags_ } /* - * Receive key from application, store it with supplied name, return a key handle. + * Receive key from application, generate a name (UUID), store it, and + * return a key handle and the name. */ static hal_error_t pkey_local_load(const hal_client_handle_t client, -- cgit v1.2.3 From 44652e52ae7b238adcac5a347ca74f2a9838ab6f Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Tue, 14 Feb 2017 20:01:04 -0500 Subject: Erase new head of free list in ks_store(). --- ks_flash.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/ks_flash.c b/ks_flash.c index b9397f8..c0179ca 100644 --- a/ks_flash.c +++ b/ks_flash.c @@ -1075,7 +1075,13 @@ static hal_error_t ks_store(hal_ks_t *ks, k->der_len = SIZEOF_FLASH_KEY_BLOCK_DER; k->attributes_len = 0; - if ((err = hal_mkm_get_kek(kek, &kek_len, sizeof(kek))) == HAL_OK) + if (db.ksi.used < db.ksi.size) + err = block_erase_maybe(db.ksi.index[db.ksi.used]); + + if (err == HAL_OK) + err = hal_mkm_get_kek(kek, &kek_len, sizeof(kek)); + + if (err == HAL_OK) err = hal_aes_keywrap(NULL, kek, kek_len, der, der_len, k->der, &k->der_len); memset(kek, 0, sizeof(kek)); -- cgit v1.2.3 From 8dd40de9dc481277d15cf4f1ff707ad620ada7ed Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Mon, 27 Feb 2017 16:07:19 -0500 Subject: Track change to stm32 keystore API. --- ks_flash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ks_flash.c b/ks_flash.c index c0179ca..7a87d5c 100644 --- a/ks_flash.c +++ b/ks_flash.c @@ -455,7 +455,7 @@ static hal_error_t block_erase(const unsigned blockno) return HAL_ERROR_IMPOSSIBLE; /* Sigh, magic numeric return codes */ - if (keystore_erase_subsectors(blockno, blockno) != 1) + if (keystore_erase_subsector(blockno) != 1) return HAL_ERROR_KEYSTORE_ACCESS; return HAL_OK; -- cgit v1.2.3 From a94d48d04137eed569c804cd0f18f5895cc79dbf Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Mon, 27 Feb 2017 16:12:10 -0500 Subject: Compile-time configuration of software-only hash cores. --- Makefile | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 6c01ab4..08215a8 100644 --- a/Makefile +++ b/Makefile @@ -48,17 +48,19 @@ KS ?= flash RPC_MODE ?= none RPC_TRANSPORT ?= none MODEXP_CORE ?= no +HASH_CORES ?= no ifeq (,$(and \ $(filter none eim i2c fmc ,${IO_BUS}),\ $(filter none server client-simple client-mixed ,${RPC_MODE}),\ $(filter mmap flash ,${KS}),\ $(filter none loopback serial daemon ,${RPC_TRANSPORT}),\ - $(filter no yes ,${MODEXP_CORE}))) + $(filter no yes ,${MODEXP_CORE}),\ + $(filter no yes ,${HASH_CORES}))) $(error ${USAGE}) endif -$(info Building libhal with configuration IO_BUS=${IO_BUS} RPC_MODE=${RPC_MODE} KS=${KS} RPC_TRANSPORT=${RPC_TRANSPORT} MODEXP_CORE=${MODEXP_CORE}) +$(info Building libhal with configuration IO_BUS=${IO_BUS} RPC_MODE=${RPC_MODE} KS=${KS} RPC_TRANSPORT=${RPC_TRANSPORT} MODEXP_CORE=${MODEXP_CORE} HASH_CORES=${HASH_CORES}) # Whether the RSA code should use the ModExp | ModExpS6 | ModExpA7 core. @@ -68,6 +70,14 @@ else RSA_USE_MODEXP_CORE := 0 endif +# Whether the hash code should use the SHA-1 / SHA-256 / SHA-512 cores. + +ifeq "${HASH_CORES}" "yes" + HASH_ONLY_USE_SOFT_CORES := 0 +else + HASH_ONLY_USE_SOFT_CORES := 1 +endif + # Object files to build, initialized with ones we always want. # There's a balance here between skipping files we don't strictly # need and reducing the number of unnecessary conditionals in this @@ -167,7 +177,7 @@ ifeq "${RPC_MODE}" "none" CFLAGS += -DHAL_RSA_USE_MODEXP=${RSA_USE_MODEXP_CORE} else ifeq "${RPC_MODE}" "server" OBJ += ${CORE_OBJ} ${RPC_SERVER_OBJ} - CFLAGS += -DRPC_CLIENT=RPC_CLIENT_LOCAL -DHAL_RSA_USE_MODEXP=${RSA_USE_MODEXP_CORE} + CFLAGS += -DRPC_CLIENT=RPC_CLIENT_LOCAL -DHAL_RSA_USE_MODEXP=${RSA_USE_MODEXP_CORE} -DHAL_ONLY_USE_SOFTWARE_HASH_CORES=${HASH_ONLY_USE_SOFT_CORES} else ifeq "${RPC_MODE}" "client-simple" OBJ += ${RPC_CLIENT_OBJ} CFLAGS += -DRPC_CLIENT=RPC_CLIENT_REMOTE -DHAL_RSA_USE_MODEXP=0 -DHAL_ONLY_USE_SOFTWARE_HASH_CORES=1 -- cgit v1.2.3 From 623ed007f5eb5fc66c24e0b3872d0912e11cf0ee Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Wed, 1 Mar 2017 14:10:31 -0500 Subject: Compute public key if necessary when loading a private key. libhal and PKCS #11 have slightly different models of private keys: in libhal, a "private key" object is really a keypair, while in PKCS #11 a private key really is a naked private key. This was a deliberate design decision in libhal, both for simplicity and to better support user interfaces other than PKCS #11, so we'd rather not change it. This difference doesn't matter very much for RSA keys in PKCS #11, where the private key components are a superset of the public key components anyway, but the PKCS #11 template for ECDSA private keys doesn't allow setting public key components with C_CreateObject(). Fortunately, computing the public components of an ECDSA key pair from the private key is straightforward, so we just do that when needed. --- ecdsa.c | 43 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/ecdsa.c b/ecdsa.c index 916a2f4..04e67b8 100644 --- a/ecdsa.c +++ b/ecdsa.c @@ -978,11 +978,9 @@ hal_error_t hal_ecdsa_key_load_public(hal_ecdsa_key_t **key_, } /* - * Load a private key from components; does all the same things as - * hal_ecdsa_key_load_public(), then loads the private key itself and - * adjusts the key type. - * - * For extra paranoia, we could check Q == dG. + * Load a private key from components: does the same things as + * hal_ecdsa_key_load_public(), but also checks the private key, and + * generates the public key from the private key if necessary. */ hal_error_t hal_ecdsa_key_load_private(hal_ecdsa_key_t **key_, @@ -992,18 +990,47 @@ hal_error_t hal_ecdsa_key_load_private(hal_ecdsa_key_t **key_, const uint8_t * const y, const size_t y_len, const uint8_t * const d, const size_t d_len) { + const ecdsa_curve_t * const curve = get_curve(curve_); hal_ecdsa_key_t *key = keybuf; hal_error_t err; - if (d == NULL) + if (key_ == NULL || key == NULL || keybuf_len < sizeof(*key) || curve == NULL || + d == NULL || d_len == 0 || (x == NULL && x_len != 0) || (y == NULL && y_len != 0)) return HAL_ERROR_BAD_ARGUMENTS; - if ((err = hal_ecdsa_key_load_public(key_, keybuf, keybuf_len, curve_, x, x_len, y, y_len)) != HAL_OK) - return err; + memset(keybuf, 0, keybuf_len); key->type = HAL_KEY_TYPE_EC_PRIVATE; + key->curve = curve_; + fp_read_unsigned_bin(key->d, unconst_uint8_t(d), d_len); + + if (fp_iszero(key->d) || fp_cmp(key->d, unconst_fp_int(curve->n)) != FP_LT) + lose(HAL_ERROR_BAD_ARGUMENTS); + + fp_set(key->Q->z, 1); + + if (x_len != 0 || y_len != 0) { + fp_read_unsigned_bin(key->Q->x, unconst_uint8_t(x), x_len); + fp_read_unsigned_bin(key->Q->y, unconst_uint8_t(y), y_len); + } + + else { + fp_copy(curve->Gx, key->Q->x); + fp_copy(curve->Gy, key->Q->y); + if ((err = point_scalar_multiply(key->d, key->Q, key->Q, curve)) != HAL_OK) + goto fail; + } + + if (!point_is_on_curve(key->Q, curve)) + lose(HAL_ERROR_KEY_NOT_ON_CURVE); + + *key_ = key; return HAL_OK; + + fail: + memset(keybuf, 0, keybuf_len); + return err; } /* -- cgit v1.2.3