From 8af7bfa982dbc855d159f2e990ea33c464f969bc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?=
 <manuel.pegourie-gonnard@arm.com>
Date: Mon, 10 Jul 2017 11:20:08 +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                                  |  7 ++++++-
 library/x509_crt.c                         | 13 ++++++++++---
 tests/suites/test_suite_x509parse.data     |  2 +-
 tests/suites/test_suite_x509parse.function |  4 ++--
 4 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 90c6e6bcf..f008cdc5f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -25,7 +25,12 @@ Bugfix
      to bypass the version verification check. Found by Peng Li/Yueh-Hsun Lin,
      KNOX Security, Samsung Research America
 
-= mbed TLS 1.3.20 branch released 2017-06-21
+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 1.3.20 released 2017-06-21
 
 Security
    * Fixed unlimited overread of heap-based buffer in ssl_read().
diff --git a/library/x509_crt.c b/library/x509_crt.c
index 2b3eef757..70ad356a9 100644
--- a/library/x509_crt.c
+++ b/library/x509_crt.c
@@ -2147,7 +2147,7 @@ int x509_crt_verify( x509_crt *crt,
         ret = x509_crt_verify_top( crt, parent, ca_crl,
                                    pathlen, selfsigned, flags, f_vrfy, p_vrfy );
         if( ret != 0 )
-            return( ret );
+            goto exit;
     }
     else
     {
@@ -2162,17 +2162,24 @@ int x509_crt_verify( x509_crt *crt,
             ret = x509_crt_verify_child( crt, parent, trust_ca, ca_crl,
                                          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,
                                        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( POLARSSL_ERR_X509_CERT_VERIFY_FAILED );
 
diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data
index a1a861e86..f68e966e2 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:0
+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 chain #1 (zero pathlen intermediate)
 depends_on:POLARSSL_SHA256_C:POLARSSL_RSA_C
diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function
index b84cf64d5..82e459603 100644
--- a/tests/suites/test_suite_x509parse.function
+++ b/tests/suites/test_suite_x509parse.function
@@ -497,7 +497,7 @@ void x509_crt_verify_max( char *ca_file, char *chain_dir, int nb_int,
 {
     char file_buf[128];
     int ret;
-    uint32_t flags;
+    int flags;
     x509_crt trusted, chain;
 
     /*
@@ -522,7 +522,7 @@ void x509_crt_verify_max( char *ca_file, char *chain_dir, int nb_int,
     ret = x509_crt_verify( &chain, &trusted, NULL, NULL, &flags,
                                    NULL, NULL );
     TEST_ASSERT( ret == ret_chk );
-    TEST_ASSERT( flags == (uint32_t) flags_chk );
+    TEST_ASSERT( flags == flags_chk );
 
 exit:
     x509_crt_free( &chain );