From 489939f829ad09dae72731729ea36240f630e49f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 22 Jun 2017 12:19:27 +0200 Subject: [PATCH] Improve behaviour on fatal errors If we didn't walk the whole chain, then there may be any kind of errors in the part of the chain we didn't check, so setting all flags looks like the safe thing to do. --- ChangeLog | 5 +++++ library/x509_crt.c | 22 ++++++++++++++++------ tests/suites/test_suite_x509parse.data | 2 +- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index 73da67a3b..dee13ea87 100644 --- a/ChangeLog +++ b/ChangeLog @@ -25,6 +25,11 @@ Bugfix to bypass the version verification check. Found by Peng Li/Yueh-Hsun Lin, KNOX Security, Samsung Research America +Changes + * Certificate verification functions now set flags to -1 in case the full + chain was not verified due to an internal error (including in the verify + callback) or chain length limitations. + = mbed TLS 2.1.8 branch released 2017-06-21 Security diff --git a/library/x509_crt.c b/library/x509_crt.c index d2aaa9202..8087ec4bb 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -2195,11 +2195,14 @@ int mbedtls_x509_crt_verify_with_profile( mbedtls_x509_crt *crt, mbedtls_x509_sequence *cur = NULL; mbedtls_pk_type_t pk_type; - if( profile == NULL ) - return( MBEDTLS_ERR_X509_BAD_INPUT_DATA ); - *flags = 0; + if( profile == NULL ) + { + ret = MBEDTLS_ERR_X509_BAD_INPUT_DATA; + goto exit; + } + if( cn != NULL ) { name = &crt->subject; @@ -2273,7 +2276,7 @@ int mbedtls_x509_crt_verify_with_profile( mbedtls_x509_crt *crt, ret = x509_crt_verify_top( crt, parent, ca_crl, profile, pathlen, selfsigned, flags, f_vrfy, p_vrfy ); if( ret != 0 ) - return( ret ); + goto exit; } else { @@ -2288,17 +2291,24 @@ int mbedtls_x509_crt_verify_with_profile( mbedtls_x509_crt *crt, ret = x509_crt_verify_child( crt, parent, trust_ca, ca_crl, profile, pathlen, selfsigned, flags, f_vrfy, p_vrfy ); if( ret != 0 ) - return( ret ); + goto exit; } else { ret = x509_crt_verify_top( crt, trust_ca, ca_crl, profile, pathlen, selfsigned, flags, f_vrfy, p_vrfy ); if( ret != 0 ) - return( ret ); + goto exit; } } +exit: + if( ret != 0 ) + { + *flags = (uint32_t) -1; + return( ret ); + } + if( *flags != 0 ) return( MBEDTLS_ERR_X509_CERT_VERIFY_FAILED ); diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index d11f5e394..17ec6caf1 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1186,7 +1186,7 @@ mbedtls_x509_crt_verify_max:"data_files/test-ca2.crt":"data_files/dir-maxpath":M X509 CRT verify long chain (max intermediate CA + 1) depends_on:MBEDTLS_SHA256_C:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED -mbedtls_x509_crt_verify_max:"data_files/dir-maxpath/00.crt":"data_files/dir-maxpath":MBEDTLS_X509_MAX_INTERMEDIATE_CA+1:MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:0 +mbedtls_x509_crt_verify_max:"data_files/dir-maxpath/00.crt":"data_files/dir-maxpath":MBEDTLS_X509_MAX_INTERMEDIATE_CA+1:MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:-1 X509 CRT verify chain #1 (zero pathlen intermediate) depends_on:MBEDTLS_SHA256_C:MBEDTLS_RSA_C