From da0afcc2fb5eab22b30d761ef86330858ba35f89 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Thu, 5 Jan 2023 16:46:59 +0100 Subject: [PATCH] x509: remove direct dependency from BIGNUM_C Signed-off-by: Valerio Setti --- include/mbedtls/x509_crt.h | 32 +++++++++- library/x509write_crt.c | 70 +++++++++++++++++++--- programs/x509/cert_write.c | 13 ++++ tests/suites/test_suite_x509write.function | 7 +++ 4 files changed, 113 insertions(+), 9 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index 661f8aa7b..f552345f4 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -197,7 +197,7 @@ mbedtls_x509_crt_profile; #define MBEDTLS_X509_CRT_VERSION_2 1 #define MBEDTLS_X509_CRT_VERSION_3 2 -#define MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN 32 +#define MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN 20 #define MBEDTLS_X509_RFC5280_UTC_TIME_LEN 15 #if !defined(MBEDTLS_X509_MAX_FILE_PATH_LEN) @@ -277,7 +277,8 @@ mbedtls_x509_crt_profile; */ typedef struct mbedtls_x509write_cert { int MBEDTLS_PRIVATE(version); - mbedtls_mpi MBEDTLS_PRIVATE(serial); + unsigned char MBEDTLS_PRIVATE(serial)[MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN]; + size_t MBEDTLS_PRIVATE(serial_len); mbedtls_pk_context *MBEDTLS_PRIVATE(subject_key); mbedtls_pk_context *MBEDTLS_PRIVATE(issuer_key); mbedtls_asn1_named_data *MBEDTLS_PRIVATE(subject); @@ -986,15 +987,42 @@ void mbedtls_x509write_crt_init(mbedtls_x509write_cert *ctx); */ void mbedtls_x509write_crt_set_version(mbedtls_x509write_cert *ctx, int version); +#if defined(MBEDTLS_BIGNUM_C) /** * \brief Set the serial number for a Certificate. * + * \deprecated This function is deprecated and will be removed in a + * future version of the library. Please use + * mbedtls_x509write_crt_set_serial_new() instead. + * + * \note Even though the MBEDTLS_BIGNUM_C guard looks redundant since + * X509 depends on PK and PK depends on BIGNUM, this emphasizes + * a direct dependency between X509 and BIGNUM which is going + * to be deprecated in the future. + * * \param ctx CRT context to use * \param serial serial number to set * * \return 0 if successful */ int mbedtls_x509write_crt_set_serial(mbedtls_x509write_cert *ctx, const mbedtls_mpi *serial); +#endif + +/** + * \brief Set the serial number for a Certificate. + * + * \param ctx CRT context to use + * \param serial_buff Input buffer containing the serial number in big + * endian format + * \param serial_buff_len Length of the previous input buffer buffer + * + * \return 0 if successful, or + * MBEDTLS_ERR_X509_BAD_INPUT_DATA if the provided input buffer: + * - is too big (longer than MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN) + * - contains invalid chars + */ +int mbedtls_x509write_crt_set_serial_new(mbedtls_x509write_cert *ctx, + char *serial_buff, size_t serial_buff_len); /** * \brief Set the validity period for a Certificate diff --git a/library/x509write_crt.c b/library/x509write_crt.c index febd0e6f3..53c2ac4ec 100644 --- a/library/x509write_crt.c +++ b/library/x509write_crt.c @@ -52,14 +52,11 @@ void mbedtls_x509write_crt_init(mbedtls_x509write_cert *ctx) { memset(ctx, 0, sizeof(mbedtls_x509write_cert)); - mbedtls_mpi_init(&ctx->serial); ctx->version = MBEDTLS_X509_CRT_VERSION_3; } void mbedtls_x509write_crt_free(mbedtls_x509write_cert *ctx) { - mbedtls_mpi_free(&ctx->serial); - mbedtls_asn1_free_named_data_list(&ctx->subject); mbedtls_asn1_free_named_data_list(&ctx->issuer); mbedtls_asn1_free_named_data_list(&ctx->extensions); @@ -103,15 +100,65 @@ int mbedtls_x509write_crt_set_issuer_name(mbedtls_x509write_cert *ctx, return mbedtls_x509_string_to_names(&ctx->issuer, issuer_name); } +#if defined(MBEDTLS_BIGNUM_C) int mbedtls_x509write_crt_set_serial(mbedtls_x509write_cert *ctx, const mbedtls_mpi *serial) { - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + int ret; + unsigned char tmp[MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN]; + size_t tmp_len; - if ((ret = mbedtls_mpi_copy(&ctx->serial, serial)) != 0) { + /* Ensure that the MPI value fits into the buffer */ + tmp_len = mbedtls_mpi_size(serial); + if (tmp_len > MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN) { + return MBEDTLS_ERR_X509_BAD_INPUT_DATA; + } + + ctx->serial_len = tmp_len; + + ret = mbedtls_mpi_write_binary(serial, tmp, + MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN); + if (ret < 0) { return ret; } + /* Reverse the string since "tmp" is in big endian format */ + for (int i = 0; i < MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN; i++) { + ctx->serial[i] = tmp[MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN - 1 - i]; + } + + return 0; +} +#endif + +int mbedtls_x509write_crt_set_serial_new(mbedtls_x509write_cert *ctx, + char *serial_buff, size_t serial_buff_len) +{ + int i, j; + char c; + unsigned char val; + + if (serial_buff_len > MBEDTLS_X509_RFC5280_MAX_SERIAL_LEN) { + return MBEDTLS_ERR_X509_BAD_INPUT_DATA; + } + + /* Store data in little endian format */ + for (i = 0, j = serial_buff_len - 1; j == 0; i++, j--) { + c = serial_buff[j]; + if (c >= 0x30 && c <= 0x39) { + val = c - 0x30; + } else if (c >= 0x41 && c <= 0x46) { + val = c - 0x37; + } else if (c >= 0x61 && c <= 0x66) { + val = c - 0x57; + } else { + return MBEDTLS_ERR_X509_BAD_INPUT_DATA; + } + + ctx->serial[i] = val; + } + ctx->serial_len = i; + return 0; } @@ -510,9 +557,18 @@ int mbedtls_x509write_crt_der(mbedtls_x509write_cert *ctx, /* * Serial ::= INTEGER + * + * Written data is: + * - [ctx->serial_len] bytes for the raw serial buffer + * - 1 byte for the length + * - 1 byte for the TAG */ - MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_mpi(&c, buf, - &ctx->serial)); + MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_raw_buffer(&c, buf, + ctx->serial, ctx->serial_len)); + MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_len(&c, buf, + ctx->serial_len)); + MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_tag(&c, buf, + MBEDTLS_ASN1_INTEGER)); /* * Version ::= INTEGER { v1(0), v2(1), v3(2) } diff --git a/programs/x509/cert_write.c b/programs/x509/cert_write.c index 3e134dd6a..363ed8d8d 100644 --- a/programs/x509/cert_write.c +++ b/programs/x509/cert_write.c @@ -514,12 +514,14 @@ usage: mbedtls_printf(" . Reading serial number..."); fflush(stdout); +#if defined(MBEDTLS_BIGNUM_C) if ((ret = mbedtls_mpi_read_string(&serial, 10, opt.serial)) != 0) { mbedtls_strerror(ret, buf, sizeof(buf)); mbedtls_printf(" failed\n ! mbedtls_mpi_read_string " "returned -0x%04x - %s\n\n", (unsigned int) -ret, buf); goto exit; } +#endif mbedtls_printf(" ok\n"); @@ -661,6 +663,7 @@ usage: mbedtls_x509write_crt_set_version(&crt, opt.version); mbedtls_x509write_crt_set_md_alg(&crt, opt.md); +#if defined(MBEDTLS_BIGNUM_C) ret = mbedtls_x509write_crt_set_serial(&crt, &serial); if (ret != 0) { mbedtls_strerror(ret, buf, sizeof(buf)); @@ -668,6 +671,16 @@ usage: "returned -0x%04x - %s\n\n", (unsigned int) -ret, buf); goto exit; } +#else + ret = mbedtls_x509write_crt_set_serial_new(&crt, opt.serial, + strlen(opt.serial)); + if (ret != 0) { + mbedtls_strerror(ret, buf, sizeof(buf)); + mbedtls_printf(" failed\n ! mbedtls_x509write_crt_set_serial_new " + "returned -0x%04x - %s\n\n", (unsigned int) -ret, buf); + goto exit; + } +#endif ret = mbedtls_x509write_crt_set_validity(&crt, opt.not_before, opt.not_after); if (ret != 0) { diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index a21ad475a..2bf8024ab 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -384,13 +384,20 @@ void x509_crt_check(char *subject_key_file, char *subject_pwd, TEST_ASSERT(mbedtls_pk_get_type(&issuer_key) == MBEDTLS_PK_OPAQUE); } +#if !defined(MBEDTLS_BIGNUM_C) TEST_ASSERT(mbedtls_test_read_mpi(&serial, serial_str) == 0); +#endif if (ver != -1) { mbedtls_x509write_crt_set_version(&crt, ver); } +#if !defined(MBEDTLS_BIGNUM_C) TEST_ASSERT(mbedtls_x509write_crt_set_serial(&crt, &serial) == 0); +#else + TEST_ASSERT(mbedtls_x509write_crt_set_serial_new(&crt, serial_str, + strlen(serial_str)) == 0); +#endif TEST_ASSERT(mbedtls_x509write_crt_set_validity(&crt, not_before, not_after) == 0); mbedtls_x509write_crt_set_md_alg(&crt, md_type);