commit 8af7bfa982dbc855d159f2e990ea33c464f969bc
author Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com> Mon Jul 10 11:20:08 2017 +0200
committer Simon Butcher <simon.butcher@arm.com> Fri Jul 28 13:15:57 2017 +0100
tree 8950ab91f5e22356a0e3e5350cef5cf66295a60f
parent 7ac50196f3e9350a93ec658237a69e74890f38a0

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

diff --git a/ChangeLog b/ChangeLog
index 90c6e6b..f008cdc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -25,7 +25,12 @@
      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().

library/x509_crt.c

diff --git a/library/x509_crt.c b/library/x509_crt.c
index 2b3eef7..70ad356 100644
--- a/library/x509_crt.c
+++ b/library/x509_crt.c
@@ -2147,7 +2147,7 @@
         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 @@
             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 );
 

tests/suites/test_suite_x509parse.data

diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data
index a1a861e..f68e966 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: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

tests/suites/test_suite_x509parse.function

diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function
index b84cf64..82e4596 100644
--- a/tests/suites/test_suite_x509parse.function
+++ b/tests/suites/test_suite_x509parse.function
@@ -497,7 +497,7 @@
 {
     char file_buf[128];
     int ret;
-    uint32_t flags;
+    int flags;
     x509_crt trusted, chain;
 
     /*
@@ -522,7 +522,7 @@
     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 );