[glib-networking/pgriffis/ocsp-openssl: 3/3] openssl: OCSP cleanups

[Patrick Griffis <pgriffis src gnome org>]

commit 41fefbb4f94b728d145128ab093f755d7c839d02
Author: Patrick Griffis <pgriffis igalia com>
Date:   Wed Aug 4 13:11:30 2021 -0500

    openssl: OCSP cleanups
    
    Cleanups after the revert such as fixing a leak and compile errors.

 tls/base/gtlsconnection-base.c             |  1 -
 tls/openssl/gtlsclientconnection-openssl.c | 24 ++++++++++++------------
 tls/openssl/gtlsdatabase-openssl.c         |  3 +++
 3 files changed, 15 insertions(+), 13 deletions(-)
---
diff --git a/tls/base/gtlsconnection-base.c b/tls/base/gtlsconnection-base.c
index 716f1631..632008b4 100644
--- a/tls/base/gtlsconnection-base.c
+++ b/tls/base/gtlsconnection-base.c
@@ -1259,7 +1259,6 @@ verify_peer_certificate (GTlsConnectionBase *tls,
 {
   GTlsConnectionBaseClass *tls_class = G_TLS_CONNECTION_BASE_GET_CLASS (tls);
   GSocketConnectable *peer_identity = NULL;
-  GTlsConnectionBaseClass *tls_class = G_TLS_CONNECTION_BASE_GET_CLASS (tls);
   GTlsDatabase *database;
   GTlsCertificateFlags errors = 0;
   gboolean is_client;
diff --git a/tls/openssl/gtlsclientconnection-openssl.c b/tls/openssl/gtlsclientconnection-openssl.c
index 7573adb1..50f67ef2 100644
--- a/tls/openssl/gtlsclientconnection-openssl.c
+++ b/tls/openssl/gtlsclientconnection-openssl.c
@@ -222,10 +222,18 @@ verify_ocsp_response (GTlsClientConnectionOpenssl *openssl,
 
   ssl = g_tls_connection_openssl_get_ssl (G_TLS_CONNECTION_OPENSSL (openssl));
   len = SSL_get_tlsext_status_ocsp_resp (ssl, &p);
-  /* Soft fail in case of no response is the best we can do
-   * FIXME: this makes it security theater, why bother with OCSP at all? */
   if (!p)
-    return 0;
+    {
+      /* OpenSSL doesn't provide an API to determine if the chain requires
+       * an OCSP response (known as MustStaple) using the status_request 
+       * X509v3 extension. We also have no way of correctly knowing the full
+       * chain OpenSSL will internally use to do this ourselves.
+       * So for now we silently continue ignoring the missing response.
+       * This does mean that this does not provide real security as an attacker
+       * can easily bypass this.
+       */
+      return 0;
+    }
 
   resp = d2i_OCSP_RESPONSE (NULL, (const unsigned char **)&p, len);
   if (!resp)
@@ -292,7 +300,6 @@ g_tls_client_connection_openssl_init (GTlsClientConnectionOpenssl *openssl)
 {
 }
 
-
 static void
 g_tls_client_connection_openssl_copy_session_state (GTlsClientConnection *conn,
                                                     GTlsClientConnection *source)
@@ -432,12 +439,6 @@ set_curve_list (GTlsClientConnectionOpenssl *client)
 }
 #endif
 
-static gboolean
-use_ocsp (void)
-{
-  return g_getenv ("G_TLS_OPENSSL_OCSP_ENABLED") != NULL;
-}
-
 static gboolean
 g_tls_client_connection_openssl_initable_init (GInitable       *initable,
                                                GCancellable    *cancellable,
@@ -531,8 +532,7 @@ g_tls_client_connection_openssl_initable_init (GInitable       *initable,
 
 #if (OPENSSL_VERSION_NUMBER >= 0x0090808fL) && !defined(OPENSSL_NO_TLSEXT) && \
     !defined(OPENSSL_NO_OCSP)
-  if (use_ocsp())
-    SSL_set_tlsext_status_type (client->ssl, TLSEXT_STATUSTYPE_ocsp);
+  SSL_set_tlsext_status_type (client->ssl, TLSEXT_STATUSTYPE_ocsp);
 #endif
 
   if (!g_tls_client_connection_openssl_parent_initable_iface->
diff --git a/tls/openssl/gtlsdatabase-openssl.c b/tls/openssl/gtlsdatabase-openssl.c
index 65b5ff42..bb42ef05 100644
--- a/tls/openssl/gtlsdatabase-openssl.c
+++ b/tls/openssl/gtlsdatabase-openssl.c
@@ -396,6 +396,9 @@ g_tls_database_openssl_verify_ocsp_response (GTlsDatabaseOpenssl *self,
     }
 
 end:
+  if (chain_openssl)
+    sk_X509_free (chain_openssl);
+
   if (basic_resp)
     OCSP_BASICRESP_free (basic_resp);