Minor fixes to CA callback tests

programs/ssl/ssl_client2.c

@@ -397,7 +397,7 @@ struct options
     int psk_opaque;
 #endif
 #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK)
-    int use_ca_callback         /* Use a callback for a trusted certificate list */
+    int ca_callback;            /* Use callback for trusted certificate list */
 #endif
     const char *psk;            /* the pre-shared key                       */
     const char *psk_identity;   /* the pre-shared key identity              */
@@ -453,21 +453,58 @@ static void my_debug( void *ctx, int level,
 }
 
 #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK)
-int ca_callback( void *data, mbedtls_x509_crt *child, mbedtls_x509_crt **candidates)
+int ca_callback( void *data, mbedtls_x509_crt const *child,
+                 mbedtls_x509_crt **candidates)
 {
+    int ret = 0;
     mbedtls_x509_crt *ca = (mbedtls_x509_crt *) data;
-    
+    mbedtls_x509_crt *first;
-    mbedtls_x509_crt *first = mbedtls_calloc( 1, sizeof( mbedtls_x509_crt ) );
+
-    TEST_ASSERT( first != NULL);
+    /* This is a test-only implementation of the CA callback
-    TEST_ASSERT( mbedtls_x509_crt_init( first ) == 0 );
+     * which always returns the entire list of trusted certificates.
-    TEST_ASSERT( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) == 0);
+     * Production implementations managing a large number of CAs
+     * should use an efficient presentation and lookup for the
+     * set of trusted certificates (such as a hashtable) and only
+     * return those trusted certificates which satisfy basic
+     * parental checks, such as the matching of child `Issuer`
+     * and parent `Subject` field. */
+    ((void) child);
+
+    first = mbedtls_calloc( 1, sizeof( mbedtls_x509_crt ) );
+    if( first == NULL )
+    {
+        ret = -1;
+        goto exit;
+    }
+    mbedtls_x509_crt_init( first );
+
+    if( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) != 0 )
+    {
+        ret = -1;
+        goto exit;
+    }
+
     while( ca->next != NULL )
     {
         ca = ca->next;
-        TEST_ASSERT( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) == 0);
+        if( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) != 0 )
+        {
+            ret = -1;
+            goto exit;
+        }
     }
+
+exit:
+
+    if( ret != 0 )
+    {
+        mbedtls_x509_crt_free( first );
+        mbedtls_free( first );
+        first = NULL;
+    }
+
     *candidates = first;
-    return 0;
+    return( ret );
 }
 #endif /* MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */
 
@@ -1641,7 +1678,7 @@ int main( int argc, char *argv[] )
     {
 #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK)
         if( opt.ca_callback != 0 )
-            mbedtls_ssl_conf_ca_cb( &conf, ca_callback, &cacert);
+            mbedtls_ssl_conf_ca_cb( &conf, ca_callback, &cacert );
         else
 #endif
             mbedtls_ssl_conf_ca_chain( &conf, &cacert, NULL );

programs/ssl/ssl_server2.c

@@ -516,7 +516,7 @@ struct options
     int psk_list_opaque;
 #endif
 #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK)
-    int use_ca_callback         /* Use a callback for a trusted certificate list */
+    int ca_callback;            /* Use callback for trusted certificate list */
 #endif
     const char *psk;            /* the pre-shared key                       */
     const char *psk_identity;   /* the pre-shared key identity              */
@@ -576,21 +576,58 @@ static void my_debug( void *ctx, int level,
 }
 
 #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK)
-int ca_callback( void *data, mbedtls_x509_crt *child, mbedtls_x509_crt **candidates)
+int ca_callback( void *data, mbedtls_x509_crt const *child,
+                 mbedtls_x509_crt **candidates)
 {
+    int ret = 0;
     mbedtls_x509_crt *ca = (mbedtls_x509_crt *) data;
-    
+    mbedtls_x509_crt *first;
-    mbedtls_x509_crt *first = mbedtls_calloc( 1, sizeof( mbedtls_x509_crt ) );
+
-    TEST_ASSERT( first != NULL);
+    /* This is a test-only implementation of the CA callback
-    TEST_ASSERT( mbedtls_x509_crt_init( first ) == 0 );
+     * which always returns the entire list of trusted certificates.
-    TEST_ASSERT( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) == 0);
+     * Production implementations managing a large number of CAs
+     * should use an efficient presentation and lookup for the
+     * set of trusted certificates (such as a hashtable) and only
+     * return those trusted certificates which satisfy basic
+     * parental checks, such as the matching of child `Issuer`
+     * and parent `Subject` field. */
+    ((void) child);
+
+    first = mbedtls_calloc( 1, sizeof( mbedtls_x509_crt ) );
+    if( first == NULL )
+    {
+        ret = -1;
+        goto exit;
+    }
+    mbedtls_x509_crt_init( first );
+
+    if( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) != 0 )
+    {
+        ret = -1;
+        goto exit;
+    }
+
     while( ca->next != NULL )
     {
         ca = ca->next;
-        TEST_ASSERT( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) == 0);
+        if( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) != 0 )
+        {
+            ret = -1;
+            goto exit;
+        }
     }
