CVE-2017-14032: Improper Authentication (CWE-287)
value: ARM mbed TLS before 1.3.21 and 2.x before 2.1.9, if optional authentication is configured, allows remote attackers to bypass peer authentication via an X.509 certificate chain with many intermediates. NOTE: although mbed TLS was formerly known as PolarSSL, the releases shipped with the PolarSSL name are not affected.
CWE description: When an actor claims to have a given identity, the product does not prove or insufficiently proves that the claim is correct.
CVSS v3 base score: 8.1
Published: 2017-08-30T20:29Z
Repository: https://github.com/ARMmbed/mbedtls
Language: C
Files changed: ChangeLog, include/mbedtls/error.h, include/mbedtls/ssl.h, include/mbedtls/x509.h, include/mbedtls/x509_crt.h, library/error.c, library/x509_crt.c, tests/suites/test_suite_x509parse.data
Commit date: 2017-07-06 11:58:41 +0200
Commit message: Only return VERIFY_FAILED from a single point

Everything else is a fatal error. Also improve documentation about that for
the vrfy callback.

Diff: diff --git a/ChangeLog b/ChangeLog
index 9bf6a1719..d35457b96 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -4,10 +4,13 @@ mbed TLS ChangeLog (Sorted per branch, date)
 
 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 2.5.1 released 2017-06-21
 
 Security
    * Fixed unlimited overread of heap-based buffer in mbedtls_ssl_read().
diff --git a/include/mbedtls/error.h b/include/mbedtls/error.h
index 5e549f6b6..31591e2d6 100644
--- a/include/mbedtls/error.h
+++ b/include/mbedtls/error.h
@@ -69,11 +69,11 @@
  *
  * High-level module nr (3 bits - 0x0...-0x7...)
  * Name      ID  Nr of Errors
  * PEM       1   9
  * PKCS#12   1   4 (Started from top)
- * X509      2   19
+ * X509      2   20
  * PKCS5     2   4 (Started from top)
  * DHM       3   9
  * PK        3   14 (Started from top)
  * RSA       4   9
  * ECP       4   8 (Started from top)
diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h
index 51c1c60d7..cc0007006 100644
--- a/include/mbedtls/ssl.h
+++ b/include/mbedtls/ssl.h
@@ -1050,11 +1050,11 @@ void mbedtls_ssl_conf_authmode( mbedtls_ssl_config *conf, int authmode );
 /**
  * \brief          Set the verification callback (Optional).
  *
  *                 If set, the verify callback is called for each
  *                 certificate in the chain. For implementation
- *                 information, please see \c x509parse_verify()
+ *                 information, please see \c mbedtls_x509_crt_verify()
  *
  * \param conf     SSL configuration
  * \param f_vrfy   verification function
  * \param p_vrfy   verification parameter
  */
diff --git a/include/mbedtls/x509.h b/include/mbedtls/x509.h
index f219bf128..128eaded6 100644
--- a/include/mbedtls/x509.h
+++ b/include/mbedtls/x509.h
@@ -74,10 +74,11 @@
 #define MBEDTLS_ERR_X509_CERT_UNKNOWN_FORMAT              -0x2780  /**< Format not recognized as DER or PEM. */
 #define MBEDTLS_ERR_X509_BAD_INPUT_DATA                   -0x2800  /**< Input invalid. */
 #define MBEDTLS_ERR_X509_ALLOC_FAILED                     -0x2880  /**< Allocation of memory failed. */
 #define MBEDTLS_ERR_X509_FILE_IO_ERROR                    -0x2900  /**< Read/write of file failed. */
 #define MBEDTLS_ERR_X509_BUFFER_TOO_SMALL                 -0x2980  /**< Destination buffer is too small. */
+#define MBEDTLS_ERR_X509_FATAL_ERROR                      -0x3000  /**< A fatal error occured, eg the chain is too long or the vrfy callback failed. */
 /* \} name */
 
 /**
  * \name X509 Verify codes
  * \{
diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h
index 383e484f7..fd203360c 100644
--- a/include/mbedtls/x509_crt.h
+++ b/include/mbedtls/x509_crt.h
@@ -265,11 +265,17 @@ int mbedtls_x509_crt_verify_info( char *buf, size_t size, const char *prefix,
  *                 that specific certificate and the certificate depth from
  *                 the bottom (Peer cert depth = 0).
  *
  *                 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 MBEDTLS_ERR_X509_CERT_VERIFY_FAILED which should not
+ *                 be returned at this point), or MBEDTLS_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 mbedtls_x509_crt_verify_info()
  *
  * \note           Same as \c mbedtls_x509_crt_verify_with_profile() with the
diff --git a/library/error.c b/library/error.c
index dd2db0c45..db42381c4 100644
--- a/library/error.c
+++ b/library/error.c
@@ -478,10 +478,12 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen )
             mbedtls_snprintf( buf, buflen, "X509 - Allocation of memory failed" );
         if( use_ret == -(MBEDTLS_ERR_X509_FILE_IO_ERROR) )
             mbedtls_snprintf( buf, buflen, "X509 - Read/write of file failed" );
         if( use_ret == -(MBEDTLS_ERR_X509_BUFFER_TOO_SMALL) )
             mbedtls_snprintf( buf, buflen, "X509 - Destination buffer is too small" );
+        if( use_ret == -(MBEDTLS_ERR_X509_FATAL_ERROR) )
+            mbedtls_snprintf( buf, buflen, "X509 - A fatal error occured, eg the chain is too long or the vrfy callback failed" );
 #endif /* MBEDTLS_X509_USE_C || MBEDTLS_X509_CREATE_C */
         // END generated code
 
         if( strlen( buf ) == 0 )
             mbedtls_snprintf( buf, buflen, "UNKNOWN ERROR CODE (%04X)", use_ret );
