From 201e643509f7ddb4a30805f48446c4712bb49dbd Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Thu, 1 Feb 2024 17:19:37 +0100 Subject: [PATCH] rsa: simplify mbedtls_rsa_parse_pubkey() input parameters In this way mbedtls_rsa_parse_pubkey() and mbedtls_rsa_parse_key() input parameter list is the same. Signed-off-by: Valerio Setti --- library/pkparse.c | 6 +-- library/psa_crypto_rsa.c | 4 +- library/rsa.c | 79 ++++++++++++++-------------- library/rsa_internal.h | 9 ++-- tests/suites/test_suite_rsa.function | 15 ++---- 5 files changed, 51 insertions(+), 62 deletions(-) diff --git a/library/pkparse.c b/library/pkparse.c index b2127b2e5..a47815b84 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -850,7 +850,7 @@ int mbedtls_pk_parse_subpubkey(unsigned char **p, const unsigned char *end, #if defined(MBEDTLS_RSA_C) if (pk_alg == MBEDTLS_PK_RSA) { - ret = mbedtls_rsa_parse_pubkey(mbedtls_pk_rsa(*pk), p, end); + ret = mbedtls_rsa_parse_pubkey(mbedtls_pk_rsa(*pk), *p, (size_t) (end - *p)); } else #endif /* MBEDTLS_RSA_C */ #if defined(MBEDTLS_PK_HAVE_ECC_KEYS) @@ -1504,7 +1504,7 @@ int mbedtls_pk_parse_public_key(mbedtls_pk_context *ctx, return ret; } - if ((ret = mbedtls_rsa_parse_pubkey(mbedtls_pk_rsa(*ctx), &p, p + pem.buflen)) != 0) { + if ((ret = mbedtls_rsa_parse_pubkey(mbedtls_pk_rsa(*ctx), p, pem.buflen)) != 0) { mbedtls_pk_free(ctx); } @@ -1551,7 +1551,7 @@ int mbedtls_pk_parse_public_key(mbedtls_pk_context *ctx, } p = (unsigned char *) key; - ret = mbedtls_rsa_parse_pubkey(mbedtls_pk_rsa(*ctx), &p, p + keylen); + ret = mbedtls_rsa_parse_pubkey(mbedtls_pk_rsa(*ctx), p, keylen); if (ret == 0) { return ret; } diff --git a/library/psa_crypto_rsa.c b/library/psa_crypto_rsa.c index 0047a26bf..4a574d1c7 100644 --- a/library/psa_crypto_rsa.c +++ b/library/psa_crypto_rsa.c @@ -73,9 +73,7 @@ psa_status_t mbedtls_psa_rsa_load_representation( if (PSA_KEY_TYPE_IS_KEY_PAIR(type)) { status = mbedtls_to_psa_error(mbedtls_rsa_parse_key(*p_rsa, data, data_length)); } else { - unsigned char *p = data; - unsigned char *end = (data + data_length); - status = mbedtls_to_psa_error(mbedtls_rsa_parse_pubkey(*p_rsa, &p, end)); + status = mbedtls_to_psa_error(mbedtls_rsa_parse_pubkey(*p_rsa, data, data_length)); } if (status != PSA_SUCCESS) { goto exit; diff --git a/library/rsa.c b/library/rsa.c index 62b56cd25..f4add9173 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -46,6 +46,34 @@ #include "mbedtls/platform.h" +/* + * Wrapper around mbedtls_asn1_get_mpi() that rejects zero. + * + * The value zero is: + * - never a valid value for an RSA parameter + * - interpreted as "omitted, please reconstruct" by mbedtls_rsa_complete(). + * + * Since values can't be omitted in PKCS#1, passing a zero value to + * rsa_complete() would be incorrect, so reject zero values early. + */ +static int asn1_get_nonzero_mpi(unsigned char **p, + const unsigned char *end, + mbedtls_mpi *X) +{ + int ret; + + ret = mbedtls_asn1_get_mpi(p, end, X); + if (ret != 0) { + return ret; + } + + if (mbedtls_mpi_cmp_int(X, 0) == 0) { + return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; + } + + return 0; +} + int mbedtls_rsa_parse_key(mbedtls_rsa_context *rsa, const unsigned char *key, size_t keylen) { int ret, version; @@ -192,9 +220,10 @@ cleanup: return ret; } -int mbedtls_rsa_parse_pubkey(mbedtls_rsa_context *rsa, unsigned char **p, - const unsigned char *end) +int mbedtls_rsa_parse_pubkey(mbedtls_rsa_context *rsa, const unsigned char *key, size_t keylen) { + unsigned char *p = (unsigned char *) key; + unsigned char *end = (unsigned char *) (key + keylen); int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; size_t len; @@ -205,45 +234,45 @@ int mbedtls_rsa_parse_pubkey(mbedtls_rsa_context *rsa, unsigned char **p, * } */ - if ((ret = mbedtls_asn1_get_tag(p, end, &len, + if ((ret = mbedtls_asn1_get_tag(&p, end, &len, MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE)) != 0) { return ret; } - if (*p + len != end) { + if (p + len != end) { return MBEDTLS_ERR_ASN1_LENGTH_MISMATCH; } /* Import N */ - if ((ret = mbedtls_asn1_get_tag(p, end, &len, MBEDTLS_ASN1_INTEGER)) != 0) { + if ((ret = mbedtls_asn1_get_tag(&p, end, &len, MBEDTLS_ASN1_INTEGER)) != 0) { return ret; } - if ((ret = mbedtls_rsa_import_raw(rsa, *p, len, NULL, 0, NULL, 0, + if ((ret = mbedtls_rsa_import_raw(rsa, p, len, NULL, 0, NULL, 0, NULL, 0, NULL, 0)) != 0) { return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; } - *p += len; + p += len; /* Import E */ - if ((ret = mbedtls_asn1_get_tag(p, end, &len, MBEDTLS_ASN1_INTEGER)) != 0) { + if ((ret = mbedtls_asn1_get_tag(&p, end, &len, MBEDTLS_ASN1_INTEGER)) != 0) { return ret; } if ((ret = mbedtls_rsa_import_raw(rsa, NULL, 0, NULL, 0, NULL, 0, - NULL, 0, *p, len)) != 0) { + NULL, 0, p, len)) != 0) { return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; } - *p += len; + p += len; if (mbedtls_rsa_complete(rsa) != 0 || mbedtls_rsa_check_pubkey(rsa) != 0) { return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; } - if (*p != end) { + if (p != end) { return MBEDTLS_ERR_ASN1_LENGTH_MISMATCH; } @@ -992,34 +1021,6 @@ size_t mbedtls_rsa_get_len(const mbedtls_rsa_context *ctx) return ctx->len; } -/* - * Wrapper around mbedtls_asn1_get_mpi() that rejects zero. - * - * The value zero is: - * - never a valid value for an RSA parameter - * - interpreted as "omitted, please reconstruct" by mbedtls_rsa_complete(). - * - * Since values can't be omitted in PKCS#1, passing a zero value to - * rsa_complete() would be incorrect, so reject zero values early. - */ -static int asn1_get_nonzero_mpi(unsigned char **p, - const unsigned char *end, - mbedtls_mpi *X) -{ - int ret; - - ret = mbedtls_asn1_get_mpi(p, end, X); - if (ret != 0) { - return ret; - } - - if (mbedtls_mpi_cmp_int(X, 0) == 0) { - return MBEDTLS_ERR_RSA_BAD_INPUT_DATA; - } - - return 0; -} - #if defined(MBEDTLS_GENPRIME) /* diff --git a/library/rsa_internal.h b/library/rsa_internal.h index 7bbdc8c2b..acf14a2ff 100644 --- a/library/rsa_internal.h +++ b/library/rsa_internal.h @@ -34,18 +34,15 @@ int mbedtls_rsa_parse_key(mbedtls_rsa_context *rsa, const unsigned char *key, si * \brief Parse a PKCS#1 (ASN.1) encoded public RSA key. * * \param rsa The RSA context where parsed data will be stored. - * \param p Beginning of the buffer containing the key to be parsed. - * On successful return, the referenced pointer will be - * updated in order to point to the end of the parsed data. - * \param end End of the buffer containing the key to be parsed. + * \param key The buffer that contains the key. + * \param keylen The length of the key buffer in bytes. * * \return 0 on success. * \return MBEDTLS_ERR_ASN1_xxx in case of ASN.1 parsing errors. * \return MBEDTLS_ERR_RSA_BAD_INPUT_DATA in case of importing or * priv/pub validation errors. */ -int mbedtls_rsa_parse_pubkey(mbedtls_rsa_context *rsa, unsigned char **p, - const unsigned char *end); +int mbedtls_rsa_parse_pubkey(mbedtls_rsa_context *rsa, const unsigned char *key, size_t keylen); /** * \brief Write a PKCS#1 (ASN.1) encoded private RSA key. diff --git a/tests/suites/test_suite_rsa.function b/tests/suites/test_suite_rsa.function index 2cc9fc17e..357c6edc2 100644 --- a/tests/suites/test_suite_rsa.function +++ b/tests/suites/test_suite_rsa.function @@ -1376,13 +1376,11 @@ exit: void rsa_parse_pkcs1_key(int is_public, data_t *input, int exp_ret_val) { mbedtls_rsa_context rsa_ctx; - unsigned char *start = input->x; - unsigned char *end = input->x + input->len; mbedtls_rsa_init(&rsa_ctx); if (is_public) { - TEST_EQUAL(mbedtls_rsa_parse_pubkey(&rsa_ctx, &start, end), exp_ret_val); + TEST_EQUAL(mbedtls_rsa_parse_pubkey(&rsa_ctx, input->x, input->len), exp_ret_val); } else { TEST_EQUAL(mbedtls_rsa_parse_key(&rsa_ctx, input->x, input->len), exp_ret_val); } @@ -1396,8 +1394,6 @@ exit: void rsa_parse_write_pkcs1_key(int is_public, data_t *input) { mbedtls_rsa_context rsa_ctx; - unsigned char *input_start = input->x; - unsigned char *input_end = input->x + input->len; unsigned char *output_buf = NULL; unsigned char *output_start; unsigned char *output_end; @@ -1410,8 +1406,7 @@ void rsa_parse_write_pkcs1_key(int is_public, data_t *input) /* Parse the key and write it back to output_buf. */ if (is_public) { - TEST_EQUAL(mbedtls_rsa_parse_pubkey(&rsa_ctx, &input_start, input_end), 0); - TEST_ASSERT(input_start == input_end); + TEST_EQUAL(mbedtls_rsa_parse_pubkey(&rsa_ctx, input->x, input->len), 0); TEST_EQUAL(mbedtls_rsa_write_pubkey(&rsa_ctx, output_start, &output_end), input->len); } else { TEST_EQUAL(mbedtls_rsa_parse_key(&rsa_ctx, input->x, input->len), 0); @@ -1434,7 +1429,7 @@ exit: void rsa_key_write_incremental(int is_public, data_t *input) { mbedtls_rsa_context rsa_ctx; - unsigned char *buf = NULL, *start, *end; + unsigned char *buf = NULL, *end; size_t i; mbedtls_rsa_init(&rsa_ctx); @@ -1442,9 +1437,7 @@ void rsa_key_write_incremental(int is_public, data_t *input) /* This is supposed to succeed as the real target of this test are the * write attempt below. */ if (is_public) { - start = input->x; - end = input->x + input->len; - TEST_EQUAL(mbedtls_rsa_parse_pubkey(&rsa_ctx, &start, end), 0); + TEST_EQUAL(mbedtls_rsa_parse_pubkey(&rsa_ctx, input->x, input->len), 0); } else { TEST_EQUAL(mbedtls_rsa_parse_key(&rsa_ctx, input->x, input->len), 0); }