From 29b482b8743e639960c459054b3ed00e4c2e3f8c Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Thu, 19 Sep 2024 05:31:06 +0200 Subject: [PATCH] ws: Use cockpit-beiboot for direct remote connections cockpit.beiboot has feature parity with cockpit-ssh, so switch the default direct remote session program to that. Use `--remote-bridge=always` mode for the time being in ws and the container; we are going to support that eventually, but let's not change everything at once. Change ws' detection of remote login availability to "cockpit-bridge and ssh are available". This involves forking a shell now (for running the `command` shell builtin), add an expected message to `TestConnection.testHttpsInstanceDoS`. Drop the `COCKPIT_SSH_BRIDGE_COMMAND` env var documentation, cockpit-beiboot does not use that. Rename `testLoginSshBeiboot` to `testSSH`. This test covers a few corner cases that other tests don't, and it's very convenient for development to have a nondestructive test which covers all aspects of bastion host SSH login. Adjust some error messages in the tests. https://issues.redhat.com/browse/COCKPIT-1029 --- containers/ws/cockpit-auth-ssh-key | 10 ++++----- doc/authentication.md | 9 +++----- doc/man/cockpit.conf.xml | 2 +- doc/urls.md | 2 +- src/ws/cockpitauth.c | 6 ++++-- src/ws/cockpithandlers.c | 26 ++++++++++++++++++++--- src/ws/cockpitws.h | 1 - src/ws/test-server.c | 3 --- test/browser/run-test.sh | 2 +- test/common/testlib.py | 3 +-- test/verify/check-connection | 1 + test/verify/check-loopback | 5 +---- test/verify/check-shell-multi-machine | 2 +- test/verify/check-shell-multi-machine-key | 4 ---- test/verify/check-static-login | 26 +++++++++++------------ test/verify/check-ws-bastion | 2 +- 16 files changed, 56 insertions(+), 48 deletions(-) diff --git a/containers/ws/cockpit-auth-ssh-key b/containers/ws/cockpit-auth-ssh-key index 5a177dcb243..f9646b468b3 100755 --- a/containers/ws/cockpit-auth-ssh-key +++ b/containers/ws/cockpit-auth-ssh-key @@ -20,8 +20,8 @@ # by cockpit-ws. It asks for authentication from cockpit-ws and expects # to receive a basic auth header in response. If COCKPIT_SSH_KEY_PATH is set # we will try to decrypt the key with the given password. If successful -# we send the decrypted key to cockpit-ws for use with cockpit-ssh. -# Once finished we exec cockpit-ssh to actually establish the ssh connection. +# we send the decrypted key to cockpit-ws for use with ssh. +# Once finished we exec cockpit.beiboot to actually establish the ssh connection. # All communication with cockpit-ws happens on stdin and stdout using the # cockpit protocol # (https://github.com/cockpit-project/cockpit/blob/main/doc/protocol.md) @@ -33,8 +33,6 @@ import subprocess import sys import time -COCKPIT_SSH_COMMAND = "/usr/libexec/cockpit-ssh" - def usage(): print("usage", sys.argv[0], "[user@]host[:port]", file=sys.stderr) @@ -189,7 +187,9 @@ def main(args): {"password": "denied"}) return - os.execlpe(COCKPIT_SSH_COMMAND, COCKPIT_SSH_COMMAND, host, os.environ) + # for the time being, we only support running an installed cockpit-bridge on the remote, + # and leave beibooting to the flatpak + os.execlpe("python3", "python3", "-m", "cockpit.beiboot", "--remote-bridge=always", host, os.environ) if __name__ == '__main__': diff --git a/doc/authentication.md b/doc/authentication.md index aba8b5be117..0a1d34d2e95 100644 --- a/doc/authentication.md +++ b/doc/authentication.md @@ -97,7 +97,8 @@ Remote machines Cockpit also supports logging directly into remote machines. The remote machine to connect to is provided by using a application name that begins with `cockpit+=`. -The default command used for this is cockpit-ssh. +The default command used for this is `python3 -m cockpit.beiboot`, which +invokes `ssh`. The section `Ssh-Login` defines the options for all ssh commands. The section has the same options as the other authentication sections with the following additions. @@ -109,8 +110,7 @@ has the same options as the other authentication sections with the following add `/etc/ssh/ssh_known_hosts`). Set this to `true` is to allow those connections to proceed. -This uses the [cockpit-ssh](https://github.com/cockpit-project/cockpit/tree/main/src/ssh) -bridge. After the user authentication with the `"*"` challenge, if the remote +After the user authentication with the `"*"` challenge, if the remote host is not already present in any local `known_hosts` file, this will send an `"x-host-key"` challenge: @@ -202,6 +202,3 @@ The following environment variables are used to set options for the `cockpit-ssh * **COCKPIT_SSH_KNOWN_HOSTS_FILE** Path to knownhost files. Defaults to `PACKAGE_SYSCONF_DIR/ssh/ssh_known_hosts` - - * **COCKPIT_SSH_BRIDGE_COMMAND** Command to launch after a ssh connection is - established. Defaults to `cockpit-bridge` if not provided. diff --git a/doc/man/cockpit.conf.xml b/doc/man/cockpit.conf.xml index 275988b347c..dcf55c66648 100644 --- a/doc/man/cockpit.conf.xml +++ b/doc/man/cockpit.conf.xml @@ -108,7 +108,7 @@ ForwardedForHeader = X-Forwarded-For on the login screen is visible and allows logging into another server. When set to false, direct remote logins are disallowed. If this option is not specified then it will be automatically detected based on whether the - cockpit-ssh process is available or not. + cockpit-bridge and ssh programs are available. If cockpit-ws is exposed to the public internet, and also has access to a private internal network, it is recommended to explicitly set LoginTo=false. This prevents diff --git a/doc/urls.md b/doc/urls.md index 8fd8ce56405..a53ecf9c81d 100644 --- a/doc/urls.md +++ b/doc/urls.md @@ -60,7 +60,7 @@ Direct to machine urls ====================== Cockpit-ws supports logging in directly to a remote machine, without first -authenticating on the machine that cockpit-ws is running on. A cockpit-ssh +authenticating on the machine that cockpit-ws is running on. A `cockpit-beiboot` processes is spawned that connects via SSH to the remote machine and all requests are proxied via that connection. diff --git a/src/ws/cockpitauth.c b/src/ws/cockpitauth.c index f603a9bdc1a..86c568bd222 100644 --- a/src/ws/cockpitauth.c +++ b/src/ws/cockpitauth.c @@ -49,9 +49,12 @@ #define ACTION_NONE "none" #define LOCAL_SESSION "local-session" +/* for the time being, we only support running an installed cockpit-bridge on the remote, + * and leave beibooting to the flatpak */ +const gchar *cockpit_ws_ssh_program = "/usr/bin/env python3 -m cockpit.beiboot --remote-bridge=always"; + /* Some tunables that can be set from tests */ const gchar *cockpit_ws_session_program = LIBEXECDIR "/cockpit-session"; -const gchar *cockpit_ws_ssh_program = LIBEXECDIR "/cockpit-ssh"; /* Timeout of authenticated session when no connections */ guint cockpit_ws_service_idle = 15; @@ -1054,7 +1057,6 @@ cockpit_session_launch (CockpitAuth *self, return NULL; } - /* this might be unset, which means "allow if cockpit-ssh is installed"; if it isn't, this will fail later on */ if (host && !cockpit_conf_bool ("WebService", "LoginTo", TRUE)) { g_set_error (error, COCKPIT_ERROR, COCKPIT_ERROR_AUTHENTICATION_FAILED, "Direct remote login is disabled"); diff --git a/src/ws/cockpithandlers.c b/src/ws/cockpithandlers.c index a70d8bb7a7a..c75c3db5a2d 100644 --- a/src/ws/cockpithandlers.c +++ b/src/ws/cockpithandlers.c @@ -245,6 +245,24 @@ add_oauth_to_environment (JsonObject *environment) } } +static bool have_command (const char *name) +{ + gint status; + g_autoptr(GError) error = NULL; + g_autofree gchar *command = g_strdup_printf ("command -v %s", name); + if (g_spawn_sync (NULL, + (gchar * []){ "/bin/sh", "-ec", command, NULL }, + NULL, + G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL, + NULL, NULL, NULL, NULL, &status, &error)) + return status == 0; + else + { + g_warning ("Failed to check for %s: %s", name, error->message); + return FALSE; + } +} + static void add_page_to_environment (JsonObject *object, gboolean is_cockpit_client) @@ -263,9 +281,11 @@ add_page_to_environment (JsonObject *object, if (page_login_to < 0) { - page_login_to = cockpit_conf_bool ("WebService", "LoginTo", - g_file_test (cockpit_ws_ssh_program, - G_FILE_TEST_IS_EXECUTABLE)); + /* cockpit.beiboot is part of cockpit-bridge package */ + gboolean have_ssh = have_command ("ssh") && have_command ("cockpit-bridge"); + if (!have_ssh) + g_info ("cockpit-bridge or ssh are not available, disabling remote logins"); + page_login_to = cockpit_conf_bool ("WebService", "LoginTo", have_ssh); } require_host = is_cockpit_client || cockpit_conf_bool ("WebService", "RequireHost", FALSE); diff --git a/src/ws/cockpitws.h b/src/ws/cockpitws.h index ea8a9b7d49c..55ae77d545f 100644 --- a/src/ws/cockpitws.h +++ b/src/ws/cockpitws.h @@ -28,7 +28,6 @@ G_BEGIN_DECLS /* From cockpitwebsocket.c */ extern const gchar *cockpit_ws_session_program; -extern const gchar *cockpit_ws_ssh_program; extern const gchar *cockpit_ws_default_host_header; extern gint cockpit_ws_specific_ssh_port; extern guint cockpit_ws_ping_interval; diff --git a/src/ws/test-server.c b/src/ws/test-server.c index acbc4b41259..f1d8c3b8cb2 100644 --- a/src/ws/test-server.c +++ b/src/ws/test-server.c @@ -1018,9 +1018,6 @@ main (int argc, bridge_argv[i++] = "cockpit.bridge"; } - // Use a local ssh session command - cockpit_ws_ssh_program = BUILDDIR "/cockpit-ssh"; - loop = g_main_loop_new (NULL, FALSE); g_bus_own_name (G_BUS_TYPE_SESSION, diff --git a/test/browser/run-test.sh b/test/browser/run-test.sh index b1f6f8eb963..f3d1f8d71a0 100644 --- a/test/browser/run-test.sh +++ b/test/browser/run-test.sh @@ -91,7 +91,7 @@ if [ "$PLAN" = "main" ]; then TestLogin.testFailingWebsocketSafari TestLogin.testFailingWebsocketSafariNoCA TestLogin.testLogging - TestLogin.testLoginSshBeiboot + TestLogin.testSSH TestLogin.testRaw TestLogin.testServer TestLogin.testUnsupportedBrowser diff --git a/test/common/testlib.py b/test/common/testlib.py index 73b084ba6fc..29a742d080a 100644 --- a/test/common/testlib.py +++ b/test/common/testlib.py @@ -1033,7 +1033,7 @@ def relogin( ) -> None: self.logout() if wait_remote_session_machine: - wait_remote_session_machine.execute("while pgrep -a cockpit-ssh; do sleep 1; done") + wait_remote_session_machine.execute("while pgrep -af '[c]ockpit.beiboot'; do sleep 1; done") self.try_login(user=user, password=password, superuser=superuser) self.wait_visible('#content') if path: @@ -2113,7 +2113,6 @@ def check_journal_messages(self, machine: testvm.Machine | None = None) -> None: "_COMM=cockpit-ws", "GLIB_DOMAIN=cockpit-ws", "GLIB_DOMAIN=cockpit-bridge", - "GLIB_DOMAIN=cockpit-ssh", "GLIB_DOMAIN=cockpit-pcp" ] diff --git a/test/verify/check-connection b/test/verify/check-connection index aab0bec184d..fdb509e569b 100755 --- a/test/verify/check-connection +++ b/test/verify/check-connection @@ -347,6 +347,7 @@ class TestConnection(testlib.MachineCase): self.allow_journal_messages(".*cockpit-ws.*dumped core.*") self.allow_journal_messages(".*Error creating thread: Resource temporarily unavailable.*") + self.allow_journal_messages(".*Failed to check for ssh: Failed to fork.*") # initial instance still works self.assertIn("HTTP/1.1 200 OK", m.execute("curl --silent --show-error -k --head https://127.0.0.1:9090")) diff --git a/test/verify/check-loopback b/test/verify/check-loopback index 399b2a1cc27..fa106b24d0f 100755 --- a/test/verify/check-loopback +++ b/test/verify/check-loopback @@ -58,10 +58,7 @@ class TestLoopback(testlib.MachineCase): b.set_val('#login-user-input', "admin") b.set_val('#login-password-input', "foobar") b.click('#login-button') - b.wait_visible('#login-fatal') - self.assertIn( - "The server refused to authenticate 'admin' using password authentication, and no other supported authentication methods are available.", - b.text('#login-fatal')) + b.wait_in_text('#login-error-message', "Wrong user name or password") self.allow_journal_messages("Cannot run program cockpit-bridge: No such file or directory", ".*server offered unsupported authentication methods: public-key.*") diff --git a/test/verify/check-shell-multi-machine b/test/verify/check-shell-multi-machine index 95e8a3bb0de..ba28000ea55 100755 --- a/test/verify/check-shell-multi-machine +++ b/test/verify/check-shell-multi-machine @@ -368,7 +368,7 @@ class TestMultiMachine(testlib.MachineCase): b.wait_not_visible("#server-group") b.click('#login-button') b.wait_visible("#server-group") - b.wait_in_text("#login-error-message", "Unable to connect") + b.wait_in_text("#login-error-message", "Refusing to connect") # Might happen when we switch away. self.allow_hostkey_messages() diff --git a/test/verify/check-shell-multi-machine-key b/test/verify/check-shell-multi-machine-key index 73da4ef77e5..354289ccfd0 100755 --- a/test/verify/check-shell-multi-machine-key +++ b/test/verify/check-shell-multi-machine-key @@ -102,10 +102,6 @@ class TestMultiMachineKeyAuth(testlib.MachineCase): self.machine2.execute("useradd user -c User", direct=True) self.machine2.execute( "sed -i 's/.*PasswordAuthentication.*/PasswordAuthentication no/' /etc/ssh/sshd_config $(ls /etc/ssh/sshd_config.d/* 2>/dev/null || true)", direct=True) - # HACK - Increase MaxAuthTries because of - # https://bugs.libssh.org/T233 and because we have a lot of - # keys and cockpit-ssh tries them all, and many of them twice. - self.machine2.write("/etc/ssh/sshd_config", "MaxAuthTries 100", append=True) self.machine2.execute(self.restart_sshd, direct=True) self.machine2.wait_execute() diff --git a/test/verify/check-static-login b/test/verify/check-static-login index a5b6e9883e8..96cdf78e545 100755 --- a/test/verify/check-static-login +++ b/test/verify/check-static-login @@ -99,7 +99,7 @@ account required pam_succeed_if.so user ingroup %s""" % m.get_admin_group # Try to login as user with correct password b.try_login("user", "abcdefg") if m.ostree_image: - b.wait_in_text("#login-error-message", "Server closed connection") + b.wait_in_text("#login-error-message", "Wrong user name or password") else: b.wait_text("#login-error-message", "Permission denied") self.assertNotIn("web", m.execute("who")) @@ -152,7 +152,7 @@ account required pam_succeed_if.so user ingroup %s""" % m.get_admin_group b.open("/=192.168.99.99/") b.wait_visible("#login") b.wait_not_visible("#option-group") - # logging in does not go via cockpit-ssh (which would cause a connection failure) + # logging in does not go via ssh (which would cause a connection failure) b.try_login() # this isn't the most helpful error message, but this is essentially hacking b.wait_text("#login-error-message", "Wrong user name or password") @@ -163,10 +163,10 @@ account required pam_succeed_if.so user ingroup %s""" % m.get_admin_group b.open("/system") b.wait_visible("#option-group") - # And now we remove cockpit-ssh which affects the default + # And now we remove ssh which affects the default if not m.ostree_image: - self.restore_file(f"{self.libexecdir}/cockpit-ssh") - m.execute(f"rm {self.libexecdir}/cockpit-ssh") + self.restore_file("/usr/bin/ssh") + m.execute("rm /usr/bin/ssh") m.restart_cockpit() b.open("/system") b.wait_visible("#login") @@ -381,8 +381,13 @@ account required pam_succeed_if.so user ingroup %s""" % m.get_admin_group b.wait_in_text("#conversation-prompt", "life the universe") b.set_val('#conversation-input', '43') b.click('#login-button') - + if m.ostree_image: + # ssh asks for password again, despite NumberOfPasswordPrompts=1 + b.wait_in_text("#conversation-prompt", "nonexisting@127.0.0.1's password") + b.set_val('#conversation-input', 'blahblah') + b.click('#login-button') b.wait_text_not("#login-error-message", "") + b.try_login("admin", "foobar") b.wait_visible("#conversation-group") b.wait_not_visible("#password-group") @@ -954,21 +959,16 @@ matchrule = ^DC=LAN,DC=COCKPIT,CN=alice$ # by IPv6 address and port check_server("[::1]:22", expect_fp_ack=True) - # check cockpit-beiboot as replacement of cockpit-ssh on the login page - # once that becomes the default, TestMultiMachine* and the other TestLogin* cover this @testlib.skipImage("needs pybridge", "rhel-8*", "centos-8*") # enable this once our cockpit/ws container can beiboot @testlib.skipOstree("client setup does not work with ws container") - def testLoginSshBeiboot(self): + def testSSH(self): m = self.machine b = self.browser # this matches our bots test VMs my_ip = "172.27.0.15" - m.write("/etc/cockpit/cockpit.conf", """ -[Ssh-Login] -Command = /usr/bin/env python3 -m cockpit.beiboot -""", append=True) + m.start_cockpit() def try_login(user, password, server=None, expect_hostkey=False): diff --git a/test/verify/check-ws-bastion b/test/verify/check-ws-bastion index 16e2277af55..18e36255ddc 100755 --- a/test/verify/check-ws-bastion +++ b/test/verify/check-ws-bastion @@ -166,7 +166,7 @@ class TestWsBastionContainer(testlib.MachineCase): "-v /root/known_hosts:/etc/ssh/ssh_known_hosts:ro,Z " "-v /root/id_bastion:/id_bastion:ro,Z " "-e COCKPIT_SSH_KEY_PATH=/id_bastion " - "-e G_MESSAGES_DEBUG=cockpit-ssh " + "-e COCKPIT_DEBUG=all " "localhost/cockpit/ws") m.execute("until curl --fail --head -k https://localhost:9090/; do sleep 1; done")