GNOME.org
 

[glib-networking/mcatanzaro/#124] openssl: drop half-baked OCSP support

commit 8add747125b1df8bf7157fb506c3098e891c2d19
Author: Michael Catanzaro <mcatanzaro gnome org>
Date:   Wed Jan 8 11:27:23 2020 -0600

    openssl: drop half-baked OCSP support
    
    This code *might* be useful in the future, to implement some effective
    form of certificate revocation checking, depending on whether OCSP
    Must-Staple becomes successful in the future. Or it might not. If it's
    useful in the future, we can bring it back in the future. But in its
    current form, it's not useful, and arguably harmful for providing a
    false sense of security. This code implements soft-fail checking of
    stapled OCSP responses. That is, if the server sends an OCSP response
    saying "my certificate is not revoked," then great it's not revoked. If
    it sends a stapled response saying "my certificate has been revoked,"
    well what a shame. But if it sends no stapled response, we shrug and
    load the connection anyway. An attacker can subvert this by simply not
    stapling a response, so there is no real security here.
    
    We should find some more effective way to perform certificate revocation
    checking in #32. Most likely, it will look similar to this, except we'll
    only check for an OCSP response if the certificate uses Must-Staple, and
    if so we'll fail the connection if it's not set. But that will be a
    project for another day. Currently the number of certificates using
    Must-Staple is miniscule, so it doesn't make sense for us to worry about
    it just yet.
    
    Fixes #124

 tls/base/gtlsconnection-base.c             |  4 --
 tls/base/gtlsconnection-base.h             |  3 -
 tls/openssl/gtlsclientconnection-openssl.c | 65 ---------------------
 tls/openssl/gtlsdatabase-openssl.c         | 93 ------------------------------
 tls/openssl/gtlsdatabase-openssl.h         |  4 --
 tls/openssl/openssl-include.h              |  3 -
 6 files changed, 172 deletions(-)
---
diff --git a/tls/base/gtlsconnection-base.c b/tls/base/gtlsconnection-base.c
index 93cfc8e..10658ad 100644
--- a/tls/base/gtlsconnection-base.c
+++ b/tls/base/gtlsconnection-base.c
@@ -1228,7 +1228,6 @@ static GTlsCertificateFlags
 verify_peer_certificate (GTlsConnectionBase *tls,
                          GTlsCertificate    *peer_certificate)
 {
-  GTlsConnectionBaseClass *tls_class = G_TLS_CONNECTION_BASE_GET_CLASS (tls);
   GSocketConnectable *peer_identity;
   GTlsDatabase *database;
   GTlsCertificateFlags errors;
@@ -1271,9 +1270,6 @@ verify_peer_certificate (GTlsConnectionBase *tls,
         }
     }
 
-  if (tls_class->verify_peer_certificate)
-    errors |= tls_class->verify_peer_certificate (tls, peer_certificate, errors);
-
   return errors;
 }
 
diff --git a/tls/base/gtlsconnection-base.h b/tls/base/gtlsconnection-base.h
index bacefab..221bbe6 100644
--- a/tls/base/gtlsconnection-base.h
+++ b/tls/base/gtlsconnection-base.h
@@ -72,9 +72,6 @@ struct _GTlsConnectionBaseClass
                                                              GCancellable         *cancellable,
                                                              GError              **error);
   GTlsCertificate            *(*retrieve_peer_certificate)  (GTlsConnectionBase   *tls);
-  GTlsCertificateFlags        (*verify_peer_certificate)    (GTlsConnectionBase   *tls,
-                                                             GTlsCertificate      *certificate,
-                                                             GTlsCertificateFlags  flags);
   void                        (*complete_handshake)         (GTlsConnectionBase   *tls,
                                                              gchar               **negotiated_protocol,
                                                              GError              **error);
diff --git a/tls/openssl/gtlsclientconnection-openssl.c b/tls/openssl/gtlsclientconnection-openssl.c
index d5fc955..56e279f 100644
--- a/tls/openssl/gtlsclientconnection-openssl.c
+++ b/tls/openssl/gtlsclientconnection-openssl.c
@@ -34,7 +34,6 @@
 #include "gtlsclientconnection-openssl.h"
 #include "gtlsbackend-openssl.h"
 #include "gtlscertificate-openssl.h"