+
+exit:
+
+    if( ret != 0 )
+    {
+        mbedtls_x509_crt_free( first );
+        mbedtls_free( first );
+        first = NULL;
+    }
+
     *candidates = first;
-    return 0;
+    return( ret );
 }
 #endif /* MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */
 

tests/suites/test_suite_x509parse.data

@@ -829,7 +829,7 @@ x509_verify:"data_files/cert_sha256.crt":"data_files/test-ca.crt":"data_files/cr
 
 X509 Certificate verification with ca callback: failure
 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_SHA1_C:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK
-x509_verify_ca_cb_failure:"data_files/server1.crt":"data_files/test-ca.crt":"NULL":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:"depth 1 - serial 03 - subject C=NL, O=PolarSSL, CN=PolarSSL Test CA - flags 0x00000000\ndepth 0 - serial 01 - subject C=NL, O=PolarSSL, CN=PolarSSL Server 1 - flags 0x00000000\n"
+x509_verify_ca_cb_failure:"data_files/server1.crt":"data_files/test-ca.crt":"NULL":MBEDTLS_ERR_X509_FATAL_ERROR
 
 X509 Certificate verification callback: bad name
 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_SECP384R1_ENABLED

tests/suites/test_suite_x509parse.function

@@ -69,7 +69,7 @@ int verify_all( void *data, mbedtls_x509_crt *crt, int certificate_depth, uint32
 }
 
 #if defined(MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK)
-int ca_callback_fail( void *data, mbedtls_x509_crt *child, mbedtls_x509_crt **candidates)
+int ca_callback_fail( void *data, mbedtls_x509_crt const *child, mbedtls_x509_crt **candidates)
 {
     ((void) data);
     ((void) child);
@@ -78,21 +78,58 @@ int ca_callback_fail( void *data, mbedtls_x509_crt *child, mbedtls_x509_crt **ca
     return -1;
 }
 
-int ca_callback( void *data, mbedtls_x509_crt *child, mbedtls_x509_crt **candidates)
+int ca_callback( void *data, mbedtls_x509_crt const *child,
+                 mbedtls_x509_crt **candidates)
 {
+    int ret = 0;
     mbedtls_x509_crt *ca = (mbedtls_x509_crt *) data;
-    
+    mbedtls_x509_crt *first;
-    mbedtls_x509_crt *first = mbedtls_calloc( 1, sizeof( mbedtls_x509_crt ) );
+
-    TEST_ASSERT( first != NULL);
+    /* This is a test-only implementation of the CA callback
-    TEST_ASSERT( mbedtls_x509_crt_init( first ) == 0 );
+     * which always returns the entire list of trusted certificates.
-    TEST_ASSERT( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) == 0);
+     * Production implementations managing a large number of CAs
+     * should use an efficient presentation and lookup for the
+     * set of trusted certificates (such as a hashtable) and only
+     * return those trusted certificates which satisfy basic
+     * parental checks, such as the matching of child `Issuer`
+     * and parent `Subject` field. */
+    ((void) child);
+
+    first = mbedtls_calloc( 1, sizeof( mbedtls_x509_crt ) );
+    if( first == NULL )
+    {
+        ret = -1;
+        goto exit;
+    }
+    mbedtls_x509_crt_init( first );
+
+    if( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) != 0 )
+    {
+        ret = -1;
+        goto exit;
+    }
+
     while( ca->next != NULL )
     {
         ca = ca->next;
-        TEST_ASSERT( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) == 0);
+        if( mbedtls_x509_crt_parse_der( first, ca->raw.p, ca->raw.len ) != 0 )
+        {
+            ret = -1;
+            goto exit;
+        }
     }
+
+exit:
+
+    if( ret != 0 )
+    {
+        mbedtls_x509_crt_free( first );
+        mbedtls_free( first );
+        first = NULL;
+    }
+
     *candidates = first;
-    return 0;
+    return( ret );
 }
 #endif /* MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */
 
@@ -419,7 +456,7 @@ exit:
 
 /* BEGIN_CASE depends_on:MBEDTLS_FS_IO:MBEDTLS_X509_CRT_PARSE_C:MBEDTLS_X509_CRL_PARSE_C:MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK */
 void x509_verify_ca_cb_failure( char *crt_file, char *ca_file, char *name,
-                                int exp_ret, char *exp_vrfy_out )
+                                int exp_ret )
 {
     int ret;
     mbedtls_x509_crt crt;
@@ -434,8 +471,10 @@ void x509_verify_ca_cb_failure( char *crt_file, char *ca_file, char *name,
 
     if( strcmp( name, "NULL" ) == 0 )
         name = NULL;
-    
+
-    ret = mbedtls_x509_crt_verify_with_cb( &crt, ca_callback_fail, &ca, &compat_profile, name, &flags, verify_all, NULL );
+    ret = mbedtls_x509_crt_verify_with_cb( &crt, ca_callback_fail, &ca,
+                                           &compat_profile, name, &flags,
+                                           verify_all, NULL );
 
     TEST_ASSERT( ret == exp_ret );
 exit: