From 5522df4f68bfa66b9b4446fdfb92783694586f70 Mon Sep 17 00:00:00 2001 From: Rob Austein Date: Wed, 13 Sep 2017 11:28:13 -0400 Subject: Sort-of-working, large (4096-bit) RSA keys broken. Snapshot of mostly but not entirely working code to include the extra ModExpA7 key components in the keystore. Need to investigate whether a more compact representation is practical for these components, as the current one bloats the key object so much that a bare 4096-bit key won't fit in a single hash block, and there may not be enough room for PKCS #11 attributes even for smaller keys. If more compact representation not possible or insufficient, the other option is to double the size of a keystore object, making it two flash subsectors for a total of 8192 octets. Which would of course halve the number of keys we can store and require a bunch of little tweaks all through the ks code (particularly flash erase), so definitely worth trying for a more compact representation first. --- hal.h | 14 ++++++- hal_internal.h | 16 ++++++++ ks.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++----------- rpc_pkey.c | 21 ++++++++-- rsa.c | 70 +++++++++++++++++++++++--------- 5 files changed, 197 insertions(+), 47 deletions(-) diff --git a/hal.h b/hal.h index b7eae72..f7a7522 100644 --- a/hal.h +++ b/hal.h @@ -479,8 +479,6 @@ extern hal_error_t hal_rsa_private_key_to_der(const hal_rsa_key_t * const key, extern hal_error_t hal_rsa_private_key_to_der_extra(const hal_rsa_key_t * const key, uint8_t *der, size_t *der_len, const size_t der_max); -extern size_t hal_rsa_private_key_to_der_len(const hal_rsa_key_t * const key); - extern hal_error_t hal_rsa_private_key_from_der(hal_rsa_key_t **key, void *keybuf, const size_t keybuf_len, const uint8_t * const der, const size_t der_len); @@ -496,6 +494,18 @@ extern hal_error_t hal_rsa_public_key_from_der(hal_rsa_key_t **key, extern int hal_rsa_key_needs_saving(const hal_rsa_key_t * const key); +static inline size_t hal_rsa_private_key_to_der_len(const hal_rsa_key_t * const key) +{ + size_t len = 0; + return hal_rsa_private_key_to_der(key, NULL, &len, 0) == HAL_OK ? len : 0; +} + +static inline size_t hal_rsa_private_key_to_der_extra_len(const hal_rsa_key_t * const key) +{ + size_t len = 0; + return hal_rsa_private_key_to_der_extra(key, NULL, &len, 0) == HAL_OK ? len : 0; +} + /* * ECDSA. */ diff --git a/hal_internal.h b/hal_internal.h index 7ab300d..a60d0b5 100644 --- a/hal_internal.h +++ b/hal_internal.h @@ -405,7 +405,19 @@ static inline hal_crc32_t hal_crc32_finalize(hal_crc32_t crc) * moment we take the easy way out and cap this at 4096-bit RSA. */ +#if 0 #define HAL_KS_WRAPPED_KEYSIZE ((2373 + 15) & ~7) +#else +#warning Temporary test hack to HAL_KS_WRAPPED_KEYSIZE, clean this up +// +// See how much of the problem we're having with pkey support for the +// new modexpa7 components is just this buffer size being too small. +// +#define HAL_KS_WRAPPED_KEYSIZE ((2373 + 6 * 4096 / 8 + 6 * 4 + 15) & ~7) +#if HAL_KS_WRAPPED_KEYSIZE + 8 > 4096 +#warning HAL_KS_WRAPPED_KEYSIZE is too big for a single 4096-octet block +#endif +#endif /* * PINs. @@ -566,6 +578,10 @@ extern hal_error_t hal_ks_get_attributes(hal_ks_t *ks, extern hal_error_t hal_ks_logout(hal_ks_t *ks, const hal_client_handle_t client); +extern hal_error_t hal_ks_rewrite_der(hal_ks_t *ks, + hal_pkey_slot_t *slot, + const uint8_t * const der, const size_t der_len); + /* * RPC lowest-level send and receive routines. These are blocking, and * transport-specific (sockets, USB). diff --git a/ks.c b/ks.c index a4e7498..2401a34 100644 --- a/ks.c +++ b/ks.c @@ -518,6 +518,46 @@ static inline int acceptable_key_type(const hal_key_type_t type) } } +/* + * Internal bits of constructing a new key block. + */ + +static hal_error_t construct_key_block(hal_ks_block_t *block, + hal_pkey_slot_t *slot, + const uint8_t * const der, const size_t der_len) +{ + if (block == NULL || slot == NULL || der == NULL || der_len == 0) + return HAL_ERROR_IMPOSSIBLE; + + hal_ks_key_block_t *k = &block->key; + hal_error_t err = HAL_OK; + uint8_t kek[KEK_LENGTH]; + size_t kek_len; + + memset(block, 0xFF, sizeof(*block)); + + block->header.block_type = HAL_KS_BLOCK_TYPE_KEY; + block->header.block_status = HAL_KS_BLOCK_STATUS_LIVE; + + k->name = slot->name; + k->type = slot->type; + k->curve = slot->curve; + k->flags = slot->flags; + k->der_len = SIZEOF_KS_KEY_BLOCK_DER; + k->attributes_len = 0; + + if ((err = hal_mkm_get_kek(kek, &kek_len, sizeof(kek))) == HAL_OK) + err = hal_aes_keywrap(NULL, kek, kek_len, der, der_len, k->der, &k->der_len); + + memset(kek, 0, sizeof(kek)); + + return err; +} + +/* + * Store a key block. + */ + hal_error_t hal_ks_store(hal_ks_t *ks, hal_pkey_slot_t *slot, const uint8_t * const der, const size_t der_len) @@ -527,9 +567,6 @@ hal_error_t hal_ks_store(hal_ks_t *ks, hal_error_t err = HAL_OK; hal_ks_block_t *block; - hal_ks_key_block_t *k; - uint8_t kek[KEK_LENGTH]; - size_t kek_len; unsigned b; hal_ks_lock(); @@ -539,35 +576,16 @@ hal_error_t hal_ks_store(hal_ks_t *ks, goto done; } - k = &block->key; - if ((err = hal_ks_index_add(ks, &slot->name, &b, &slot->hint)) != HAL_OK) goto done; hal_ks_cache_mark_used(ks, block, b); - memset(block, 0xFF, sizeof(*block)); - - block->header.block_type = HAL_KS_BLOCK_TYPE_KEY; - block->header.block_status = HAL_KS_BLOCK_STATUS_LIVE; - - k->name = slot->name; - k->type = slot->type; - k->curve = slot->curve; - k->flags = slot->flags; - k->der_len = SIZEOF_KS_KEY_BLOCK_DER; - k->attributes_len = 0; - if (ks->used < ks->size) err = hal_ks_block_erase_maybe(ks, ks->index[ks->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)); + err = construct_key_block(block, slot, der, der_len); if (err == HAL_OK) err = hal_ks_block_write(ks, b, block); @@ -931,6 +949,65 @@ hal_error_t hal_ks_get_attributes(hal_ks_t *ks, return err; } +hal_error_t hal_ks_rewrite_der(hal_ks_t *ks, + hal_pkey_slot_t *slot, + const uint8_t * const der, const size_t der_len) +{ + if (ks == NULL || slot == NULL || der == NULL || der_len == 0 || !acceptable_key_type(slot->type)) + return HAL_ERROR_BAD_ARGUMENTS; + + hal_ks_block_t *block = NULL; + hal_error_t err = HAL_OK; + unsigned b; + + hal_ks_lock(); + + { + if ((err = hal_ks_index_find(ks, &slot->name, &b, &slot->hint)) != HAL_OK || + (err = hal_ks_block_test_owner(ks, b, slot->client, slot->session)) != HAL_OK || + (err = hal_ks_block_read_cached(ks, b, &block)) != HAL_OK) + goto done; + + hal_ks_cache_mark_used(ks, block, b); + + size_t bytes_len = 0, attributes_len = 0; + unsigned *count = NULL; + uint8_t *bytes = NULL; + + if ((err = locate_attributes(block, &bytes, &bytes_len, &count)) != HAL_OK || + (err = hal_ks_attribute_scan(bytes, bytes_len, NULL, *count, &attributes_len)) != HAL_OK) + goto done; + + if (der_len + attributes_len > SIZEOF_KS_KEY_BLOCK_DER) { + err = HAL_ERROR_RESULT_TOO_LONG; + goto done; + } + + uint8_t attributes[attributes_len > 0 ? attributes_len : 1]; + hal_ks_key_block_t *k = &block->key; + unsigned attributes_count = *count; + + memcpy(attributes, bytes, attributes_len); + + if ((err = construct_key_block(block, slot, der, der_len)) != HAL_OK) + goto done; + + if (k->der_len + attributes_len > SIZEOF_KS_KEY_BLOCK_DER) { + err = HAL_ERROR_IMPOSSIBLE; + goto done; + } + + memcpy(k->der + k->der_len, attributes, attributes_len); + k->attributes_len = attributes_count; + + err = hal_ks_block_update(ks, b, block, &slot->name, &slot->hint); + } + + done: + hal_ks_unlock(); + return err; +} + /* * Local variables: * indent-tabs-mode: nil diff --git a/rpc_pkey.c b/rpc_pkey.c index 3d4a379..53d3214 100644 --- a/rpc_pkey.c +++ b/rpc_pkey.c @@ -734,7 +734,8 @@ static hal_error_t pkey_local_get_public_key(const hal_pkey_handle_t pkey, * algorithm-specific functions. */ -static hal_error_t pkey_local_sign_rsa(uint8_t *keybuf, const size_t keybuf_len, +static hal_error_t pkey_local_sign_rsa(hal_pkey_slot_t *slot, + uint8_t *keybuf, const size_t keybuf_len, const uint8_t * const der, const size_t der_len, const hal_hash_handle_t hash, const uint8_t * input, size_t input_len, @@ -763,10 +764,21 @@ static hal_error_t pkey_local_sign_rsa(uint8_t *keybuf, const size_t keybuf_len, (err = hal_rsa_decrypt(NULL, key, signature, *signature_len, signature, *signature_len)) != HAL_OK) return err; + if (hal_rsa_key_needs_saving(key)) { + uint8_t pkcs8[hal_rsa_private_key_to_der_extra_len(key)]; + size_t pkcs8_len = 0; + if ((err = hal_rsa_private_key_to_der_extra(key, pkcs8, &pkcs8_len, sizeof(pkcs8))) == HAL_OK) + err = hal_ks_rewrite_der(ks_from_flags(slot->flags), slot, pkcs8, pkcs8_len); + memset(pkcs8, 0, sizeof(pkcs8)); + if (err != HAL_OK) + return err; + } + return HAL_OK; } -static hal_error_t pkey_local_sign_ecdsa(uint8_t *keybuf, const size_t keybuf_len, +static hal_error_t pkey_local_sign_ecdsa(hal_pkey_slot_t *slot, + uint8_t *keybuf, const size_t keybuf_len, const uint8_t * const der, const size_t der_len, const hal_hash_handle_t hash, const uint8_t * input, size_t input_len, @@ -813,7 +825,8 @@ static hal_error_t pkey_local_sign(const hal_pkey_handle_t pkey, if (slot == NULL) return HAL_ERROR_KEY_NOT_FOUND; - hal_error_t (*signer)(uint8_t *keybuf, const size_t keybuf_len, + hal_error_t (*signer)(hal_pkey_slot_t *slot, + uint8_t *keybuf, const size_t keybuf_len, const uint8_t * const der, const size_t der_len, const hal_hash_handle_t hash, const uint8_t * const input, const size_t input_len, @@ -840,7 +853,7 @@ static hal_error_t pkey_local_sign(const hal_pkey_handle_t pkey, hal_error_t err; if ((err = ks_fetch_from_flags(slot, der, &der_len, sizeof(der))) == HAL_OK) - err = signer(keybuf, sizeof(keybuf), der, der_len, hash, input, input_len, + err = signer(slot, keybuf, sizeof(keybuf), der, der_len, hash, input, input_len, signature, signature_len, signature_max); memset(keybuf, 0, sizeof(keybuf)); diff --git a/rsa.c b/rsa.c index 24dc66f..dace19b 100644 --- a/rsa.c +++ b/rsa.c @@ -871,9 +871,9 @@ int hal_rsa_key_needs_saving(const hal_rsa_key_t * const key) _(ASN1_PRIVATE + 4, qC, RSA_FLAG_PRECALC_PQ_DONE); \ _(ASN1_PRIVATE + 5, qF, RSA_FLAG_PRECALC_PQ_DONE); -hal_error_t hal_rsa_private_key_to_der_extra_maybe(const hal_rsa_key_t * const key, - const int include_extra, - uint8_t *der, size_t *der_len, const size_t der_max) +hal_error_t hal_rsa_private_key_to_der_internal(const hal_rsa_key_t * const key, + const int include_extra, + uint8_t *der, size_t *der_len, const size_t der_max) { hal_error_t err = HAL_OK; @@ -888,7 +888,15 @@ hal_error_t hal_rsa_private_key_to_der_extra_maybe(const hal_rsa_key_t * const k size_t hlen = 0, vlen = 0; -#define _(x) { size_t n = 0; if ((err = hal_asn1_encode_integer(x, NULL, &n, der_max - vlen)) != HAL_OK) return err; vlen += n; } +#define _(x) \ + { \ + size_t n = 0; \ + err = hal_asn1_encode_integer(x, NULL, &n, der_max - vlen); \ + if (err != HAL_OK) \ + return err; \ + vlen += n; \ + } + RSAPrivateKey_fields; #undef _ @@ -926,7 +934,16 @@ hal_error_t hal_rsa_private_key_to_der_extra_maybe(const hal_rsa_key_t * const k uint8_t *d = der + hlen; memset(d, 0, vlen); -#define _(x) { size_t n = 0; if ((err = hal_asn1_encode_integer(x, d, &n, vlen)) != HAL_OK) return err; d += n; vlen -= n; } +#define _(x) \ + { \ + size_t n = 0; \ + err = hal_asn1_encode_integer(x, d, &n, vlen); \ + if (err != HAL_OK) \ + return err; \ + d += n; \ + vlen -= n; \ + } + RSAPrivateKey_fields; #undef _ @@ -936,8 +953,11 @@ hal_error_t hal_rsa_private_key_to_der_extra_maybe(const hal_rsa_key_t * const k if ((err = hal_asn1_encode_header(x, sizeof(key->y), d, \ &n, vlen)) != HAL_OK) \ return err; \ - d += n + sizeof(key->y); \ - vlen -= n + sizeof(key->y); \ + d += n; \ + vlen -= n; \ + memcpy(d, key->y, sizeof(key->y)); \ + d += sizeof(key->y); \ + vlen -= sizeof(key->y); \ } if (include_extra) { @@ -952,19 +972,13 @@ hal_error_t hal_rsa_private_key_to_der_extra_maybe(const hal_rsa_key_t * const k hal_error_t hal_rsa_private_key_to_der(const hal_rsa_key_t * const key, uint8_t *der, size_t *der_len, const size_t der_max) { - return hal_rsa_private_key_to_der_extra_maybe(key, 0, der, der_len, der_max); + return hal_rsa_private_key_to_der_internal(key, 0, der, der_len, der_max); } hal_error_t hal_rsa_private_key_to_der_extra(const hal_rsa_key_t * const key, uint8_t *der, size_t *der_len, const size_t der_max) { - return hal_rsa_private_key_to_der_extra_maybe(key, 1, der, der_len, der_max); -} - -size_t hal_rsa_private_key_to_der_len(const hal_rsa_key_t * const key) -{ - size_t len = 0; - return hal_rsa_private_key_to_der(key, NULL, &len, 0) == HAL_OK ? len : 0; + return hal_rsa_private_key_to_der_internal(key, 1, der, der_len, der_max); } hal_error_t hal_rsa_private_key_from_der(hal_rsa_key_t **key_, @@ -1002,7 +1016,16 @@ hal_error_t hal_rsa_private_key_from_der(hal_rsa_key_t **key_, fp_int version[1] = INIT_FP_INT; -#define _(x) { size_t n; if ((err = hal_asn1_decode_integer(x, d, &n, vlen)) != HAL_OK) return err; d += n; vlen -= n; } +#define _(x) \ + { \ + size_t n; \ + err = hal_asn1_decode_integer(x, d, &n, vlen); \ + if (err != HAL_OK) \ + return err; \ + d += n; \ + vlen -= n; \ + } + RSAPrivateKey_fields; #undef _ @@ -1011,8 +1034,11 @@ hal_error_t hal_rsa_private_key_from_der(hal_rsa_key_t **key_, size_t hl = 0, vl = 0; \ if ((err = hal_asn1_decode_header(x, d, vlen, &hl, &vl)) != HAL_OK) \ return err; \ - if (vl > sizeof(key->y)) \ + if (vl > sizeof(key->y)) { \ + hal_log(HAL_LOG_DEBUG, "extra factor %s too big (%lu > %lu)", \ + #y, (unsigned long) vl, (unsigned long) sizeof(key->y)); \ return HAL_ERROR_ASN1_PARSE_FAILED; \ + } \ memcpy(key->y, d + hl, vl); \ key->flags |= z; \ d += hl + vl; \ @@ -1022,8 +1048,16 @@ hal_error_t hal_rsa_private_key_from_der(hal_rsa_key_t **key_, RSAPrivateKey_extra_fields; #undef _ - if (d != privkey + privkey_len || !fp_iszero(version)) + if (d != privkey + privkey_len) { + hal_log(HAL_LOG_DEBUG, "not at end of buffer (0x%lx != 0x%lx)", + (unsigned long) d, (unsigned long) privkey + privkey_len); return HAL_ERROR_ASN1_PARSE_FAILED; + } + + if (!fp_iszero(version)) { + hal_log(HAL_LOG_DEBUG, "nonzero version"); + return HAL_ERROR_ASN1_PARSE_FAILED; + } *key_ = key; -- cgit v1.2.3