From 8dcb2d7d7e6275ad2d669f531fb1390e467ea564 Mon Sep 17 00:00:00 2001 From: Paul Bakker Date: Fri, 8 Aug 2014 12:22:30 +0200 Subject: [PATCH] Support escaping of commas in x509_string_to_names() --- ChangeLog | 4 ++ include/polarssl/x509.h | 2 + library/x509.c | 2 +- library/x509_create.c | 32 ++++++++++++++-- tests/suites/test_suite_x509write.data | 18 +++++++++ tests/suites/test_suite_x509write.function | 44 ++++++++++++++++++++++ 6 files changed, 98 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 163bdb796..52c8bd1ec 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ PolarSSL ChangeLog (Sorted per branch, date) += 1.3 branch +Bugfix + * Support escaping of commas in x509_string_to_names() + = PolarSSL 1.3.8 released 2014-07-11 Security * Fix length checking for AEAD ciphersuites (found by Codenomicon). diff --git a/include/polarssl/x509.h b/include/polarssl/x509.h index 583cb8320..9b0bcb78a 100644 --- a/include/polarssl/x509.h +++ b/include/polarssl/x509.h @@ -143,6 +143,8 @@ #define X509_FORMAT_DER 1 #define X509_FORMAT_PEM 2 +#define X509_MAX_DN_NAME_SIZE 256 /**< Maximum value size of a DN entry */ + #ifdef __cplusplus extern "C" { #endif diff --git a/library/x509.c b/library/x509.c index 17c7a7db0..13081e35d 100644 --- a/library/x509.c +++ b/library/x509.c @@ -750,7 +750,7 @@ int x509_dn_gets( char *buf, size_t size, const x509_name *dn ) unsigned char c; const x509_name *name; const char *short_name = NULL; - char s[128], *p; + char s[X509_MAX_DN_NAME_SIZE], *p; memset( s, 0, sizeof( s ) ); diff --git a/library/x509_create.c b/library/x509_create.c index 101931332..747dc8280 100644 --- a/library/x509_create.c +++ b/library/x509_create.c @@ -100,6 +100,8 @@ int x509_string_to_names( asn1_named_data **head, const char *name ) const char *end = s + strlen( s ); const char *oid = NULL; int in_tag = 1; + char data[X509_MAX_DN_NAME_SIZE]; + char *d = data; /* Clear existing chain if present */ asn1_free_named_data_list( head ); @@ -116,13 +118,25 @@ int x509_string_to_names( asn1_named_data **head, const char *name ) s = c + 1; in_tag = 0; + d = data; } - if( !in_tag && ( *c == ',' || c == end ) ) + if( !in_tag && *c == '\\' && c != end ) + { + c++; + + /* Check for valid escaped characters */ + if( c == end || *c != ',' ) + { + ret = POLARSSL_ERR_X509_INVALID_NAME; + goto exit; + } + } + else if( !in_tag && ( *c == ',' || c == end ) ) { if( asn1_store_named_data( head, oid, strlen( oid ), - (unsigned char *) s, - c - s ) == NULL ) + (unsigned char *) data, + d - data ) == NULL ) { return( POLARSSL_ERR_X509_MALLOC_FAILED ); } @@ -133,6 +147,18 @@ int x509_string_to_names( asn1_named_data **head, const char *name ) s = c + 1; in_tag = 1; } + + if( !in_tag && s != c + 1 ) + { + *(d++) = *c; + + if( d - data == X509_MAX_DN_NAME_SIZE ) + { + ret = POLARSSL_ERR_X509_INVALID_NAME; + goto exit; + } + } + c++; } diff --git a/tests/suites/test_suite_x509write.data b/tests/suites/test_suite_x509write.data index b244bbbe3..66f099387 100644 --- a/tests/suites/test_suite_x509write.data +++ b/tests/suites/test_suite_x509write.data @@ -57,3 +57,21 @@ x509_crt_check:"data_files/server1.key":"":"C=NL,O=PolarSSL,CN=PolarSSL Server 1 Certificate write check Server1 SHA1, version 1 depends_on:POLARSSL_SHA1_C:POLARSSL_RSA_C:POLARSSL_PKCS1_V15:POLARSSL_DES_C:POLARSSL_CIPHER_MODE_CBC:POLARSSL_MD5_C x509_crt_check:"data_files/server1.key":"":"C=NL,O=PolarSSL,CN=PolarSSL Server 1":"data_files/test-ca.key":"PolarSSLTest":"C=NL,O=PolarSSL,CN=PolarSSL Test CA":"1":"20110212144406":"20210212144406":POLARSSL_MD_SHA1:0:0:X509_CRT_VERSION_1:"data_files/server1.v1.crt" + +X509 String to Names #1 +x509_string_to_names:"C=NL,O=Offspark\, Inc., OU=PolarSSL":"C=NL, O=Offspark, Inc., OU=PolarSSL":0 + +X509 String to Names #2 +x509_string_to_names:"C=NL, O=Offspark, Inc., OU=PolarSSL":"":POLARSSL_ERR_X509_UNKNOWN_OID + +X509 String to Names #3 (Name precisely 255 bytes) +x509_string_to_names:"C=NL, O=123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345,OU=PolarSSL":"C=NL, O=123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345, OU=PolarSSL":0 + +X509 String to Names #4 (Name larger than 255 bytes) +x509_string_to_names:"C=NL, O=1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456, OU=PolarSSL":"":POLARSSL_ERR_X509_INVALID_NAME + +X509 String to Names #5 (Escape non-allowed characters) +x509_string_to_names:"C=NL, O=Offspark\a Inc., OU=PolarSSL":"":POLARSSL_ERR_X509_INVALID_NAME + +X509 String to Names #6 (Escape at end) +x509_string_to_names:"C=NL, O=Offspark\":"":POLARSSL_ERR_X509_INVALID_NAME diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index 340629e99..701ed0040 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -132,3 +132,47 @@ exit: mpi_free( &serial ); } /* END_CASE */ + +/* BEGIN_CASE depends_on:POLARSSL_X509_CREATE_C:POLARSSL_X509_USE_C */ +void x509_string_to_names( char *name, char *parsed_name, int result ) +{ + int ret; + size_t len = 0; + asn1_named_data *names = NULL; + x509_name parsed, *parsed_cur, *parsed_prv; + unsigned char buf[2048], *c; + + memset( &parsed, 0, sizeof( parsed ) ); + memset( buf, 0, sizeof( buf ) ); + c = buf + sizeof( buf ); + + ret = x509_string_to_names( &names, name ); + TEST_ASSERT( ret == result ); + + if( ret != 0 ) + goto exit; + + ret = x509_write_names( &c, buf, names ); + TEST_ASSERT( ret > 0 ); + + TEST_ASSERT( asn1_get_tag( &c, buf + sizeof( buf ), &len, + ASN1_CONSTRUCTED | ASN1_SEQUENCE ) == 0 ); + TEST_ASSERT( x509_get_name( &c, buf + sizeof( buf ), &parsed ) == 0 ); + + ret = x509_dn_gets( (char *) buf, sizeof( buf ), &parsed ); + TEST_ASSERT( ret > 0 ); + + TEST_ASSERT( strcmp( (char *) buf, parsed_name ) == 0 ); + +exit: + asn1_free_named_data_list( &names ); + + parsed_cur = parsed.next; + while( parsed_cur != 0 ) + { + parsed_prv = parsed_cur; + parsed_cur = parsed_cur->next; + polarssl_free( parsed_prv ); + } +} +/* END_CASE */