ws: Drop TLS support

That has been deprecated since 2019 when we introduced cockpit-tls in
commit ace21c8. It's now getting in the way with the upcoming
support for post-quantum certificates, which we'll only do in c-tls.

Now the only thing that the `--no-tls` option does is to disable the TLS
redirection.

Part of https://issues.redhat.com/browse/COCKPIT-1330

Author: martinpitt

doc/man/pages/cockpit-ws.8.adoc

@@ -48,7 +48,7 @@ seconds if nobody logs in, or after the last user is disconnected.
   *ListenStream* directive in the *cockpit.socket* file in the usual
   *systemd* manner.
 *--no-tls*::
-  Don't use TLS.
+  Disable http to https redirection.
 *--for-tls-proxy*::
   Tell *cockpit-ws* that it is running behind a local reverse proxy
   that does the TLS termination. Then Cockpit puts https:// URLs into

src/common/cockpitwebserver.c

@@ -40,16 +40,12 @@
 
 #include "cockpitwebrequest-private.h"
 
-/* Used during testing */
-gboolean cockpit_webserver_want_certificate = FALSE;
-
 guint cockpit_webserver_request_timeout = 30;
 const gsize cockpit_webserver_request_maximum = 8192;
 
 struct _CockpitWebServer {
   GObject parent_instance;
 
-  GTlsCertificate *certificate;
   GString *ssl_exception_prefix;
   GString *url_root;
   gint request_timeout;
@@ -67,7 +63,6 @@ struct _CockpitWebServer {
 enum
 {
   PROP_0,
-  PROP_CERTIFICATE,
   PROP_SSL_EXCEPTION_PREFIX,
   PROP_FLAGS,
   PROP_URL_ROOT,
@@ -132,7 +127,6 @@ cockpit_web_server_finalize (GObject *object)
 {
   CockpitWebServer *server = COCKPIT_WEB_SERVER (object);
 
-  g_clear_object (&server->certificate);
   g_hash_table_destroy (server->requests);
   if (server->main_context)
     g_main_context_unref (server->main_context);
@@ -155,10 +149,6 @@ cockpit_web_server_get_property (GObject *object,
 
   switch (prop_id)
     {
-    case PROP_CERTIFICATE:
-      g_value_set_object (value, server->certificate);
-      break;
-
     case PROP_SSL_EXCEPTION_PREFIX:
       g_value_set_string (value, server->ssl_exception_prefix->str);
       break;
@@ -191,10 +181,6 @@ cockpit_web_server_set_property (GObject *object,
 
   switch (prop_id)
     {
-    case PROP_CERTIFICATE:
-      server->certificate = g_value_dup_object (value);
-      break;
-
     case PROP_SSL_EXCEPTION_PREFIX:
       g_string_assign (server->ssl_exception_prefix, g_value_get_string (value));
       break;
@@ -345,15 +331,6 @@ cockpit_web_server_class_init (CockpitWebServerClass *klass)
   gobject_class->set_property = cockpit_web_server_set_property;
   gobject_class->get_property = cockpit_web_server_get_property;
 
-  g_object_class_install_property (gobject_class,
-                                   PROP_CERTIFICATE,
-                                   g_param_spec_object ("certificate", NULL, NULL,
-                                                        G_TYPE_TLS_CERTIFICATE,
-                                                        G_PARAM_READABLE |
-                                                        G_PARAM_WRITABLE |
-                                                        G_PARAM_CONSTRUCT_ONLY |
-                                                        G_PARAM_STATIC_STRINGS));
-
   g_object_class_install_property (gobject_class, PROP_SSL_EXCEPTION_PREFIX,
                                    g_param_spec_string ("ssl-exception-prefix", NULL, NULL, "",
                                                         G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));
@@ -396,11 +373,9 @@ cockpit_web_server_class_init (CockpitWebServerClass *klass)
 }
 
 CockpitWebServer *
-cockpit_web_server_new (GTlsCertificate *certificate,
-                        CockpitWebServerFlags flags)
+cockpit_web_server_new (CockpitWebServerFlags flags)
 {
   return g_object_new (COCKPIT_TYPE_WEB_SERVER,
-                       "certificate", certificate,
                        "flags", flags,
                        NULL);
 }
@@ -957,13 +932,6 @@ static gboolean
 should_suppress_request_error (GError *error,
                                gsize received)
 {
-  if (g_error_matches (error, G_TLS_ERROR, G_TLS_ERROR_EOF) ||
-      g_error_matches (error, G_TLS_ERROR, G_TLS_ERROR_NOT_TLS))
-    {
-      g_debug ("request error: %s", error->message);
-      return TRUE;
-    }
-
   /* If no bytes received, then don't worry about ECONNRESET and friends */
   if (received > 0)
     return FALSE;
@@ -1085,17 +1053,6 @@ cockpit_web_request_start_input (CockpitWebRequest *self)
   g_source_attach (self->source, self->web_server->main_context);
 }
 
-static gboolean
-cockpit_web_request_on_accept_certificate (GTlsConnection *conn,
-                                           GTlsCertificate *peer_cert,
-                                           GTlsCertificateFlags errors,
-                                           gpointer user_data)
-{
-  /* Only used during testing */
-  g_assert (cockpit_webserver_want_certificate == TRUE);
-  return TRUE;
-}
-
 static gboolean
 cockpit_web_request_on_socket_input (GSocket *socket,
                                      GIOCondition condition,
@@ -1106,7 +1063,6 @@ cockpit_web_request_on_socket_input (GSocket *socket,
   GInputVector vector[1] = { { &first_byte, 1 } };
   gint flags = G_SOCKET_MSG_PEEK;
   GError *error = NULL;
-  GIOStream *tls_stream;
   gssize num_read;
   g_auto(CockpitControlMessages) ccm = COCKPIT_CONTROL_MESSAGES_INIT;
 
@@ -1153,47 +1109,19 @@ cockpit_web_request_on_socket_input (GSocket *socket,
 
   /*
    * TLS streams are guaranteed to start with octet 22.. this way we can distinguish them
-   * from regular HTTP requests
+   * from regular HTTP requests. cockpit-ws no longer handles TLS.
    */
   if (first_byte == 22 || first_byte == 0x80)
     {
-      if (self->web_server->certificate == NULL)
-        {
-          g_warning ("Received unexpected TLS connection and no certificate was configured");
-          cockpit_web_request_finish (self);
-          return FALSE;
-        }
-
-      tls_stream = g_tls_server_connection_new (self->io,
-                                                self->web_server->certificate,
-                                                &error);
-      if (tls_stream == NULL)
-        {
-          g_warning ("couldn't create new TLS stream: %s", error->message);
-          cockpit_web_request_finish (self);
-          g_error_free (error);
-          return FALSE;
-        }
-
-      if (cockpit_webserver_want_certificate)
-        {
-          g_object_set (tls_stream, "authentication-mode", G_TLS_AUTHENTICATION_REQUESTED, NULL);
-          g_signal_connect (tls_stream, "accept-certificate", G_CALLBACK (cockpit_web_request_on_accept_certificate), NULL);
-        }
-
-      g_object_unref (self->io);
-      self->io = G_IO_STREAM (tls_stream);
-    }
-  else
-    {
-      if (self->web_server->certificate || self->web_server->flags & COCKPIT_WEB_SERVER_REDIRECT_TLS)
-        {
-          /* non-TLS stream; defer redirection check until after header parsing */
-          if (cockpit_web_server_get_flags (self->web_server) & COCKPIT_WEB_SERVER_REDIRECT_TLS)
-            self->check_tls_redirect = TRUE;
-        }
+      g_warning ("Received unexpected TLS connection; use cockpit-tls for TLS termination");
+      cockpit_web_request_finish (self);
+      return FALSE;
     }
 
+  /* Defer redirection check until after header parsing */
+  if (cockpit_web_server_get_flags (self->web_server) & COCKPIT_WEB_SERVER_REDIRECT_TLS)
+    self->check_tls_redirect = TRUE;
+
   cockpit_web_request_start_input (self);
 
   /* No longer run *this* source */
@@ -1322,9 +1250,6 @@ cockpit_web_request_get_host (CockpitWebRequest *self)
 const gchar *
 cockpit_web_request_get_protocol (CockpitWebRequest *self)
 {
-  if (G_IS_TLS_CONNECTION (self->io))
-    return "https";
-
   if (self->web_server && self->web_server->flags & COCKPIT_WEB_SERVER_FOR_TLS_PROXY)
     return "https";
 
@@ -1369,17 +1294,11 @@ cockpit_web_request_get_remote_address (CockpitWebRequest *self)
         return g_strdup (tmp);
     }
 
-  g_autoptr(GIOStream) base = NULL;
-  if (G_IS_TLS_CONNECTION (self->io))
-    g_object_get (self->io, "base-io-stream", &base, NULL);
-  else
-    base = g_object_ref (self->io);
-
   /* This is definitely a socket */
-  g_return_val_if_fail (G_IS_SOCKET_CONNECTION (base), NULL);
+  g_return_val_if_fail (G_IS_SOCKET_CONNECTION (self->io), NULL);
 
   /* ...but it might be a unix socket.  NB: GInetSocketAddress includes IPv6. */
-  g_autoptr(GSocketAddress) remote = g_socket_connection_get_remote_address (G_SOCKET_CONNECTION (base), NULL);
+  g_autoptr(GSocketAddress) remote = g_socket_connection_get_remote_address (G_SOCKET_CONNECTION (self->io), NULL);
   if (remote && G_IS_INET_SOCKET_ADDRESS (remote))
     return g_inet_address_to_string (g_inet_socket_address_get_address (G_INET_SOCKET_ADDRESS (remote)));
 

src/common/cockpitwebserver.h

@@ -90,8 +90,7 @@ typedef enum {
 } CockpitWebServerFlags;
 
 
-CockpitWebServer * cockpit_web_server_new           (GTlsCertificate *certificate,
-                                                     CockpitWebServerFlags flags);
+CockpitWebServer * cockpit_web_server_new           (CockpitWebServerFlags flags);
 
 void               cockpit_web_server_start         (CockpitWebServer *self);
 

src/common/test-webserver.c

@@ -75,26 +75,16 @@ static void
 fixture_setup (Fixture *fixture,
                const TestCase *test_case)
 {
-  GTlsCertificate *cert = NULL;
-  GError *error = NULL;
   GInetAddress *inet;
   gchar *str = NULL;
   gint port;
+  GError *error = NULL;
 
   inet = cockpit_test_find_non_loopback_address ();
   /* this can fail in environments with only localhost */
   if (inet != NULL)
     str = g_inet_address_to_string (inet);
 
-  if (test_case->use_cert)
-    {
-      cert = g_tls_certificate_new_from_file (SRCDIR "/src/ws/mock-combined.crt", &error);
-      g_assert_no_error (error);
-
-      /* don't require system SSL cert database in build environments */
-      cockpit_expect_possible_log ("GLib-Net", G_LOG_LEVEL_WARNING, "couldn't load TLS file database: * No such file or directory");
-    }
-
   gchar *address;
   if (test_case->local_only)
     address = "127.0.0.1";
@@ -103,8 +93,7 @@ fixture_setup (Fixture *fixture,
   else
     address = NULL;
 
-  fixture->web_server = cockpit_web_server_new (cert, test_case->server_flags);
-  g_clear_object (&cert);
+  fixture->web_server = cockpit_web_server_new (test_case->server_flags);
 
   if (test_case && test_case->forwarded_for_header)
     cockpit_web_server_set_forwarded_for_header (fixture->web_server, test_case->forwarded_for_header);
@@ -470,14 +459,6 @@ perform_http_request (const gchar *hostport,
   return perform_request (hostport, request, length, FALSE);
 }
 
-static gchar *
-perform_https_request (const gchar *hostport,
-                       const gchar *request,
-                       gsize *length)
-{
-  return perform_request (hostport, request, length, TRUE);
-}
-
 static gboolean
 on_shell_index_html (CockpitWebServer *server,
                      CockpitWebRequest *request,
@@ -534,81 +515,7 @@ test_webserver_not_found (Fixture *fixture,
   g_free (resp);
 }
 
-static void
-test_webserver_tls (Fixture *fixture,
-                    const TestCase *test_case)
-{
-  gchar *resp;
-  gsize length;
-
-  g_signal_connect (fixture->web_server, "handle-resource", G_CALLBACK (on_shell_index_html), NULL);
-  resp = perform_https_request (fixture->localport, "GET /shell/index.html HTTP/1.0\r\nHost:test\r\n\r\n", &length);
-  g_assert (resp != NULL);
-  g_assert_cmpuint (length, >, 0);
 
-  cockpit_assert_strmatch (resp, "HTTP/* 200 *\r\nContent-Length: *\r\n\r\n<!DOCTYPE html>*");
-  g_free (resp);
-}
-
-static gboolean
-on_big_header (CockpitWebServer *server,
-               CockpitWebRequest *request,
-               const gchar *path,
-               GHashTable *headers,
-               CockpitWebResponse *response,
-               gpointer user_data)
-{
-  GBytes *bytes;
-  const gchar *big_header;
-
-  big_header = g_hash_table_lookup (headers, "BigHeader");
-  g_assert (big_header);
-  g_assert_cmpint (strlen (big_header), ==, 7000);
-  g_assert_cmpint (big_header[strlen (big_header) - 1], ==, '1');
-
-  bytes = g_bytes_new_static ("OK", 2);
-  cockpit_web_response_content (response, NULL, bytes, NULL);
-  g_bytes_unref (bytes);
-  return TRUE;
-}
-
-static void
-test_webserver_tls_big_header (Fixture *fixture,
-                               const TestCase *test_case)
-{
-  g_autofree gchar *req = NULL;
-  g_autofree gchar *resp = NULL;
-  gsize length;
-
-  /* max request size is 8KiB (2 * cockpit_webserver_request_maximum), stay slightly below that */
-  req = g_strdup_printf ("GET /test HTTP/1.0\r\nHost:test\r\nBigHeader: %07000i\r\n\r\n", 1);
-
-  g_signal_connect (fixture->web_server, "handle-resource", G_CALLBACK (on_big_header), NULL);
-  resp = perform_https_request (fixture->localport, req, &length);
-  g_assert (resp != NULL);
-  g_assert_cmpuint (length, >, 0);
-
-  cockpit_assert_strmatch (resp, "HTTP/* 200 *\r\nContent-Length: 2\r\n*\r\n\r\nOK");
-}
-
-static void
-test_webserver_tls_request_too_large (Fixture *fixture,
-                                      const TestCase *test_case)
-{
-  g_autofree gchar *req = NULL;
-  g_autofree gchar *resp = NULL;
-  gsize length;
-
-  /* request bigger than 16 KiB should be rejected */
-  /* FIXME: This really should be 8 KiB, but due to pipelining we reserve twice
-   * that amount in the buffer */
-  cockpit_expect_log ("cockpit-protocol", G_LOG_LEVEL_MESSAGE, "received HTTP request that was too large");
-  req = g_strdup_printf ("GET /test HTTP/1.0\r\nHost:test\r\nBigHeader: %016500i\r\n\r\n", 1);
-  resp = perform_https_request (fixture->localport, req, &length);
-  g_assert (resp != NULL);
-  g_assert_cmpuint (length, ==, 0);
-  g_assert_cmpstr (resp, ==, "");
-}
 
 static void
 test_webserver_redirect_notls (Fixture *fixture,
@@ -1008,7 +915,7 @@ test_bad_address (Fixture *fixture,
 {
   gint port;
 
-  g_autoptr(CockpitWebServer) server = cockpit_web_server_new (NULL, COCKPIT_WEB_SERVER_NONE);
+  g_autoptr(CockpitWebServer) server = cockpit_web_server_new (COCKPIT_WEB_SERVER_NONE);
   g_autoptr(GError) error = NULL;
   port = cockpit_web_server_add_inet_listener (server, "bad", 0, &error);
   g_assert (port == 0);
@@ -1072,20 +979,13 @@ main (int argc,
   cockpit_test_add ("/web-server/host-header", test_webserver_host_header);
   cockpit_test_add ("/web-server/not-found", test_webserver_not_found);
 
-  cockpit_test_add ("/web-server/tls", test_webserver_tls,
-                    .use_cert=TRUE, .expected_protocol="https");
-  cockpit_test_add ("/web-server/tls-big-header", test_webserver_tls_big_header,
-                    .use_cert=TRUE, .expected_protocol="https");
-  cockpit_test_add ("/web-server/tls-request-too-large", test_webserver_tls_request_too_large,
-                    .use_cert=TRUE, .expected_protocol="https");
-
-  cockpit_test_add ("/web-server/redirect-notls", test_webserver_redirect_notls, .use_cert=TRUE,
+  cockpit_test_add ("/web-server/redirect-notls", test_webserver_redirect_notls,
                     .server_flags=COCKPIT_WEB_SERVER_REDIRECT_TLS);
-  cockpit_test_add ("/web-server/no-redirect-localhost", test_webserver_noredirect_localhost, .use_cert=TRUE,
+  cockpit_test_add ("/web-server/no-redirect-localhost", test_webserver_noredirect_localhost,
                     .server_flags=COCKPIT_WEB_SERVER_REDIRECT_TLS);
-  cockpit_test_add ("/web-server/no-redirect-exception", test_webserver_noredirect_exception, .use_cert=TRUE,
+  cockpit_test_add ("/web-server/no-redirect-exception", test_webserver_noredirect_exception,
                     .server_flags=COCKPIT_WEB_SERVER_REDIRECT_TLS);
-  cockpit_test_add ("/web-server/no-redirect-override", test_webserver_noredirect_override, .use_cert=TRUE,
+  cockpit_test_add ("/web-server/no-redirect-override", test_webserver_noredirect_override,
                     .server_flags=COCKPIT_WEB_SERVER_NONE);
 
   cockpit_test_add ("/web-server/handle-resource", test_handle_resource);