[Cryptech-Commits] [sw/libhal] 03/03: Sort-of-working, large (4096-bit) RSA keys broken.
git at cryptech.is
git at cryptech.is
Wed Sep 13 15:57:01 UTC 2017
This is an automated email from the git hooks/post-receive script.
sra at hactrn.net pushed a commit to branch systolic_crt
in repository sw/libhal.
commit 5522df4f68bfa66b9b4446fdfb92783694586f70
Author: Rob Austein <sra at hactrn.net>
AuthorDate: Wed Sep 13 11:28:13 2017 -0400
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;
More information about the Commits
mailing list