[glib-networking/mcatanzaro/insecure-renegotiation-check: 2/2] Fail unsafe rehandshake attempts initiated by API request (take two)
- From: Ignacio Casal Quinteiro <icq src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [glib-networking/mcatanzaro/insecure-renegotiation-check: 2/2] Fail unsafe rehandshake attempts initiated by API request (take two)
- Date: Thu, 4 Jul 2019 09:48:11 +0000 (UTC)
commit 03bc29b91f66efdedc31dd8dd1f8dd20a0a7c480
Author: Michael Catanzaro <mcatanzaro igalia com>
Date: Mon Jul 1 19:59:49 2019 -0500
Fail unsafe rehandshake attempts initiated by API request (take two)
This reimplements 21f765e24c9a9f7da6860ce6b4affc74a46648ee, which got
lost when I ported the GnuTLS code to the base class. I've had a pending
TODO to restore this check for a while now.
It's not possible to test because we don't test against an old peer that
lacks secure renegotiation support.
tls/base/gtlsconnection-base.c | 8 +++
tls/base/gtlsconnection-base.h | 135 ++++++++++++++++++-----------------
tls/gnutls/gtlsconnection-gnutls.c | 11 +++
tls/openssl/gtlsconnection-openssl.c | 13 ++++
tls/tests/connection.c | 1 +
5 files changed, 104 insertions(+), 64 deletions(-)
---
diff --git a/tls/base/gtlsconnection-base.c b/tls/base/gtlsconnection-base.c
index b1225f5..5230152 100644
--- a/tls/base/gtlsconnection-base.c
+++ b/tls/base/gtlsconnection-base.c
@@ -1332,6 +1332,14 @@ handshake_thread (GTask *task,
{
GTlsConnectionBaseStatus status;
+ if (priv->rehandshake_mode != G_TLS_REHANDSHAKE_UNSAFELY &&
+ tls_class->safe_renegotiation_status (tls) != G_TLS_SAFE_RENEGOTIATION_SUPPORTED_BY_PEER)
+ {
+ g_task_return_new_error (task, G_TLS_ERROR, G_TLS_ERROR_MISC,
+ _("Peer does not support safe renegotiation"));
+ return;
+ }
+
/* Adjust the timeout for the next operation in the sequence. */
if (timeout > 0)
{
diff --git a/tls/base/gtlsconnection-base.h b/tls/base/gtlsconnection-base.h
index 3cbb14c..98632ad 100644
--- a/tls/base/gtlsconnection-base.h
+++ b/tls/base/gtlsconnection-base.h
@@ -47,76 +47,83 @@ typedef enum {
G_TLS_DIRECTION_WRITE = 1 << 1,
} GTlsDirection;
+typedef enum {
+ G_TLS_SAFE_RENEGOTIATION_SUPPORTED_BY_PEER,
+ G_TLS_SAFE_RENEGOTIATION_UNSUPPORTED
+} GTlsSafeRenegotiationStatus;
+
#define G_TLS_DIRECTION_BOTH (G_TLS_DIRECTION_READ | G_TLS_DIRECTION_WRITE)
struct _GTlsConnectionBaseClass
{
GTlsConnectionClass parent_class;
- GTlsConnectionBaseStatus (*request_rehandshake) (GTlsConnectionBase *tls,
- gint64 timeout,
- GCancellable *cancellable,
- GError **error);
-
- void (*prepare_handshake) (GTlsConnectionBase *tls,
- gchar **advertised_protocols);
- GTlsConnectionBaseStatus (*handshake_thread_handshake) (GTlsConnectionBase *tls,
- gint64 timeout,
- 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);
-
- gboolean (*is_session_resumed) (GTlsConnectionBase *tls);
-
- void (*push_io) (GTlsConnectionBase *tls,
- GIOCondition direction,
- gint64 timeout,
- GCancellable *cancellable);
- GTlsConnectionBaseStatus (*pop_io) (GTlsConnectionBase *tls,
- GIOCondition direction,
- gboolean success,
- GError **error);
-
- GTlsConnectionBaseStatus (*read_fn) (GTlsConnectionBase *tls,
- void *buffer,
- gsize count,
- gint64 timeout,
- gssize *nread,
- GCancellable *cancellable,
- GError **error);
- GTlsConnectionBaseStatus (*read_message_fn) (GTlsConnectionBase *tls,
- GInputVector *vectors,
- guint num_vectors,
- gint64 timeout,
- gssize *nread,
- GCancellable *cancellable,
- GError **error);
-
- GTlsConnectionBaseStatus (*write_fn) (GTlsConnectionBase *tls,
- const void *buffer,
- gsize count,
- gint64 timeout,
- gssize *nwrote,
- GCancellable *cancellable,
- GError **error);
- GTlsConnectionBaseStatus (*write_message_fn) (GTlsConnectionBase *tls,
- GOutputVector *vectors,
- guint num_vectors,
- gint64 timeout,
- gssize *nwrote,
- GCancellable *cancellable,
- GError **error);
-
- GTlsConnectionBaseStatus (*close_fn) (GTlsConnectionBase *tls,
- gint64 timeout,
- GCancellable *cancellable,
- GError **error);
+ GTlsConnectionBaseStatus (*request_rehandshake) (GTlsConnectionBase *tls,
+ gint64 timeout,
+ GCancellable *cancellable,
+ GError **error);
+
+ void (*prepare_handshake) (GTlsConnectionBase *tls,
+ gchar **advertised_protocols);
+ GTlsConnectionBaseStatus (*handshake_thread_handshake) (GTlsConnectionBase *tls,
+ gint64 timeout,
+ 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);
+
+ gboolean (*is_session_resumed) (GTlsConnectionBase *tls);
+
+ void (*push_io) (GTlsConnectionBase *tls,
+ GIOCondition direction,
+ gint64 timeout,
+ GCancellable *cancellable);
+ GTlsConnectionBaseStatus (*pop_io) (GTlsConnectionBase *tls,
+ GIOCondition direction,
+ gboolean success,
+ GError **error);
+
+ GTlsConnectionBaseStatus (*read_fn) (GTlsConnectionBase *tls,
+ void *buffer,
+ gsize count,
+ gint64 timeout,
+ gssize *nread,
+ GCancellable *cancellable,
+ GError **error);
+ GTlsConnectionBaseStatus (*read_message_fn) (GTlsConnectionBase *tls,
+ GInputVector *vectors,
+ guint num_vectors,
+ gint64 timeout,
+ gssize *nread,
+ GCancellable *cancellable,
+ GError **error);
+
+ GTlsConnectionBaseStatus (*write_fn) (GTlsConnectionBase *tls,
+ const void *buffer,
+ gsize count,
+ gint64 timeout,
+ gssize *nwrote,
+ GCancellable *cancellable,
+ GError **error);
+ GTlsConnectionBaseStatus (*write_message_fn) (GTlsConnectionBase *tls,
+ GOutputVector *vectors,
+ guint num_vectors,
+ gint64 timeout,
+ gssize *nwrote,
+ GCancellable *cancellable,
+ GError **error);
+
+ GTlsConnectionBaseStatus (*close_fn) (GTlsConnectionBase *tls,
+ gint64 timeout,
+ GCancellable *cancellable,
+ GError **error);
+
+ GTlsSafeRenegotiationStatus (*safe_renegotiation_status) (GTlsConnectionBase *tls);
};
gboolean g_tls_connection_base_handshake_thread_verify_certificate
diff --git a/tls/gnutls/gtlsconnection-gnutls.c b/tls/gnutls/gtlsconnection-gnutls.c
index 1ac803a..0990006 100644
--- a/tls/gnutls/gtlsconnection-gnutls.c
+++ b/tls/gnutls/gtlsconnection-gnutls.c
@@ -1096,6 +1096,16 @@ g_tls_connection_gnutls_close (GTlsConnectionBase *tls,
return status;
}
+static GTlsSafeRenegotiationStatus
+g_tls_connection_gnutls_safe_renegotiation_status (GTlsConnectionBase *tls)
+{
+ GTlsConnectionGnutls *gnutls = G_TLS_CONNECTION_GNUTLS (tls);
+ GTlsConnectionGnutlsPrivate *priv = g_tls_connection_gnutls_get_instance_private (gnutls);
+
+ return gnutls_safe_renegotiation_status (priv->session) ? G_TLS_SAFE_RENEGOTIATION_SUPPORTED_BY_PEER
+ : G_TLS_SAFE_RENEGOTIATION_UNSUPPORTED;
+}
+
static void
g_tls_connection_gnutls_class_init (GTlsConnectionGnutlsClass *klass)
{
@@ -1115,6 +1125,7 @@ g_tls_connection_gnutls_class_init (GTlsConnectionGnutlsClass *klass)
base_class->write_fn = g_tls_connection_gnutls_write;
base_class->write_message_fn = g_tls_connection_gnutls_write_message;
base_class->close_fn = g_tls_connection_gnutls_close;
+ base_class->safe_renegotiation_status = g_tls_connection_gnutls_safe_renegotiation_status;
}
static void
diff --git a/tls/openssl/gtlsconnection-openssl.c b/tls/openssl/gtlsconnection-openssl.c
index d3c39b1..3745766 100644
--- a/tls/openssl/gtlsconnection-openssl.c
+++ b/tls/openssl/gtlsconnection-openssl.c
@@ -497,6 +497,18 @@ g_tls_connection_openssl_close (GTlsConnectionBase *tls,
return status;
}
+static GTlsSafeRenegotiationStatus
+g_tls_connection_openssl_safe_renegotiation_status (GTlsConnectionBase *tls)
+{
+ GTlsConnectionOpenssl *openssl = G_TLS_CONNECTION_OPENSSL (tls);
+ SSL *ssl;
+
+ ssl = g_tls_connection_openssl_get_ssl (openssl);
+
+ return SSL_get_secure_renegotiation_support (ssl) ? G_TLS_SAFE_RENEGOTIATION_SUPPORTED_BY_PEER
+ : G_TLS_SAFE_RENEGOTIATION_UNSUPPORTED;
+}
+
static void
g_tls_connection_openssl_class_init (GTlsConnectionOpensslClass *klass)
{
@@ -513,6 +525,7 @@ g_tls_connection_openssl_class_init (GTlsConnectionOpensslClass *klass)
base_class->read_fn = g_tls_connection_openssl_read;
base_class->write_fn = g_tls_connection_openssl_write;
base_class->close_fn = g_tls_connection_openssl_close;
+ base_class->safe_renegotiation_status = g_tls_connection_openssl_safe_renegotiation_status;
}
static int data_index = -1;
diff --git a/tls/tests/connection.c b/tls/tests/connection.c
index e5cd537..31aaa1c 100644
--- a/tls/tests/connection.c
+++ b/tls/tests/connection.c
@@ -1053,6 +1053,7 @@ test_client_auth_rehandshake (TestConnection *test,
gconstpointer data)
{
#ifdef BACKEND_IS_OPENSSL
+ /* FIXME: this doesn't make sense, we should support safe renegotation */
g_test_skip ("the server avoids rehandshake to avoid the security problem CVE-2009-3555");
return;
#endif
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
