[glib-networking/mcatanzaro/session-resumption: 13/13] gnutls: support TLS 1.3 session resumption client-side

[Michael Catanzaro <mcatanzaro src gnome org>]

commit 91668ce7f3f4cdee6361fc29d1e94348de31718a
Author: Michael Catanzaro <mcatanzaro gnome org>
Date:   Mon Nov 18 14:16:18 2019 -0600

    gnutls: support TLS 1.3 session resumption client-side
    
    This removes support for session resumption with TLS 1.2, which is
    sufficiently different that it's annoying to try to support them both.
    
    The main difference is that in TLS 1.3 we can receive any number of
    session tickets from the server, and we should use each one at most
    once.

 tls/gnutls/gtlsbackend-gnutls.c          |  81 +++++++--------------
 tls/gnutls/gtlsbackend-gnutls.h          |  10 +--
 tls/gnutls/gtlsclientconnection-gnutls.c | 117 ++++++++++++++++---------------
 tls/gnutls/gtlsconnection-gnutls.c       |   6 +-
 tls/gnutls/gtlsconnection-gnutls.h       |   2 -
 5 files changed, 91 insertions(+), 125 deletions(-)
---
diff --git a/tls/gnutls/gtlsbackend-gnutls.c b/tls/gnutls/gtlsbackend-gnutls.c
index f8890e5..bcc7f92 100644
--- a/tls/gnutls/gtlsbackend-gnutls.c
+++ b/tls/gnutls/gtlsbackend-gnutls.c
@@ -172,14 +172,13 @@ g_tls_backend_gnutls_interface_init (GTlsBackendInterface *iface)
  */
 
 G_LOCK_DEFINE_STATIC (session_cache_lock);
-GHashTable *client_session_cache, *server_session_cache;
+GHashTable *client_session_cache; /* (owned) GBytes -> (owned) GTlsBackendGnutlsCacheData */
 
 #define SESSION_CACHE_MAX_SIZE 50
 #define SESSION_CACHE_MAX_AGE (60ll * 60ll * G_USEC_PER_SEC) /* one hour */
 
 typedef struct {
-  GBytes *session_id;
-  GBytes *session_data;
+  GQueue *session_tickets; /* (owned) GBytes */
   gint64  last_used;
 } GTlsBackendGnutlsCacheData;
 
@@ -189,96 +188,63 @@ session_cache_cleanup (GHashTable *cache)
   GHashTableIter iter;
   gpointer key, value;
   GTlsBackendGnutlsCacheData *cache_data;
-  gint64 expired = g_get_monotonic_time () - SESSION_CACHE_MAX_AGE;
 
   g_hash_table_iter_init (&iter, cache);
   while (g_hash_table_iter_next (&iter, &key, &value))
     {
       cache_data = value;
-      if (cache_data->last_used < expired)
+      if (cache_data->last_used + SESSION_CACHE_MAX_AGE < g_get_monotonic_time ())
         g_hash_table_iter_remove (&iter);
     }
 }
 
 static void
-cache_data_free (gpointer data)
+cache_data_free (GTlsBackendGnutlsCacheData *data)
 {
-  GTlsBackendGnutlsCacheData *cache_data = data;
-
-  g_bytes_unref (cache_data->session_id);
-  g_bytes_unref (cache_data->session_data);
-  g_free (cache_data);
+  g_queue_free_full (data->session_tickets, (GDestroyNotify)g_bytes_unref);
+  g_free (data);
 }
 
 static GHashTable *
-get_session_cache (unsigned int type,
-                   gboolean     create)
+get_session_cache (gboolean create)
 {
-  GHashTable **cache_p;
-
-  cache_p = (type == GNUTLS_CLIENT) ? &client_session_cache : &server_session_cache;
-  if (!*cache_p && create)
+  if (!client_session_cache && create)
     {
-      *cache_p = g_hash_table_new_full (g_bytes_hash, g_bytes_equal,
-                                        NULL, cache_data_free);
+      client_session_cache = g_hash_table_new_full (g_bytes_hash, g_bytes_equal,
+                                                    (GDestroyNotify)g_bytes_unref, 
(GDestroyNotify)cache_data_free);
     }
-  return *cache_p;
+  return client_session_cache;
 }
 
 void
-g_tls_backend_gnutls_store_session (unsigned int  type,
-                                    GBytes       *session_id,
-                                    GBytes       *session_data)
+g_tls_backend_gnutls_store_session_data (GBytes *session_id,
+                                         GBytes *session_data)
 {
   GTlsBackendGnutlsCacheData *cache_data;
   GHashTable *cache;
 
   G_LOCK (session_cache_lock);
 
-  cache = get_session_cache (type, TRUE);
+  cache = get_session_cache (TRUE);
   cache_data = g_hash_table_lookup (cache, session_id);
-  if (cache_data)
-    {
-      if (!g_bytes_equal (cache_data->session_data, session_data))
-        {
-          g_bytes_unref (cache_data->session_data);
-          cache_data->session_data = g_bytes_ref (session_data);
-        }
-    }
-  else
+  if (!cache_data)
     {
       if (g_hash_table_size (cache) >= SESSION_CACHE_MAX_SIZE)
         session_cache_cleanup (cache);
 
       cache_data = g_new (GTlsBackendGnutlsCacheData, 1);
-      cache_data->session_id = g_bytes_ref (session_id);
-      cache_data->session_data = g_bytes_ref (session_data);
-
-      g_hash_table_insert (cache, cache_data->session_id, cache_data);
+      cache_data->session_tickets = g_queue_new ();
+      g_hash_table_insert (cache, g_bytes_ref (session_id), cache_data);
     }
-  cache_data->last_used = g_get_monotonic_time ();
-
-  G_UNLOCK (session_cache_lock);
-}
-
-void
-g_tls_backend_gnutls_remove_session (unsigned int  type,
-                                     GBytes       *session_id)
-{
-  GHashTable *cache;
-
-  G_LOCK (session_cache_lock);
 
-  cache = get_session_cache (type, FALSE);
-  if (cache)
-    g_hash_table_remove (cache, session_id);
+  g_queue_push_tail (cache_data->session_tickets, g_bytes_ref (session_data));
+  cache_data->last_used = g_get_monotonic_time ();
 
   G_UNLOCK (session_cache_lock);
 }
 
 GBytes *
