From e888c176da31d608a7db36d9e2367cdd0a13b963 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 | 69 +++++++++++++++++++--- programs/x509/cert_write.c | 14 +++++ tests/suites/test_suite_x509write.function | 7 +++ 4 files changed, 112 insertions(+), 10 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index d2b7648c41..5a9da98bf1 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -203,7 +203,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 ) @@ -284,7 +284,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); @@ -995,15 +996,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 254c4b2fca..618c51e18e 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,16 +100,63 @@ 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 ) - return( ret ); + /* 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; iserial[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; +} int mbedtls_x509write_crt_set_validity( mbedtls_x509write_cert *ctx, const char *not_before, @@ -504,9 +548,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 a8910d7f68..be3464e02d 100644 --- a/programs/x509/cert_write.c +++ b/programs/x509/cert_write.c @@ -559,6 +559,7 @@ int main( int argc, char *argv[] ) 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) ); @@ -566,6 +567,7 @@ int main( int argc, char *argv[] ) "returned -0x%04x - %s\n\n", (unsigned int) -ret, buf ); goto exit; } +#endif mbedtls_printf( " ok\n" ); @@ -721,6 +723,7 @@ int main( int argc, char *argv[] ) 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 ) { @@ -729,6 +732,17 @@ int main( int argc, char *argv[] ) "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 5bd814ad8f..c6ebc229e5 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -377,12 +377,19 @@ void x509_crt_check( char *subject_key_file, char *subject_pwd, if( pk_wrap == 2 ) 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 );