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

Change-Id: I354e8163001cdb4bdd8d06b0d9863debc3a0bf50

diff --git a/tls/openssl/gtlscertificate-openssl.c b/tls/openssl/gtlscertificate-openssl.c
index 4bd9f80..d5c0412 100644 (file)
--- a/tls/openssl/gtlscertificate-openssl.c
+++ b/tls/openssl/gtlscertificate-openssl.c
@@ -287,7 +287,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);
   priv = g_tls_certificate_openssl_get_instance_private (cert_openssl);
@@ -336,22 +335,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/gtlsfiledatabase-openssl.c b/tls/openssl/gtlsfiledatabase-openssl.c
index e45a619..6eb1c65 100644 (file)
--- a/tls/openssl/gtlsfiledatabase-openssl.c
+++ b/tls/openssl/gtlsfiledatabase-openssl.c
@@ -527,34 +527,6 @@ g_tls_file_database_openssl_lookup_certificates_issued_by (GTlsDatabase
   return issued;
 }
 
-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)
 {
@@ -626,11 +598,6 @@ g_tls_file_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 3310269..9ca543b 100644 (file)
--- a/tls/tests/certificate.c
+++ b/tls/tests/certificate.c
@@ -510,13 +510,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);