From 1c38550bbdc7819263afaa6910bd26a432c27fde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 1 Sep 2015 16:35:00 +0200 Subject: [PATCH] Skip to trusted certs early in the chain This helps in the case where an intermediate certificate is directly trusted. In that case we want to ignore what comes after it in the chain, not only for performance but also to avoid false negatives (eg an old root being no longer trusted while the newer intermediate is directly trusted). see #220 backport of fdbdd72 --- library/x509_crt.c | 73 ++++++++++++++++++++------ tests/suites/test_suite_x509parse.data | 12 +++++ 2 files changed, 68 insertions(+), 17 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index b94f21322..3fb2b686e 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1930,8 +1930,8 @@ static int x509_crt_verify_child( *flags |= x509_crt_verifycrl(child, parent, ca_crl); #endif - /* Look for a grandparent upwards the chain */ - for( grandparent = parent->next; + /* Look for a grandparent in trusted CAs */ + for( grandparent = trust_ca; grandparent != NULL; grandparent = grandparent->next ) { @@ -1940,20 +1940,42 @@ static int x509_crt_verify_child( break; } - /* Is our parent part of the chain or at the top? */ if( grandparent != NULL ) { - ret = x509_crt_verify_child( parent, grandparent, trust_ca, ca_crl, + ret = x509_crt_verify_top( parent, grandparent, ca_crl, path_cnt + 1, &parent_flags, f_vrfy, p_vrfy ); if( ret != 0 ) return( ret ); } else { - ret = x509_crt_verify_top( parent, trust_ca, ca_crl, - path_cnt + 1, &parent_flags, f_vrfy, p_vrfy ); - if( ret != 0 ) - return( ret ); + /* Look for a grandparent upwards the chain */ + for( grandparent = parent->next; + grandparent != NULL; + grandparent = grandparent->next ) + { + if( x509_crt_check_parent( parent, grandparent, + 0, path_cnt == 0 ) == 0 ) + break; + } + + /* Is our parent part of the chain or at the top? */ + if( grandparent != NULL ) + { + ret = x509_crt_verify_child( parent, grandparent, trust_ca, ca_crl, + path_cnt + 1, &parent_flags, + f_vrfy, p_vrfy ); + if( ret != 0 ) + return( ret ); + } + else + { + ret = x509_crt_verify_top( parent, trust_ca, ca_crl, + path_cnt + 1, &parent_flags, + f_vrfy, p_vrfy ); + if( ret != 0 ) + return( ret ); + } } /* child is verified to be a child of the parent, call verify callback */ @@ -2035,27 +2057,44 @@ int x509_crt_verify( x509_crt *crt, } } - /* Look for a parent upwards the chain */ - for( parent = crt->next; parent != NULL; parent = parent->next ) + /* Look for a parent in trusted CAs */ + for( parent = trust_ca; parent != NULL; parent = parent->next ) { if( x509_crt_check_parent( crt, parent, 0, pathlen == 0 ) == 0 ) break; } - /* Are we part of the chain or at the top? */ if( parent != NULL ) { - ret = x509_crt_verify_child( crt, parent, trust_ca, ca_crl, - pathlen, flags, f_vrfy, p_vrfy ); + ret = x509_crt_verify_top( crt, parent, ca_crl, + pathlen, flags, f_vrfy, p_vrfy ); if( ret != 0 ) return( ret ); } else { - ret = x509_crt_verify_top( crt, trust_ca, ca_crl, - pathlen, flags, f_vrfy, p_vrfy ); - if( ret != 0 ) - return( ret ); + /* Look for a parent upwards the chain */ + for( parent = crt->next; parent != NULL; parent = parent->next ) + { + if( x509_crt_check_parent( crt, parent, 0, pathlen == 0 ) == 0 ) + break; + } + + /* Are we part of the chain or at the top? */ + if( parent != NULL ) + { + ret = x509_crt_verify_child( crt, parent, trust_ca, ca_crl, + pathlen, flags, f_vrfy, p_vrfy ); + if( ret != 0 ) + return( ret ); + } + else + { + ret = x509_crt_verify_top( crt, trust_ca, ca_crl, + pathlen, flags, f_vrfy, p_vrfy ); + if( ret != 0 ) + return( ret ); + } } if( *flags != 0 ) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 50a76f22a..73f550cd2 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -723,6 +723,10 @@ X509 Certificate verification callback: intermediate ca, root included depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_ECDSA_C:POLARSSL_RSA_C:POLARSSL_ECP_DP_SECP256R1_ENABLED:POLARSSL_ECP_DP_SECP384R1_ENABLED:POLARSSL_PKCS1_V15:POLARSSL_SHA256_C x509_verify_callback:"data_files/server7_int-ca_ca2.crt":"data_files/test-ca_cat12.crt":0:"depth 2 - serial C1\:43\:E2\:7E\:62\:43\:CC\:E8 - subject C=NL, O=PolarSSL, CN=Polarssl Test EC CA\ndepth 1 - serial 0E - subject C=NL, O=PolarSSL, CN=PolarSSL Test Intermediate CA\ndepth 0 - serial 10 - subject C=NL, O=PolarSSL, CN=localhost\n" +X509 Certificate verification callback: intermediate ca trusted +depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_ECDSA_C:POLARSSL_RSA_C:POLARSSL_ECP_DP_SECP256R1_ENABLED:POLARSSL_ECP_DP_SECP384R1_ENABLED:POLARSSL_PKCS1_V15:POLARSSL_SHA256_C +x509_verify_callback:"data_files/server7_int-ca_ca2.crt":"data_files/test-int-ca.crt":0:"depth 1 - serial 0E - subject C=NL, O=PolarSSL, CN=PolarSSL Test Intermediate CA\ndepth 0 - serial 10 - subject C=NL, O=PolarSSL, CN=localhost\n" + X509 Certificate verification callback: two intermediates depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_ECDSA_C:POLARSSL_RSA_C:POLARSSL_ECP_DP_SECP256R1_ENABLED:POLARSSL_ECP_DP_SECP384R1_ENABLED:POLARSSL_PKCS1_V15:POLARSSL_SHA256_C x509_verify_callback:"data_files/server10_int3_int-ca2.crt":"data_files/test-ca_cat21.crt":0:"depth 3 - serial 00 - subject C=NL, O=PolarSSL, CN=PolarSSL Test CA\ndepth 2 - serial 0F - subject C=NL, O=PolarSSL, CN=PolarSSL Test Intermediate EC CA\ndepth 1 - serial 4D - subject C=UK, O=mbed TLS, CN=mbed TLS Test intermediate CA 3\ndepth 0 - serial 4B - subject CN=localhost\n" @@ -731,6 +735,14 @@ X509 Certificate verification callback: two intermediates, root included depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_ECDSA_C:POLARSSL_RSA_C:POLARSSL_ECP_DP_SECP256R1_ENABLED:POLARSSL_ECP_DP_SECP384R1_ENABLED:POLARSSL_PKCS1_V15:POLARSSL_SHA256_C x509_verify_callback:"data_files/server10_int3_int-ca2_ca.crt":"data_files/test-ca_cat21.crt":0:"depth 3 - serial 00 - subject C=NL, O=PolarSSL, CN=PolarSSL Test CA\ndepth 2 - serial 0F - subject C=NL, O=PolarSSL, CN=PolarSSL Test Intermediate EC CA\ndepth 1 - serial 4D - subject C=UK, O=mbed TLS, CN=mbed TLS Test intermediate CA 3\ndepth 0 - serial 4B - subject CN=localhost\n" +X509 Certificate verification callback: two intermediates, top int trusted +depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_ECDSA_C:POLARSSL_RSA_C:POLARSSL_ECP_DP_SECP256R1_ENABLED:POLARSSL_ECP_DP_SECP384R1_ENABLED:POLARSSL_PKCS1_V15:POLARSSL_SHA256_C +x509_verify_callback:"data_files/server10_int3_int-ca2.crt":"data_files/test-int-ca2.crt":0:"depth 2 - serial 0F - subject C=NL, O=PolarSSL, CN=PolarSSL Test Intermediate EC CA\ndepth 1 - serial 4D - subject C=UK, O=mbed TLS, CN=mbed TLS Test intermediate CA 3\ndepth 0 - serial 4B - subject CN=localhost\n" + +X509 Certificate verification callback: two intermediates, low int trusted +depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_ECDSA_C:POLARSSL_RSA_C:POLARSSL_ECP_DP_SECP256R1_ENABLED:POLARSSL_ECP_DP_SECP384R1_ENABLED:POLARSSL_PKCS1_V15:POLARSSL_SHA256_C +x509_verify_callback:"data_files/server10_int3_int-ca2_ca.crt":"data_files/test-int-ca3.crt":0:"depth 1 - serial 4D - subject C=UK, O=mbed TLS, CN=mbed TLS Test intermediate CA 3\ndepth 0 - serial 4B - subject CN=localhost\n" + X509 Parse Selftest depends_on:POLARSSL_SHA1_C:POLARSSL_PEM_PARSE_C:POLARSSL_CERTS_C x509_selftest: