[glib-networking/mcatanzaro/gnutls3.6] Rework GnuTLS priority string generation...

[Michael Catanzaro <mcatanzaro src gnome org>]

commit 01efebf6a10d2f96cb1f04b7fa80089d64c299e6
Author: Michael Catanzaro <mcatanzaro gnome org>
Date:   Sat Oct 26 17:02:56 2019 -0500

    Rework GnuTLS priority string generation...
    
    ...and drop support for unsafe renegotiation and protocol version
    fallback, both of which are already deprecated and haven't been needed
    for several years.
    
    In particular, this allows us to stop hardcoding the NORMAL priority and
    append to the default priority instead, which makes no difference on
    most distros but is important for Fedora and RHEL, where we have to use
    the Fedora/RHEL-specific @SYSTEM@ priority instead of NORMAL. This means
    we can stop patching glib-networking downstream to change NORMAL to
    @SYSTEM@.
    
    OpenSSL should like this too, since it never supported either of these
    properties, and since this allows removing the protocol version fallback
    test that it never passed.

 tls/base/gtlsconnection-base.c     |   3 +-
 tls/gnutls/gtlsconnection-gnutls.c | 119 +++++++++++--------------------------
 tls/tests/connection.c             |  62 -------------------
 3 files changed, 35 insertions(+), 149 deletions(-)
---
diff --git a/tls/base/gtlsconnection-base.c b/tls/base/gtlsconnection-base.c
index 6b6b6f2..aa83d8f 100644
--- a/tls/base/gtlsconnection-base.c
+++ b/tls/base/gtlsconnection-base.c
@@ -1376,8 +1376,7 @@ handshake_thread (GTask        *task,
     {
       GTlsConnectionBaseStatus status;
 
-      if (priv->rehandshake_mode != G_TLS_REHANDSHAKE_UNSAFELY &&
-          tls_class->handshake_thread_safe_renegotiation_status (tls) != 
G_TLS_SAFE_RENEGOTIATION_SUPPORTED_BY_PEER)
+      if (tls_class->handshake_thread_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"));
diff --git a/tls/gnutls/gtlsconnection-gnutls.c b/tls/gnutls/gtlsconnection-gnutls.c
index 86c4839..b06c99e 100644
--- a/tls/gnutls/gtlsconnection-gnutls.c
+++ b/tls/gnutls/gtlsconnection-gnutls.c
@@ -65,10 +65,10 @@ static int     g_tls_connection_gnutls_pull_timeout_func (gnutls_transport_ptr_t
 
 static void g_tls_connection_gnutls_initable_iface_init (GInitableIface *iface);
 
-static void g_tls_connection_gnutls_init_priorities (void);
-
 static int verify_certificate_cb (gnutls_session_t session);
 
+static gnutls_priority_t priority;
+
 typedef struct
 {
   gnutls_certificate_credentials_t creds;
@@ -81,7 +81,6 @@ G_DEFINE_ABSTRACT_TYPE_WITH_CODE (GTlsConnectionGnutls, g_tls_connection_gnutls,
                                   G_ADD_PRIVATE (GTlsConnectionGnutls);
                                   G_IMPLEMENT_INTERFACE (G_TYPE_INITABLE,
                                                          g_tls_connection_gnutls_initable_iface_init);
-                                  g_tls_connection_gnutls_init_priorities ();
                                   );
 
 static gint unique_interaction_id = 0;
@@ -100,93 +99,17 @@ g_tls_connection_gnutls_init (GTlsConnectionGnutls *gnutls)
   priv->cancellable = g_cancellable_new ();
 }
 
-/* First field is "fallback", second is "allow unsafe rehandshaking" */
-static gnutls_priority_t priorities[2][2];
-
-/* TODO: Get rid of this in favor of gnutls_set_default_priority_append()
- * when upgrading to GnuTLS 3.6.3.
- */
-#define DEFAULT_BASE_PRIORITY "NORMAL:%COMPAT:-VERS-TLS1.1:-VERS-TLS1.0"
-
-static void
-g_tls_connection_gnutls_init_priorities (void)
-{
-  const gchar *base_priority;
-  gchar *fallback_priority, *unsafe_rehandshake_priority, *fallback_unsafe_rehandshake_priority;
-  const guint *protos;
-  int ret, i, nprotos, fallback_proto;
-
-  base_priority = g_getenv ("G_TLS_GNUTLS_PRIORITY");
-  if (!base_priority)
-    base_priority = DEFAULT_BASE_PRIORITY;
-  ret = gnutls_priority_init (&priorities[FALSE][FALSE], base_priority, NULL);
-  if (ret == GNUTLS_E_INVALID_REQUEST)
-    {
-      g_warning ("G_TLS_GNUTLS_PRIORITY is invalid; ignoring!");
-      base_priority = DEFAULT_BASE_PRIORITY;
-      ret = gnutls_priority_init (&priorities[FALSE][FALSE], base_priority, NULL);
-      g_warn_if_fail (ret == 0);
-    }
-
-  unsafe_rehandshake_priority = g_strdup_printf ("%s:%%UNSAFE_RENEGOTIATION", base_priority);
-  ret = gnutls_priority_init (&priorities[FALSE][TRUE], unsafe_rehandshake_priority, NULL);
-  g_warn_if_fail (ret == 0);
-  g_free (unsafe_rehandshake_priority);
-
-  /* Figure out the lowest SSl/TLS version supported by base_priority */
-  nprotos = gnutls_priority_protocol_list (priorities[FALSE][FALSE], &protos);
-  fallback_proto = G_MAXUINT;
-  for (i = 0; i < nprotos; i++)
-    {
-      if (protos[i] < fallback_proto)
-        fallback_proto = protos[i];
-    }
-  if (fallback_proto == G_MAXUINT)
-    {
-      g_warning ("All GNUTLS protocol versions disabled?");
-      fallback_priority = g_strdup (base_priority);
-    }
-  else
-    {
-      /* %COMPAT is intentionally duplicated here, to ensure it gets added for
-       * the fallback even if the default priority has been changed. */
-      fallback_priority = g_strdup_printf ("%s:%%COMPAT:!VERS-TLS-ALL:+VERS-%s:%%FALLBACK_SCSV",
-                                           DEFAULT_BASE_PRIORITY,
-                                           gnutls_protocol_get_name (fallback_proto));
-    }
-  fallback_unsafe_rehandshake_priority = g_strdup_printf ("%s:%%UNSAFE_RENEGOTIATION",
-                                                          fallback_priority);
-
-  ret = gnutls_priority_init (&priorities[TRUE][FALSE], fallback_priority, NULL);
-  g_warn_if_fail (ret == 0);
-  ret = gnutls_priority_init (&priorities[TRUE][TRUE], fallback_unsafe_rehandshake_priority, NULL);
-  g_warn_if_fail (ret == 0);
-  g_free (fallback_priority);
-  g_free (fallback_unsafe_rehandshake_priority);
-}
-
 static void
 g_tls_connection_gnutls_set_handshake_priority (GTlsConnectionGnutls *gnutls)
 {
   GTlsConnectionGnutlsPrivate *priv = g_tls_connection_gnutls_get_instance_private (gnutls);
-  gboolean fallback, unsafe_rehandshake;
+  int ret;
 
-  if (G_IS_TLS_CLIENT_CONNECTION (gnutls))
-    {
-#if defined(__GNUC__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#endif
-      fallback = g_tls_client_connection_get_use_ssl3 (G_TLS_CLIENT_CONNECTION (gnutls));
-#if defined(__GNUC__)
-#pragma GCC diagnostic pop
-#endif
-    }
-  else
-    fallback = FALSE;
-  unsafe_rehandshake = g_tls_connection_get_rehandshake_mode (G_TLS_CONNECTION (gnutls)) == 
G_TLS_REHANDSHAKE_UNSAFELY;
-  gnutls_priority_set (priv->session,
-                       priorities[fallback][unsafe_rehandshake]);
+  g_assert (priority);
+
+  ret = gnutls_priority_set (priv->session, priority);
+  if (ret != GNUTLS_E_SUCCESS)
+    g_warning ("Failed to set GnuTLS session priority: %s", gnutls_strerror (ret));
 }
 
 static gboolean
@@ -1186,6 +1109,30 @@ g_tls_connection_gnutls_close (GTlsConnectionBase  *tls,
   return status;
 }
 
+static void
+initialize_gnutls_priority (void)
+{
+  const gchar *priority_override;
+  const gchar *error_pos = NULL;
+  int ret;
+
+  g_assert (!priority);
+
+  priority_override = g_getenv ("G_TLS_GNUTLS_PRIORITY");
+  if (priority_override)
+    {
+      ret = gnutls_priority_init2 (&priority, priority_override, &error_pos, 0);
+      if (ret != GNUTLS_E_SUCCESS)
+        g_warning ("Failed to set GnuTLS session priority with beginning at %s: %s", error_pos, 
gnutls_strerror (ret));
+      return;
+    }
+
+  /* TODO: Generally we should expect GnuTLS to define appropriate security levels. */
+  ret = gnutls_priority_init2 (&priority, "%COMPAT:-VERS-TLS1.1:-VERS-TLS1.0", &error_pos, 
GNUTLS_PRIORITY_INIT_DEF_APPEND);
+  if (ret != GNUTLS_E_SUCCESS)
+    g_warning ("Failed to set GnuTLS session priority with error beginning at %s: %s", error_pos, 
gnutls_strerror (ret));
+}
+
 static void
 g_tls_connection_gnutls_class_init (GTlsConnectionGnutlsClass *klass)
 {
@@ -1206,6 +1153,8 @@ 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;
+
+  initialize_gnutls_priority ();
 }
 
 static void
diff --git a/tls/tests/connection.c b/tls/tests/connection.c
index 719845b..9677cee 100644
--- a/tls/tests/connection.c
+++ b/tls/tests/connection.c
@@ -2127,66 +2127,6 @@ test_async_implicit_handshake (TestConnection *test, gconstpointer   data)
   test->client_connection = NULL;
 }
 
-static void
-quit_on_handshake_complete (GObject      *object,
-                            GAsyncResult *result,
-                            gpointer      user_data)
-{
-  TestConnection *test = user_data;
-  GError *error = NULL;
-
-  g_tls_connection_handshake_finish (G_TLS_CONNECTION (object), result, &error);
-  g_assert_error (error, G_TLS_ERROR, G_TLS_ERROR_NOT_TLS);
-  g_error_free (error);
-
-  g_main_loop_quit (test->loop);
-  return;
-}
-
-static void
-test_fallback (TestConnection *test,
-               gconstpointer   data)
-{
-  GIOStream *connection;
-  GTlsConnection *tlsconn;
-  GError *error = NULL;
-
-#ifdef BACKEND_IS_OPENSSL
-  g_test_skip ("this needs more research on openssl");
-  return;
-#endif
-
-  connection = start_async_server_and_connect_to_it (test, G_TLS_AUTHENTICATION_NONE);
-  test->client_connection = g_tls_client_connection_new (connection, NULL, &error);
-  g_assert_no_error (error);
-  tlsconn = G_TLS_CONNECTION (test->client_connection);
-  g_object_unref (connection);
-
-  g_tls_client_connection_set_validation_flags (G_TLS_CLIENT_CONNECTION (test->client_connection),
-                                                0);
-#if defined(__GNUC__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-#endif
-  g_tls_client_connection_set_use_ssl3 (G_TLS_CLIENT_CONNECTION (test->client_connection),
-                                        TRUE);
-#if defined(__GNUC__)
-#pragma GCC diagnostic pop
-#endif
-
-  g_tls_connection_handshake_async (tlsconn, G_PRIORITY_DEFAULT, NULL,
-                                    quit_on_handshake_complete, test);
-  g_main_loop_run (test->loop);
-  wait_until_server_finished (test);
-
-  /* The server should detect a protocol downgrade attack and terminate the connection.
-   */
-  g_assert_error (test->server_error, G_TLS_ERROR, G_TLS_ERROR_INAPPROPRIATE_FALLBACK);
-
-  g_io_stream_close (test->client_connection, NULL, &error);
-  g_assert_no_error (error);
-}
-
 static void
 handshake_completed (GObject      *object,
                      GAsyncResult *result,
@@ -2615,8 +2555,6 @@ main (int   argc,
               setup_connection, test_async_implicit_handshake, teardown_connection);
   g_test_add ("/tls/" BACKEND "/connection/output-stream-close", TestConnection, NULL,
               setup_connection, test_output_stream_close, teardown_connection);
-  g_test_add ("/tls/" BACKEND "/connection/fallback", TestConnection, NULL,
-              setup_connection, test_fallback, teardown_connection);
   g_test_add ("/tls/" BACKEND "/connection/garbage-database", TestConnection, NULL,
               setup_connection, test_garbage_database, teardown_connection);
   g_test_add ("/tls/" BACKEND "/connection/readwrite-after-connection-destroyed", TestConnection, NULL,