From 954ef4bbd5727a92113732e51622af374d2f736f Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Mon, 5 Feb 2024 12:06:46 +0100 Subject: [PATCH] psa_util: improve convert_raw_to_der_single_int() Allow the function to support DER buffers than what it is nominally required by the provided coordinates. In other words let's ignore padding zeros in the raw number. Signed-off-by: Valerio Setti --- include/mbedtls/psa_util.h | 7 +++++- library/psa_util.c | 25 ++++++++++--------- tests/suites/test_suite_psa_crypto_util.data | 4 +++ .../test_suite_psa_crypto_util.function | 13 +++++++--- 4 files changed, 33 insertions(+), 16 deletions(-) diff --git a/include/mbedtls/psa_util.h b/include/mbedtls/psa_util.h index 06732d8c5c..132c73f230 100644 --- a/include/mbedtls/psa_util.h +++ b/include/mbedtls/psa_util.h @@ -191,7 +191,12 @@ static inline mbedtls_md_type_t mbedtls_md_type_from_psa_alg(psa_algorithm_t psa * \param raw_len Length of \p raw in bytes. * \param[out] der Buffer that will be filled with the converted DER * output. It can overlap with raw buffer. - * \param der_size Size of \p der in bytes. + * \param der_size Size of \p der in bytes. Given \p bits parameter: + * * #MBEDTLS_ECDSA_MAX_SIG_LEN(\p bits) can be used + * to determine a large enough buffer for any + * \p raw input vector. + * * The minimum size might be smaller in case + * \p raw input vector contains padding zeros. * \param[out] der_len On success it contains the amount of valid data * (in bytes) written to \p der. It's undefined * in case of failure. diff --git a/library/psa_util.c b/library/psa_util.c index 2491f2e45a..4e350c097b 100644 --- a/library/psa_util.c +++ b/library/psa_util.c @@ -365,9 +365,21 @@ static int convert_raw_to_der_single_int(const unsigned char *raw_buf, size_t ra unsigned char *der_buf_end) { unsigned char *p = der_buf_end; - int len = (int) raw_len; + int len; int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + /* ASN.1 DER encoding requires minimal length, so skip leading 0s. + * Provided input MPIs should not be 0, but as a failsafe measure, still + * detect that and return error in case. */ + while (*raw_buf == 0x00) { + ++raw_buf; + --raw_len; + if (raw_len == 0) { + return MBEDTLS_ERR_ASN1_INVALID_DATA; + } + } + len = (int) raw_len; + /* Copy the raw coordinate to the end of der_buf. */ if ((p - der_buf_start) < len) { return MBEDTLS_ERR_ASN1_BUF_TOO_SMALL; @@ -375,17 +387,6 @@ static int convert_raw_to_der_single_int(const unsigned char *raw_buf, size_t ra p -= len; memcpy(p, raw_buf, len); - /* ASN.1 DER encoding requires minimal length, so skip leading 0s. - * Provided input MPIs should not be 0, but as a failsafe measure, still - * detect that and return error in case. */ - while (*p == 0x00) { - ++p; - --len; - if (len == 0) { - return MBEDTLS_ERR_ASN1_INVALID_DATA; - } - } - /* If MSb is 1, ASN.1 requires that we prepend a 0. */ if (*p & 0x80) { if ((p - der_buf_start) < 1) { diff --git a/tests/suites/test_suite_psa_crypto_util.data b/tests/suites/test_suite_psa_crypto_util.data index c92b5fcc17..606e563990 100644 --- a/tests/suites/test_suite_psa_crypto_util.data +++ b/tests/suites/test_suite_psa_crypto_util.data @@ -115,3 +115,7 @@ ecdsa_raw_to_der_incremental:512:"9111111111111111111111111111111111111111111111 ECDSA Raw -> DER, 521bit, Incremental DER buffer sizes depends_on:PSA_WANT_ECC_SECP_R1_521 ecdsa_raw_to_der_incremental:528:"911111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222":"3081890243009111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111110242222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222" + +ECDSA Raw -> DER, 256bit, DER buffer of minimal length (1 byte per integer) +depends_on:PSA_WANT_ECC_SECP_K1_256 +ecdsa_raw_to_der_incremental:256:"00000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000002":"3006020101020102" diff --git a/tests/suites/test_suite_psa_crypto_util.function b/tests/suites/test_suite_psa_crypto_util.function index c102b07615..51f42a7bd7 100644 --- a/tests/suites/test_suite_psa_crypto_util.function +++ b/tests/suites/test_suite_psa_crypto_util.function @@ -32,6 +32,7 @@ void ecdsa_raw_to_der_incremental(int key_bits, data_t *input, data_t *exp_resul size_t ret_len; size_t i; + /* Test with an output buffer smaller than required (expexted to fail). */ for (i = 1; i < tmp_buf_len; i++) { TEST_CALLOC(tmp_buf, i); TEST_ASSERT(mbedtls_ecdsa_raw_to_der(key_bits, input->x, input->len, @@ -39,10 +40,16 @@ void ecdsa_raw_to_der_incremental(int key_bits, data_t *input, data_t *exp_resul mbedtls_free(tmp_buf); tmp_buf = NULL; } + /* Test with an output buffer larger/equal than required (expexted to + * succeed). */ + for (i = tmp_buf_len; i < (2 * tmp_buf_len); i++) { + TEST_CALLOC(tmp_buf, i); + TEST_ASSERT(mbedtls_ecdsa_raw_to_der(key_bits, input->x, input->len, + tmp_buf, i, &ret_len) == 0); + mbedtls_free(tmp_buf); + tmp_buf = NULL; + } - TEST_CALLOC(tmp_buf, i); - TEST_EQUAL(mbedtls_ecdsa_raw_to_der(key_bits, input->x, input->len, - tmp_buf, i, &ret_len), 0); exit: mbedtls_free(tmp_buf); }