From 92fb6041391bca83b0e50836991c980066d9a66c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 1 Feb 2024 22:33:06 +0100 Subject: [PATCH] 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 --- ChangeLog.d/rsa-bitlen.txt | 4 ++++ library/pk_wrap.c | 2 +- tests/suites/test_suite_pk.data | 16 +++++++++++++++- tests/suites/test_suite_pk.function | 22 +++++++++++++++------- tests/suites/test_suite_pkparse.data | 12 ++++++++++++ tests/suites/test_suite_pkparse.function | 4 ++++ 6 files changed, 51 insertions(+), 9 deletions(-) diff --git a/ChangeLog.d/rsa-bitlen.txt b/ChangeLog.d/rsa-bitlen.txt index 85a989442..9f0b3243f 100644 --- a/ChangeLog.d/rsa-bitlen.txt +++ b/ChangeLog.d/rsa-bitlen.txt @@ -1,3 +1,7 @@ +Bugfix + * Fix mbedtls_pk_get_bitlen() for RSA keys whose size is not a + multiple of 8. Fixes #868. + Features * The new function mbedtls_rsa_get_bitlen() returns the length of the modulus in bits, i.e. the key size for an RSA key. diff --git a/library/pk_wrap.c b/library/pk_wrap.c index 2e00d4a25..69e1baf2e 100644 --- a/library/pk_wrap.c +++ b/library/pk_wrap.c @@ -58,7 +58,7 @@ static int rsa_can_do(mbedtls_pk_type_t type) static size_t rsa_get_bitlen(mbedtls_pk_context *pk) { const mbedtls_rsa_context *rsa = (const mbedtls_rsa_context *) pk->pk_ctx; - return 8 * mbedtls_rsa_get_len(rsa); + return mbedtls_rsa_get_bitlen(rsa); } #if defined(MBEDTLS_USE_PSA_CRYPTO) diff --git a/tests/suites/test_suite_pk.data b/tests/suites/test_suite_pk.data index 341495883..35f02cb81 100644 --- a/tests/suites/test_suite_pk.data +++ b/tests/suites/test_suite_pk.data @@ -10,7 +10,21 @@ valid_parameters_pkwrite:"308204a20201000282010100a9021f3d406ad555538bfd36ee8265 PK utils: RSA Minimum key depends_on:MBEDTLS_RSA_C:MBEDTLS_GENPRIME -pk_utils:MBEDTLS_PK_RSA:MBEDTLS_RSA_GEN_KEY_MIN_BITS:MBEDTLS_RSA_GEN_KEY_MIN_BITS:(MBEDTLS_RSA_GEN_KEY_MIN_BITS /8):"RSA" +pk_utils:MBEDTLS_PK_RSA:MBEDTLS_RSA_GEN_KEY_MIN_BITS:MBEDTLS_RSA_GEN_KEY_MIN_BITS:(MBEDTLS_RSA_GEN_KEY_MIN_BITS + 7) / 8:"RSA" + +# mbedtls_rsa_gen_key() only supports even sizes, so we don't test min+1, +# min+3, etc. +PK utils: RSA Minimum key + 2 bits +depends_on:MBEDTLS_RSA_C:MBEDTLS_GENPRIME +pk_utils:MBEDTLS_PK_RSA:MBEDTLS_RSA_GEN_KEY_MIN_BITS + 2:MBEDTLS_RSA_GEN_KEY_MIN_BITS + 2:(MBEDTLS_RSA_GEN_KEY_MIN_BITS + 2 + 7) / 8:"RSA" + +PK utils: RSA Minimum key + 4 bits +depends_on:MBEDTLS_RSA_C:MBEDTLS_GENPRIME +pk_utils:MBEDTLS_PK_RSA:MBEDTLS_RSA_GEN_KEY_MIN_BITS + 4:MBEDTLS_RSA_GEN_KEY_MIN_BITS + 4:(MBEDTLS_RSA_GEN_KEY_MIN_BITS + 4 + 7) / 8:"RSA" + +PK utils: RSA Minimum key + 6 bits +depends_on:MBEDTLS_RSA_C:MBEDTLS_GENPRIME +pk_utils:MBEDTLS_PK_RSA:MBEDTLS_RSA_GEN_KEY_MIN_BITS + 6:MBEDTLS_RSA_GEN_KEY_MIN_BITS + 6:(MBEDTLS_RSA_GEN_KEY_MIN_BITS + 6 + 7) / 8:"RSA" PK utils: ECKEY SECP192R1 depends_on:MBEDTLS_PK_HAVE_ECC_KEYS:MBEDTLS_ECP_HAVE_SECP192R1 diff --git a/tests/suites/test_suite_pk.function b/tests/suites/test_suite_pk.function index 257430702..681de0ff0 100644 --- a/tests/suites/test_suite_pk.function +++ b/tests/suites/test_suite_pk.function @@ -427,7 +427,7 @@ void pk_psa_utils(int key_is_rsa) 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); if (key_is_rsa) { TEST_ASSERT(mbedtls_pk_can_do(&pk, MBEDTLS_PK_ECKEY) == 0); @@ -822,7 +822,7 @@ void pk_rsa_verify_test_vec(data_t *message_str, int digest, int mod, 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); @@ -862,7 +862,7 @@ void pk_rsa_verify_ext_test_vec(data_t *message_str, int digest, 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); @@ -1143,7 +1143,7 @@ void pk_rsa_encrypt_decrypt_test(data_t *message, int mod, rsa = mbedtls_pk_rsa(pk); /* load public key */ - 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); @@ -1169,9 +1169,12 @@ void pk_rsa_encrypt_decrypt_test(data_t *message, int mod, 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_len(&pk), (mod + 7) / 8); + TEST_EQUAL(mbedtls_pk_get_bitlen(&pk), mod); + memset(result, 0, sizeof(result)); rlen = 0; TEST_ASSERT(mbedtls_pk_decrypt(&pk, output, olen, @@ -1222,9 +1225,12 @@ void pk_rsa_decrypt_test_vec(data_t *cipher, int mod, 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; @@ -1278,7 +1284,7 @@ void pk_wrap_rsa_decrypt_test_vec(data_t *cipher, int mod, TEST_EQUAL(mbedtls_test_read_mpi(&P, input_P), 0); TEST_EQUAL(mbedtls_test_read_mpi(&Q, input_Q), 0); TEST_EQUAL(mbedtls_rsa_import(rsa, &N, &P, &Q, NULL, &E), 0); - TEST_EQUAL(mbedtls_rsa_get_len(rsa), (size_t) (mod / 8)); + TEST_EQUAL(mbedtls_rsa_get_len(rsa), (mod + 7) / 8); TEST_EQUAL(mbedtls_rsa_complete(rsa), 0); /* Turn PK context into an opaque one. */ @@ -1287,6 +1293,8 @@ void pk_wrap_rsa_decrypt_test_vec(data_t *cipher, int mod, PSA_KEY_USAGE_DECRYPT, PSA_ALG_NONE), 0); + TEST_EQUAL(mbedtls_pk_get_bitlen(&pk), mod); + /* 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 707da7f38..e526311b5 100644 --- a/tests/suites/test_suite_pkparse.data +++ b/tests/suites/test_suite_pkparse.data @@ -914,6 +914,18 @@ Parse RSA Key #99.8 (PKCS#8 encrypted v2 PBKDF2 AES-256-CBC hmacWithSHA384 DER, depends_on:MBEDTLS_AES_C:MBEDTLS_MD_CAN_SHA384:MBEDTLS_PKCS5_C:MBEDTLS_CIPHER_C:MBEDTLS_CIPHER_MODE_CBC:!MBEDTLS_AES_ONLY_128_BIT_KEY_LENGTH pk_parse_keyfile_rsa:"data_files/rsa_pkcs8_pbes2_pbkdf2_2048_aes256cbc_sha384.der":"PolarSSLTest":0 +Parse RSA Key #100.1 (512-bit) +pk_parse_keyfile_rsa:"data_files/rsa512.key":"":0 + +Parse RSA Key #100.1 (521-bit) +pk_parse_keyfile_rsa:"data_files/rsa521.key":"":0 + +Parse RSA Key #100.1 (522-bit) +pk_parse_keyfile_rsa:"data_files/rsa522.key":"":0 + +Parse RSA Key #100.1 (528-bit) +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 14afef6e9..f4bbb215a 100644 --- a/tests/suites/test_suite_pkparse.function +++ b/tests/suites/test_suite_pkparse.function @@ -42,6 +42,10 @@ void pk_parse_keyfile_rsa(char *key_file, char *password, int result) rsa = mbedtls_pk_rsa(ctx); TEST_EQUAL(mbedtls_rsa_check_privkey(rsa), 0); + size_t bitlen = mbedtls_rsa_get_bitlen(rsa); + TEST_EQUAL(mbedtls_pk_get_bitlen(&ctx), bitlen); + TEST_EQUAL(mbedtls_pk_get_len(&ctx), (bitlen + 7) / 8); + #if defined(MBEDTLS_PSA_CRYPTO_C) psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; TEST_EQUAL(mbedtls_pk_get_psa_attributes(&ctx,