[glib-networking/mcatanzaro/#124] openssl: drop half-baked OCSP support
- From: Michael Catanzaro <mcatanzaro src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [glib-networking/mcatanzaro/#124] openssl: drop half-baked OCSP support
- Date: Wed, 8 Jan 2020 17:32:18 +0000 (UTC)
commit cb84ea8d3162ed030abc9fb59c3c7e64c64b6fdd
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 | 53 -----------------
tls/openssl/gtlsdatabase-openssl.c | 93 ------------------------------
tls/openssl/gtlsdatabase-openssl.h | 4 --
tls/openssl/openssl-include.h | 3 -
6 files changed, 160 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..89a59ea 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)
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]
