[glib-networking/mcatanzaro/copy-session-state: 3/3] gnutls: bring back copy_session_state() support for TLS 1.2



commit 800ce16eac7ddaab494b70b393e33dd968ffbee7
Author: Michael Catanzaro <mcatanzaro gnome org>
Date:   Sat Feb 1 18:59:12 2020 -0600

    gnutls: bring back copy_session_state() support for TLS 1.2
    
    This should probably avoid breaking GVfs's FTPS support.
    
    Although it shouldn't be required, we check if the handshake succeeded
    before trying to get the session data to avoid a memory leak in GnuTLS
    that occurs if called without a successful handshake, see gnutls!1185.
    This is OK because it's confusing for complete_handshake to not indicate
    whether the handshake was successful, so and it's a tiny optimization to
    avoid calling GnuTLS functions when not required.

 tls/base/gtlsconnection-base.c             |  7 ++++--
 tls/base/gtlsconnection-base.h             |  1 +
 tls/gnutls/gtlsclientconnection-gnutls.c   | 39 +++++++++++++++++++++++++++---
 tls/gnutls/gtlsconnection-gnutls.c         |  5 +++-
 tls/openssl/gtlsclientconnection-openssl.c |  3 ++-
 5 files changed, 47 insertions(+), 8 deletions(-)
---
diff --git a/tls/base/gtlsconnection-base.c b/tls/base/gtlsconnection-base.c
index 4fb224a..d6c4e37 100644
--- a/tls/base/gtlsconnection-base.c
+++ b/tls/base/gtlsconnection-base.c
@@ -1555,13 +1555,15 @@ finish_handshake (GTlsConnectionBase  *tls,
   GTlsConnectionBasePrivate *priv = g_tls_connection_base_get_instance_private (tls);
   GTlsConnectionBaseClass *tls_class = G_TLS_CONNECTION_BASE_GET_CLASS (tls);
   gchar *original_negotiated_protocol;
+  gboolean success;
   GError *my_error = NULL;
 
   g_tls_log_debug (tls, "finishing TLS handshake");
 
   original_negotiated_protocol = g_steal_pointer (&priv->negotiated_protocol);
 
-  if (g_task_propagate_boolean (task, &my_error))
+  success = g_task_propagate_boolean (task, &my_error);
+  if (success)
     {
       if (tls_class->is_session_resumed && tls_class->is_session_resumed (tls))
         {
@@ -1585,13 +1587,14 @@ finish_handshake (GTlsConnectionBase  *tls,
         {
           g_set_error_literal (&my_error, G_TLS_ERROR, G_TLS_ERROR_BAD_CERTIFICATE,
                                _("Unacceptable TLS certificate"));
+          success = FALSE;
         }
     }
 
   if (tls_class->complete_handshake)
     {
       /* If we already have an error, ignore further errors. */
-      tls_class->complete_handshake (tls, &priv->negotiated_protocol, my_error ? NULL : &my_error);
+      tls_class->complete_handshake (tls, success, &priv->negotiated_protocol, my_error ? NULL : &my_error);
 
       if (g_strcmp0 (original_negotiated_protocol, priv->negotiated_protocol) != 0)
         g_object_notify (G_OBJECT (tls), "negotiated-protocol");
diff --git a/tls/base/gtlsconnection-base.h b/tls/base/gtlsconnection-base.h
index 221bbe6..45c14e5 100644
--- a/tls/base/gtlsconnection-base.h
+++ b/tls/base/gtlsconnection-base.h
@@ -73,6 +73,7 @@ struct _GTlsConnectionBaseClass
                                                              GError              **error);
   GTlsCertificate            *(*retrieve_peer_certificate)  (GTlsConnectionBase   *tls);
   void                        (*complete_handshake)         (GTlsConnectionBase   *tls,
+                                                             gboolean              handshake_succeeded,
                                                              gchar               **negotiated_protocol,
                                                              GError              **error);
 
diff --git a/tls/gnutls/gtlsclientconnection-gnutls.c b/tls/gnutls/gtlsclientconnection-gnutls.c
index 734ad75..902857c 100644
--- a/tls/gnutls/gtlsclientconnection-gnutls.c
+++ b/tls/gnutls/gtlsclientconnection-gnutls.c
@@ -491,18 +491,48 @@ g_tls_client_connection_gnutls_prepare_handshake (GTlsConnectionBase  *tls,
 
 static void
 g_tls_client_connection_gnutls_complete_handshake (GTlsConnectionBase  *tls,
+                                                   gboolean             handshake_succeeded,
                                                    gchar              **negotiated_protocol,
                                                    GError             **error)
 {
   GTlsClientConnectionGnutls *gnutls = G_TLS_CLIENT_CONNECTION_GNUTLS (tls);
+  gnutls_session_t session;
+  gnutls_protocol_t version;
 
-  G_TLS_CONNECTION_BASE_CLASS (g_tls_client_connection_gnutls_parent_class)->complete_handshake (tls, 
negotiated_protocol, error);
+  G_TLS_CONNECTION_BASE_CLASS (g_tls_client_connection_gnutls_parent_class)->complete_handshake (tls, 
handshake_succeeded, negotiated_protocol, error);
 
   /* It may have changed during the handshake, but we have to wait until here
    * because we can't emit notifies on the handshake thread.
    */
   if (gnutls->accepted_cas_changed)
     g_object_notify (G_OBJECT (gnutls), "accepted-cas");
+
+  if (handshake_succeeded)
+    {
+      /* If we're not using TLS 1.3, store the session ticket here. We
+       * don't normally perform session resumption in TLS 1.2, but we still
+       * support it if the application calls copy_session_state() (which
+       * doesn't exist for DTLS, so do this for TLS only).
+       *
+       * Note to distant future: remove this when dropping TLS 1.2 support.
+       */
+      session = g_tls_connection_gnutls_get_session (G_TLS_CONNECTION_GNUTLS (tls));
+      version = gnutls_protocol_get_version (session);
+      if (version <= GNUTLS_TLS1_2 && !g_tls_connection_base_is_dtls (tls))
+        {
+          gnutls_datum_t session_datum;
+
+          if (gnutls_session_get_data2 (g_tls_connection_gnutls_get_session (G_TLS_CONNECTION_GNUTLS (tls)),
+                                        &session_datum) == 0)
+            {
+              g_clear_pointer (&gnutls->session_data, g_bytes_unref);
+              gnutls->session_data = g_bytes_new_with_free_func (session_datum.data,
+                                                                 session_datum.size,
+                                                                 (GDestroyNotify)gnutls_free,
+                                                                 session_datum.data);
+            }
+        }
+  }
 }
 
 static void
@@ -515,6 +545,7 @@ g_tls_client_connection_gnutls_copy_session_state (GTlsClientConnection *conn,
   /* Precondition: source has handshaked, conn has not. */
   g_return_if_fail (!gnutls->session_id);
   g_return_if_fail (gnutls_source->session_id);
+  g_return_if_fail (!gnutls->session_data);
 
   /* Prefer to use a new session ticket, if possible. */
   gnutls->session_data = g_tls_backend_gnutls_lookup_session_data (gnutls_source->session_id);
@@ -522,9 +553,9 @@ g_tls_client_connection_gnutls_copy_session_state (GTlsClientConnection *conn,
   if (!gnutls->session_data && gnutls_source->session_data)
     {
       /* If it's not possible, we'll try to reuse the old ticket, even though
-       * this is a privacy risk since TLS 1.3. Applications should not use this
-       * function unless they need us to try as hard as possible to resume a
-       * session, even at the cost of privacy.
+       * this is a privacy risk. Applications should not use this function
+       * unless they need us to try as hard as possible to resume a session,
+       * even at the cost of privacy.
        */
       gnutls->session_data = g_bytes_ref (gnutls_source->session_data);
     }
diff --git a/tls/gnutls/gtlsconnection-gnutls.c b/tls/gnutls/gtlsconnection-gnutls.c
index e8e399c..99a22b6 100644
--- a/tls/gnutls/gtlsconnection-gnutls.c
+++ b/tls/gnutls/gtlsconnection-gnutls.c
@@ -822,6 +822,7 @@ g_tls_connection_gnutls_handshake_thread_handshake (GTlsConnectionBase  *tls,
 
 static void
 g_tls_connection_gnutls_complete_handshake (GTlsConnectionBase  *tls,
+                                            gboolean             handshake_succeeded,
                                             gchar              **negotiated_protocol,
                                             GError             **error)
 {
@@ -829,7 +830,9 @@ g_tls_connection_gnutls_complete_handshake (GTlsConnectionBase  *tls,
   GTlsConnectionGnutlsPrivate *priv = g_tls_connection_gnutls_get_instance_private (gnutls);
   gnutls_datum_t protocol;
 
-  if (gnutls_alpn_get_selected_protocol (priv->session, &protocol) == 0 && protocol.size > 0)
+  if (handshake_succeeded &&
+      gnutls_alpn_get_selected_protocol (priv->session, &protocol) == 0 &&
+      protocol.size > 0)
     {
       g_assert (!*negotiated_protocol);
       *negotiated_protocol = g_strndup ((gchar *)protocol.data, protocol.size);
diff --git a/tls/openssl/gtlsclientconnection-openssl.c b/tls/openssl/gtlsclientconnection-openssl.c
index 56e279f..aa533b7 100644
--- a/tls/openssl/gtlsclientconnection-openssl.c
+++ b/tls/openssl/gtlsclientconnection-openssl.c
@@ -186,13 +186,14 @@ g_tls_client_connection_openssl_set_property (GObject      *object,
 
 static void
 g_tls_client_connection_openssl_complete_handshake (GTlsConnectionBase  *tls,
+                                                    gboolean             handshake_succeeded,
                                                     gchar              **negotiated_protocol,
                                                     GError             **error)
 {
   GTlsClientConnectionOpenssl *client = G_TLS_CLIENT_CONNECTION_OPENSSL (tls);
 
   if (G_TLS_CONNECTION_BASE_CLASS (g_tls_client_connection_openssl_parent_class)->complete_handshake)
-    G_TLS_CONNECTION_BASE_CLASS (g_tls_client_connection_openssl_parent_class)->complete_handshake (tls, 
negotiated_protocol, error);
+    G_TLS_CONNECTION_BASE_CLASS (g_tls_client_connection_openssl_parent_class)->complete_handshake (tls, 
handshake_succeeded, negotiated_protocol, error);
 
   /* It may have changed during the handshake, but we have to wait until here
    * because we can't emit notifies on the handshake thread.


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