Shared code to free x509 structs

Signed-off-by: Glenn Strauss <gstrauss@gluelogic.com>

ChangeLog.d/mbedtls_asn1_type_free.txt

@@ -0,0 +1,2 @@
+Features
+   * Shared code to free x509 structs like mbedtls_x509_named_data

include/mbedtls/asn1.h

@@ -625,6 +625,15 @@ void mbedtls_asn1_free_named_data( mbedtls_asn1_named_data *entry );
  */
 void mbedtls_asn1_free_named_data_list( mbedtls_asn1_named_data **head );
 
+/**
+ * \brief       Free all shallow entries in a mbedtls_asn1_named_data list,
+ *              but do not free internal pointer targets.
+ *
+ * \param name  Head of the list of named data entries to free.
+ *              This function calls mbedtls_free() on each list element.
+ */
+void mbedtls_asn1_free_named_data_list_shallow( mbedtls_asn1_named_data *name );
+
 /** \} name Functions to parse ASN.1 data structures */
 /** \} addtogroup asn1_module */
 

library/asn1parse.c

@@ -455,6 +455,16 @@ void mbedtls_asn1_free_named_data_list( mbedtls_asn1_named_data **head )
     }
 }
 
+void mbedtls_asn1_free_named_data_list_shallow( mbedtls_asn1_named_data *name )
+{
+    for( mbedtls_asn1_named_data *next; name != NULL; name = next )
+    {
+        next = name->next;
+        mbedtls_platform_zeroize( name, sizeof( *name ) );
+        mbedtls_free( name );
+    }
+}
+
 const mbedtls_asn1_named_data *mbedtls_asn1_find_named_data( const mbedtls_asn1_named_data *list,
                                        const char *oid, size_t len )
 {

library/ssl_tls12_client.c

@@ -2680,7 +2680,6 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl )
     {
         unsigned char *p = dn + i + 2;
         mbedtls_x509_name name;
-        mbedtls_x509_name *name_cur, *name_prv;
         size_t asn1_len;
         char s[MBEDTLS_X509_MAX_DN_NAME_SIZE];
         memset( &name, 0, sizeof( name ) );
@@ -2700,14 +2699,7 @@ static int ssl_parse_certificate_request( mbedtls_ssl_context *ssl )
         MBEDTLS_SSL_DEBUG_MSG( 3,
             ( "DN hint: %.*s",
               mbedtls_x509_dn_gets( s, sizeof(s), &name ), s ) );
-        name_cur = name.next;
+        mbedtls_asn1_free_named_data_list_shallow( name.next );
-        while( name_cur != NULL )
-        {
-            name_prv = name_cur;
-            name_cur = name_cur->next;
-            mbedtls_platform_zeroize( name_prv, sizeof( mbedtls_x509_name ) );
-            mbedtls_free( name_prv );
-        }
     }
 #endif
 

library/x509.c

@@ -472,7 +472,6 @@ int mbedtls_x509_get_name( unsigned char **p, const unsigned char *end,
     size_t set_len;
     const unsigned char *end_set;
     mbedtls_x509_name *head = cur;
-    mbedtls_x509_name *prev, *allocated;
 
     /* don't use recursion, we'd risk stack overflow if not optimized */
     while( 1 )
@@ -530,18 +529,8 @@ int mbedtls_x509_get_name( unsigned char **p, const unsigned char *end,
 
 error:
     /* Skip the first element as we did not allocate it */
-    allocated = head->next;
+    mbedtls_asn1_free_named_data_list_shallow( head->next );
-
+    head->next = NULL;
-    while( allocated != NULL )
-    {
-        prev = allocated;
-        allocated = allocated->next;
-
-        mbedtls_platform_zeroize( prev, sizeof( *prev ) );
-        mbedtls_free( prev );
-    }
-
-    mbedtls_platform_zeroize( head, sizeof( *head ) );
 
     return( ret );
 }

library/x509_crl.c

@@ -705,28 +705,16 @@ void mbedtls_x509_crl_free( mbedtls_x509_crl *crl )
 {
     mbedtls_x509_crl *crl_cur = crl;
     mbedtls_x509_crl *crl_prv;
-    mbedtls_x509_name *name_cur;
-    mbedtls_x509_name *name_prv;
     mbedtls_x509_crl_entry *entry_cur;
     mbedtls_x509_crl_entry *entry_prv;
 
-    if( crl == NULL )
+    while( crl_cur != NULL )
-        return;
-
-    do
     {
 #if defined(MBEDTLS_X509_RSASSA_PSS_SUPPORT)
         mbedtls_free( crl_cur->sig_opts );
 #endif
 
-        name_cur = crl_cur->issuer.next;
+        mbedtls_asn1_free_named_data_list_shallow( crl_cur->issuer.next );
-        while( name_cur != NULL )
-        {
-            name_prv = name_cur;
-            name_cur = name_cur->next;
-            mbedtls_platform_zeroize( name_prv, sizeof( mbedtls_x509_name ) );
-            mbedtls_free( name_prv );
-        }
 
         entry_cur = crl_cur->entry.next;
         while( entry_cur != NULL )
@@ -744,13 +732,6 @@ void mbedtls_x509_crl_free( mbedtls_x509_crl *crl )
             mbedtls_free( crl_cur->raw.p );
         }
 
-        crl_cur = crl_cur->next;
-    }
-    while( crl_cur != NULL );
-
-    crl_cur = crl;
-    do
-    {
         crl_prv = crl_cur;
         crl_cur = crl_cur->next;
 
@@ -758,7 +739,6 @@ void mbedtls_x509_crl_free( mbedtls_x509_crl *crl )
         if( crl_prv != crl )
             mbedtls_free( crl_prv );
     }
-    while( crl_cur != NULL );
 }
 
 #endif /* MBEDTLS_X509_CRL_PARSE_C */