-g_tls_backend_gnutls_lookup_session (unsigned int  type,
-                                     GBytes       *session_id)
+g_tls_backend_gnutls_lookup_session_data (GBytes *session_id)
 {
   GTlsBackendGnutlsCacheData *cache_data;
   GBytes *session_data = NULL;
@@ -286,14 +252,17 @@ g_tls_backend_gnutls_lookup_session (unsigned int  type,
 
   G_LOCK (session_cache_lock);
 
-  cache = get_session_cache (type, FALSE);
+  cache = get_session_cache (FALSE);
   if (cache)
     {
       cache_data = g_hash_table_lookup (cache, session_id);
       if (cache_data)
         {
+          /* Note that session tickets should be used only once since TLS 1.3,
+           * so we remove from the queue after retrieval. See RFC 8446 §C.4.
+           */
+          session_data = g_queue_pop_head (cache_data->session_tickets);
           cache_data->last_used = g_get_monotonic_time ();
-          session_data = g_bytes_ref (cache_data->session_data);
         }
     }
 
diff --git a/tls/gnutls/gtlsbackend-gnutls.h b/tls/gnutls/gtlsbackend-gnutls.h
index 7b92bf3..fb33361 100644
--- a/tls/gnutls/gtlsbackend-gnutls.h
+++ b/tls/gnutls/gtlsbackend-gnutls.h
@@ -35,12 +35,8 @@ G_DECLARE_FINAL_TYPE (GTlsBackendGnutls, g_tls_backend_gnutls, G, TLS_BACKEND_GN
 
 void  g_tls_backend_gnutls_register (GIOModule *module);
 
-void    g_tls_backend_gnutls_store_session  (unsigned int             type,
-                                             GBytes                  *session_id,
-                                             GBytes                  *session_data);
-void    g_tls_backend_gnutls_remove_session (unsigned int             type,
-                                             GBytes                  *session_id);
-GBytes *g_tls_backend_gnutls_lookup_session (unsigned int             type,
-                                             GBytes                  *session_id);
+void    g_tls_backend_gnutls_store_session_data  (GBytes *session_id,
+                                                  GBytes *session_data);
+GBytes *g_tls_backend_gnutls_lookup_session_data (GBytes *session_id);
 
 G_END_DECLS
diff --git a/tls/gnutls/gtlsclientconnection-gnutls.c b/tls/gnutls/gtlsclientconnection-gnutls.c
index 5e1b76a..1d2ba78 100644
--- a/tls/gnutls/gtlsclientconnection-gnutls.c
+++ b/tls/gnutls/gtlsclientconnection-gnutls.c
@@ -52,10 +52,15 @@ struct _GTlsClientConnectionGnutls
   GTlsCertificateFlags validation_flags;
   GSocketConnectable *server_identity;
   gboolean use_ssl3;
-  gboolean session_data_override;
 
+  /* session_data is either the session ticket that was used to resume this
+   * connection, or the most recent session ticket received from the server.
+   * Because session ticket reuse is generally undesirable, it should only be
+   * accessed if session_data_override is set.
+   */
   GBytes *session_id;
   GBytes *session_data;
+  gboolean session_data_override;
 
   GPtrArray *accepted_cas;
   gboolean accepted_cas_changed;
@@ -65,7 +70,7 @@ struct _GTlsClientConnectionGnutls
   gnutls_privkey_t pkey;
 };
 
-static void     g_tls_client_connection_gnutls_initable_interface_init (GInitableIface  *iface);
+static void g_tls_client_connection_gnutls_initable_interface_init (GInitableIface  *iface);
 
 static void g_tls_client_connection_gnutls_client_connection_interface_init (GTlsClientConnectionInterface 
*iface);
 static void g_tls_client_connection_gnutls_dtls_client_connection_interface_init 
(GDtlsClientConnectionInterface *iface);
@@ -137,12 +142,17 @@ g_tls_client_connection_gnutls_compute_session_id (GTlsClientConnectionGnutls *g
   if (g_test_initialized ())
     return;
 
-  /* Create a TLS session ID. We base it on the IP address since
+  /* Create a TLS "session ID." We base it on the IP address since
    * different hosts serving the same hostname/service will probably
    * not share the same session cache. We base it on the
    * server-identity because at least some servers will fail (rather
    * than just failing to resume the session) if we don't.
    * (https://bugs.launchpad.net/bugs/823325)
+   *
+   * Note that our session IDs have no relation to TLS protocol
+   * session IDs, e.g. as provided by gnutls_session_get_id2(). Unlike
+   * our session IDs, actual TLS session IDs can no longer be used for
+   * session resumption.
    */
   g_object_get (G_OBJECT (gnutls), "base-io-stream", &base_conn, NULL);
   if (G_IS_SOCKET_CONNECTION (base_conn))
@@ -191,6 +201,34 @@ g_tls_client_connection_gnutls_compute_session_id (GTlsClientConnectionGnutls *g
   g_clear_object (&base_conn);
 }
 
+static int
+handshake_thread_session_ticket_received_cb (gnutls_session_t      session,
+                                             guint                 htype,
+                                             guint                 when,
+                                             guint                 incoming,
+                                             const gnutls_datum_t *msg)
+{
+  GTlsClientConnectionGnutls *gnutls = G_TLS_CLIENT_CONNECTION_GNUTLS (gnutls_session_get_ptr (session));
+  gnutls_datum_t session_datum;
+
+  if (gnutls_session_get_data2 (session, &session_datum) == GNUTLS_E_SUCCESS)
+    {
+      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);
+
+      if (gnutls->session_id)
+        {
+          g_tls_backend_gnutls_store_session_data (gnutls->session_id,
+                                                   gnutls->session_data);
+        }
+    }
+
+  return 0;
+}
+
 static void
 g_tls_client_connection_gnutls_finalize (GObject *object)
 {
@@ -237,6 +275,9 @@ g_tls_client_connection_gnutls_initable_init (GInitable       *initable,
       g_free (normalized_hostname);
     }
 
+  gnutls_handshake_set_hook_function (session, GNUTLS_HANDSHAKE_NEW_SESSION_TICKET,
+                                      GNUTLS_HOOK_POST, handshake_thread_session_ticket_received_cb);
+
   return TRUE;
 }
 
@@ -407,21 +448,6 @@ g_tls_client_connection_gnutls_handshake_thread_retrieve_function (gnutls_sessio
   return 0;
 }
 
-static void
-g_tls_client_connection_gnutls_clear_session_data (GTlsClientConnectionGnutls *gnutls)
-{
-  gnutls->session_data_override = FALSE;
-  g_clear_pointer (&gnutls->session_data, g_bytes_unref);
-  if (gnutls->session_id)
-    g_tls_backend_gnutls_remove_session (GNUTLS_CLIENT, gnutls->session_id);
-}
-
-static void
-g_tls_client_connection_gnutls_failed (GTlsConnectionGnutls *gnutls)
-{
-  g_tls_client_connection_gnutls_clear_session_data (G_TLS_CLIENT_CONNECTION_GNUTLS (gnutls));
-}
-
 static void
 g_tls_client_connection_gnutls_prepare_handshake (GTlsConnectionBase  *tls,
                                                   gchar              **advertised_protocols)
@@ -430,9 +456,9 @@ g_tls_client_connection_gnutls_prepare_handshake (GTlsConnectionBase  *tls,
 
   g_tls_client_connection_gnutls_compute_session_id (gnutls);
 
-  /* Try to get a cached session */
   if (gnutls->session_data_override)
     {
+      g_assert (gnutls->session_data);
       gnutls_session_set_data (g_tls_connection_gnutls_get_session (G_TLS_CONNECTION_GNUTLS (tls)),
                                g_bytes_get_data (gnutls->session_data, NULL),
                                g_bytes_get_size (gnutls->session_data));
@@ -441,14 +467,14 @@ g_tls_client_connection_gnutls_prepare_handshake (GTlsConnectionBase  *tls,
     {
       GBytes *session_data;
 
-      session_data = g_tls_backend_gnutls_lookup_session (GNUTLS_CLIENT, gnutls->session_id);
+      session_data = g_tls_backend_gnutls_lookup_session_data (gnutls->session_id);
       if (session_data)
         {
           gnutls_session_set_data (g_tls_connection_gnutls_get_session (G_TLS_CONNECTION_GNUTLS (tls)),
                                    g_bytes_get_data (session_data, NULL),
                                    g_bytes_get_size (session_data));
           g_clear_pointer (&gnutls->session_data, g_bytes_unref);
-          gnutls->session_data = session_data;
+          gnutls->session_data = g_steal_pointer (&session_data);
         }
     }
 
@@ -462,7 +488,6 @@ g_tls_client_connection_gnutls_complete_handshake (GTlsConnectionBase  *tls,
                                                    GError             **error)
 {
   GTlsClientConnectionGnutls *gnutls = G_TLS_CLIENT_CONNECTION_GNUTLS (tls);
-  int resumed;
 
   G_TLS_CONNECTION_BASE_CLASS (g_tls_client_connection_gnutls_parent_class)->complete_handshake (tls, 
negotiated_protocol, error);
 
@@ -471,29 +496,6 @@ g_tls_client_connection_gnutls_complete_handshake (GTlsConnectionBase  *tls,
    */
   if (gnutls->accepted_cas_changed)
     g_object_notify (G_OBJECT (gnutls), "accepted-cas");
-
-  resumed = gnutls_session_is_resumed (g_tls_connection_gnutls_get_session (G_TLS_CONNECTION_GNUTLS (tls)));
-  if (!resumed)
-    {
-      gnutls_datum_t session_datum;
-
-      g_tls_client_connection_gnutls_clear_session_data (G_TLS_CLIENT_CONNECTION_GNUTLS (tls));
-
-      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);
-
-          if (gnutls->session_id)
-            g_tls_backend_gnutls_store_session (GNUTLS_CLIENT,
-                                                gnutls->session_id,
-                                                gnutls->session_data);
-        }
-    }
 }
 
 static void
@@ -503,16 +505,24 @@ g_tls_client_connection_gnutls_copy_session_state (GTlsClientConnection *conn,
   GTlsClientConnectionGnutls *gnutls = G_TLS_CLIENT_CONNECTION_GNUTLS (conn);
   GTlsClientConnectionGnutls *gnutls_source = G_TLS_CLIENT_CONNECTION_GNUTLS (source);
 
-  if (gnutls_source->session_data)
+  /* Precondition: source has handshaked, conn has not. */
+  g_return_if_fail (!gnutls->session_id);
+  g_return_if_fail (gnutls_source->session_id);
+
+  /* Prefer to use a new session ticket, if possible. */
+  gnutls->session_data = g_tls_backend_gnutls_lookup_session_data (gnutls_source->session_id);
+
+  if (!gnutls->session_data && gnutls_source->session_data)
     {
-      gnutls->session_data_override = TRUE;
+      /* 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.
+       */
       gnutls->session_data = g_bytes_ref (gnutls_source->session_data);
-
-      if (gnutls->session_id)
-        g_tls_backend_gnutls_store_session (GNUTLS_CLIENT,
-                                            gnutls->session_id,
-                                            gnutls->session_data);
     }
+
+  gnutls->session_data_override = !!gnutls->session_data;
 }
 
 static void
@@ -520,7 +530,6 @@ g_tls_client_connection_gnutls_class_init (GTlsClientConnectionGnutlsClass *klas
 {
   GObjectClass *gobject_class = G_OBJECT_CLASS (klass);
   GTlsConnectionBaseClass *base_class = G_TLS_CONNECTION_BASE_CLASS (klass);
-  GTlsConnectionGnutlsClass *gnutls_class = G_TLS_CONNECTION_GNUTLS_CLASS (klass);
 
   gobject_class->get_property = g_tls_client_connection_gnutls_get_property;
   gobject_class->set_property = g_tls_client_connection_gnutls_set_property;
@@ -529,8 +538,6 @@ g_tls_client_connection_gnutls_class_init (GTlsClientConnectionGnutlsClass *klas
   base_class->prepare_handshake  = g_tls_client_connection_gnutls_prepare_handshake;
   base_class->complete_handshake = g_tls_client_connection_gnutls_complete_handshake;
 
-  gnutls_class->failed             = g_tls_client_connection_gnutls_failed;
-
   g_object_class_override_property (gobject_class, PROP_VALIDATION_FLAGS, "validation-flags");
   g_object_class_override_property (gobject_class, PROP_SERVER_IDENTITY, "server-identity");
   g_object_class_override_property (gobject_class, PROP_USE_SSL3, "use-ssl3");
diff --git a/tls/gnutls/gtlsconnection-gnutls.c b/tls/gnutls/gtlsconnection-gnutls.c
index bc6ec72..8dd73d0 100644
--- a/tls/gnutls/gtlsconnection-gnutls.c
+++ b/tls/gnutls/gtlsconnection-gnutls.c
@@ -471,11 +471,7 @@ end_gnutls_io (GTlsConnectionGnutls  *gnutls,
 
 #define END_GNUTLS_IO(gnutls, direction, ret, status, errmsg, err)      \
     status = end_gnutls_io (gnutls, direction, ret, err, errmsg);       \
-  } while (status == G_TLS_CONNECTION_BASE_TRY_AGAIN);                  \
-                                                                        \
-  if (status == G_TLS_CONNECTION_BASE_ERROR &&                          \
-      G_TLS_CONNECTION_GNUTLS_GET_CLASS (gnutls)-> failed)              \
-    G_TLS_CONNECTION_GNUTLS_GET_CLASS (gnutls)->failed (gnutls);
+  } while (status == G_TLS_CONNECTION_BASE_TRY_AGAIN);
 
 static void
 set_gnutls_error (GTlsConnectionGnutls *gnutls,
diff --git a/tls/gnutls/gtlsconnection-gnutls.h b/tls/gnutls/gtlsconnection-gnutls.h
index db3b726..55ae5ee 100644
--- a/tls/gnutls/gtlsconnection-gnutls.h
+++ b/tls/gnutls/gtlsconnection-gnutls.h
@@ -39,8 +39,6 @@ G_DECLARE_DERIVABLE_TYPE (GTlsConnectionGnutls, g_tls_connection_gnutls, G, TLS_
 struct _GTlsConnectionGnutlsClass
 {
   GTlsConnectionBaseClass parent_class;
-
-  void (*failed) (GTlsConnectionGnutls *tls);
 };
 
 gnutls_certificate_credentials_t g_tls_connection_gnutls_get_credentials (GTlsConnectionGnutls *connection);