Revert "ws: report cockpit-ssh stderr to cockpit-client"

With the improved error handling in the previous commit we don't need this special case any more. This reverts commit 05978ba and also the follow-up "ws: Don't capture stderr for Cockpit Client when debugging" commit 9ad3f39.
cockpit-project · Mar 21, 2024 · 8454d65 · 8454d65
1 parent 6f24455
commit 8454d65
Show file tree

Hide file tree

Showing 2 changed files with 5 additions and 44 deletions.
diff --git a/pkg/static/login.js b/pkg/static/login.js
@@ -126,23 +126,6 @@ import "./login.scss";
         hideToggle(arguments, true);
     }
 
-    function show_captured_stderr(msg) {
-        if (window.console)
-            console.warn("stderr:", msg);
-
-        hide("#login-wait-validating");
-
-        hide("#login", "#login-details");
-        show("#login-fatal");
-
-        id("login-again").onclick = () => { hide('#login-fatal'); show_login() };
-        show("#login-again");
-
-        const el = id("login-fatal-message");
-        el.textContent = "";
-        el.appendChild(document.createTextNode(msg));
-    }
-
     function fatal(msg) {
         if (window.console)
             console.warn("fatal:", msg);
@@ -908,9 +891,7 @@ import "./login.scss";
                 } else {
                     if (window.console)
                         console.log(xhr.statusText);
-                    if (xhr.statusText.startsWith("captured-stderr:")) {
-                        show_captured_stderr(decodeURIComponent(xhr.statusText.replace(/^captured-stderr:/, '')));
-                    } else if (xhr.statusText.indexOf("authentication-not-supported") > -1) {
+                    if (xhr.statusText.indexOf("authentication-not-supported") > -1) {
                         const user = trim(id("login-user-input").value);
                         fatal(format(_("The server refused to authenticate '$0' using password authentication, and no other supported authentication methods are available."), user));
                     } else if (xhr.statusText.indexOf("terminated") > -1) {

diff --git a/src/ws/cockpitauth.c b/src/ws/cockpitauth.c
@@ -464,8 +464,7 @@ session_child_setup (gpointer data)
 
 static CockpitPipe *
 session_start_process (const gchar **argv,
-                       const gchar **env,
-                       gboolean capture_stderr)
+                       const gchar **env)
 {
   GError *error = NULL;
   ChildData child;
@@ -484,12 +483,11 @@ session_start_process (const gchar **argv,
       return NULL;
     }
 
-  int stderr_fd = -1;
   child.io = fds[0];
   ret = g_spawn_async_with_pipes (NULL, (gchar **)argv, (gchar **)env,
                                   G_SPAWN_DO_NOT_REAP_CHILD | G_SPAWN_LEAVE_DESCRIPTORS_OPEN,
                                   session_child_setup, &child,
-                                  &pid, NULL, NULL, capture_stderr ? &stderr_fd : NULL, &error);
+                                  &pid, NULL, NULL, NULL, &error);
 
   close (fds[0]);
 
@@ -504,7 +502,6 @@ session_start_process (const gchar **argv,
   return g_object_new (COCKPIT_TYPE_PIPE,
                        "in-fd", fds[1],
                        "out-fd", fds[1],
-                       "err-fd", stderr_fd,
                        "pid", pid,
                        "name", argv[0],
                        NULL);
@@ -824,20 +821,13 @@ on_transport_closed (CockpitTransport *transport,
   else if (!session->initialized)
     {
       pipe = cockpit_pipe_transport_get_pipe (COCKPIT_PIPE_TRANSPORT (transport));
-      g_autofree gchar *captured_error = cockpit_pipe_take_stderr_as_utf8 (pipe);
-
       if (cockpit_pipe_get_pid (pipe, NULL))
         status = cockpit_pipe_exit_status (pipe);
       g_debug ("%s: authentication process exited: %d; problem %s", session->name, status, problem);
 
-      if (captured_error)
-        {
-          g_set_error (&error, COCKPIT_ERROR, COCKPIT_ERROR_AUTHENTICATION_FAILED,
-                       "captured-stderr:%s", captured_error);
-        }
       /* we get "access-denied" both if cockpit-session cannot execute cockpit-bridge (common case)
        * and if cockpit-session itself is not executable (corner case, messed up install) */
-      else if (problem && (!session->authorize || g_strcmp0 (problem, "access-denied") != 0))
+      if (problem && (!session->authorize || g_strcmp0 (problem, "access-denied") != 0))
         {
           g_set_error (&error, COCKPIT_ERROR, COCKPIT_ERROR_FAILED,
                        g_strcmp0 (problem, "no-cockpit") == 0
@@ -1070,21 +1060,11 @@ cockpit_session_launch (CockpitAuth *self,
   const gchar *command = cockpit_conf_string (section, "Command");
   const gchar *unix_path = cockpit_conf_string (section, "UnixPath");
 
-  gboolean capture_stderr = FALSE;
   if (g_str_equal (section, COCKPIT_CONF_SSH_SECTION))
     {
       if (!host)
         host = cockpit_conf_string (COCKPIT_CONF_SSH_SECTION, "host") ?: "127.0.0.1";
 
-      /* We capture stderr only for Cockpit Client; we don't want to
-       * send log messages to potential remote attackers.
-       *
-       * Only do that if COCKPIT_DEBUG is off, though: otherwise the
-       * stderr is going to be too long to make sense of in the browser.
-       */
-      if (g_getenv("COCKPIT_DEBUG") == NULL)
-          capture_stderr = cockpit_conf_bool ("WebService", "X-For-CockpitClient", FALSE);
-
       if (command == NULL && unix_path == NULL)
         command = cockpit_ws_ssh_program;
     }
@@ -1124,7 +1104,7 @@ cockpit_session_launch (CockpitAuth *self,
       argv[argc++] = g_strdup (host ?: "localhost");
       argv[argc] = NULL;
 
-      pipe = session_start_process ((const gchar **) argv, (const gchar **)env, capture_stderr);
+      pipe = session_start_process ((const gchar **) argv, (const gchar **)env);
     }
   else if (unix_path != NULL)
     {