-#include "gtlsdatabase-openssl.h"
 #include <glib/gi18n-lib.h>
 
 #define DEFAULT_CIPHER_LIST "HIGH:!DSS:!aNULL@STRENGTH"
@@ -201,56 +200,6 @@ g_tls_client_connection_openssl_complete_handshake (GTlsConnectionBase  *tls,
   g_object_notify (G_OBJECT (client), "accepted-cas");
 }
 
-static GTlsCertificateFlags
-verify_ocsp_response (GTlsClientConnectionOpenssl *openssl,
-                      GTlsCertificate             *peer_certificate)
-{
-#if (OPENSSL_VERSION_NUMBER >= 0x0090808fL) && !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_OCSP)
-  SSL *ssl = NULL;
-  OCSP_RESPONSE *resp = NULL;
-  GTlsDatabase *database;
-  long len = 0;
-  unsigned char *p = NULL;
-
-  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;
-
-  resp = d2i_OCSP_RESPONSE (NULL, (const unsigned char **)&p, len);
-  if (!resp)
-    return G_TLS_CERTIFICATE_GENERIC_ERROR;
-
-  database = g_tls_connection_get_database (G_TLS_CONNECTION (openssl));
-
-  /* If there's no database, then G_TLS_CERTIFICATE_UNKNOWN_CA must be flagged,
-   * and this function is only called if there are no flags.
-   */
-  g_assert (database);
-
-  return g_tls_database_openssl_verify_ocsp_response (G_TLS_DATABASE_OPENSSL (database),
-                                                      peer_certificate,
-                                                      resp);
-#else
-  return 0;
-#endif
-}
-
-static GTlsCertificateFlags
-g_tls_client_connection_openssl_verify_peer_certificate (GTlsConnectionBase   *tls,
-                                                         GTlsCertificate      *certificate,
-                                                         GTlsCertificateFlags  flags)
-{
-  GTlsClientConnectionOpenssl *openssl = G_TLS_CLIENT_CONNECTION_OPENSSL (tls);
-
-  if (flags == 0)
-    flags = verify_ocsp_response (openssl, certificate);
-
-  return flags;
-}
-
 static SSL *
 g_tls_client_connection_openssl_get_ssl (GTlsConnectionOpenssl *connection)
 {
@@ -269,7 +218,6 @@ g_tls_client_connection_openssl_class_init (GTlsClientConnectionOpensslClass *kl
   gobject_class->set_property         = g_tls_client_connection_openssl_set_property;
 
   base_class->complete_handshake      = g_tls_client_connection_openssl_complete_handshake;
-  base_class->verify_peer_certificate = g_tls_client_connection_openssl_verify_peer_certificate;
 
   openssl_class->get_ssl              = g_tls_client_connection_openssl_get_ssl;
 
@@ -284,7 +232,6 @@ g_tls_client_connection_openssl_init (GTlsClientConnectionOpenssl *openssl)
 {
 }
 
-
 static void
 g_tls_client_connection_openssl_copy_session_state (GTlsClientConnection *conn,
                                                     GTlsClientConnection *source)
@@ -392,12 +339,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,
@@ -481,12 +422,6 @@ g_tls_client_connection_openssl_initable_init (GInitable       *initable,
 
   SSL_set_connect_state (client->ssl);
 
-#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);
-#endif
-
   if (!g_tls_client_connection_openssl_parent_initable_iface->
       init (initable, cancellable, error))
     return FALSE;
diff --git a/tls/openssl/gtlsdatabase-openssl.c b/tls/openssl/gtlsdatabase-openssl.c
index 232bcef..61d607d 100644
--- a/tls/openssl/gtlsdatabase-openssl.c
+++ b/tls/openssl/gtlsdatabase-openssl.c
@@ -270,96 +270,3 @@ g_tls_database_openssl_new (GError **error)
 
   return g_initable_new (G_TYPE_TLS_DATABASE_OPENSSL, NULL, error, NULL);
 }
-
-GTlsCertificateFlags
-g_tls_database_openssl_verify_ocsp_response (GTlsDatabaseOpenssl *self,
-                                             GTlsCertificate     *chain,
-                                             OCSP_RESPONSE       *resp)
-{
-  GTlsCertificateFlags errors = 0;
-#if (OPENSSL_VERSION_NUMBER >= 0x0090808fL) && !defined(OPENSSL_NO_TLSEXT) && \
-  !defined(OPENSSL_NO_OCSP)
-  GTlsDatabaseOpensslPrivate *priv;
-  STACK_OF(X509) *chain_openssl = NULL;
-  OCSP_BASICRESP *basic_resp = NULL;
-  int ocsp_status = 0;
-  int i;
-
-  ocsp_status = OCSP_response_status (resp);
-  if (ocsp_status != OCSP_RESPONSE_STATUS_SUCCESSFUL)
-    {
-      errors = G_TLS_CERTIFICATE_GENERIC_ERROR;
-      goto end;
-    }
-
-  basic_resp = OCSP_response_get1_basic (resp);
-  if (basic_resp == NULL)
-    {
-      errors = G_TLS_CERTIFICATE_GENERIC_ERROR;
-      goto end;
-    }
-
-  chain_openssl = convert_certificate_chain_to_openssl (G_TLS_CERTIFICATE_OPENSSL (chain));
-  priv = g_tls_database_openssl_get_instance_private (self);
-  if ((chain_openssl == NULL) ||
-      (priv->store == NULL))
-    {
-      errors = G_TLS_CERTIFICATE_GENERIC_ERROR;
-      goto end;
-    }
-
-  if (OCSP_basic_verify (basic_resp, chain_openssl, priv->store, 0) <= 0)
-    {
-      errors = G_TLS_CERTIFICATE_GENERIC_ERROR;
-      goto end;
-    }
-
-  for (i = 0; i < OCSP_resp_count (basic_resp); i++)
-    {
-      OCSP_SINGLERESP *single_resp = OCSP_resp_get0 (basic_resp, i);
-      ASN1_GENERALIZEDTIME *revocation_time = NULL;
-      ASN1_GENERALIZEDTIME *this_update_time = NULL;
-      ASN1_GENERALIZEDTIME *next_update_time = NULL;
-      int crl_reason = 0;
-      int cert_status = 0;
-
-      if (single_resp == NULL)
-        continue;
-
-      cert_status = OCSP_single_get0_status (single_resp,
-                                             &crl_reason,
-                                             &revocation_time,
-                                             &this_update_time,
-                                             &next_update_time);
-      if (!OCSP_check_validity (this_update_time,
-                                next_update_time,
-                                300L,
-                                -1L))
-        {
-          errors = G_TLS_CERTIFICATE_GENERIC_ERROR;
-          goto end;
-        }
-
-      switch (cert_status)
-        {
-        case V_OCSP_CERTSTATUS_GOOD:
-          break;
-        case V_OCSP_CERTSTATUS_REVOKED:
-          errors = G_TLS_CERTIFICATE_REVOKED;
-          goto end;
-        case V_OCSP_CERTSTATUS_UNKNOWN:
-          errors = G_TLS_CERTIFICATE_GENERIC_ERROR;
-          goto end;
-        }
-    }
-
-end:
-  if (basic_resp)
-    OCSP_BASICRESP_free (basic_resp);
-
-  if (resp)
-    OCSP_RESPONSE_free (resp);
-
-#endif
-  return errors;
-}
diff --git a/tls/openssl/gtlsdatabase-openssl.h b/tls/openssl/gtlsdatabase-openssl.h
index f3203c0..092fdee 100644
--- a/tls/openssl/gtlsdatabase-openssl.h
+++ b/tls/openssl/gtlsdatabase-openssl.h
@@ -46,8 +46,4 @@ struct _GTlsDatabaseOpensslClass
 
 GTlsDatabaseOpenssl      *g_tls_database_openssl_new                      (GError **error);
 
-GTlsCertificateFlags      g_tls_database_openssl_verify_ocsp_response     (GTlsDatabaseOpenssl *self,
-                                                                           GTlsCertificate     *chain,
-                                                                           OCSP_RESPONSE       *resp);
-
 G_END_DECLS
diff --git a/tls/openssl/openssl-include.h b/tls/openssl/openssl-include.h
index a0083d5..8f57042 100644
--- a/tls/openssl/openssl-include.h
+++ b/tls/openssl/openssl-include.h
@@ -50,6 +50,3 @@
 #include <openssl/x509_vfy.h>
 #include <openssl/x509v3.h>
 #include <openssl/crypto.h>
-#if (OPENSSL_VERSION_NUMBER >= 0x0090808fL) && !defined(OPENSSL_NO_OCSP)
-#include <openssl/ocsp.h>
-#endif
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]