Fix mbedtls_pk_get_bitlen() for RSA with non-byte-aligned sizes
Add non-regression tests. Update some test functions to not assume that
byte_length == bit_length / 8.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
diff --git a/ChangeLog.d/rsa-bitlen.txt b/ChangeLog.d/rsa-bitlen.txt
new file mode 100644
index 0000000..9cb8689
--- /dev/null
+++ b/ChangeLog.d/rsa-bitlen.txt
@@ -0,0 +1,3 @@
+Bugfix
+ * Fix mbedtls_pk_get_bitlen() for RSA keys whose size is not a
+ multiple of 8. Fixes #868.
diff --git a/library/pk_wrap.c b/library/pk_wrap.c
index 14c6d3f..dd460a6 100644
--- a/library/pk_wrap.c
+++ b/library/pk_wrap.c
@@ -53,7 +53,23 @@
static size_t rsa_get_bitlen(const void *ctx)
{
const mbedtls_rsa_context *rsa = (const mbedtls_rsa_context *) ctx;
- return 8 * mbedtls_rsa_get_len(rsa);
+ /* Unfortunately, the rsa.h interface does not have a direct way
+ * to access the bit-length that works with MBEDTLS_RSA_ALT.
+ * So we have to do a little work here.
+ */
+ mbedtls_mpi N;
+ mbedtls_mpi_init(&N);
+ int ret = mbedtls_rsa_export(rsa, &N, NULL, NULL, NULL, NULL);
+ /* If the export fails for some reason (e.g. the RSA_ALT implementation
+ * does not support export, or there is not enough memory),
+ * we have no way of returning an error from this function.
+ * As a fallback, return the byte-length converted in bits, which is
+ * the correct value if the modulus size is a multiple of 8 bits, which
+ * is very often the case in practice. */
+ size_t bitlen = (ret == 0 ? mbedtls_mpi_bitlen(&N) :
+ 8 * mbedtls_rsa_get_len(rsa));
+ mbedtls_mpi_free(&N);
+ return bitlen;
}
static int rsa_verify_wrap(void *ctx, mbedtls_md_type_t md_alg,
diff --git a/tests/suites/test_suite_pk.data b/tests/suites/test_suite_pk.data
index 5248074..ee54529 100644
--- a/tests/suites/test_suite_pk.data
+++ b/tests/suites/test_suite_pk.data
@@ -12,6 +12,19 @@
depends_on:MBEDTLS_RSA_C:MBEDTLS_GENPRIME
pk_utils:MBEDTLS_PK_RSA:512:512:64:"RSA"
+# mbedtls_rsa_gen_key() only supports even sizes, so we don't test 513 etc.
+PK utils: RSA 514-bit
+depends_on:MBEDTLS_RSA_C:MBEDTLS_GENPRIME
+pk_utils:MBEDTLS_PK_RSA:514:514:65:"RSA"
+
+PK utils: RSA 516-bit
+depends_on:MBEDTLS_RSA_C:MBEDTLS_GENPRIME
+pk_utils:MBEDTLS_PK_RSA:516:516:65:"RSA"
+
+PK utils: RSA 518-bit
+depends_on:MBEDTLS_RSA_C:MBEDTLS_GENPRIME
+pk_utils:MBEDTLS_PK_RSA:518:518:65:"RSA"
+
PK utils: ECKEY SECP192R1
depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED
pk_utils:MBEDTLS_PK_ECKEY:MBEDTLS_ECP_DP_SECP192R1:192:24:"EC"
diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function
index c17037f..2111926 100644
--- a/tests/suites/test_suite_pk.function
+++ b/tests/suites/test_suite_pk.function
@@ -155,7 +155,7 @@
TEST_ASSERT(strcmp(mbedtls_pk_get_name(&pk), name) == 0);
TEST_ASSERT(mbedtls_pk_get_bitlen(&pk) == bitlen);
- TEST_ASSERT(mbedtls_pk_get_len(&pk) == bitlen / 8);
+ TEST_ASSERT(mbedtls_pk_get_len(&pk) == (bitlen + 7) / 8);
TEST_ASSERT(mbedtls_pk_can_do(&pk, MBEDTLS_PK_ECKEY) == 1);
TEST_ASSERT(mbedtls_pk_can_do(&pk, MBEDTLS_PK_ECDSA) == 1);
@@ -683,7 +683,7 @@
TEST_ASSERT(mbedtls_pk_setup(&pk, mbedtls_pk_info_from_type(MBEDTLS_PK_RSA)) == 0);
rsa = mbedtls_pk_rsa(pk);
- rsa->len = mod / 8;
+ rsa->len = (mod + 7) / 8;
TEST_ASSERT(mbedtls_test_read_mpi(&rsa->N, input_N) == 0);
TEST_ASSERT(mbedtls_test_read_mpi(&rsa->E, input_E) == 0);
@@ -731,7 +731,7 @@
TEST_ASSERT(mbedtls_pk_setup(&pk, mbedtls_pk_info_from_type(MBEDTLS_PK_RSA)) == 0);
rsa = mbedtls_pk_rsa(pk);
- rsa->len = mod / 8;
+ rsa->len = (mod + 7) / 8;
TEST_ASSERT(mbedtls_test_read_mpi(&rsa->N, input_N) == 0);
TEST_ASSERT(mbedtls_test_read_mpi(&rsa->E, input_E) == 0);
@@ -1004,7 +1004,7 @@
TEST_ASSERT(mbedtls_pk_setup(&pk, mbedtls_pk_info_from_type(MBEDTLS_PK_RSA)) == 0);
rsa = mbedtls_pk_rsa(pk);
- rsa->len = mod / 8;
+ rsa->len = (mod + 7) / 8;
TEST_ASSERT(mbedtls_test_read_mpi(&rsa->N, input_N) == 0);
TEST_ASSERT(mbedtls_test_read_mpi(&rsa->E, input_E) == 0);
@@ -1053,9 +1053,12 @@
TEST_ASSERT(mbedtls_test_read_mpi(&P, input_P) == 0);
TEST_ASSERT(mbedtls_test_read_mpi(&Q, input_Q) == 0);
TEST_ASSERT(mbedtls_rsa_import(rsa, &N, &P, &Q, NULL, &E) == 0);
- TEST_ASSERT(mbedtls_rsa_get_len(rsa) == (size_t) (mod / 8));
+ TEST_EQUAL(mbedtls_rsa_get_len(rsa), (mod + 7) / 8);
TEST_ASSERT(mbedtls_rsa_complete(rsa) == 0);
+ TEST_EQUAL(mbedtls_pk_get_bitlen(&pk), mod);
+ TEST_EQUAL(mbedtls_pk_get_len(&pk), (mod + 7) / 8);
+
/* decryption test */
memset(output, 0, sizeof(output));
olen = 0;
diff --git a/tests/suites/test_suite_pkparse.data b/tests/suites/test_suite_pkparse.data
index 9108a21..0837636 100644
--- a/tests/suites/test_suite_pkparse.data
+++ b/tests/suites/test_suite_pkparse.data
@@ -938,6 +938,22 @@
depends_on:MBEDTLS_DES_C:MBEDTLS_SHA512_C:MBEDTLS_PKCS5_C
pk_parse_keyfile_rsa:"data_files/rsa_pkcs8_pbes2_pbkdf2_4096_des_sha512.der":"":MBEDTLS_ERR_PK_KEY_INVALID_FORMAT
+Parse RSA Key #100.1 (512-bit)
+depends_on:MBEDTLS_PEM_C
+pk_parse_keyfile_rsa:"data_files/rsa512.key":"":0
+
+Parse RSA Key #100.1 (521-bit)
+depends_on:MBEDTLS_PEM_C
+pk_parse_keyfile_rsa:"data_files/rsa521.key":"":0
+
+Parse RSA Key #100.1 (522-bit)
+depends_on:MBEDTLS_PEM_C
+pk_parse_keyfile_rsa:"data_files/rsa522.key":"":0
+
+Parse RSA Key #100.1 (528-bit)
+depends_on:MBEDTLS_PEM_C
+pk_parse_keyfile_rsa:"data_files/rsa528.key":"":0
+
Parse Public RSA Key #1 (PKCS#8 wrapped)
depends_on:MBEDTLS_PEM_PARSE_C
pk_parse_public_keyfile_rsa:"data_files/rsa_pkcs8_2048_public.pem":0
diff --git a/tests/suites/test_suite_pkparse.function b/tests/suites/test_suite_pkparse.function
index d6b6984..08f2763 100644
--- a/tests/suites/test_suite_pkparse.function
+++ b/tests/suites/test_suite_pkparse.function
@@ -32,6 +32,10 @@
TEST_ASSERT(mbedtls_pk_can_do(&ctx, MBEDTLS_PK_RSA));
rsa = mbedtls_pk_rsa(ctx);
TEST_ASSERT(mbedtls_rsa_check_privkey(rsa) == 0);
+
+ /* Test consistency between get_len and get_bitlen */
+ size_t bitlen = mbedtls_pk_get_bitlen(&ctx);
+ TEST_EQUAL(mbedtls_pk_get_len(&ctx), (bitlen + 7) / 8);
}
exit:
@@ -58,6 +62,10 @@
TEST_ASSERT(mbedtls_pk_can_do(&ctx, MBEDTLS_PK_RSA));
rsa = mbedtls_pk_rsa(ctx);
TEST_ASSERT(mbedtls_rsa_check_pubkey(rsa) == 0);
+
+ /* Test consistency between get_len and get_bitlen */
+ size_t bitlen = mbedtls_pk_get_bitlen(&ctx);
+ TEST_EQUAL(mbedtls_pk_get_len(&ctx), (bitlen + 7) / 8);
}
exit: