[glib-networking/enable-openssl-tests: 1/2] openssl: reimplement read/write to be always non blocking



commit 11e82ae5afa9d3bb82dca052e81e7f12c3d7f413
Author: Ignacio Casal Quinteiro <icq gnome org>
Date:   Thu May 23 12:40:57 2019 +0200

    openssl: reimplement read/write to be always non blocking
    
    And we block ourselves until the socket is available. This is to
    be able to lock over the ssl object and avoid deadlocks when writing
    and reading at the same time.

 tls/openssl/gtlsbio.c                | 47 +++++++++++++++++
 tls/openssl/gtlsbio.h                |  4 ++
 tls/openssl/gtlsconnection-openssl.c | 97 +++++++++++++++++++++++++++++-------
 3 files changed, 129 insertions(+), 19 deletions(-)
---
diff --git a/tls/openssl/gtlsbio.c b/tls/openssl/gtlsbio.c
index 02ccf21..826cb50 100644
--- a/tls/openssl/gtlsbio.c
+++ b/tls/openssl/gtlsbio.c
@@ -397,3 +397,50 @@ g_tls_bio_set_write_error (BIO     *bio,
 #endif
   gbio->write_error = error;
 }
+
+static gboolean
+on_source_ready (GObject *pollable_stream,
+                 gpointer user_data)
+{
+  GMainLoop *loop = user_data;
+
+  g_main_loop_quit (loop);
+
+  return G_SOURCE_REMOVE;
+}
+
+void
+g_tls_bio_wait_available (BIO          *bio,
+                          GIOCondition  condition,
+                          GCancellable *cancellable)
+{
+  GTlsBio *gbio;
+  GMainLoop *loop;
+  GSource *source;
+
+  g_return_if_fail (bio != NULL);
+
+#if OPENSSL_VERSION_NUMBER < 0x10100000L || defined (LIBRESSL_VERSION_NUMBER)
+  gbio = (GTlsBio *)bio->ptr;
+#else
+  gbio = BIO_get_data (bio);
+#endif
+
+  loop = g_main_loop_new (g_main_context_get_thread_default (), FALSE);
+
+  if (condition & G_IO_IN)
+    source = g_pollable_input_stream_create_source (G_POLLABLE_INPUT_STREAM (g_io_stream_get_input_stream 
(gbio->io_stream)),
+                                                    cancellable);
+  else
+    source = g_pollable_output_stream_create_source (G_POLLABLE_OUTPUT_STREAM (g_io_stream_get_output_stream 
(gbio->io_stream)),
+                                                     cancellable);
+
+  g_source_set_callback (source, (GSourceFunc)on_source_ready, loop, NULL);
+  g_source_attach (source, g_main_context_get_thread_default ());
+
+  g_main_loop_run (loop);
+  g_main_loop_unref (loop);
+
+  g_source_destroy (source);
+  g_source_unref (source);
+}
diff --git a/tls/openssl/gtlsbio.h b/tls/openssl/gtlsbio.h
index b6812fe..4809c17 100644
--- a/tls/openssl/gtlsbio.h
+++ b/tls/openssl/gtlsbio.h
@@ -51,6 +51,10 @@ void       g_tls_bio_set_write_blocking    (BIO          *bio,
 void       g_tls_bio_set_write_error       (BIO          *bio,
                                             GError      **error);
 
+void       g_tls_bio_wait_available        (BIO          *bio,
+                                            GIOCondition  condition,
+                                            GCancellable *cancellable);
+
 G_END_DECLS
 
 #endif /* __G_TLS_BIO_H__ */
diff --git a/tls/openssl/gtlsconnection-openssl.c b/tls/openssl/gtlsconnection-openssl.c
index 6eb803b..515f4fb 100644
--- a/tls/openssl/gtlsconnection-openssl.c
+++ b/tls/openssl/gtlsconnection-openssl.c
@@ -41,6 +41,7 @@
 typedef struct _GTlsConnectionOpensslPrivate
 {
   BIO *bio;
+  GMutex ssl_mutex;
 
   gboolean shutting_down;
 } GTlsConnectionOpensslPrivate;
@@ -52,6 +53,19 @@ G_DEFINE_ABSTRACT_TYPE_WITH_CODE (GTlsConnectionOpenssl, g_tls_connection_openss
                                   G_IMPLEMENT_INTERFACE (G_TYPE_INITABLE,
                                                          g_tls_connection_openssl_initable_iface_init))
 
+static void
+g_tls_connection_openssl_finalize (GObject *object)
+{
+  GTlsConnectionOpenssl *openssl = G_TLS_CONNECTION_OPENSSL (object);
+  GTlsConnectionOpensslPrivate *priv;
+
+  priv = g_tls_connection_openssl_get_instance_private (openssl);
+
+  g_mutex_clear (&priv->ssl_mutex);
+
+  G_OBJECT_CLASS (g_tls_connection_openssl_parent_class)->finalize (object);
+}
+
 static GTlsConnectionBaseStatus
 end_openssl_io (GTlsConnectionOpenssl  *openssl,
                 GIOCondition            direction,
@@ -75,17 +89,8 @@ end_openssl_io (GTlsConnectionOpenssl  *openssl,
 
   status = g_tls_connection_base_pop_io (tls, direction, ret > 0, &my_error);
 
-  /* NOTE: this is tricky! The tls bio will set to retry if the operation
-   * would block, and we would get an error code with WANT_READ or WANT_WRITE,
-   * though if in that case we try again we would end up in an infinite loop
-   * since we will not let the glib main loop to do its stuff and we would
-   * be getting a would block forever. Instead we need to also check the error
-   * we get from the socket operation to understand whether to try again. See
-   * that we propagate the WOULD_BLOCK error a bit more down.
-   */
-  if ((err_code == SSL_ERROR_WANT_READ ||
-       err_code == SSL_ERROR_WANT_WRITE) &&
-      status != G_TLS_CONNECTION_BASE_WOULD_BLOCK)
+  if (err_code == SSL_ERROR_WANT_READ ||
+      err_code == SSL_ERROR_WANT_WRITE)
     {
       if (my_error)
         g_error_free (my_error);
@@ -343,6 +348,8 @@ g_tls_connection_openssl_push_io (GTlsConnectionBase *tls,
       g_clear_error (error);
       g_tls_bio_set_write_error (priv->bio, error);
     }
+
+  g_mutex_lock (&priv->ssl_mutex);
 }
 
 static GTlsConnectionBaseStatus
@@ -356,6 +363,8 @@ g_tls_connection_openssl_pop_io (GTlsConnectionBase  *tls,
 
   priv = g_tls_connection_openssl_get_instance_private (openssl);
 
+  g_mutex_unlock (&priv->ssl_mutex);
+
   if (direction & G_IO_IN)
     g_tls_bio_set_read_cancellable (priv->bio, NULL);
 
@@ -376,16 +385,35 @@ g_tls_connection_openssl_read (GTlsConnectionBase    *tls,
                                GError               **error)
 {
   GTlsConnectionOpenssl *openssl = G_TLS_CONNECTION_OPENSSL (tls);
+  GTlsConnectionOpensslPrivate *priv;
   GTlsConnectionBaseStatus status;
   SSL *ssl;
   gssize ret;
 
+  priv = g_tls_connection_openssl_get_instance_private (openssl);
+
   ssl = g_tls_connection_openssl_get_ssl (openssl);
 
-  BEGIN_OPENSSL_IO (openssl, G_IO_IN, timeout, cancellable);
-  ret = SSL_read (ssl, buffer, count);
-  END_OPENSSL_IO (openssl, G_IO_IN, ret, status,
-                  _("Error reading data from TLS socket"), error);
+  while (TRUE)
+    {
+      char error_str[256];
+
+      /* We want to always be non blocking here to avoid deadlocks */
+      g_tls_connection_base_push_io (G_TLS_CONNECTION_BASE (openssl),
+                                     G_IO_IN, 0, cancellable);
+
+      ret = SSL_read (ssl, buffer, count);
+
+      ERR_error_string_n (SSL_get_error (ssl, ret), error_str, sizeof (error_str));
+      status = end_openssl_io (openssl, G_IO_IN, ret, error,
+                               _("Error reading data from TLS socket"), error_str);
+
+      if (status != G_TLS_CONNECTION_BASE_TRY_AGAIN)
+        break;
+
+      /* Wait for the socket to be available again to avoid an infinite loop */
+      g_tls_bio_wait_available (priv->bio, G_IO_IN, cancellable);
+    }
 
   if (ret >= 0)
     *nread = ret;
@@ -402,16 +430,35 @@ g_tls_connection_openssl_write (GTlsConnectionBase    *tls,
                                 GError               **error)
 {
   GTlsConnectionOpenssl *openssl = G_TLS_CONNECTION_OPENSSL (tls);
+  GTlsConnectionOpensslPrivate *priv;
   GTlsConnectionBaseStatus status;
   SSL *ssl;
   gssize ret;
 
+  priv = g_tls_connection_openssl_get_instance_private (openssl);
+
   ssl = g_tls_connection_openssl_get_ssl (openssl);
 
-  BEGIN_OPENSSL_IO (openssl, G_IO_OUT, timeout, cancellable);
-  ret = SSL_write (ssl, buffer, count);
-  END_OPENSSL_IO (openssl, G_IO_OUT, ret, status,
-                  _("Error writing data to TLS socket"), error);
+  while (TRUE)
+    {
+      char error_str[256];
+
+      /* We want to always be non blocking here to avoid deadlocks */
+      g_tls_connection_base_push_io (G_TLS_CONNECTION_BASE (openssl),
+                                     G_IO_OUT, 0, cancellable);
+
+      ret = SSL_write (ssl, buffer, count);
+
+      ERR_error_string_n (SSL_get_error (ssl, ret), error_str, sizeof (error_str));
+      status = end_openssl_io (openssl, G_IO_OUT, ret, error,
+                               _("Error writing data to TLS socket"), error_str);
+
+      if (status != G_TLS_CONNECTION_BASE_TRY_AGAIN)
+        break;
+
+      /* Wait for the socket to be available again to avoid an infinite loop */
+      g_tls_bio_wait_available (priv->bio, G_IO_OUT, cancellable);
+    }
 
   if (ret >= 0)
     *nwrote = ret;
@@ -437,6 +484,10 @@ g_tls_connection_openssl_close (GTlsConnectionBase  *tls,
 
   BEGIN_OPENSSL_IO (openssl, G_IO_IN | G_IO_OUT, timeout, cancellable);
   ret = SSL_shutdown (ssl);
+  /* Note it is documented that getting 0 is correct when shutting down since
+   * it means it will close the write direction
+   */
+  ret = ret == 0 ? 1 : ret;
   END_OPENSSL_IO (openssl, G_IO_IN | G_IO_OUT, ret, status,
                   _("Error performing TLS close"), error);
 
@@ -446,8 +497,11 @@ g_tls_connection_openssl_close (GTlsConnectionBase  *tls,
 static void
 g_tls_connection_openssl_class_init (GTlsConnectionOpensslClass *klass)
 {
+  GObjectClass *object_class = G_OBJECT_CLASS (klass);
   GTlsConnectionBaseClass *base_class = G_TLS_CONNECTION_BASE_CLASS (klass);
 
+  object_class->finalize                 = g_tls_connection_openssl_finalize;
+
   base_class->request_rehandshake        = g_tls_connection_openssl_request_rehandshake;
   base_class->handshake_thread_handshake = g_tls_connection_openssl_handshake_thread_handshake;
   base_class->retrieve_peer_certificate  = g_tls_connection_openssl_retrieve_peer_certificate;
@@ -502,6 +556,11 @@ g_tls_connection_openssl_initable_iface_init (GInitableIface *iface)
 static void
 g_tls_connection_openssl_init (GTlsConnectionOpenssl *openssl)
 {
+  GTlsConnectionOpensslPrivate *priv;
+
+  priv = g_tls_connection_openssl_get_instance_private (openssl);
+
+  g_mutex_init (&priv->ssl_mutex);
 }
 
 SSL *


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]