Merge branch 'iotssl-1071-ca-flags'

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 MBEDTLS_X509_BADCERT_EXPIRED and
MBEDTLS_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.

5 files changed, 86 insertions, 10 deletions

diff --git a/ChangeLog b/ChangeLog
index 1c111ec..a299b80 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -13,6 +13,11 @@ Security
      Introduced by interoperability fix for #513.
 
 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,
+     MBEDTLS_X509_BADCERT_NOT_TRUSTED and MBEDTLS_X509_BADCERT_EXPIRED, to be
+     set when the verification conditions are not met regardless of the cause.
+     Found by Harm Verhagen and inestlerode. #665 #561
    * Fix the redefinition of macro ssl_set_bio to an undefined symbol
      mbedtls_ssl_set_bio_timeout in compat-1.3.h, by removing it.
      Found by omlib-lin. #673
diff --git a/library/x509_crt.c b/library/x509_crt.c
index 056dc16..234f145 100644
--- a/library/x509_crt.c
+++ b/library/x509_crt.c
@@ -1904,6 +1904,7 @@ static int x509_crt_verify_top(
     int check_path_cnt;
     unsigned char hash[MBEDTLS_MD_MAX_SIZE];
     const mbedtls_md_info_t *md_info;
+    mbedtls_x509_crt *future_past_ca = NULL;
 
     if( mbedtls_x509_time_is_past( &child->valid_to ) )
         *flags |= MBEDTLS_X509_BADCERT_EXPIRED;
@@ -1958,23 +1959,27 @@ static int x509_crt_verify_top(
             continue;
         }
 
-        if( mbedtls_x509_time_is_past( &trust_ca->valid_to ) )
+        if( mbedtls_pk_verify_ext( child->sig_pk, child->sig_opts, &trust_ca->pk,
+                           child->sig_md, hash, mbedtls_md_get_size( md_info ),
+                           child->sig.p, child->sig.len ) != 0 )
         {
             continue;
         }
 
-        if( mbedtls_x509_time_is_future( &trust_ca->valid_from ) )
+        if( mbedtls_x509_time_is_past( &trust_ca->valid_to ) ||
+            mbedtls_x509_time_is_future( &trust_ca->valid_from ) )
         {
-            continue;
-        }
+            if ( future_past_ca == NULL )
+                future_past_ca = trust_ca;
 
-        if( mbedtls_pk_verify_ext( child->sig_pk, child->sig_opts, &trust_ca->pk,
-                           child->sig_md, hash, mbedtls_md_get_size( md_info ),
-                           child->sig.p, child->sig.len ) != 0 )
-        {
             continue;
         }
 
+        break;
+    }
+
+    if( trust_ca != NULL || ( trust_ca = future_past_ca ) != NULL )
+    {
         /*
          * Top of chain is signed by a trusted CA
          */
@@ -1982,8 +1987,6 @@ static int x509_crt_verify_top(
 
         if( x509_profile_check_key( profile, child->sig_pk, &trust_ca->pk ) != 0 )
             *flags |= MBEDTLS_X509_BADCERT_BAD_KEY;
-
-        break;
     }
 
     /*
@@ -2003,6 +2006,12 @@ static int x509_crt_verify_top(
         ((void) ca_crl);
 #endif
 
+        if( mbedtls_x509_time_is_past( &trust_ca->valid_to ) )
+            ca_flags |= MBEDTLS_X509_BADCERT_EXPIRED;
+
+        if( mbedtls_x509_time_is_future( &trust_ca->valid_from ) )
+            ca_flags |= MBEDTLS_X509_BADCERT_FUTURE;
+
         if( NULL != f_vrfy )
         {
             if( ( ret = f_vrfy( p_vrfy, trust_ca, path_cnt + 1,
diff --git a/tests/data_files/test-ca2_cat-future-invalid.crt b/tests/data_files/test-ca2_cat-future-invalid.crt
new file mode 100644
index 0000000..b1cfbf0
--- /dev/null
+++ b/tests/data_files/test-ca2_cat-future-invalid.crt
@@ -0,0 +1,27 @@
+-----BEGIN CERTIFICATE-----
+MIICIDCCAaWgAwIBAgIBCjAKBggqhkjOPQQDAjA+MQswCQYDVQQGEwJOTDERMA8G
+A1UEChMIUG9sYXJTU0wxHDAaBgNVBAMTE1BvbGFyc3NsIFRlc3QgRUMgQ0EwHhcN
+MTMwOTI0MTU1MjA0WhcNMjMwOTIyMTU1MjA0WjA0MQswCQYDVQQGEwJOTDERMA8G
+A1UEChMIUG9sYXJTU0wxEjAQBgNVBAMTCWxvY2FsaG9zdDBZMBMGByqGSM49AgEG
+CCqGSM49AwEHA0IABIFZMXZJJPoVraugMW4O7TMR+pElVcGwwZwDcj6Yui2kcjeJ
+H0M3jR+OOtjwV+gvT8kApPfbcw+yxgSU0UA7OOOjgZ0wgZowCQYDVR0TBAIwADAd
+BgNVHQ4EFgQUfmWPPjMDFOXhvmCy4IV/jOdgK3swbgYDVR0jBGcwZYAUnW0gJEkB
+PyvLeLUZvH4kydv7NnyhQqRAMD4xCzAJBgNVBAYTAk5MMREwDwYDVQQKEwhQb2xh
+clNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQYIJAMFD4n5iQ8zoMAoG
+CCqGSM49BAMCA2kAMGYCMQCsYTyleBFuI4nizuxo/ie5dxJnD0ynwCnRJ+84PZP4
+AQA3HdUz0qNYs4CZ2am9Gz0CMQDr2TNLFA3C3S3pmgXMT0eKzR1Ca1/Nulf0llQZ
+Xj09kLboxuemP40IIqhQnpYptMg=
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIIB+zCCAYCgAwIBAgIBATAMBggqhkjOPQQDAgUAMD4xCzAJBgNVBAYTAk5MMREw
+DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTAe
+Fw0yMzA5MjIxNTQ5NDlaFw0zMDEyMzEyMzU5NTlaMD4xCzAJBgNVBAYTAk5MMREw
+DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTB2
+MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBuww5XUzM5
+WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiyaY7zQa0p
+w7RfdadHb9UZKVVpmlM7ILRmFmAzHqNQME4wDAYDVR0TBAUwAwEB/zAdBgNVHQ4E
+FgQUnW0gJEkBPyvLeLUZvH4kydv7NnwwHwYDVR0jBBgwFoAUnW0gJEkBPyvLeLUZ
+vH4kydv7NnwwDAYIKoZIzj0EAwIFAANnADBkAjB1ZNdOM7KRJiPo45hP17A1sJSH
+qHFPEJbml6KdNevoVZ1HqvP8AoFGcPJRpQVtzC0CMDa7JEqn0dOss8EmW9pVF/N2
++XvzNczj89mWMgPhJJlT+MONQx3LFQO+TMSI9hLdkw==
+-----END CERTIFICATE-----
diff --git a/tests/data_files/test-ca2_cat-past-invalid.crt b/tests/data_files/test-ca2_cat-past-invalid.crt
new file mode 100644
index 0000000..febad74
--- /dev/null
+++ b/tests/data_files/test-ca2_cat-past-invalid.crt
@@ -0,0 +1,27 @@
+-----BEGIN CERTIFICATE-----
+MIIB/TCCAYCgAwIBAgIBATAMBggqhkjOPQQDAgUAMD4xCzAJBgNVBAYTAk5MMREw
+DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTAe
+Fw0wMzA5MjQxNTQ5NDhaFw0xMzA5MjQxNTQ5NDhaMD4xCzAJBgNVBAYTAk5MMREw
+DwYDVQQKEwhQb2xhclNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQTB2
+MBAGByqGSM49AgEGBSuBBAAiA2IABMPaKzRBN1gvh1b+/Im6KUNLTuBuww5XUzM5
+WNRStJGVOQsj318XJGJI/BqVKc4sLYfCiFKAr9ZqqyHduNMcbli4yuiyaY7zQa0p
+w7RfdadHb9UZKVVpmlM7ILRmFmAzHqNQME4wDAYDVR0TBAUwAwEB/zAdBgNVHQ4E
+FgQUnW0gJEkBPyvLeLUZvH4kydv7NnwwHwYDVR0jBBgwFoAUnW0gJEkBPyvLeLUZ
+vH4kydv7NnwwDAYIKoZIzj0EAwIFAANpADBmAjEAvQ/49lXXrLYdOIGtTaYWjpZP
+tRBXQiGPMzUvmKBk7gM7bF4iFPsdJikyXHmuwv3RAjEA8vtUX8fAAB3fbh5dEXRm
+l7tz0Sw/RW6AHFtaIauGkhHqeKIaKIi6WSgHu6x97uyg
+-----END CERTIFICATE-----
+-----BEGIN CERTIFICATE-----
+MIICIDCCAaWgAwIBAgIBCjAKBggqhkjOPQQDAjA+MQswCQYDVQQGEwJOTDERMA8G
+A1UEChMIUG9sYXJTU0wxHDAaBgNVBAMTE1BvbGFyc3NsIFRlc3QgRUMgQ0EwHhcN
+MTMwOTI0MTU1MjA0WhcNMjMwOTIyMTU1MjA0WjA0MQswCQYDVQQGEwJOTDERMA8G
+A1UEChMIUG9sYXJTU0wxEjAQBgNVBAMTCWxvY2FsaG9zdDBZMBMGByqGSM49AgEG
+CCqGSM49AwEHA0IABIFZMXZJJPoVraugMW4O7TMR+pElVcGwwZwDcj6Yui2kcjeJ
+H0M3jR+OOtjwV+gvT8kApPfbcw+yxgSU0UA7OOOjgZ0wgZowCQYDVR0TBAIwADAd
+BgNVHQ4EFgQUfmWPPjMDFOXhvmCy4IV/jOdgK3swbgYDVR0jBGcwZYAUnW0gJEkB
+PyvLeLUZvH4kydv7NnyhQqRAMD4xCzAJBgNVBAYTAk5MMREwDwYDVQQKEwhQb2xh
+clNTTDEcMBoGA1UEAxMTUG9sYXJzc2wgVGVzdCBFQyBDQYIJAMFD4n5iQ8zoMAoG
+CCqGSM49BAMCA2kAMGYCMQCsYTyleBFuI4nizuxo/ie5dxJnD0ynwCnRJ+84PZP4
+AQA3HdUz0qNYs4CZ2am9Gz0CMQDr2TNLFA3C3S3pmgXMT0eKzR1Ca1/Nulf0llQZ
+Xj09kLboxuemP40IIqhQnpYptMg=
+-----END CERTIFICATE-----
diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data
index 9170438..1031bd9 100644
--- a/tests/suites/test_suite_x509parse.data
+++ b/tests/suites/test_suite_x509parse.data
@@ -719,6 +719,14 @@ X509 Certificate verification #85 (Not yet valid CA and valid CA)
 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_SECP384R1_ENABLED:MBEDTLS_SHA1_C:MBEDTLS_SHA256_C
 x509_verify:"data_files/server5.crt":"data_files/test-ca2_cat-past-present.crt":"data_files/crl-ec-sha1.pem":"NULL":0:0:"NULL"
 
+X509 Certificate verification #86 (Not yet valid CA and invalid CA)
+depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_SECP384R1_ENABLED:MBEDTLS_SHA1_C:MBEDTLS_SHA256_C
+x509_verify:"data_files/server5.crt":"data_files/test-ca2_cat-future-invalid.crt":"data_files/crl-ec-sha1.pem":"NULL":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_FUTURE:"NULL"
+
+X509 Certificate verification #87 (Expired CA and invalid CA)
+depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_ECP_DP_SECP384R1_ENABLED:MBEDTLS_SHA1_C:MBEDTLS_SHA256_C
+x509_verify:"data_files/server5.crt":"data_files/test-ca2_cat-past-invalid.crt":"data_files/crl-ec-sha1.pem":"NULL":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_EXPIRED:"NULL"
+
 X509 Certificate verification callback: trusted EE cert
 depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
 x509_verify_callback:"data_files/server5-selfsigned.crt":"data_files/server5-selfsigned.crt":0:"depth 0 - serial 53\:A2\:CB\:4B\:12\:4E\:AD\:83\:7D\:A8\:94\:B2 - subject CN=selfsigned, OU=testing, O=PolarSSL, C=NL\n"