From 7c61f43d516dff9f1047d1c08a9bb778cb8edc68 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Wed, 24 May 2017 21:44:56 -0400 Subject: Checkpoint, not expected to work yet, includes a lot of notes. --- ks.h | 241 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 241 insertions(+) create mode 100644 ks.h (limited to 'ks.h') diff --git a/ks.h b/ks.h new file mode 100644 index 0000000..ff6d382 --- /dev/null +++ b/ks.h @@ -0,0 +1,241 @@ +// Notes towards unified keystore code (drivers become just low-level +// "disk" I/O and perhaps a bit of local init/shutdown). +// +// Most of the structure definitions in ks_flash.c and ks_volatile.c +// become common and go in ks.h (or wherever, but probably be enough +// stuff that separate .h file might be easier to read). +// +// We already have +// +// typedef struct hal_ks hal_ks_t; +// +// which we "subclass" to get ks_t (ks_volatile) and db_t (ks_flash). +// We can move more common stuff there. +// +// flash_block_t (etc) becomes ks_block_t (etc) as these data +// structures will be used by all keystores, not just flash. +// +// We might want to fold hal_ks_index_t into hal_ks_t as everything +// will be using it. Then again, it's relatively harmless as it is, a +// bit more verbose trading for a bit more isolation. Probably go for +// less verbose, for readability. +// +// Each keystore will still have some weird private stuff, like the +// RAM for the keys themselves in the volatile case and the PIN stuff +// in the flash case. +// +// The ks_flash cache, however, probably wants to become common code. +// Yes we could get a bit more efficient if we skipped caching in the +// volatile case, but that's not our bottleneck and there are some +// cases where the code relies on knowing that mucking with the cache +// copy is harmless until we write the block to "disk", don't want to +// mess with that, so keep the flash model for volatile. Cache size +// will need to become another hal_ks_t field. +// +// Don't remember exactly where we're doing the "subclassing" casts, +// should be easy enough to find...except that ks_flash is mostly +// ignoring that argument and using the static db variable directly. +// ks_volatile may be closer to write on this point, as it already had +// ks_to_ksv(). But most of the code will be in a driver-agnostic +// ks.c (or whatever) and will be calling functions that care through +// the driver, maybe this doesn't matter very much. +// +// Tedious though it sounds, might be simplest just to check each +// function in ks_*.c to see whether it moves to ks.[ch] or becomes +// something called by the new lower-level driver API. Need a sketch +// of the lower-level driver API, chicken and egg there but probably +// is init(), shutdown(), block_read(), block_deprecate(), +// block_zero(), block_erase(), block_erase_maybe(), block-write(). +// Possible that some of these don't really need to be driver, was +// mostly basing this on which things in ks_flash touch flash +// directly-ish via the keystore_*() functions. +// +// Would be nice if we can make the API regular enough (inline +// functions?) that user need not really care which functions are +// driver-specific and which are layered on top, but that may be +// impractical (or silly). +// +// Hmm, hal_ks_open() and hal_ks_close() don't quite fit new model, +// what was I thinking there? Not much, existing implementations just +// use that to get back a (hal_ks_t*), so really just checking the +// binding between driver and keystore object. +// +// I think this boils down to another instance of the confusion +// between what in Python would be Keystore.__new__() and +// Keystore.__init__(). This even sort of fits with the weird `alloc` +// parameter in ks_init(). +// +// Maybe we can trust C memory initialization enough to use a zeroed +// static variable as test for whether a keystore has been +// initialized, and just have the low-level (driver) methods check +// that and fail if trying to use an uninitialized keystore? +// +// Pythonesque view might be the right way to handle ks_init(0 and +// ks_shutdown() too: in most cases we have inline functions which +// call the driver function, but for these methods the subclass needs +// to extend the abstract method, which translates, in C, to the +// generic method calling the driver method of the same name at the +// right time. Not quite what Python does but close enough. + + +#ifndef _KS_H_ +#define _KS_H_ + +#include "hal.h" +#include "hal_internal.h" + +/* + * Size of a keystore "block". + * + * This must be an integer multiple of the flash subsector size, among + * other reasons because that's the minimum erasable unit. + */ + +#ifndef HAL_KS_BLOCK_SIZE +#define HAL_KS_BLOCK_SIZE (KEYSTORE_SUBSECTOR_SIZE * 1) +#endif + +/* + * Known block states. + * + * C does not guarantee any particular representation for enums, so + * including enums directly in the block header isn't safe. Instead, + * we use an access method which casts when reading from the header. + * Writing to the header isn't a problem, because C does guarantee + * that enum is compatible with *some* integer type, it just doesn't + * specify which one. + */ + +typedef enum { + KS_BLOCK_TYPE_ERASED = 0xFF, /* Pristine erased block (candidate for reuse) */ + KS_BLOCK_TYPE_ZEROED = 0x00, /* Zeroed block (recently used) */ + KS_BLOCK_TYPE_KEY = 0x55, /* Block contains key material */ + KS_BLOCK_TYPE_PIN = 0xAA, /* Block contains PINs */ + KS_BLOCK_TYPE_UNKNOWN = -1, /* Internal code for "I have no clue what this is" */ +} ks_block_type_t; + +/* + * Block status. + */ + +typedef enum { + KS_BLOCK_STATUS_LIVE = 0x66, /* This is a live block */ + KS_BLOCK_STATUS_TOMBSTONE = 0x44, /* This is a tombstone left behind during an update */ + KS_BLOCK_STATUS_UNKNOWN = -1, /* Internal code for "I have no clue what this is" */ +} ks_block_status_t; + +/* + * Common header for all keystore block types. + * A few of these fields are deliberately omitted from the CRC. + */ + +typedef struct { + uint8_t block_type; + uint8_t block_status; + hal_crc32_t crc; +} ks_block_header_t; + +/* + * Key block. Tail end of "der" field (after der_len) used for attributes. + */ + +typedef struct { + ks_block_header_t header; + hal_uuid_t name; + hal_key_type_t type; + hal_curve_name_t curve; + hal_key_flags_t flags; + size_t der_len; + unsigned attributes_len; + uint8_t der[]; /* Must be last field -- C99 "flexible array member" */ +} ks_blockkey_block_t; + +#define SIZEOF_KS_BLOCKKEY_BLOCK_DER \ + (HAL_KS_BLOCK_SIZE - offsetof(ks_blockkey_block_t, der)) + +/* + * PIN block. Also includes space for backing up the KEK when + * HAL_MKM_FLASH_BACKUP_KLUDGE is enabled. + */ + +typedef struct { + ks_block_header_t header; + hal_ks_pin_t wheel_pin; + hal_ks_pin_t so_pin; + hal_ks_pin_t user_pin; +#if HAL_MKM_FLASH_BACKUP_KLUDGE + uint32_t kek_set; + uint8_t kek[KEK_LENGTH]; +#endif +} ks_blockpin_block_t; + +#define FLASH_KEK_SET 0x33333333 + +/* + * One keystore block. + */ + +typedef union { + uint8_t bytes[HAL_KS_BLOCK_SIZE]; + ks_block_header_t header; + ks_blockkey_block_t key; + ks_blockpin_block_t pin; +} ks_block_t; + +/* + * In-memory cache. + */ + +typedef struct { + unsigned blockno; + unsigned lru; + ks_block_t block; +} ks_cache_block_t; + +/* + * Medium-specific driver and in-memory database. + * + * The top-level structure is a static variable; the arrays are + * allocated at runtime using hal_allocate_static_memory() because + * they can get kind of large. + * + * Driver-specific stuff is handled by a form of subclassing: the + * driver embeds the hal_ks_t structure at the head of whatever else + * it needs, and performs (controlled, type-safe) casts as needed. + */ + +typedef struct hal_ks_driver hal_ks_driver_t; +typedef struct hal_ks hal_ks_t; + +struct hal_ks { + const hal_ks_driver_t *driver; + unsigned size; /* Blocks in keystore */ + unsigned used; /* How many blocks are in use */ + uint16_t *index; /* Index/freelist array */ + hal_uuid_t *names; /* Keyname array */ + unsigned cache_lru; /* Cache LRU counter */ + unsigned cache_size; /* Size (how many blocks) in cache */ + ks_cache_block_t *cache; /* Cache */ + int per_session; /* Whether objects have per-session semantics (PKCS #11, sigh) */ +}; + +struct hal_ks_driver { + hal_error_t (*init) (hal_ks_t *, const int alloc); + hal_error_t (*shutdown) (hal_ks_t *); + hal_error_t (*read) (hal_ks_t *, const unsigned blockno, ks_block_t *); + hal_error_t (*write) (hal_ks_t *, const unsigned blockno, ks_block_t *) + hal_error_t (*deprecate) (hal_ks_t *, const unsigned blockno); + hal_error_t (*zero) (hal_ks_t *, const unsigned blockno); + hal_error_t (*erase) (hal_ks_t *, const unsigned blockno); + hal_error_t (*erase_maybe) (hal_ks_t *, const unsigned blockno); + hal_error_t (*get_owner) (hal_ks_t *, const unsigned blockno, hal_client_handle_t *, hal_session_handle_t *); + hal_error_t (*set_owner) (hal_ks_t *, const unsigned blockno, const hal_client_handle_t, const hal_session_handle_t); +}; + +#endif /* _KS_H_ */ + +/* + * Local variables: + * indent-tabs-mode: nil + * End: + */ -- cgit v1.2.3 From c1b19879e13e5717867c73b3273b0fbdeea88c01 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Wed, 24 May 2017 22:36:55 -0400 Subject: Type name cleanup, key visibility. --- ks.h | 168 ++++++++++++++++++++++++------------------------------------------- 1 file changed, 61 insertions(+), 107 deletions(-) (limited to 'ks.h') diff --git a/ks.h b/ks.h index ff6d382..3db2e66 100644 --- a/ks.h +++ b/ks.h @@ -1,82 +1,36 @@ -// Notes towards unified keystore code (drivers become just low-level -// "disk" I/O and perhaps a bit of local init/shutdown). -// -// Most of the structure definitions in ks_flash.c and ks_volatile.c -// become common and go in ks.h (or wherever, but probably be enough -// stuff that separate .h file might be easier to read). -// -// We already have -// -// typedef struct hal_ks hal_ks_t; -// -// which we "subclass" to get ks_t (ks_volatile) and db_t (ks_flash). -// We can move more common stuff there. -// -// flash_block_t (etc) becomes ks_block_t (etc) as these data -// structures will be used by all keystores, not just flash. -// -// We might want to fold hal_ks_index_t into hal_ks_t as everything -// will be using it. Then again, it's relatively harmless as it is, a -// bit more verbose trading for a bit more isolation. Probably go for -// less verbose, for readability. -// -// Each keystore will still have some weird private stuff, like the -// RAM for the keys themselves in the volatile case and the PIN stuff -// in the flash case. -// -// The ks_flash cache, however, probably wants to become common code. -// Yes we could get a bit more efficient if we skipped caching in the -// volatile case, but that's not our bottleneck and there are some -// cases where the code relies on knowing that mucking with the cache -// copy is harmless until we write the block to "disk", don't want to -// mess with that, so keep the flash model for volatile. Cache size -// will need to become another hal_ks_t field. -// -// Don't remember exactly where we're doing the "subclassing" casts, -// should be easy enough to find...except that ks_flash is mostly -// ignoring that argument and using the static db variable directly. -// ks_volatile may be closer to write on this point, as it already had -// ks_to_ksv(). But most of the code will be in a driver-agnostic -// ks.c (or whatever) and will be calling functions that care through -// the driver, maybe this doesn't matter very much. -// -// Tedious though it sounds, might be simplest just to check each -// function in ks_*.c to see whether it moves to ks.[ch] or becomes -// something called by the new lower-level driver API. Need a sketch -// of the lower-level driver API, chicken and egg there but probably -// is init(), shutdown(), block_read(), block_deprecate(), -// block_zero(), block_erase(), block_erase_maybe(), block-write(). -// Possible that some of these don't really need to be driver, was -// mostly basing this on which things in ks_flash touch flash -// directly-ish via the keystore_*() functions. -// -// Would be nice if we can make the API regular enough (inline -// functions?) that user need not really care which functions are -// driver-specific and which are layered on top, but that may be -// impractical (or silly). -// -// Hmm, hal_ks_open() and hal_ks_close() don't quite fit new model, -// what was I thinking there? Not much, existing implementations just -// use that to get back a (hal_ks_t*), so really just checking the -// binding between driver and keystore object. -// -// I think this boils down to another instance of the confusion -// between what in Python would be Keystore.__new__() and -// Keystore.__init__(). This even sort of fits with the weird `alloc` -// parameter in ks_init(). -// -// Maybe we can trust C memory initialization enough to use a zeroed -// static variable as test for whether a keystore has been -// initialized, and just have the low-level (driver) methods check -// that and fail if trying to use an uninitialized keystore? -// -// Pythonesque view might be the right way to handle ks_init(0 and -// ks_shutdown() too: in most cases we have inline functions which -// call the driver function, but for these methods the subclass needs -// to extend the abstract method, which translates, in C, to the -// generic method calling the driver method of the same name at the -// right time. Not quite what Python does but close enough. - +/* + * ks.h + * ---- + * Keystore, generic parts anyway. This is internal within libhal. + * + * Copyright (c) 2015-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. + */ #ifndef _KS_H_ #define _KS_H_ @@ -107,22 +61,22 @@ */ typedef enum { - KS_BLOCK_TYPE_ERASED = 0xFF, /* Pristine erased block (candidate for reuse) */ - KS_BLOCK_TYPE_ZEROED = 0x00, /* Zeroed block (recently used) */ - KS_BLOCK_TYPE_KEY = 0x55, /* Block contains key material */ - KS_BLOCK_TYPE_PIN = 0xAA, /* Block contains PINs */ - KS_BLOCK_TYPE_UNKNOWN = -1, /* Internal code for "I have no clue what this is" */ -} ks_block_type_t; + HAL_KS_BLOCK_TYPE_ERASED = 0xFF, /* Pristine erased block (candidate for reuse) */ + HAL_KS_BLOCK_TYPE_ZEROED = 0x00, /* Zeroed block (recently used) */ + HAL_KS_BLOCK_TYPE_KEY = 0x55, /* Block contains key material */ + HAL_KS_BLOCK_TYPE_PIN = 0xAA, /* Block contains PINs */ + HAL_KS_BLOCK_TYPE_UNKNOWN = -1, /* Internal code for "I have no clue what this is" */ +} hal_ks_block_type_t; /* * Block status. */ typedef enum { - KS_BLOCK_STATUS_LIVE = 0x66, /* This is a live block */ - KS_BLOCK_STATUS_TOMBSTONE = 0x44, /* This is a tombstone left behind during an update */ - KS_BLOCK_STATUS_UNKNOWN = -1, /* Internal code for "I have no clue what this is" */ -} ks_block_status_t; + HAL_KS_BLOCK_STATUS_LIVE = 0x66, /* This is a live block */ + HAL_KS_BLOCK_STATUS_TOMBSTONE = 0x44, /* This is a tombstone left behind during an update */ + HAL_KS_BLOCK_STATUS_UNKNOWN = -1, /* Internal code for "I have no clue what this is" */ +} hal_ks_block_status_t; /* * Common header for all keystore block types. @@ -133,14 +87,14 @@ typedef struct { uint8_t block_type; uint8_t block_status; hal_crc32_t crc; -} ks_block_header_t; +} hal_ks_block_header_t; /* * Key block. Tail end of "der" field (after der_len) used for attributes. */ typedef struct { - ks_block_header_t header; + hal_ks_block_header_t header; hal_uuid_t name; hal_key_type_t type; hal_curve_name_t curve; @@ -148,10 +102,10 @@ typedef struct { size_t der_len; unsigned attributes_len; uint8_t der[]; /* Must be last field -- C99 "flexible array member" */ -} ks_blockkey_block_t; +} hal_ks_blockkey_block_t; #define SIZEOF_KS_BLOCKKEY_BLOCK_DER \ - (HAL_KS_BLOCK_SIZE - offsetof(ks_blockkey_block_t, der)) + (HAL_KS_BLOCK_SIZE - offsetof(hal_ks_blockkey_block_t, der)) /* * PIN block. Also includes space for backing up the KEK when @@ -159,7 +113,7 @@ typedef struct { */ typedef struct { - ks_block_header_t header; + hal_ks_block_header_t header; hal_ks_pin_t wheel_pin; hal_ks_pin_t so_pin; hal_ks_pin_t user_pin; @@ -167,7 +121,7 @@ typedef struct { uint32_t kek_set; uint8_t kek[KEK_LENGTH]; #endif -} ks_blockpin_block_t; +} hal_ks_blockpin_block_t; #define FLASH_KEK_SET 0x33333333 @@ -176,11 +130,11 @@ typedef struct { */ typedef union { - uint8_t bytes[HAL_KS_BLOCK_SIZE]; - ks_block_header_t header; - ks_blockkey_block_t key; - ks_blockpin_block_t pin; -} ks_block_t; + uint8_t bytes[HAL_KS_BLOCK_SIZE]; + hal_ks_block_header_t header; + hal_ks_blockkey_block_t key; + hal_ks_blockpin_block_t pin; +} hal_ks_block_t; /* * In-memory cache. @@ -189,8 +143,8 @@ typedef union { typedef struct { unsigned blockno; unsigned lru; - ks_block_t block; -} ks_cache_block_t; + hal_ks_block_t block; +} hal_ks_cache_block_t; /* * Medium-specific driver and in-memory database. @@ -215,21 +169,21 @@ struct hal_ks { hal_uuid_t *names; /* Keyname array */ unsigned cache_lru; /* Cache LRU counter */ unsigned cache_size; /* Size (how many blocks) in cache */ - ks_cache_block_t *cache; /* Cache */ + hal_ks_cache_block_t *cache; /* Cache */ int per_session; /* Whether objects have per-session semantics (PKCS #11, sigh) */ }; struct hal_ks_driver { hal_error_t (*init) (hal_ks_t *, const int alloc); hal_error_t (*shutdown) (hal_ks_t *); - hal_error_t (*read) (hal_ks_t *, const unsigned blockno, ks_block_t *); - hal_error_t (*write) (hal_ks_t *, const unsigned blockno, ks_block_t *) + hal_error_t (*read) (hal_ks_t *, const unsigned blockno, hal_ks_block_t *); + hal_error_t (*write) (hal_ks_t *, const unsigned blockno, hal_ks_block_t *) hal_error_t (*deprecate) (hal_ks_t *, const unsigned blockno); hal_error_t (*zero) (hal_ks_t *, const unsigned blockno); hal_error_t (*erase) (hal_ks_t *, const unsigned blockno); hal_error_t (*erase_maybe) (hal_ks_t *, const unsigned blockno); - hal_error_t (*get_owner) (hal_ks_t *, const unsigned blockno, hal_client_handle_t *, hal_session_handle_t *); - hal_error_t (*set_owner) (hal_ks_t *, const unsigned blockno, const hal_client_handle_t, const hal_session_handle_t); + hal_error_t (*set_owner) (hal_ks_t *, const unsigned blockno, const hal_client_handle_t, const hal_session_handle_t); + hal_error_t (*test_owner) (hal_ks_t *, const unsigned blockno, const hal_client_handle_t, const hal_session_handle_t); }; #endif /* _KS_H_ */ -- cgit v1.2.3 From b9565626187cca926c21120786ec575c59f06a05 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Thu, 25 May 2017 01:05:10 -0400 Subject: Fix up ks driver calls and inline wrappers. --- ks.h | 43 +++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) (limited to 'ks.h') diff --git a/ks.h b/ks.h index 3db2e66..2f48738 100644 --- a/ks.h +++ b/ks.h @@ -173,18 +173,37 @@ struct hal_ks { int per_session; /* Whether objects have per-session semantics (PKCS #11, sigh) */ }; -struct hal_ks_driver { - hal_error_t (*init) (hal_ks_t *, const int alloc); - hal_error_t (*shutdown) (hal_ks_t *); - hal_error_t (*read) (hal_ks_t *, const unsigned blockno, hal_ks_block_t *); - hal_error_t (*write) (hal_ks_t *, const unsigned blockno, hal_ks_block_t *) - hal_error_t (*deprecate) (hal_ks_t *, const unsigned blockno); - hal_error_t (*zero) (hal_ks_t *, const unsigned blockno); - hal_error_t (*erase) (hal_ks_t *, const unsigned blockno); - hal_error_t (*erase_maybe) (hal_ks_t *, const unsigned blockno); - hal_error_t (*set_owner) (hal_ks_t *, const unsigned blockno, const hal_client_handle_t, const hal_session_handle_t); - hal_error_t (*test_owner) (hal_ks_t *, const unsigned blockno, const hal_client_handle_t, const hal_session_handle_t); -}; +#define KS_DRIVER_END_LIST +#define KS_DRIVER_METHODS \ + KS_DRIVER_METHOD(read, hal_ks_t *ks, const unsigned blockno, hal_ks_block_t *block) \ + KS_DRIVER_METHOD(write, hal_ks_t *ks, const unsigned blockno, hal_ks_block_t *block) \ + KS_DRIVER_METHOD(deprecate, hal_ks_t *ks, const unsigned blockno) \ + KS_DRIVER_METHOD(zero, hal_ks_t *ks, const unsigned blockno) \ + KS_DRIVER_METHOD(erase, hal_ks_t *ks, const unsigned blockno) \ + KS_DRIVER_METHOD(erase_maybe, hal_ks_t *ks, const unsigned blockno) \ + KS_DRIVER_METHOD(set_owner, hal_ks_t *ks, const unsigned blockno, \ + const hal_client_handle_t client, const hal_session_handle_t session) \ + KS_DRIVER_METHOD(test_owner, hal_ks_t *ks, const unsigned blockno, \ + const hal_client_handle_t client, const hal_session_handle_t session) \ + KS_DRIVER_END_LIST + +#define KS_DRIVER_METHOD(_name_, ...) hal_error_t (*_name_)(__VA_ARGS__) +struct hal_ks_driver { KS_DRIVER_METHODS }; +#undef KS_DRIVER_METHOD + +#define KS_DRIVER_METHOD(_name_, ...) \ + static inline hal_error_t hal_ks_block_##_name_(__VA_ARGS__) \ + { \ + return \ + ks == NULL || ks->driver == NULL ? HAL_ERROR_BAD_ARGUMENTS : \ + ks->driver->_name_ == NULL ? HAL_ERROR_NOT_IMPLEMENTED : \ + ks->driver->_name_(__VA_ARGS__); \ + } +KS_DRIVER_METHODS +#undef KS_DRIVER_METHOD + +#undef KS_DRIVER_METHODS +#undef KS_DRIVER_END_LIST #endif /* _KS_H_ */ -- cgit v1.2.3 From 5eccb3e6d7c27149a0092de48eb21baa495879cb Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Thu, 25 May 2017 11:18:39 -0400 Subject: Checkpoint while refactoring. Almost certainly will not compile. --- ks.h | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 105 insertions(+), 8 deletions(-) (limited to 'ks.h') diff --git a/ks.h b/ks.h index 2f48738..b24f3c0 100644 --- a/ks.h +++ b/ks.h @@ -147,20 +147,45 @@ typedef struct { } hal_ks_cache_block_t; /* - * Medium-specific driver and in-memory database. + * Keystore object. hal_internal.h typedefs this to hal_ks_t. * - * The top-level structure is a static variable; the arrays are - * allocated at runtime using hal_allocate_static_memory() because - * they can get kind of large. + * We expect this to be a static variable, but we expect the arrays in + * it to be allocated at runtime using hal_allocate_static_memory() + * because they can get kind of large. * * Driver-specific stuff is handled by a form of subclassing: the * driver embeds the hal_ks_t structure at the head of whatever else * it needs, and performs (controlled, type-safe) casts as needed. + * + * Core of this is the keystore index. This is intended to be usable + * by both memory-based and flash-based keystores. Some of the + * features aren't necessary for memory-based keystores, but should be + * harmless, and let us keep the drivers simple. + * + * General approach is multiple arrays, all but one of which are + * indexed by "block" numbers, where a block number might be a slot in + * yet another static array, the number of a flash sub-sector, or + * whatever is the appropriate unit for holding one keystore record. + * + * The index array only contains block numbers. This is a small data + * structure so that moving data within it is relatively cheap. + * + * The index array is divided into two portions: the index proper, and + * the free queue. The index proper is ordered according to the names + * (UUIDs) of the corresponding blocks; the free queue is a FIFO, to + * support a simplistic form of wear leveling in flash-based keystores. + * + * Key names are kept in a separate array, indexed by block number. + * + * The all-zeros UUID, which (by definition) cannot be a valid key + * UUID, is reserved for the (non-key) block used to stash PINs and + * other small data which aren't really part of the keystore proper + * but are kept with it because the keystore is the flash we have. + * + * Note that this API deliberately says nothing about how the keys + * themselves are stored, that's up to the keystore driver. */ -typedef struct hal_ks_driver hal_ks_driver_t; -typedef struct hal_ks hal_ks_t; - struct hal_ks { const hal_ks_driver_t *driver; unsigned size; /* Blocks in keystore */ @@ -173,6 +198,19 @@ struct hal_ks { int per_session; /* Whether objects have per-session semantics (PKCS #11, sigh) */ }; +/* + * Keystore driver. This is just a dispatch vector for low-level + * keystore operations, and the code is very repetitive. We opt for + * expressing this in a terse form via C macros over expressing it + * as huge chunks of repetitive code: both are difficult to read, but + * the terse form has the advantage of fitting in a single screen. + * The KS_DRIVER_METHODS macro is the protein, the rest is just the + * machinery to expand the method definitions into a struct of typed + * function pointers and a set of static inline wrapper functions. + */ + +typedef struct hal_ks_driver hal_ks_driver_t; + #define KS_DRIVER_END_LIST #define KS_DRIVER_METHODS \ KS_DRIVER_METHOD(read, hal_ks_t *ks, const unsigned blockno, hal_ks_block_t *block) \ @@ -187,7 +225,7 @@ struct hal_ks { const hal_client_handle_t client, const hal_session_handle_t session) \ KS_DRIVER_END_LIST -#define KS_DRIVER_METHOD(_name_, ...) hal_error_t (*_name_)(__VA_ARGS__) +#define KS_DRIVER_METHOD(_name_, ...) hal_error_t (*_name_)(__VA_ARGS__); struct hal_ks_driver { KS_DRIVER_METHODS }; #undef KS_DRIVER_METHOD @@ -207,6 +245,65 @@ KS_DRIVER_METHODS #endif /* _KS_H_ */ +/* + * Keystore utilities. Some or all of these may end up static within ks.c. + */ + +extern hal_error_t hal_ks_alloc_common(hal_ks_t *ks, + const unsigned ks_blocks, + const unsigned cache_blocks); + +extern hal_error_t hal_ks_init_common(hal_ks_t *ks, + const hal_ks_driver_t * const driver); + +extern hal_error_t hal_ks_index_heapsort(hal_ks_t *ks); + +extern hal_error_t hal_ks_index_find(hal_ks_t *ks, + const hal_uuid_t * const name, + unsigned *blockno, + int *hint); + +extern hal_error_t hal_ks_index_add(hal_ks_t *ks, + const hal_uuid_t * const name, + unsigned *blockno, + int *hint); + +extern hal_error_t hal_ks_index_delete(hal_ks_t *ks, + const hal_uuid_t * const name, + unsigned *blockno, + int *hint); + +extern hal_error_t hal_ks_index_replace(hal_ks_t *ks, + const hal_uuid_t * const name, + unsigned *blockno, + int *hint); + +extern hal_error_t hal_ks_index_fsck(hal_ks_t *ks); + +extern const size_t hal_ks_attribute_header_size; + +extern hal_error_t hal_ks_attribute_scan(const uint8_t * const bytes, + const size_t bytes_len, + hal_pkey_attribute_t *attributes, + const unsigned attributes_len, + size_t *total_len); + +extern hal_error_t hal_ks_attribute_delete(uint8_t *bytes, + const size_t bytes_len, + hal_pkey_attribute_t *attributes, + unsigned *attributes_len, + size_t *total_len, + const uint32_t type); + +extern hal_error_t hal_ks_attribute_insert(uint8_t *bytes, const size_t bytes_len, + hal_pkey_attribute_t *attributes, + unsigned *attributes_len, + size_t *total_len, + const uint32_t type, + const uint8_t * const value, + const size_t value_len); + + /* * Local variables: * indent-tabs-mode: nil -- cgit v1.2.3 From f59533ee9807832ea5ca7dd5492592c8991a9f34 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Sun, 28 May 2017 12:11:31 -0400 Subject: Further keystore cleanup and consolidation. Still not yet expected to compile, much less run, but getting closer. --- ks.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'ks.h') diff --git a/ks.h b/ks.h index b24f3c0..29965cc 100644 --- a/ks.h +++ b/ks.h @@ -186,8 +186,10 @@ typedef struct { * themselves are stored, that's up to the keystore driver. */ +typedef struct hal_ks_driver hal_ks_driver_t; + struct hal_ks { - const hal_ks_driver_t *driver; + const hal_ks_driver_t *driver;/* Must be first */ unsigned size; /* Blocks in keystore */ unsigned used; /* How many blocks are in use */ uint16_t *index; /* Index/freelist array */ @@ -209,10 +211,9 @@ struct hal_ks { * function pointers and a set of static inline wrapper functions. */ -typedef struct hal_ks_driver hal_ks_driver_t; - #define KS_DRIVER_END_LIST #define KS_DRIVER_METHODS \ + KS_DRIVER_METHOD(init, hal_ks_t *ks, const int alloc) \ KS_DRIVER_METHOD(read, hal_ks_t *ks, const unsigned blockno, hal_ks_block_t *block) \ KS_DRIVER_METHOD(write, hal_ks_t *ks, const unsigned blockno, hal_ks_block_t *block) \ KS_DRIVER_METHOD(deprecate, hal_ks_t *ks, const unsigned blockno) \ -- cgit v1.2.3 From 2caa6c72640877abc5f3572c4d926a23ff672ab1 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Sun, 28 May 2017 16:11:25 -0400 Subject: Almost compiles. Need to refactor init sequence slightly (again), this time to humor the bootloader, which has its own special read-only view of the PIN block in the token keystore. --- ks.h | 205 +++++++++++++++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 150 insertions(+), 55 deletions(-) (limited to 'ks.h') diff --git a/ks.h b/ks.h index 29965cc..6db0bd7 100644 --- a/ks.h +++ b/ks.h @@ -46,9 +46,16 @@ */ #ifndef HAL_KS_BLOCK_SIZE -#define HAL_KS_BLOCK_SIZE (KEYSTORE_SUBSECTOR_SIZE * 1) +#define HAL_KS_BLOCK_SIZE (4096) #endif +/* + * PIN block gets the all-zeros UUID, which will never be returned by + * the UUID generation code (by definition -- it's not a version 4 UUID). + */ + +const hal_uuid_t hal_ks_pin_uuid; + /* * Known block states. * @@ -94,7 +101,7 @@ typedef struct { */ typedef struct { - hal_ks_block_header_t header; + hal_ks_block_header_t header; hal_uuid_t name; hal_key_type_t type; hal_curve_name_t curve; @@ -102,10 +109,10 @@ typedef struct { size_t der_len; unsigned attributes_len; uint8_t der[]; /* Must be last field -- C99 "flexible array member" */ -} hal_ks_blockkey_block_t; +} hal_ks_key_block_t; -#define SIZEOF_KS_BLOCKKEY_BLOCK_DER \ - (HAL_KS_BLOCK_SIZE - offsetof(hal_ks_blockkey_block_t, der)) +#define SIZEOF_KS_KEY_BLOCK_DER \ + (HAL_KS_BLOCK_SIZE - offsetof(hal_ks_key_block_t, der)) /* * PIN block. Also includes space for backing up the KEK when @@ -113,7 +120,7 @@ typedef struct { */ typedef struct { - hal_ks_block_header_t header; + hal_ks_block_header_t header; hal_ks_pin_t wheel_pin; hal_ks_pin_t so_pin; hal_ks_pin_t user_pin; @@ -121,7 +128,7 @@ typedef struct { uint32_t kek_set; uint8_t kek[KEK_LENGTH]; #endif -} hal_ks_blockpin_block_t; +} hal_ks_pin_block_t; #define FLASH_KEK_SET 0x33333333 @@ -130,10 +137,10 @@ typedef struct { */ typedef union { - uint8_t bytes[HAL_KS_BLOCK_SIZE]; + uint8_t bytes[HAL_KS_BLOCK_SIZE]; hal_ks_block_header_t header; - hal_ks_blockkey_block_t key; - hal_ks_blockpin_block_t pin; + hal_ks_key_block_t key; + hal_ks_pin_block_t pin; } hal_ks_block_t; /* @@ -143,7 +150,7 @@ typedef union { typedef struct { unsigned blockno; unsigned lru; - hal_ks_block_t block; + hal_ks_block_t block; } hal_ks_cache_block_t; /* @@ -201,50 +208,112 @@ struct hal_ks { }; /* - * Keystore driver. This is just a dispatch vector for low-level - * keystore operations, and the code is very repetitive. We opt for - * expressing this in a terse form via C macros over expressing it - * as huge chunks of repetitive code: both are difficult to read, but - * the terse form has the advantage of fitting in a single screen. - * The KS_DRIVER_METHODS macro is the protein, the rest is just the - * machinery to expand the method definitions into a struct of typed - * function pointers and a set of static inline wrapper functions. + * Keystore driver. */ -#define KS_DRIVER_END_LIST -#define KS_DRIVER_METHODS \ - KS_DRIVER_METHOD(init, hal_ks_t *ks, const int alloc) \ - KS_DRIVER_METHOD(read, hal_ks_t *ks, const unsigned blockno, hal_ks_block_t *block) \ - KS_DRIVER_METHOD(write, hal_ks_t *ks, const unsigned blockno, hal_ks_block_t *block) \ - KS_DRIVER_METHOD(deprecate, hal_ks_t *ks, const unsigned blockno) \ - KS_DRIVER_METHOD(zero, hal_ks_t *ks, const unsigned blockno) \ - KS_DRIVER_METHOD(erase, hal_ks_t *ks, const unsigned blockno) \ - KS_DRIVER_METHOD(erase_maybe, hal_ks_t *ks, const unsigned blockno) \ - KS_DRIVER_METHOD(set_owner, hal_ks_t *ks, const unsigned blockno, \ - const hal_client_handle_t client, const hal_session_handle_t session) \ - KS_DRIVER_METHOD(test_owner, hal_ks_t *ks, const unsigned blockno, \ - const hal_client_handle_t client, const hal_session_handle_t session) \ - KS_DRIVER_END_LIST - -#define KS_DRIVER_METHOD(_name_, ...) hal_error_t (*_name_)(__VA_ARGS__); -struct hal_ks_driver { KS_DRIVER_METHODS }; -#undef KS_DRIVER_METHOD - -#define KS_DRIVER_METHOD(_name_, ...) \ - static inline hal_error_t hal_ks_block_##_name_(__VA_ARGS__) \ - { \ - return \ - ks == NULL || ks->driver == NULL ? HAL_ERROR_BAD_ARGUMENTS : \ - ks->driver->_name_ == NULL ? HAL_ERROR_NOT_IMPLEMENTED : \ - ks->driver->_name_(__VA_ARGS__); \ - } -KS_DRIVER_METHODS -#undef KS_DRIVER_METHOD - -#undef KS_DRIVER_METHODS -#undef KS_DRIVER_END_LIST +struct hal_ks_driver { + hal_error_t (*init) (hal_ks_t *ks, const int alloc); + hal_error_t (*read) (hal_ks_t *ks, const unsigned blockno, hal_ks_block_t *block); + hal_error_t (*write) (hal_ks_t *ks, const unsigned blockno, hal_ks_block_t *block); + hal_error_t (*deprecate) (hal_ks_t *ks, const unsigned blockno); + hal_error_t (*zero) (hal_ks_t *ks, const unsigned blockno); + hal_error_t (*erase) (hal_ks_t *ks, const unsigned blockno); + hal_error_t (*erase_maybe) (hal_ks_t *ks, const unsigned blockno); + hal_error_t (*set_owner) (hal_ks_t *ks, const unsigned blockno, + const hal_client_handle_t client, const hal_session_handle_t session); + hal_error_t (*test_owner) (hal_ks_t *ks, const unsigned blockno, + const hal_client_handle_t client, const hal_session_handle_t session); +}; -#endif /* _KS_H_ */ +/* + * Wrappers around keystore driver methods. + * + * hal_ks_init() is missing here because we expose it to the rest of libhal. + */ + +static inline hal_error_t hal_ks_block_read(hal_ks_t *ks, const unsigned blockno, hal_ks_block_t *block) +{ + return + ks == NULL || ks->driver == NULL ? HAL_ERROR_BAD_ARGUMENTS : + ks->driver->read == NULL ? HAL_ERROR_NOT_IMPLEMENTED : + ks->driver->read(ks, blockno, block); +} + +static inline hal_error_t hal_ks_block_write(hal_ks_t *ks, const unsigned blockno, hal_ks_block_t *block) +{ + return + ks == NULL || ks->driver == NULL ? HAL_ERROR_BAD_ARGUMENTS : + ks->driver->write == NULL ? HAL_ERROR_NOT_IMPLEMENTED : + ks->driver->write(ks, blockno, block); +} + +static inline hal_error_t hal_ks_block_deprecate(hal_ks_t *ks, const unsigned blockno) +{ + return + ks == NULL || ks->driver == NULL ? HAL_ERROR_BAD_ARGUMENTS : + ks->driver->deprecate == NULL ? HAL_ERROR_NOT_IMPLEMENTED : + ks->driver->deprecate(ks, blockno); +} + +static inline hal_error_t hal_ks_block_zero(hal_ks_t *ks, const unsigned blockno) +{ + return + ks == NULL || ks->driver == NULL ? HAL_ERROR_BAD_ARGUMENTS : + ks->driver->zero == NULL ? HAL_ERROR_NOT_IMPLEMENTED : + ks->driver->zero(ks, blockno); +} + +static inline hal_error_t hal_ks_block_erase(hal_ks_t *ks, const unsigned blockno) +{ + return + ks == NULL || ks->driver == NULL ? HAL_ERROR_BAD_ARGUMENTS : + ks->driver->erase == NULL ? HAL_ERROR_NOT_IMPLEMENTED : + ks->driver->erase(ks, blockno); +} + +static inline hal_error_t hal_ks_block_erase_maybe(hal_ks_t *ks, const unsigned blockno) +{ + return + ks == NULL || ks->driver == NULL ? HAL_ERROR_BAD_ARGUMENTS : + ks->driver->erase_maybe == NULL ? HAL_ERROR_NOT_IMPLEMENTED : + ks->driver->erase_maybe(ks, blockno); +} + +static inline hal_error_t hal_ks_block_set_owner(hal_ks_t *ks, const unsigned blockno, + const hal_client_handle_t client, + const hal_session_handle_t session) +{ + return + ks == NULL || ks->driver == NULL ? HAL_ERROR_BAD_ARGUMENTS : + ks->driver->set_owner == NULL ? HAL_ERROR_NOT_IMPLEMENTED : + ks->driver->set_owner(ks, blockno, client, session); +} + +static inline hal_error_t hal_ks_block_test_owner(hal_ks_t *ks, const unsigned blockno, + const hal_client_handle_t client, + const hal_session_handle_t session) +{ + return + ks == NULL || ks->driver == NULL ? HAL_ERROR_BAD_ARGUMENTS : + ks->driver->test_owner == NULL ? HAL_ERROR_NOT_IMPLEMENTED : + ks->driver->test_owner(ks, blockno, client, session); +} + +/* + * Type safe casts. + */ + +static inline hal_ks_block_type_t hal_ks_block_get_type(const hal_ks_block_t * const block) +{ + return block == NULL ? HAL_KS_BLOCK_TYPE_UNKNOWN : + (hal_ks_block_type_t) block->header.block_type; +} + +static inline hal_ks_block_status_t hal_ks_block_get_status(const hal_ks_block_t * const block) +{ + return block == NULL ? HAL_KS_BLOCK_STATUS_UNKNOWN : + (hal_ks_block_status_t) block->header.block_status; +} /* * Keystore utilities. Some or all of these may end up static within ks.c. @@ -252,10 +321,13 @@ KS_DRIVER_METHODS extern hal_error_t hal_ks_alloc_common(hal_ks_t *ks, const unsigned ks_blocks, - const unsigned cache_blocks); + const unsigned cache_blocks, + void **extra, + const size_t extra_len); -extern hal_error_t hal_ks_init_common(hal_ks_t *ks, - const hal_ks_driver_t * const driver); +extern hal_error_t hal_ks_init_common(hal_ks_t *ks); + +extern hal_crc32_t hal_ks_block_calculate_crc(const hal_ks_block_t * const block); extern hal_error_t hal_ks_index_heapsort(hal_ks_t *ks); @@ -304,6 +376,29 @@ extern hal_error_t hal_ks_attribute_insert(uint8_t *bytes, const size_t bytes_le const uint8_t * const value, const size_t value_len); +extern hal_ks_block_t *hal_ks_cache_pick_lru(hal_ks_t *ks); + +extern hal_ks_block_t *hal_ks_cache_find_block(const hal_ks_t * const ks, + const unsigned blockno); + +extern void hal_ks_cache_mark_used(hal_ks_t *ks, + const hal_ks_block_t * const block, + const unsigned blockno); + +extern void hal_ks_cache_release(hal_ks_t *ks, + const hal_ks_block_t * const block); + +extern hal_error_t hal_ks_block_read_cached(hal_ks_t *ks, + const unsigned blockno, + hal_ks_block_t **block); + +extern hal_error_t hal_ks_block_update(hal_ks_t *ks, + const unsigned b1, + hal_ks_block_t *block, + const hal_uuid_t * const uuid, + int *hint); + +#endif /* _KS_H_ */ /* * Local variables: -- cgit v1.2.3 From 5cee716555db92942c5b11c824839bb00aaf35b9 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Mon, 29 May 2017 00:44:18 -0400 Subject: Debug per-session keys. --- ks.h | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'ks.h') diff --git a/ks.h b/ks.h index 6db0bd7..25f2acf 100644 --- a/ks.h +++ b/ks.h @@ -223,6 +223,7 @@ struct hal_ks_driver { const hal_client_handle_t client, const hal_session_handle_t session); hal_error_t (*test_owner) (hal_ks_t *ks, const unsigned blockno, const hal_client_handle_t client, const hal_session_handle_t session); + hal_error_t (*copy_owner) (hal_ks_t *ks, const unsigned source, const unsigned target); }; /* @@ -299,6 +300,16 @@ static inline hal_error_t hal_ks_block_test_owner(hal_ks_t *ks, const unsigned b ks->driver->test_owner(ks, blockno, client, session); } +static inline hal_error_t hal_ks_block_copy_owner(hal_ks_t *ks, + const unsigned source, + const unsigned target) +{ + return + ks == NULL || ks->driver == NULL ? HAL_ERROR_BAD_ARGUMENTS : + ks->driver->copy_owner == NULL ? HAL_ERROR_NOT_IMPLEMENTED : + ks->driver->copy_owner(ks, source, target); +} + /* * Type safe casts. */ -- cgit v1.2.3 From d2633bb4155c6798949e92d8113bc036b942a018 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Mon, 29 May 2017 12:44:12 -0400 Subject: Indentation. --- ks.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'ks.h') diff --git a/ks.h b/ks.h index 25f2acf..44581a2 100644 --- a/ks.h +++ b/ks.h @@ -111,7 +111,7 @@ typedef struct { uint8_t der[]; /* Must be last field -- C99 "flexible array member" */ } hal_ks_key_block_t; -#define SIZEOF_KS_KEY_BLOCK_DER \ +#define SIZEOF_KS_KEY_BLOCK_DER \ (HAL_KS_BLOCK_SIZE - offsetof(hal_ks_key_block_t, der)) /* @@ -390,14 +390,14 @@ extern hal_error_t hal_ks_attribute_insert(uint8_t *bytes, const size_t bytes_le extern hal_ks_block_t *hal_ks_cache_pick_lru(hal_ks_t *ks); extern hal_ks_block_t *hal_ks_cache_find_block(const hal_ks_t * const ks, - const unsigned blockno); + const unsigned blockno); extern void hal_ks_cache_mark_used(hal_ks_t *ks, - const hal_ks_block_t * const block, - const unsigned blockno); + const hal_ks_block_t * const block, + const unsigned blockno); extern void hal_ks_cache_release(hal_ks_t *ks, - const hal_ks_block_t * const block); + const hal_ks_block_t * const block); extern hal_error_t hal_ks_block_read_cached(hal_ks_t *ks, const unsigned blockno, -- cgit v1.2.3 From 776c4e8cfed92bc2d894f002cb7d222abc65bb50 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Mon, 29 May 2017 13:16:14 -0400 Subject: Simplify per-session keys. Cosmetic cleanup of pkey_slot along the way. --- ks.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'ks.h') diff --git a/ks.h b/ks.h index 44581a2..240d3e6 100644 --- a/ks.h +++ b/ks.h @@ -204,7 +204,6 @@ struct hal_ks { unsigned cache_lru; /* Cache LRU counter */ unsigned cache_size; /* Size (how many blocks) in cache */ hal_ks_cache_block_t *cache; /* Cache */ - int per_session; /* Whether objects have per-session semantics (PKCS #11, sigh) */ }; /* @@ -280,7 +279,8 @@ static inline hal_error_t hal_ks_block_erase_maybe(hal_ks_t *ks, const unsigned ks->driver->erase_maybe(ks, blockno); } -static inline hal_error_t hal_ks_block_set_owner(hal_ks_t *ks, const unsigned blockno, +static inline hal_error_t hal_ks_block_set_owner(hal_ks_t *ks, + const unsigned blockno, const hal_client_handle_t client, const hal_session_handle_t session) { @@ -290,7 +290,8 @@ static inline hal_error_t hal_ks_block_set_owner(hal_ks_t *ks, const unsigned bl ks->driver->set_owner(ks, blockno, client, session); } -static inline hal_error_t hal_ks_block_test_owner(hal_ks_t *ks, const unsigned blockno, +static inline hal_error_t hal_ks_block_test_owner(hal_ks_t *ks, + const unsigned blockno, const hal_client_handle_t client, const hal_session_handle_t session) { -- cgit v1.2.3 From a83d9dfba5f882ca75eaab9a166e6ad9794f2f90 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Sun, 4 Jun 2017 12:21:45 -0400 Subject: Tweak CRC input to be backwards compatabile with ksng. Except for torture tests, we never really used the hideously complex multi-block capabilities of the ksng version of the flash keystore, among other reasons because the only keys large enough to trigger the multi-block code were slow enough to constitute torture on their own. So we can preserve backwards compatabliity simply by including the former *chunk fields (renamed legacy* here) in the CRC and checking for the expected single-block key values. We probably want to include everything in the CRC in any case except when there's an explicit reason omit something, so, this is cheap, just a bit obscure. At some point in the future we can phase out support for the backwards compatible values, but there's no particular hurry about it unless we want to reuse those fields for some other purpose. --- ks.h | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'ks.h') diff --git a/ks.h b/ks.h index 1c09b53..b95216d 100644 --- a/ks.h +++ b/ks.h @@ -86,13 +86,22 @@ typedef enum { } hal_ks_block_status_t; /* - * Common header for all keystore block types. - * A few of these fields are deliberately omitted from the CRC. + * Common header for all keystore block types. A few of these fields + * are deliberately omitted from the CRC. + * + * The legacy_1 and legacy_2 fields were used in the more complex + * "chunked" layout used in an earlier iteration of this keystore + * design, which proved more complex than it was worth. At the + * moment, the only thing we do with these fields is include them in + * the CRC and check them for allowed values, to avoid gratuitously + * breaking backwards compatability with the earlier design. */ typedef struct { uint8_t block_type; uint8_t block_status; + uint8_t legacy_1; + uint8_t legacy_2; hal_crc32_t crc; } hal_ks_block_header_t; -- cgit v1.2.3