Only return VERIFY_FAILED from a single point
Everything else is a fatal error. Also improve documentation about that for
the vrfy callback.
diff --git a/ChangeLog b/ChangeLog
index f008cdc..354fcab 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -29,6 +29,9 @@
* 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 adaacff..de487ce 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 24f7c7a..eef7f3a 100644
--- a/include/polarssl/x509_crt.h
+++ b/include/polarssl/x509_crt.h
@@ -232,7 +232,13 @@
*
* 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 98fba5c..be642ca 100644
--- a/library/error.c
+++ b/library/error.c
@@ -496,6 +496,8 @@
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 70ad356..fb09253 100644
--- a/library/x509_crt.c
+++ b/library/x509_crt.c
@@ -1957,8 +1957,8 @@
/* 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 @@
}
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 c0d9685..4697438 100755
--- a/scripts/generate_errors.pl
+++ b/scripts/generate_errors.pl
@@ -94,7 +94,7 @@
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 f68e966..135ec60 100644
--- a/tests/suites/test_suite_x509parse.data
+++ b/tests/suites/test_suite_x509parse.data
@@ -1182,7 +1182,7 @@
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