diff --git a/library/x509_crt.c b/library/x509_crt.c
index ee5f27e46..ec5f77268 100644
--- a/library/x509_crt.c
+++ b/library/x509_crt.c
@@ -2055,12 +2055,12 @@ static int x509_crt_verify_child(
         self_cnt++;
 
     /* path_cnt is 0 for the first intermediate CA */
     if( 1 + path_cnt > MBEDTLS_X509_MAX_INTERMEDIATE_CA )
     {
-        *flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED;
-        return( MBEDTLS_ERR_X509_CERT_VERIFY_FAILED );
+        /* return immediately as the goal is to avoid unbounded recursion */
+        return( MBEDTLS_ERR_X509_FATAL_ERROR );
     }
 
     if( mbedtls_x509_time_is_past( &child->valid_to ) )
         *flags |= MBEDTLS_X509_BADCERT_EXPIRED;
 
@@ -2308,10 +2308,14 @@ int mbedtls_x509_crt_verify_with_profile( mbedtls_x509_crt *crt,
                 goto exit;
         }
     }
 
 exit:
+    /* prevent misuse of the vrfy callback */
+    if( ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED )
+        ret = MBEDTLS_ERR_X509_FATAL_ERROR;
+
     if( ret != 0 )
     {
         *flags = (uint32_t) -1;
         return( ret );
     }
diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data
index 6df529875..ea56f3fbc 100644
--- a/tests/suites/test_suite_x509parse.data
+++ b/tests/suites/test_suite_x509parse.data
@@ -1202,11 +1202,11 @@ X509 CRT verify long chain (max intermediate CA, untrusted)
 depends_on:MBEDTLS_SHA256_C:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_SECP384R1_ENABLED
 mbedtls_x509_crt_verify_max:"data_files/test-ca2.crt":"data_files/dir-maxpath":MBEDTLS_X509_MAX_INTERMEDIATE_CA-1:MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_NOT_TRUSTED
 
 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:-1
+mbedtls_x509_crt_verify_max:"data_files/dir-maxpath/00.crt":"data_files/dir-maxpath":MBEDTLS_X509_MAX_INTERMEDIATE_CA+1:MBEDTLS_ERR_X509_FATAL_ERROR:-1
 
 X509 CRT verify chain #1 (zero pathlen intermediate)
 depends_on:MBEDTLS_SHA256_C:MBEDTLS_RSA_C
 mbedtls_x509_crt_verify_chain:"data_files/dir4/cert14.crt data_files/dir4/cert13.crt data_files/dir4/cert12.crt":"data_files/dir4/cert11.crt":MBEDTLS_X509_BADCERT_NOT_TRUSTED
 

Vulnerable code:  * X509      2   19
 *                 information, please see \c x509parse_verify()
 *                 return 0 for anything but a fatal error.
        *flags |= MBEDTLS_X509_BADCERT_NOT_TRUSTED;
        return( MBEDTLS_ERR_X509_CERT_VERIFY_FAILED );
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
Fixed code:    * 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.
 * X509      2   20
 *                 information, please see \c mbedtls_x509_crt_verify()
#define MBEDTLS_ERR_X509_FATAL_ERROR                      -0x3000  /**< A fatal error occured, eg the chain is too long or the vrfy callback failed. */
 *                 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 MBEDTLS_ERR_X509_CERT_VERIFY_FAILED which should not
 *                 be returned at this point), or MBEDTLS_ERR_X509_FATAL_ERROR
 *                 can be used if no better code is available.
        if( use_ret == -(MBEDTLS_ERR_X509_FATAL_ERROR) )
            mbedtls_snprintf( buf, buflen, "X509 - A fatal error occured, eg the chain is too long or the vrfy callback failed" );
        /* return immediately as the goal is to avoid unbounded recursion */
        return( MBEDTLS_ERR_X509_FATAL_ERROR );
    /* prevent misuse of the vrfy callback */
    if( ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED )
        ret = MBEDTLS_ERR_X509_FATAL_ERROR;

mbedtls_x509_crt_verify_max:"data_files/dir-maxpath/00.crt":"data_files/dir-maxpath":MBEDTLS_X509_MAX_INTERMEDIATE_CA+1:MBEDTLS_ERR_X509_FATAL_ERROR:-1