[glib-networking/mcatanzaro/openssl-expiration: 19/19] openssl: remove manual check for certificate expiration

[Michael Catanzaro <mcatanzaro src gnome org>]

commit a268c6a22043968c34640534fdc6c2dff7f1d824
Author: Michael Catanzaro <mcatanzaro redhat com>
Date:   Wed Nov 24 07:37:44 2021 -0600

    openssl: remove manual check for certificate expiration
    
    We should rely on OpenSSL to do this for us instead. Doing it here is
    wrong because we wind up checking certificates that may not actually be
    used in the final certificate chain constructed by OpenSSL. We don't
    have any way to know which chain OpenSSL will build from the
    certificates that we pass to it, so there is no way to safely perform
    certificate validity checks at the glib-networking level.
    
    Fixes #179
    
    Corresponding change for GTlsDatabaseGnutls:
    https://gitlab.gnome.org/GNOME/glib-networking/-/commit/a2cc9b8e08063745d9ba1091e030fbe43fc5a055
    
    Corresponding change for GTlsCertificateGnutls:
    https://gitlab.gnome.org/GNOME/glib-networking/-/commit/e1a8d06648328f3c5cb2de5ca016de8ac3ddc2b2
    
    Documented by:
    https://gitlab.gnome.org/GNOME/glib/-/commit/780af9cff3cc636525a973c3f0c0244f2422a39e

 tls/openssl/gtlscertificate-openssl.c | 17 -----------------
 tls/openssl/gtlsdatabase-openssl.c    | 33 ---------------------------------
 tls/tests/certificate.c               |  8 ++++++--
 3 files changed, 6 insertions(+), 52 deletions(-)
---
diff --git a/tls/openssl/gtlscertificate-openssl.c b/tls/openssl/gtlscertificate-openssl.c
index ec30ab0a..2e3148c2 100644
--- a/tls/openssl/gtlscertificate-openssl.c
+++ b/tls/openssl/gtlscertificate-openssl.c
@@ -472,7 +472,6 @@ g_tls_certificate_openssl_verify (GTlsCertificate     *cert,
   GTlsCertificateFlags gtls_flags;
   X509 *x;
   STACK_OF(X509) *untrusted;
-  gint i;
 
   cert_openssl = G_TLS_CERTIFICATE_OPENSSL (cert);
   x = cert_openssl->cert;
@@ -514,22 +513,6 @@ g_tls_certificate_openssl_verify (GTlsCertificate     *cert,
       X509_STORE_free (store);
     }
 
-  /* We have to check these ourselves since openssl
-   * does not give us flags and UNKNOWN_CA will take priority.
-   */
-  for (i = 0; i < sk_X509_num (untrusted); i++)
-    {
-      X509 *c = sk_X509_value (untrusted, i);
-      ASN1_TIME *not_before = X509_get_notBefore (c);
-      ASN1_TIME *not_after = X509_get_notAfter (c);
-
-      if (X509_cmp_current_time (not_before) > 0)
-        gtls_flags |= G_TLS_CERTIFICATE_NOT_ACTIVATED;
-
-      if (X509_cmp_current_time (not_after) < 0)
-        gtls_flags |= G_TLS_CERTIFICATE_EXPIRED;
-    }
-
   sk_X509_free (untrusted);
 
   if (identity)
diff --git a/tls/openssl/gtlsdatabase-openssl.c b/tls/openssl/gtlsdatabase-openssl.c
index 2ee65398..52a6c335 100644
--- a/tls/openssl/gtlsdatabase-openssl.c
+++ b/tls/openssl/gtlsdatabase-openssl.c
@@ -84,34 +84,6 @@ g_tls_database_openssl_init (GTlsDatabaseOpenssl *self)
   g_mutex_init (&priv->mutex);
 }
 
-static GTlsCertificateFlags
-double_check_before_after_dates (GTlsCertificateOpenssl *chain)
-{
-  GTlsCertificateFlags gtls_flags = 0;
-  X509 *cert;
-
-  while (chain)
-    {
-      ASN1_TIME *not_before;
-      ASN1_TIME *not_after;
-
-      cert = g_tls_certificate_openssl_get_cert (chain);
-      not_before = X509_get_notBefore (cert);
-      not_after = X509_get_notAfter (cert);
-
-      if (X509_cmp_current_time (not_before) > 0)
-        gtls_flags |= G_TLS_CERTIFICATE_NOT_ACTIVATED;
-
-      if (X509_cmp_current_time (not_after) < 0)
-        gtls_flags |= G_TLS_CERTIFICATE_EXPIRED;
-
-      chain = G_TLS_CERTIFICATE_OPENSSL (g_tls_certificate_get_issuer
-                                         (G_TLS_CERTIFICATE (chain)));
-    }
-
-  return gtls_flags;
-}
-
 static STACK_OF(X509) *
 convert_certificate_chain_to_openssl (GTlsCertificateOpenssl *chain)
 {
@@ -172,11 +144,6 @@ g_tls_database_openssl_verify_chain (GTlsDatabase             *database,
   if (g_cancellable_set_error_if_cancelled (cancellable, error))
     return G_TLS_CERTIFICATE_GENERIC_ERROR;
 
-  /* We have to check these ourselves since openssl
-   * does not give us flags and UNKNOWN_CA will take priority.
-   */
-  result |= double_check_before_after_dates (G_TLS_CERTIFICATE_OPENSSL (chain));
-
   if (identity)
     result |= g_tls_certificate_openssl_verify_identity (G_TLS_CERTIFICATE_OPENSSL (chain),
                                                          identity);
diff --git a/tls/tests/certificate.c b/tls/tests/certificate.c
index d216c710..01c3c1a8 100644
--- a/tls/tests/certificate.c
+++ b/tls/tests/certificate.c
@@ -610,13 +610,17 @@ test_verify_certificate_bad_combo (TestVerify      *test,
    * - Use unrelated cert as CA
    * - Use wrong identity.
    * - Use expired certificate.
+   *
+   * Once upon a time, we might have asserted to see that all of these errors
+   * are set. But this is impossible to do correctly, so nowadays we only
+   * guarantee that at least one error will be set. See glib-networking#179 and
+   * glib!2214 for rationale.
    */
 
   identity = g_network_address_new ("other.example.com", 80);
 
   errors = g_tls_certificate_verify (cert, identity, cacert);
-  g_assert_cmpuint (errors, ==, G_TLS_CERTIFICATE_UNKNOWN_CA |
-                    G_TLS_CERTIFICATE_BAD_IDENTITY | G_TLS_CERTIFICATE_EXPIRED);
+  g_assert_cmpuint (errors, !=, 0);
 
   g_object_unref (cert);
   g_object_unref (cacert);