diff --git a/ChangeLog b/ChangeLog index f008cdc5f..354fcaba5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -29,6 +29,9 @@ 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. + * With authmode set to optional, handshake is now aborted if the + verification of the peer's certificate failed due to an overlong chain or + a fatal error in the vrfy callback. = mbed TLS 1.3.20 released 2017-06-21 diff --git a/include/polarssl/x509.h b/include/polarssl/x509.h index adaacfff1..de487cee4 100644 --- a/include/polarssl/x509.h +++ b/include/polarssl/x509.h @@ -76,6 +76,7 @@ #define POLARSSL_ERR_X509_BAD_INPUT_DATA -0x2800 /**< Input invalid. */ #define POLARSSL_ERR_X509_MALLOC_FAILED -0x2880 /**< Allocation of memory failed. */ #define POLARSSL_ERR_X509_FILE_IO_ERROR -0x2900 /**< Read/write of file failed. */ +#define POLARSSL_ERR_X509_FATAL_ERROR -0x3000 /**< A fatal error occured, eg the chain is too long or the vrfy callback failed. */ /* \} name */ /** diff --git a/include/polarssl/x509_crt.h b/include/polarssl/x509_crt.h index 24f7c7ab6..eef7f3afb 100644 --- a/include/polarssl/x509_crt.h +++ b/include/polarssl/x509_crt.h @@ -232,7 +232,13 @@ int x509_crt_verify_info( char *buf, size_t size, const char *prefix, * * All flags left after returning from the callback * are also returned to the application. The function should - * return 0 for anything but a fatal error. + * return 0 for anything (including invalid certificates) + * other than fatal error, as a non-zero return code + * immediately aborts the verification process. For fatal + * errors, a specific error code should be used (different + * from POLARSSL_ERR_X509_CERT_VERIFY_FAILED which should not + * be returned at this point), or POLARSSL_ERR_X509_FATAL_ERROR + * can be used if no better code is available. * * \note In case verification failed, the results can be displayed * using \c x509_crt_verify_info() diff --git a/library/error.c b/library/error.c index 98fba5cbd..be642cada 100644 --- a/library/error.c +++ b/library/error.c @@ -496,6 +496,8 @@ void polarssl_strerror( int ret, char *buf, size_t buflen ) polarssl_snprintf( buf, buflen, "X509 - Allocation of memory failed" ); if( use_ret == -(POLARSSL_ERR_X509_FILE_IO_ERROR) ) polarssl_snprintf( buf, buflen, "X509 - Read/write of file failed" ); + if( use_ret == -(POLARSSL_ERR_X509_FATAL_ERROR) ) + polarssl_snprintf( buf, buflen, "X509 - A fatal error occured, eg the chain is too long or the vrfy callback failed" ); #endif /* POLARSSL_X509_USE,X509_CREATE_C */ // END generated code diff --git a/library/x509_crt.c b/library/x509_crt.c index 70ad356a9..fb09253e6 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1957,8 +1957,8 @@ static int x509_crt_verify_child( /* path_cnt is 0 for the first intermediate CA */ if( 1 + path_cnt > POLARSSL_X509_MAX_INTERMEDIATE_CA ) { - *flags |= BADCERT_NOT_TRUSTED; - return( POLARSSL_ERR_X509_CERT_VERIFY_FAILED ); + /* return immediately as the goal is to avoid unbounded recursion */ + return( POLARSSL_ERR_X509_FATAL_ERROR ); } if( x509_time_expired( &child->valid_to ) ) @@ -2174,6 +2174,10 @@ int x509_crt_verify( x509_crt *crt, } exit: + /* prevent misuse of the vrfy callback */ + if( ret == POLARSSL_ERR_X509_CERT_VERIFY_FAILED ) + ret = POLARSSL_ERR_X509_FATAL_ERROR; + if( ret != 0 ) { *flags = (uint32_t) -1; diff --git a/scripts/generate_errors.pl b/scripts/generate_errors.pl index c0d9685f6..469743833 100755 --- a/scripts/generate_errors.pl +++ b/scripts/generate_errors.pl @@ -94,7 +94,7 @@ while (my $line = ) my $found_hl = grep $_ eq $module_name, @high_level_modules; if (!$found_ll && !$found_hl) { - polarssl_printf("Error: Do not know how to handle: $module_name\n"); + printf("Error: Do not know how to handle: $module_name\n"); exit 1; } diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index f68e966e2..135ec60bc 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1182,7 +1182,7 @@ x509_crt_verify_max:"data_files/test-ca2.crt":"data_files/dir-maxpath":POLARSSL_ X509 CRT verify long chain (max intermediate CA + 1) depends_on:POLARSSL_SHA256_C:POLARSSL_ECDSA_C:POLARSSL_ECP_DP_SECP256R1_ENABLED -x509_crt_verify_max:"data_files/dir-maxpath/00.crt":"data_files/dir-maxpath":POLARSSL_X509_MAX_INTERMEDIATE_CA+1:POLARSSL_ERR_X509_CERT_VERIFY_FAILED:-1 +x509_crt_verify_max:"data_files/dir-maxpath/00.crt":"data_files/dir-maxpath":POLARSSL_X509_MAX_INTERMEDIATE_CA+1:POLARSSL_ERR_X509_FATAL_ERROR:-1 X509 CRT verify chain #1 (zero pathlen intermediate) depends_on:POLARSSL_SHA256_C:POLARSSL_RSA_C