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

[Ignacio Casal Quinteiro <icq src gnome org>]

commit ddcc5dad26e8f2277d12018d1872a6b41d39acc9
Author: Ignacio Casal Quinteiro <icq gnome org>
Date:   Thu May 23 16:42:42 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/gtlsconnection-openssl.c | 109 +++++++++++++++++++++++++----------
 1 file changed, 78 insertions(+), 31 deletions(-)
---
diff --git a/tls/openssl/gtlsconnection-openssl.c b/tls/openssl/gtlsconnection-openssl.c
index 6eb803b..f5fb1dd 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,
@@ -61,31 +75,19 @@ end_openssl_io (GTlsConnectionOpenssl  *openssl,
                 const char             *err_str)
 {
   GTlsConnectionBase *tls = G_TLS_CONNECTION_BASE (openssl);
-  GTlsConnectionOpensslPrivate *priv;
   int err_code, err, err_lib, reason;
   GError *my_error = NULL;
   GTlsConnectionBaseStatus status;
   SSL *ssl;
 
-  priv = g_tls_connection_openssl_get_instance_private (openssl);
-
   ssl = g_tls_connection_openssl_get_ssl (openssl);
 
   err_code = SSL_get_error (ssl, ret);
 
   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);
@@ -104,15 +106,6 @@ end_openssl_io (GTlsConnectionOpenssl  *openssl,
       return status;
     }
 
-  /* This case is documented that it may happen and that is perfectly fine */
-  if (err_code == SSL_ERROR_SYSCALL &&
-      ((priv->shutting_down && !my_error) ||
-       g_error_matches (my_error, G_IO_ERROR, G_IO_ERROR_BROKEN_PIPE)))
-    {
-      g_clear_error (&my_error);
-      return G_TLS_CONNECTION_BASE_OK;
-    }
-
   err = ERR_get_error ();
   err_lib = ERR_GET_LIB (err);
   reason = ERR_GET_REASON (err);
@@ -343,6 +336,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 +351,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 +373,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 +418,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 +472,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 +485,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 +544,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 *