library/x509_crt.c

@@ -685,16 +685,7 @@ static int x509_get_subject_alt_name( unsigned char **p,
          */
         if( ret != 0 && ret != MBEDTLS_ERR_X509_FEATURE_UNAVAILABLE )
         {
-            mbedtls_x509_sequence *seq_cur = subject_alt_name->next;
+            mbedtls_asn1_sequence_free( subject_alt_name->next );
-            mbedtls_x509_sequence *seq_prv;
-            while( seq_cur != NULL )
-            {
-                seq_prv = seq_cur;
-                seq_cur = seq_cur->next;
-                mbedtls_platform_zeroize( seq_prv,
-                                          sizeof( mbedtls_x509_sequence ) );
-                mbedtls_free( seq_prv );
-            }
             subject_alt_name->next = NULL;
             return( ret );
         }
@@ -3300,15 +3291,8 @@ void mbedtls_x509_crt_free( mbedtls_x509_crt *crt )
 {
     mbedtls_x509_crt *cert_cur = crt;
     mbedtls_x509_crt *cert_prv;
-    mbedtls_x509_name *name_cur;
-    mbedtls_x509_name *name_prv;
-    mbedtls_x509_sequence *seq_cur;
-    mbedtls_x509_sequence *seq_prv;
 
-    if( crt == NULL )
+    while( cert_cur != NULL )
-        return;
-
-    do
     {
         mbedtls_pk_free( &cert_cur->pk );
 
@@ -3316,53 +3300,11 @@ void mbedtls_x509_crt_free( mbedtls_x509_crt *crt )
         mbedtls_free( cert_cur->sig_opts );
 #endif
 
-        name_cur = cert_cur->issuer.next;
+        mbedtls_asn1_free_named_data_list_shallow( cert_cur->issuer.next );
-        while( name_cur != NULL )
+        mbedtls_asn1_free_named_data_list_shallow( cert_cur->subject.next );
-        {
+        mbedtls_asn1_sequence_free( cert_cur->ext_key_usage.next );
-            name_prv = name_cur;
+        mbedtls_asn1_sequence_free( cert_cur->subject_alt_names.next );
-            name_cur = name_cur->next;
+        mbedtls_asn1_sequence_free( cert_cur->certificate_policies.next );
-            mbedtls_platform_zeroize( name_prv, sizeof( mbedtls_x509_name ) );
-            mbedtls_free( name_prv );
-        }
-
-        name_cur = cert_cur->subject.next;
-        while( name_cur != NULL )
-        {
-            name_prv = name_cur;
-            name_cur = name_cur->next;
-            mbedtls_platform_zeroize( name_prv, sizeof( mbedtls_x509_name ) );
-            mbedtls_free( name_prv );
-        }
-
-        seq_cur = cert_cur->ext_key_usage.next;
-        while( seq_cur != NULL )
-        {
-            seq_prv = seq_cur;
-            seq_cur = seq_cur->next;
-            mbedtls_platform_zeroize( seq_prv,
-                                      sizeof( mbedtls_x509_sequence ) );
-            mbedtls_free( seq_prv );
-        }
-
-        seq_cur = cert_cur->subject_alt_names.next;
-        while( seq_cur != NULL )
-        {
-            seq_prv = seq_cur;
-            seq_cur = seq_cur->next;
-            mbedtls_platform_zeroize( seq_prv,
-                                      sizeof( mbedtls_x509_sequence ) );
-            mbedtls_free( seq_prv );
-        }
-
-        seq_cur = cert_cur->certificate_policies.next;
-        while( seq_cur != NULL )
-        {
-            seq_prv = seq_cur;
-            seq_cur = seq_cur->next;
-            mbedtls_platform_zeroize( seq_prv,
-                                      sizeof( mbedtls_x509_sequence ) );
-            mbedtls_free( seq_prv );
-        }
 
         if( cert_cur->raw.p != NULL && cert_cur->own_buffer )
         {
@@ -3370,13 +3312,6 @@ void mbedtls_x509_crt_free( mbedtls_x509_crt *crt )
             mbedtls_free( cert_cur->raw.p );
         }
 
-        cert_cur = cert_cur->next;
-    }
-    while( cert_cur != NULL );
-
-    cert_cur = crt;
-    do
-    {
         cert_prv = cert_cur;
         cert_cur = cert_cur->next;
 
@@ -3384,7 +3319,6 @@ void mbedtls_x509_crt_free( mbedtls_x509_crt *crt )
         if( cert_prv != crt )
             mbedtls_free( cert_prv );
     }
-    while( cert_cur != NULL );
 }
 
 #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE)

library/x509_csr.c

@@ -375,9 +375,6 @@ void mbedtls_x509_csr_init( mbedtls_x509_csr *csr )
  */
 void mbedtls_x509_csr_free( mbedtls_x509_csr *csr )
 {
-    mbedtls_x509_name *name_cur;
-    mbedtls_x509_name *name_prv;
-
     if( csr == NULL )
         return;
 
@@ -387,14 +384,7 @@ void mbedtls_x509_csr_free( mbedtls_x509_csr *csr )
     mbedtls_free( csr->sig_opts );
 #endif
 
-    name_cur = csr->subject.next;
+    mbedtls_asn1_free_named_data_list_shallow( csr->subject.next );
-    while( name_cur != NULL )
-    {
-        name_prv = name_cur;
-        name_cur = name_cur->next;
-        mbedtls_platform_zeroize( name_prv, sizeof( mbedtls_x509_name ) );
-        mbedtls_free( name_prv );
-    }
 
     if( csr->raw.p != NULL )
     {

tests/suites/test_suite_x509parse.function

@@ -825,7 +825,6 @@ void mbedtls_x509_get_name( char * rdn_sequence, int exp_ret )
     unsigned char *p;
     size_t name_len;
     mbedtls_x509_name head;
-    mbedtls_x509_name *allocated, *prev;
     int ret;
 
     memset( &head, 0, sizeof( head ) );
@@ -835,17 +834,7 @@ void mbedtls_x509_get_name( char * rdn_sequence, int exp_ret )
 
     ret = mbedtls_x509_get_name( &p, ( name + name_len ), &head );
     if( ret == 0 )
-    {
+        mbedtls_asn1_free_named_data_list_shallow( head.next );
-        allocated = head.next;
-
-        while( allocated != NULL )
-        {
-            prev = allocated;
-            allocated = allocated->next;
-
-            mbedtls_free( prev );
-        }
-    }
 
     TEST_EQUAL( ret, exp_ret );
 
@@ -859,7 +848,7 @@ void mbedtls_x509_dn_get_next( char * name_str, int next_merged, char * expected
     int ret = 0, i;
     size_t len = 0, out_size;
     mbedtls_asn1_named_data *names = NULL;
-    mbedtls_x509_name parsed, *parsed_cur, *parsed_prv;
+    mbedtls_x509_name parsed, *parsed_cur;
     // Size of buf is maximum required for test cases
     unsigned char buf[80], *out = NULL, *c;
     const char *short_name;
@@ -913,14 +902,7 @@ void mbedtls_x509_dn_get_next( char * name_str, int next_merged, char * expected
 exit:
     mbedtls_free( out );
     mbedtls_asn1_free_named_data_list( &names );
-
+    mbedtls_asn1_free_named_data_list_shallow( parsed.next );
-    parsed_cur = parsed.next;
-    while( parsed_cur != 0 )
-    {
-        parsed_prv = parsed_cur;
-        parsed_cur = parsed_cur->next;
-        mbedtls_free( parsed_prv );
-    }
 }
 /* END_CASE */