From b3acb053fb94921eade19c5a374139916896220a Mon Sep 17 00:00:00 2001 From: Werner Lewis Date: Fri, 17 Jun 2022 15:59:58 +0100 Subject: [PATCH 1/6] Add mbedtls_x509_dn_get_next function Allow iteration through relative DNs when X509 name contains multi- value RDNs. Signed-off-by: Werner Lewis --- ChangeLog.d/add_dn_get_next.txt | 3 ++ include/mbedtls/x509.h | 10 ++++ library/x509.c | 9 ++++ tests/suites/test_suite_x509parse.data | 12 +++++ tests/suites/test_suite_x509parse.function | 63 ++++++++++++++++++++++ 5 files changed, 97 insertions(+) create mode 100644 ChangeLog.d/add_dn_get_next.txt diff --git a/ChangeLog.d/add_dn_get_next.txt b/ChangeLog.d/add_dn_get_next.txt new file mode 100644 index 000000000..04ee954f6 --- /dev/null +++ b/ChangeLog.d/add_dn_get_next.txt @@ -0,0 +1,3 @@ +Bugfix + * Add mbedtls_x509_dn_get_next function to return the next relative DN in + an X509 name, to allow walking the name list. Fixes #5431. diff --git a/include/mbedtls/x509.h b/include/mbedtls/x509.h index 3c76fec78..2ad90de48 100644 --- a/include/mbedtls/x509.h +++ b/include/mbedtls/x509.h @@ -266,6 +266,16 @@ mbedtls_x509_time; */ int mbedtls_x509_dn_gets( char *buf, size_t size, const mbedtls_x509_name *dn ); +/** + * \brief Return the next relative DN in an X509 name. + * + * \param dn Current node in the X509 name + * + * \return Pointer to the first attribute-value pair of the + * next RDN in sequence, or NULL if end is reached. + */ +mbedtls_x509_name * mbedtls_x509_dn_get_next( mbedtls_x509_name * start ); + /** * \brief Store the certificate serial in printable form into buf; * no more than size characters will be written. diff --git a/library/x509.c b/library/x509.c index 2e11c7fce..17d103038 100644 --- a/library/x509.c +++ b/library/x509.c @@ -796,6 +796,15 @@ int mbedtls_x509_dn_gets( char *buf, size_t size, const mbedtls_x509_name *dn ) return( (int) ( size - n ) ); } +/* + * Return the next relative DN in an X509 name. + */ +mbedtls_x509_name * mbedtls_x509_dn_get_next( mbedtls_x509_name * dn ) +{ + for( ; dn->next != NULL && dn->next_merged; dn = dn->next ); + return( dn->next ); +} + /* * Store the serial in printable form into buf; no more * than size characters will be written diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index d04b7d84b..5bb847967 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -375,6 +375,18 @@ X509 Get Distinguished Name #4 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_SHA1_C mbedtls_x509_dn_gets:"data_files/server2.crt":"issuer":"C=NL, O=PolarSSL, CN=PolarSSL Test CA" +X509 Get Next DN #1 No Multivalue RDNs +mbedtls_x509_dn_get_next:"C=NL, O=PolarSSL, CN=PolarSSL Server 1":0:"C O CN":3:"C=NL, O=PolarSSL, CN=PolarSSL Server 1" + +X509 Get Next DN #2 Initial Multivalue RDN +mbedtls_x509_dn_get_next:"C=NL, O=PolarSSL, CN=PolarSSL Server 1":0b001:"C CN":2:"C=NL + O=PolarSSL, CN=PolarSSL Server 1" + +X509 Get Next DN #3 Single Multivalue RDN +mbedtls_x509_dn_get_next:"C=NL, O=PolarSSL, CN=PolarSSL Server 1":0b011:"C":1:"C=NL + O=PolarSSL + CN=PolarSSL Server 1" + +X509 Get Next DN #4 Consecutive MV RDNs +mbedtls_x509_dn_get_next:"C=NL, O=PolarSSL, title=Example, CN=PolarSSL Server 1":0b0101:"C title":2:"C=NL + O=PolarSSL, title=Example + CN=PolarSSL Server 1" + X509 Time Expired #1 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_HAVE_TIME_DATE:MBEDTLS_SHA1_C mbedtls_x509_time_is_past:"data_files/server1.crt":"valid_from":1 diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index 1d06fe3c4..249527015 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -785,6 +785,69 @@ exit: } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_X509_CREATE_C:MBEDTLS_X509_USE_C:MBEDTLS_X509_CRT_PARSE_C:!MBEDTLS_X509_REMOVE_INFO */ +void mbedtls_x509_dn_get_next( char * name_str, int next_merged, char * expected_oids, int exp_count, char * exp_dn_gets ) +{ + int ret = 0, i; + size_t len = 0; + mbedtls_asn1_named_data *names = NULL; + mbedtls_x509_name parsed, *parsed_cur, *parsed_prv; + unsigned char buf[1024], out[1024], *c; + const char *short_name; + + memset( &parsed, 0, sizeof( parsed ) ); + memset( out, 0, sizeof( out ) ); + memset( buf, 0, sizeof( buf ) ); + c = buf + sizeof( buf ); + + TEST_ASSERT( mbedtls_x509_string_to_names( &names, name_str ) == 0 ); + + ret = mbedtls_x509_write_names( &c, buf, names ); + TEST_ASSERT( ret > 0 ); + + TEST_ASSERT( mbedtls_asn1_get_tag( &c, buf + sizeof( buf ), &len, + MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) == 0 ); + TEST_ASSERT( mbedtls_x509_get_name( &c, buf + sizeof( buf ), &parsed ) == 0 ); + + // Iterate over names and set next_merged nodes + parsed_cur = &parsed; + for( ; next_merged != 0 && parsed_cur != NULL; next_merged = next_merged >> 1 ) + { + parsed_cur->next_merged = next_merged & 0b1; + parsed_cur = parsed_cur->next; + } + + // Iterate over RDN nodes and print OID of first element to buffer + parsed_cur = &parsed; + len = 0; + for( i = 0; parsed_cur != NULL; i++ ) + { + TEST_ASSERT( mbedtls_oid_get_attr_short_name( &parsed_cur->oid, + &short_name ) == 0 ); + len += mbedtls_snprintf( (char*) out + len, 1024 - len, "%s ", short_name ); + parsed_cur = mbedtls_x509_dn_get_next( parsed_cur ); + } + out[len-1] = 0; + + TEST_ASSERT( exp_count == i ); + TEST_ASSERT( strcmp( (char *) out, expected_oids ) == 0 ); + + TEST_ASSERT( mbedtls_x509_dn_gets( (char *) out, sizeof( out ), &parsed ) >= 0 ); + TEST_ASSERT( strcmp( (char *) out, exp_dn_gets ) == 0 ); +exit: + mbedtls_asn1_free_named_data_list( &names ); + + parsed_cur = parsed.next; + while( parsed_cur != 0 ) + { + parsed_prv = parsed_cur; + parsed_cur = parsed_cur->next; + mbedtls_free( parsed_prv ); + } +} + +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_FS_IO:MBEDTLS_X509_CRT_PARSE_C */ void mbedtls_x509_time_is_past( char * crt_file, char * entity, int result ) { From 2f1d51070c3921ffaf08ba508e06c9fbed2f1bb6 Mon Sep 17 00:00:00 2001 From: Werner Lewis Date: Mon, 20 Jun 2022 11:45:01 +0100 Subject: [PATCH 2/6] Fix incorrect param in function declaration Signed-off-by: Werner Lewis --- include/mbedtls/x509.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/mbedtls/x509.h b/include/mbedtls/x509.h index 2ad90de48..b7e364527 100644 --- a/include/mbedtls/x509.h +++ b/include/mbedtls/x509.h @@ -274,7 +274,7 @@ int mbedtls_x509_dn_gets( char *buf, size_t size, const mbedtls_x509_name *dn ); * \return Pointer to the first attribute-value pair of the * next RDN in sequence, or NULL if end is reached. */ -mbedtls_x509_name * mbedtls_x509_dn_get_next( mbedtls_x509_name * start ); +mbedtls_x509_name * mbedtls_x509_dn_get_next( mbedtls_x509_name *dn ); /** * \brief Store the certificate serial in printable form into buf; From 90c46c376ba9cd60f91063fa3deeb3af43c362d1 Mon Sep 17 00:00:00 2001 From: Werner Lewis Date: Mon, 20 Jun 2022 11:46:58 +0100 Subject: [PATCH 3/6] Use consistent test case names Signed-off-by: Werner Lewis --- tests/suites/test_suite_x509parse.data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 5bb847967..e07acd17e 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -384,7 +384,7 @@ mbedtls_x509_dn_get_next:"C=NL, O=PolarSSL, CN=PolarSSL Server 1":0b001:"C CN":2 X509 Get Next DN #3 Single Multivalue RDN mbedtls_x509_dn_get_next:"C=NL, O=PolarSSL, CN=PolarSSL Server 1":0b011:"C":1:"C=NL + O=PolarSSL + CN=PolarSSL Server 1" -X509 Get Next DN #4 Consecutive MV RDNs +X509 Get Next DN #4 Consecutive Multivalue RDNs mbedtls_x509_dn_get_next:"C=NL, O=PolarSSL, title=Example, CN=PolarSSL Server 1":0b0101:"C title":2:"C=NL + O=PolarSSL, title=Example + CN=PolarSSL Server 1" X509 Time Expired #1 From 12657cdcc646aefeaeae89c52812e0b224731dd9 Mon Sep 17 00:00:00 2001 From: Werner Lewis Date: Mon, 20 Jun 2022 11:47:57 +0100 Subject: [PATCH 4/6] Remove binary int use Signed-off-by: Werner Lewis --- tests/suites/test_suite_x509parse.data | 6 +++--- tests/suites/test_suite_x509parse.function | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index e07acd17e..f62fba397 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -379,13 +379,13 @@ X509 Get Next DN #1 No Multivalue RDNs mbedtls_x509_dn_get_next:"C=NL, O=PolarSSL, CN=PolarSSL Server 1":0:"C O CN":3:"C=NL, O=PolarSSL, CN=PolarSSL Server 1" X509 Get Next DN #2 Initial Multivalue RDN -mbedtls_x509_dn_get_next:"C=NL, O=PolarSSL, CN=PolarSSL Server 1":0b001:"C CN":2:"C=NL + O=PolarSSL, CN=PolarSSL Server 1" +mbedtls_x509_dn_get_next:"C=NL, O=PolarSSL, CN=PolarSSL Server 1":0x01:"C CN":2:"C=NL + O=PolarSSL, CN=PolarSSL Server 1" X509 Get Next DN #3 Single Multivalue RDN -mbedtls_x509_dn_get_next:"C=NL, O=PolarSSL, CN=PolarSSL Server 1":0b011:"C":1:"C=NL + O=PolarSSL + CN=PolarSSL Server 1" +mbedtls_x509_dn_get_next:"C=NL, O=PolarSSL, CN=PolarSSL Server 1":0x03:"C":1:"C=NL + O=PolarSSL + CN=PolarSSL Server 1" X509 Get Next DN #4 Consecutive Multivalue RDNs -mbedtls_x509_dn_get_next:"C=NL, O=PolarSSL, title=Example, CN=PolarSSL Server 1":0b0101:"C title":2:"C=NL + O=PolarSSL, title=Example + CN=PolarSSL Server 1" +mbedtls_x509_dn_get_next:"C=NL, O=PolarSSL, title=Example, CN=PolarSSL Server 1":0x05:"C title":2:"C=NL + O=PolarSSL, title=Example + CN=PolarSSL Server 1" X509 Time Expired #1 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_HAVE_TIME_DATE:MBEDTLS_SHA1_C diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index 249527015..d831eb155 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -813,7 +813,7 @@ void mbedtls_x509_dn_get_next( char * name_str, int next_merged, char * expected parsed_cur = &parsed; for( ; next_merged != 0 && parsed_cur != NULL; next_merged = next_merged >> 1 ) { - parsed_cur->next_merged = next_merged & 0b1; + parsed_cur->next_merged = next_merged & 0x01; parsed_cur = parsed_cur->next; } From ac80a66395877988d56af97a5dfdc3a0da1a1fb9 Mon Sep 17 00:00:00 2001 From: Werner Lewis Date: Thu, 23 Jun 2022 11:58:02 +0100 Subject: [PATCH 5/6] Reduce buffer sizes to expected size Signed-off-by: Werner Lewis --- tests/suites/test_suite_x509parse.function | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index d831eb155..fed0cdaa6 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -789,16 +789,19 @@ exit: void mbedtls_x509_dn_get_next( char * name_str, int next_merged, char * expected_oids, int exp_count, char * exp_dn_gets ) { int ret = 0, i; - size_t len = 0; + size_t len = 0, out_size; mbedtls_asn1_named_data *names = NULL; mbedtls_x509_name parsed, *parsed_cur, *parsed_prv; - unsigned char buf[1024], out[1024], *c; + // Size of buf is maximum required for test cases + unsigned char buf[80], *out = NULL, *c; const char *short_name; memset( &parsed, 0, sizeof( parsed ) ); - memset( out, 0, sizeof( out ) ); memset( buf, 0, sizeof( buf ) ); c = buf + sizeof( buf ); + // Additional size required for trailing space + out_size = strlen( expected_oids ) + 2; + ASSERT_ALLOC( out, out_size ); TEST_ASSERT( mbedtls_x509_string_to_names( &names, name_str ) == 0 ); @@ -824,17 +827,23 @@ void mbedtls_x509_dn_get_next( char * name_str, int next_merged, char * expected { TEST_ASSERT( mbedtls_oid_get_attr_short_name( &parsed_cur->oid, &short_name ) == 0 ); - len += mbedtls_snprintf( (char*) out + len, 1024 - len, "%s ", short_name ); + len += mbedtls_snprintf( (char*) out + len, out_size - len, "%s ", short_name ); parsed_cur = mbedtls_x509_dn_get_next( parsed_cur ); } out[len-1] = 0; TEST_ASSERT( exp_count == i ); TEST_ASSERT( strcmp( (char *) out, expected_oids ) == 0 ); + mbedtls_free( out ); + out = NULL; - TEST_ASSERT( mbedtls_x509_dn_gets( (char *) out, sizeof( out ), &parsed ) >= 0 ); + out_size = strlen( exp_dn_gets ) + 1; + ASSERT_ALLOC( out, out_size ); + + TEST_ASSERT( mbedtls_x509_dn_gets( (char *) out, out_size, &parsed ) >= 0 ); TEST_ASSERT( strcmp( (char *) out, exp_dn_gets ) == 0 ); exit: + mbedtls_free( out ); mbedtls_asn1_free_named_data_list( &names ); parsed_cur = parsed.next; From 3e5585b45d5c4ffc0f7a40594dd089808c46c84a Mon Sep 17 00:00:00 2001 From: Werner Lewis Date: Thu, 23 Jun 2022 15:12:10 +0100 Subject: [PATCH 6/6] Replace TEST_ASSERT macro uses Signed-off-by: Werner Lewis --- tests/suites/test_suite_x509parse.function | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index fed0cdaa6..a9d7e47c8 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -803,14 +803,14 @@ void mbedtls_x509_dn_get_next( char * name_str, int next_merged, char * expected out_size = strlen( expected_oids ) + 2; ASSERT_ALLOC( out, out_size ); - TEST_ASSERT( mbedtls_x509_string_to_names( &names, name_str ) == 0 ); + TEST_EQUAL( mbedtls_x509_string_to_names( &names, name_str ), 0 ); ret = mbedtls_x509_write_names( &c, buf, names ); - TEST_ASSERT( ret > 0 ); + TEST_LE_S( 0, ret ); - TEST_ASSERT( mbedtls_asn1_get_tag( &c, buf + sizeof( buf ), &len, - MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ) == 0 ); - TEST_ASSERT( mbedtls_x509_get_name( &c, buf + sizeof( buf ), &parsed ) == 0 ); + TEST_EQUAL( mbedtls_asn1_get_tag( &c, buf + sizeof( buf ), &len, + MBEDTLS_ASN1_CONSTRUCTED | MBEDTLS_ASN1_SEQUENCE ), 0 ); + TEST_EQUAL( mbedtls_x509_get_name( &c, buf + sizeof( buf ), &parsed ), 0 ); // Iterate over names and set next_merged nodes parsed_cur = &parsed; @@ -825,23 +825,23 @@ void mbedtls_x509_dn_get_next( char * name_str, int next_merged, char * expected len = 0; for( i = 0; parsed_cur != NULL; i++ ) { - TEST_ASSERT( mbedtls_oid_get_attr_short_name( &parsed_cur->oid, - &short_name ) == 0 ); + TEST_EQUAL( mbedtls_oid_get_attr_short_name( &parsed_cur->oid, + &short_name ), 0 ); len += mbedtls_snprintf( (char*) out + len, out_size - len, "%s ", short_name ); parsed_cur = mbedtls_x509_dn_get_next( parsed_cur ); } out[len-1] = 0; - TEST_ASSERT( exp_count == i ); - TEST_ASSERT( strcmp( (char *) out, expected_oids ) == 0 ); + TEST_EQUAL( exp_count, i ); + TEST_EQUAL( strcmp( (char *) out, expected_oids ), 0 ); mbedtls_free( out ); out = NULL; out_size = strlen( exp_dn_gets ) + 1; ASSERT_ALLOC( out, out_size ); - TEST_ASSERT( mbedtls_x509_dn_gets( (char *) out, out_size, &parsed ) >= 0 ); - TEST_ASSERT( strcmp( (char *) out, exp_dn_gets ) == 0 ); + TEST_LE_S( 0, mbedtls_x509_dn_gets( (char *) out, out_size, &parsed ) ); + TEST_EQUAL( strcmp( (char *) out, exp_dn_gets ), 0 ); exit: mbedtls_free( out ); mbedtls_asn1_free_named_data_list( &names );