From 02c9d3b9c2098ecd5a5affe1c229c8400f1fcd6e Mon Sep 17 00:00:00 2001 From: Werner Lewis Date: Fri, 20 May 2022 12:48:46 +0100 Subject: [PATCH 1/4] Fix parsing of special chars in X509 DN values Use escape mechanism defined in RFC 1779 when parsing commas and other special characters in X509 DN values. Resolves failures when generating a certificate with a CSR containing a comma in subject value. Fixes #769. Signed-off-by: Werner Lewis --- ChangeLog.d/fix-csr_subject_commas.txt | 3 +++ library/x509.c | 21 ++++++++++++++------- tests/data_files/Makefile | 8 +++++++- tests/data_files/server1.commas.crt | 20 ++++++++++++++++++++ tests/data_files/server1.req.commas.sha256 | 16 ++++++++++++++++ tests/suites/test_suite_x509parse.data | 8 ++++++++ tests/suites/test_suite_x509write.data | 2 +- 7 files changed, 69 insertions(+), 9 deletions(-) create mode 100644 ChangeLog.d/fix-csr_subject_commas.txt create mode 100644 tests/data_files/server1.commas.crt create mode 100644 tests/data_files/server1.req.commas.sha256 diff --git a/ChangeLog.d/fix-csr_subject_commas.txt b/ChangeLog.d/fix-csr_subject_commas.txt new file mode 100644 index 0000000000..d497b1211f --- /dev/null +++ b/ChangeLog.d/fix-csr_subject_commas.txt @@ -0,0 +1,3 @@ +Bugfix + * Fix string representation of DNs when parsing values containing commas, + conforming to RFC 1779. Fixes #769. diff --git a/library/x509.c b/library/x509.c index f21e9e6944..c55d0387b6 100644 --- a/library/x509.c +++ b/library/x509.c @@ -741,7 +741,7 @@ int mbedtls_x509_get_ext( unsigned char **p, const unsigned char *end, int mbedtls_x509_dn_gets( char *buf, size_t size, const mbedtls_x509_name *dn ) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - size_t i, n; + size_t i, j, n; unsigned char c, merge = 0; const mbedtls_x509_name *name; const char *short_name = NULL; @@ -775,17 +775,24 @@ int mbedtls_x509_dn_gets( char *buf, size_t size, const mbedtls_x509_name *dn ) ret = mbedtls_snprintf( p, n, "\?\?=" ); MBEDTLS_X509_SAFE_SNPRINTF; - for( i = 0; i < name->val.len; i++ ) + for( i = 0, j = 0; i < name->val.len; i++, j++ ) { - if( i >= sizeof( s ) - 1 ) - break; + if( j >= sizeof( s ) - 1 ) + return( MBEDTLS_ERR_X509_BUFFER_TOO_SMALL ); c = name->val.p[i]; + // Special characters requiring escaping, RFC 1779 + if( c && strchr( ",=+<>#;\"\\", c ) ) + { + if( j + 1 >= sizeof( s ) - 1 ) + continue; + s[j++] = '\\'; + } if( c < 32 || c >= 127 ) - s[i] = '?'; - else s[i] = c; + s[j] = '?'; + else s[j] = c; } - s[i] = '\0'; + s[j] = '\0'; ret = mbedtls_snprintf( p, n, "%s", s ); MBEDTLS_X509_SAFE_SNPRINTF; diff --git a/tests/data_files/Makefile b/tests/data_files/Makefile index f3cba5acb8..49db4cab27 100644 --- a/tests/data_files/Makefile +++ b/tests/data_files/Makefile @@ -937,6 +937,10 @@ server1.req.cert_type_empty: server1.key $(MBEDTLS_CERT_REQ) output_file=$@ filename=$< subject_name="C=NL,O=PolarSSL,CN=PolarSSL Server 1" md=SHA1 force_ns_cert_type=1 all_final += server1.req.cert_type_empty +server1.req.commas.sha256: server1.key + $(MBEDTLS_CERT_REQ) output_file=$@ filename=$< subject_name="C=NL,O=PolarSSL\, Commas,CN=PolarSSL Server 1" md=SHA256 +all_final += server1.req.commas.sha256 + # server2* server2_pwd_ec = PolarSSLTest @@ -994,7 +998,9 @@ server1.crt.der: server1.crt $(MBEDTLS_CERT_WRITE) request_file=server1.req.sha256 issuer_crt=$(test_ca_crt) issuer_key=$(test_ca_key_file_rsa) issuer_pwd=$(test_ca_pwd_rsa) not_before=20190210144406 not_after=20290210144406 md=SHA1 authority_identifier=0 version=3 output_file=$@ server1.der: server1.crt $(OPENSSL) x509 -inform PEM -in $< -outform DER -out $@ -all_final += server1.crt server1.noauthid.crt server1.crt.der +server1.commas.crt: server1.key server1.req.commas.sha256 $(test_ca_crt) $(test_ca_key_file_rsa) + $(MBEDTLS_CERT_WRITE) request_file=server1.req.commas.sha256 issuer_crt=$(test_ca_crt) issuer_key=$(test_ca_key_file_rsa) issuer_pwd=$(test_ca_pwd_rsa) version=1 not_before=20190210144406 not_after=20290210144406 md=SHA1 version=3 output_file=$@ +all_final += server1.crt server1.noauthid.crt server1.crt.der server1.commas.crt server1.key_usage.crt: server1.key server1.req.sha256 $(test_ca_crt) $(test_ca_key_file_rsa) $(MBEDTLS_CERT_WRITE) request_file=server1.req.sha256 issuer_crt=$(test_ca_crt) issuer_key=$(test_ca_key_file_rsa) issuer_pwd=$(test_ca_pwd_rsa) version=1 not_before=20190210144406 not_after=20290210144406 md=SHA1 key_usage=digital_signature,non_repudiation,key_encipherment version=3 output_file=$@ diff --git a/tests/data_files/server1.commas.crt b/tests/data_files/server1.commas.crt new file mode 100644 index 0000000000..5acd2555d4 --- /dev/null +++ b/tests/data_files/server1.commas.crt @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDRzCCAi+gAwIBAgIBATANBgkqhkiG9w0BAQUFADA7MQswCQYDVQQGEwJOTDER +MA8GA1UECgwIUG9sYXJTU0wxGTAXBgNVBAMMEFBvbGFyU1NMIFRlc3QgQ0EwHhcN +MTkwMjEwMTQ0NDA2WhcNMjkwMjEwMTQ0NDA2WjBEMQswCQYDVQQGEwJOTDEZMBcG +A1UECgwQUG9sYXJTU0wsIENvbW1hczEaMBgGA1UEAwwRUG9sYXJTU0wgU2VydmVy +IDEwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCpAh89QGrVVVOL/Tbu +gmUuFWFeib+46EWQ2+6IFlLT8UNQR5YSWWSHa/0r4Eb5c77dz5LhkVvtZqBviSl5 +RYDQg2rVQUN3Xzl8CQRHgrBXOXDto+wVGR6oMwhHwQVCqf1Mw7Tf3QYfTRBRQGdz +Ew9A+G2BJV8KsVPGMH4VOaz5Wu5/kp6mBVvnE5eFtSOS2dQkBtUJJYl1B92mGo8/ +CRm+rWUsZOuVm9z+QV4XptpsW2nMAroULBYknErczdD3Umdz8S2gI/1+9DHKLXDK +iQsE2y6mT3Buns69WIniU1meblqSZeKIPwyUGaPd5eidlRPtKdurcBLcWsprF6tS +glSxAgMBAAGjTTBLMAkGA1UdEwQCMAAwHQYDVR0OBBYEFB901j8pwXR0RTsFEiw9 +qL1DWQKmMB8GA1UdIwQYMBaAFLRa5KWz3tJS9rnVppUP6z68x/3/MA0GCSqGSIb3 +DQEBBQUAA4IBAQA1Ecg+VVJRmgFF9cnlztnXj4y9QKj8MCf2uZA3nTNe1Deh9l17 +ZNNWdPkXzVzf0IeR3LQRKT+daTzxuOOCSV9OxOcN0dIODBwa97BtNQfuWw2eWC9I +3UOVXbx8Ga+bXnD8ouatpyEG0FfhLO5YgEP0K9TyyN/nFa9kkB2Kvpy8yWm3w9WG +WgsOr2fpIExfC2ZFaiu3NVGTpT9fLv8RTatSC1XLA5Sr8NNHia3zCvEJEAlTuFHs +wm8apIAHlb44bbgW+7UwBIH9r2A21gQFy3v4cTLtlbnaUBbHUJvarK4ru70J+gew +OO3NZ1ocvnV+qGIcc7LgyNA8pZW5Jbewb/gN +-----END CERTIFICATE----- diff --git a/tests/data_files/server1.req.commas.sha256 b/tests/data_files/server1.req.commas.sha256 new file mode 100644 index 0000000000..0287a31109 --- /dev/null +++ b/tests/data_files/server1.req.commas.sha256 @@ -0,0 +1,16 @@ +-----BEGIN CERTIFICATE REQUEST----- +MIICiTCCAXECAQAwRDELMAkGA1UEBhMCTkwxGTAXBgNVBAoMEFBvbGFyU1NMLCBD +b21tYXMxGjAYBgNVBAMMEVBvbGFyU1NMIFNlcnZlciAxMIIBIjANBgkqhkiG9w0B +AQEFAAOCAQ8AMIIBCgKCAQEAqQIfPUBq1VVTi/027oJlLhVhXom/uOhFkNvuiBZS +0/FDUEeWEllkh2v9K+BG+XO+3c+S4ZFb7Wagb4kpeUWA0INq1UFDd185fAkER4Kw +Vzlw7aPsFRkeqDMIR8EFQqn9TMO0390GH00QUUBncxMPQPhtgSVfCrFTxjB+FTms ++Vruf5KepgVb5xOXhbUjktnUJAbVCSWJdQfdphqPPwkZvq1lLGTrlZvc/kFeF6ba +bFtpzAK6FCwWJJxK3M3Q91Jnc/EtoCP9fvQxyi1wyokLBNsupk9wbp7OvViJ4lNZ +nm5akmXiiD8MlBmj3eXonZUT7Snbq3AS3FrKaxerUoJUsQIDAQABoAAwDQYJKoZI +hvcNAQELBQADggEBAI7ZtRJYX6cMuwVhwXOizPV+WD17wby+273V4R8e9/6QA4nY +RrSciAse+nWZz9Y6toBzLWr0K/9SCzwBX4OzMvLqu4A1G/wApODCDnbGaUPNUxRt +6qbg8y7faBWvDGjk4+OpQ0suR/pdbM/L7pImqWRNwYdSPbJumNqIdB/Ewtso0TlA +QVZ992RPe1LovXpDCfPP2p123L7/UHezNCtu5QmzLsDfQmN/rLhCJ2NZzTsnIdnP +jp6XYU4kRV2BPDL65k38k8CSVWb6fw9XwPNUiyO3q1Zs6jpGJRYMLj9qTEoRN1np +RME09CN2siMcgkv8UqDeDJ4Oa9qyXS6VXsDmSNI= +-----END CERTIFICATE REQUEST----- diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 7ac91f641c..3edfc29833 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -318,6 +318,10 @@ X509 CSR Information RSA with SHA512 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_SHA512_C:MBEDTLS_RSA_C mbedtls_x509_csr_info:"data_files/server1.req.sha512":"CSR version \: 1\nsubject name \: C=NL, O=PolarSSL, CN=PolarSSL Server 1\nsigned using \: RSA with SHA-512\nRSA key size \: 2048 bits\n" +X509 CSR Information RSA with SHA-256, containing commas +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_SHA256_C:MBEDTLS_RSA_C:MBEDTS_X509_INFO +mbedtls_x509_csr_info:"data_files/server1.req.commas.sha256":"CSR version \: 1\nsubject name \: C=NL, O=PolarSSL\, Commas, CN=PolarSSL Server 1\nsigned using \: RSA with SHA-256\nRSA key size \: 2048 bits\n" + X509 CSR Information EC with SHA1 depends_on:MBEDTLS_ECDSA_C:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA1_C mbedtls_x509_csr_info:"data_files/server5.req.sha1":"CSR version \: 1\nsubject name \: C=NL, O=PolarSSL, CN=localhost\nsigned using \: ECDSA with SHA1\nEC key size \: 256 bits\n" @@ -399,6 +403,10 @@ 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 Distinguished Name #5 +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_SHA1_C +mbedtls_x509_dn_gets:"data_files/server1.commas.crt":"subject":"C=NL, O=PolarSSL\, Commas, 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_x509write.data b/tests/suites/test_suite_x509write.data index e0f80be943..cce99d459c 100644 --- a/tests/suites/test_suite_x509write.data +++ b/tests/suites/test_suite_x509write.data @@ -96,7 +96,7 @@ x509_crt_check:"data_files/server1.key":"":"C=NL,O=PolarSSL,CN=PolarSSL Server 1 X509 String to Names #1 -mbedtls_x509_string_to_names:"C=NL,O=Offspark\, Inc., OU=PolarSSL":"C=NL, O=Offspark, Inc., OU=PolarSSL":0 +mbedtls_x509_string_to_names:"C=NL,O=Offspark\, Inc., OU=PolarSSL":"C=NL, O=Offspark\, Inc., OU=PolarSSL":0 X509 String to Names #2 mbedtls_x509_string_to_names:"C=NL, O=Offspark, Inc., OU=PolarSSL":"":MBEDTLS_ERR_X509_UNKNOWN_OID From 9a2356b1909ccdf015c8b6032ebffb61a84e6097 Mon Sep 17 00:00:00 2001 From: Werner Lewis Date: Fri, 17 Jun 2022 15:51:55 +0100 Subject: [PATCH 2/4] Add tests for exceeded buffer size Signed-off-by: Werner Lewis --- tests/suites/test_suite_x509parse.data | 16 +++++++++++ tests/suites/test_suite_x509parse.function | 31 ++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 3edfc29833..0671606c74 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -407,6 +407,22 @@ X509 Get Distinguished Name #5 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_SHA1_C mbedtls_x509_dn_gets:"data_files/server1.commas.crt":"subject":"C=NL, O=PolarSSL\, Commas, CN=PolarSSL Server 1" +X509 Get Modified DN #1 +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_SHA1_C +mbedtls_x509_dn_gets_subject_replace:"data_files/server1.crt":"Modified":"C=NL, O=Modified, CN=PolarSSL Server 1":0 + +X509 Get Modified DN #2 Name exactly 255 bytes +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_SHA1_C +mbedtls_x509_dn_gets_subject_replace:"data_files/server1.crt":"123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345":"C=NL, O=123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345, CN=PolarSSL Server 1":0 + +X509 Get Modified DN #3 Name exceeds 255 bytes +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_SHA1_C +mbedtls_x509_dn_gets_subject_replace:"data_files/server1.crt":"1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456":"":MBEDTLS_ERR_X509_BUFFER_TOO_SMALL + +X509 Get Modified DN #4 Name exactly 255 bytes, with comma requiring escaping +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_SHA1_C +mbedtls_x509_dn_gets_subject_replace:"data_files/server1.crt":"1234567890,1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234":"":MBEDTLS_ERR_X509_BUFFER_TOO_SMALL + 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 00b68637c8..77f3d2338f 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -768,6 +768,37 @@ exit: } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_FS_IO:MBEDTLS_X509_CRT_PARSE_C:!MBEDTLS_X509_REMOVE_INFO */ +void mbedtls_x509_dn_gets_subject_replace( char * crt_file, char * new_subject_ou, char * result_str, int ret ) +{ + mbedtls_x509_crt crt; + char buf[2000]; + int res = 0; + + mbedtls_x509_crt_init( &crt ); + memset( buf, 0, 2000 ); + + TEST_ASSERT( mbedtls_x509_crt_parse_file( &crt, crt_file ) == 0 ); + crt.subject.next->val.p = (unsigned char *) new_subject_ou; + crt.subject.next->val.len = strlen( new_subject_ou ); + + res = mbedtls_x509_dn_gets( buf, 2000, &crt.subject ); + + if ( ret != 0 ) + { + TEST_ASSERT( res == ret ); + } + else + { + TEST_ASSERT( res != -1 ); + TEST_ASSERT( res != -2 ); + TEST_ASSERT( strcmp( buf, result_str ) == 0 ); + } +exit: + mbedtls_x509_crt_free( &crt ); +} +/* 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 2ee1e2dd2217afd253e3fb9705212ea00951a181 Mon Sep 17 00:00:00 2001 From: Werner Lewis Date: Mon, 27 Jun 2022 10:03:10 +0100 Subject: [PATCH 3/4] Replace parsing with outputting Signed-off-by: Werner Lewis --- ChangeLog.d/fix-csr_subject_commas.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ChangeLog.d/fix-csr_subject_commas.txt b/ChangeLog.d/fix-csr_subject_commas.txt index d497b1211f..e01c9a88ce 100644 --- a/ChangeLog.d/fix-csr_subject_commas.txt +++ b/ChangeLog.d/fix-csr_subject_commas.txt @@ -1,3 +1,3 @@ Bugfix - * Fix string representation of DNs when parsing values containing commas, - conforming to RFC 1779. Fixes #769. + * Fix string representation of DNs when outputting values containing commas + and other special characters, conforming to RFC 1779. Fixes #769. From 1421efa25e34f39d3acf1e9e16bec228a1440442 Mon Sep 17 00:00:00 2001 From: Werner Lewis Date: Mon, 27 Jun 2022 12:01:22 +0100 Subject: [PATCH 4/4] Fix case where final special char exceeds buffer Signed-off-by: Werner Lewis --- library/x509.c | 2 +- tests/suites/test_suite_x509parse.data | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/library/x509.c b/library/x509.c index c55d0387b6..3997ebd1f3 100644 --- a/library/x509.c +++ b/library/x509.c @@ -785,7 +785,7 @@ int mbedtls_x509_dn_gets( char *buf, size_t size, const mbedtls_x509_name *dn ) if( c && strchr( ",=+<>#;\"\\", c ) ) { if( j + 1 >= sizeof( s ) - 1 ) - continue; + return( MBEDTLS_ERR_X509_BUFFER_TOO_SMALL ); s[j++] = '\\'; } if( c < 32 || c >= 127 ) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 0671606c74..066d6e49f5 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -423,6 +423,10 @@ X509 Get Modified DN #4 Name exactly 255 bytes, with comma requiring escaping depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_SHA1_C mbedtls_x509_dn_gets_subject_replace:"data_files/server1.crt":"1234567890,1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234":"":MBEDTLS_ERR_X509_BUFFER_TOO_SMALL +X509 Get Modified DN #5 Name exactly 255 bytes, ending with comma requiring escaping +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_RSA_C:MBEDTLS_SHA1_C +mbedtls_x509_dn_gets_subject_replace:"data_files/server1.crt":"12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234,":"":MBEDTLS_ERR_X509_BUFFER_TOO_SMALL + 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