From 2f3fe70f7e362caf1ac88a258d2a370a988486c9 Mon Sep 17 00:00:00 2001
From: Andres AG <andres.amayagarcia@arm.com>
Date: Fri, 20 Jan 2017 17:07:46 +0000
Subject: [PATCH] Fix verify out flags from x509_crt_verify_top()

This change fixes a regression introduced by an earlier commit that
modified x509_crt_verify_top() to ensure that valid certificates
that are after past or future valid in the chain are processed. However
the change introduced a change in behaviour that caused the
verification flags BADCERT_EXPIRED and BADCERT_FUTURE to always be set
whenever there is a failure in the verification regardless of the cause.

The fix maintains both behaviours:
  * Ensure that valid certificates after future and past are verified
  * Ensure that the correct verification flags are set.

To do so, a temporary pointer to the first future or past valid
certificate is maintained while traversing the chain. If a truly valid
certificate is found then that one is used, otherwise if no valid
certificate is found and the end of the chain is reached, the program
reverts back to using the future or past valid certificate.
---
 ChangeLog          |  9 +++++++++
 library/x509_crt.c | 31 ++++++++++++++++++++-----------
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d4cf85b7e..20c8eaf95 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 mbed TLS ChangeLog (Sorted per branch, date)
 
+= mbed TLS x.x.x branch released xxxx-xx-xx
+
+Bugfix
+   * Fix output certificate verification flags set by x509_crt_verify_top() when
+     traversing a chain of trusted CA. The issue would cause both flags,
+     BADCERT_NOT_TRUSTED and BADCERT_EXPIRED, to be set when the verification
+     conditions are not met regardless of the cause. Found by Harm Verhagen and
+     inestlerode. #665 #561
+
 = mbed TLS 1.3.18 branch 2016-10-17
 
 Security
diff --git a/library/x509_crt.c b/library/x509_crt.c
index 4b831aed3..a3517f64f 100644
--- a/library/x509_crt.c
+++ b/library/x509_crt.c
@@ -1775,6 +1775,7 @@ static int x509_crt_verify_top(
     int ca_flags = 0, check_path_cnt;
     unsigned char hash[POLARSSL_MD_MAX_SIZE];
     const md_info_t *md_info;
+    x509_crt *future_past_ca = NULL;
 
     if( x509_time_expired( &child->valid_to ) )
         *flags |= BADCERT_EXPIRED;
@@ -1823,16 +1824,6 @@ static int x509_crt_verify_top(
             continue;
         }
 
-        if( x509_time_expired( &trust_ca->valid_to ) )
-        {
-            continue;
-        }
-
-        if( x509_time_future( &trust_ca->valid_from ) )
-        {
-            continue;
-        }
-
         if( pk_verify_ext( child->sig_pk, child->sig_opts, &trust_ca->pk,
                            child->sig_md, hash, md_info->size,
                            child->sig.p, child->sig.len ) != 0 )
@@ -1840,11 +1831,23 @@ static int x509_crt_verify_top(
             continue;
         }
 
+        if( x509_time_expired( &trust_ca->valid_to ) ||
+            x509_time_future( &trust_ca->valid_from ) )
+        {
+            if( future_past_ca == NULL )
+                future_past_ca = trust_ca;
+            continue;
+        }
+
+        break;
+    }
+
+    if( trust_ca != NULL || ( trust_ca = future_past_ca ) != NULL )
+    {
         /*
          * Top of chain is signed by a trusted CA
          */
         *flags &= ~BADCERT_NOT_TRUSTED;
-        break;
     }
 
     /*
@@ -1864,6 +1867,12 @@ static int x509_crt_verify_top(
         ((void) ca_crl);
 #endif
 
+        if( x509_time_expired( &trust_ca->valid_to ) )
+            ca_flags |= BADCERT_EXPIRED;
+
+        if( x509_time_future( &trust_ca->valid_from ) )
+            ca_flags |= BADCERT_FUTURE;
+
         if( NULL != f_vrfy )
         {
             if( ( ret = f_vrfy( p_vrfy, trust_ca, path_cnt + 1,