From c34aa521e7be3d6f61d6f8926ac1492db090cfb5 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Thu, 5 Oct 2023 06:39:38 +0200 Subject: [PATCH] ssh: Replace cockpit-ssh with cockpit.beiboot cockpit.beiboot has feature parity with cockpit-ssh. Entirely stop building and shipping cockpit-ssh with the pybridge, and use cockpit.beiboot by default for direct remote SSH logins from the login page. This gets rid of the libssh build dependency. Drop the `Provides: cockpit-ssh` from Debian. No package ever related to that virtual package name, and it's meaningless these days. Change ws' detection of remote login availability to `type cockpit-bridge` with the pybridge, as the existence of cockpit-ssh is not relevant any more. This is much cheaper than actually trying to run the bridge with `--version` or call Python to check the module. We still need to do this, as a system could only have the cockpit-ws package installed but not cockpit-bridge. https://issues.redhat.com/browse/COCKPIT-1029 --- Makefile.am | 1 - configure.ac | 15 ------ containers/flatpak/prepare | 1 - containers/ws/cockpit-auth-ssh-key | 8 ++- src/bridge/Makefile.am | 1 - src/ssh/Makefile-ssh.am | 78 ----------------------------- src/ws/cockpitauth.c | 3 +- src/ws/cockpithandlers.c | 21 ++++++-- test/verify/check-static-login | 5 +- test/verify/check-ws-bastion | 2 +- tools/cockpit.spec | 3 -- tools/debian/cockpit-bridge.install | 1 - tools/debian/control | 1 - tools/make-dist | 1 - 14 files changed, 24 insertions(+), 117 deletions(-) delete mode 100644 src/ssh/Makefile-ssh.am diff --git a/Makefile.am b/Makefile.am index 65b1b47d175..ce67f50b76d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -176,7 +176,6 @@ include src/client/Makefile.am include src/common/Makefile-common.am include src/pam-ssh-add/Makefile.am include src/session/Makefile-session.am -include src/ssh/Makefile-ssh.am include src/systemd/Makefile.am include src/tls/Makefile-tls.am include src/websocket/Makefile-websocket.am diff --git a/configure.ac b/configure.ac index 1887a462cc2..25549037b4d 100644 --- a/configure.ac +++ b/configure.ac @@ -70,12 +70,6 @@ if test "$enable_polkit" != 'no'; then fi AC_MSG_RESULT(${enable_polkit:=yes}) -# --disable-ssh -AC_MSG_CHECKING([whether to build cockpit-ssh]) -AC_ARG_ENABLE(ssh, AS_HELP_STRING([--disable-ssh], [Disable cockpit-ssh build and libssh dependency])) -AM_CONDITIONAL(WITH_COCKPIT_SSH, test "$enable_ssh" != "no") -AC_MSG_RESULT(${enable_ssh:=yes}) - # TODO: remove this and clean up the old bridge in src/bridge/ AM_CONDITIONAL(WITH_OLD_BRIDGE, false) @@ -108,14 +102,6 @@ PKG_CHECK_MODULES(krb5, [krb5-gssapi >= 1.11 krb5 >= 1.11]) if test "$enable_polkit" != "no"; then PKG_CHECK_MODULES(polkit, [polkit-agent-1 >= 0.105]) fi -if test "$enable_ssh" != "no"; then - PKG_CHECK_MODULES(libssh, [libssh >= 0.8.5]) - old_CFLAGS=$CFLAGS; CFLAGS=$libssh_CFLAGS - old_LIBS=$LIBS; LIBS=$libssh_LIBS - AC_CHECK_FUNCS(ssh_userauth_publickey_auto_get_current_identity) - CFLAGS=$old_CFLAGS - LIBS=$old_LIBS -fi # pam AC_CHECK_HEADER([security/pam_appl.h], , @@ -442,7 +428,6 @@ echo " SELinux Policy: ${enable_selinux_policy} cockpit-client: ${enable_cockpit_client} - cockpit-ssh: ${enable_ssh} ssh-add: ${SSH_ADD} ssh-agent: ${SSH_AGENT} diff --git a/containers/flatpak/prepare b/containers/flatpak/prepare index 3b8620f5c15..97988766b2f 100755 --- a/containers/flatpak/prepare +++ b/containers/flatpak/prepare @@ -104,7 +104,6 @@ def create_manifest( 'config-opts': [ '--enable-cockpit-client', '--disable-polkit', - '--disable-ssh', '--disable-pcp', '--with-systemdunitdir=/invalid', 'CPPFLAGS=-Itools/mock-build-env', diff --git a/containers/ws/cockpit-auth-ssh-key b/containers/ws/cockpit-auth-ssh-key index 5a177dcb243..ceba79153f8 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,7 @@ def main(args): {"password": "denied"}) return - os.execlpe(COCKPIT_SSH_COMMAND, COCKPIT_SSH_COMMAND, host, os.environ) + os.execlpe("python3", "python3", "-m", "cockpit.beiboot", host, os.environ) if __name__ == '__main__': diff --git a/src/bridge/Makefile.am b/src/bridge/Makefile.am index 96481bf726b..fdb1ceaf1ad 100644 --- a/src/bridge/Makefile.am +++ b/src/bridge/Makefile.am @@ -236,7 +236,6 @@ endif # ----------------------------------------------------------------------------- # polkit -# make sure this ends up in the tarball, even with --disable-ssh polkit_in_files = src/bridge/org.cockpit-project.cockpit-bridge.policy.in EXTRA_DIST += $(polkit_in_files) diff --git a/src/ssh/Makefile-ssh.am b/src/ssh/Makefile-ssh.am deleted file mode 100644 index 195eec6e9c0..00000000000 --- a/src/ssh/Makefile-ssh.am +++ /dev/null @@ -1,78 +0,0 @@ -if WITH_COCKPIT_SSH - -# ----------------------------------------------------------------------------- -# libcockpit-ssh.a: code used in cockpit-ssh and its tests - -noinst_LIBRARIES += libcockpit-ssh.a - -libcockpit_ssh_a_CPPFLAGS = \ - -DG_LOG_DOMAIN=\"cockpit-ssh\" \ - $(glib_CFLAGS) \ - $(json_glib_CFLAGS) \ - $(libssh_CFLAGS) \ - $(AM_CPPFLAGS) - -libcockpit_ssh_a_LIBS = \ - libcockpit-ssh.a \ - $(libcockpit_common_a_LIBS) \ - $(libssh_LIBS) \ - $(NULL) - -libcockpit_ssh_a_SOURCES = \ - src/ssh/cockpitsshoptions.c \ - src/ssh/cockpitsshoptions.h \ - src/ssh/cockpitsshrelay.h \ - src/ssh/cockpitsshrelay.c \ - $(NULL) - -# ----------------------------------------------------------------------------- -# cockpit-ssh - -libexec_PROGRAMS += cockpit-ssh -cockpit_ssh_CPPFLAGS = $(libcockpit_ssh_a_CPPFLAGS) -cockpit_ssh_LDADD = $(libcockpit_ssh_a_LIBS) -cockpit_ssh_SOURCES = src/ssh/ssh.c - -# ----------------------------------------------------------------------------- -# mock-ssh - -check_PROGRAMS += mock-sshd -mock_sshd_CPPFLAGS = $(libcockpit_ssh_a_CPPFLAGS) $(TEST_CPP) -mock_sshd_LDADD = $(libcockpit_ssh_a_LIBS) $(TEST_LIBS) -mock_sshd_SOURCES = src/ssh/mock-sshd.c - -# ----------------------------------------------------------------------------- -# Unit tests - -dist_check_DATA += \ - src/ssh/mock_rsa_key \ - src/ssh/mock_ecdsa_key \ - src/ssh/test_rsa \ - src/ssh/test_rsa.pub \ - src/ssh/mock_known_hosts \ - src/ssh/mock-pid-cat \ - src/ssh/mock-config \ - src/ssh/invalid_known_hosts \ - $(NULL) - -TEST_PROGRAM += test-sshbridge -test_sshbridge_CPPFLAGS = $(libcockpit_ssh_a_CPPFLAGS) $(TEST_CPP) -test_sshbridge_LDADD = $(libcockpit_ssh_a_LIBS) $(TEST_LIBS) -test_sshbridge_SOURCES = src/ssh/test-sshbridge.c - -TEST_PROGRAM += test-sshoptions -test_sshoptions_CPPFLAGS = $(libcockpit_ssh_a_CPPFLAGS) $(TEST_CPP) -test_sshoptions_LDADD = $(libcockpit_ssh_a_LIBS) $(TEST_LIBS) -test_sshoptions_SOURCES = src/ssh/test-sshoptions.c - -check_DATA += test_rsa_key -CLEANFILES += test_rsa_key -test_rsa_key: src/ssh/test_rsa - $(AM_V_GEN) cp $< $@ && chmod 600 $@ - -update-known-hosts: - cat $(srcdir)/src/ssh/mock_*.pub | \ - sed -ne 's/\(.*\) [^ ]\+$$/[localhost]:*,[127.0.0.1]:* \1/p' > \ - $(srcdir)/src/ssh/mock_known_hosts - -endif diff --git a/src/ws/cockpitauth.c b/src/ws/cockpitauth.c index f603a9bdc1a..6ba5b1340d0 100644 --- a/src/ws/cockpitauth.c +++ b/src/ws/cockpitauth.c @@ -51,7 +51,7 @@ /* 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"; +const gchar *cockpit_ws_ssh_program = "/usr/bin/env python3 -m cockpit.beiboot"; /* Timeout of authenticated session when no connections */ guint cockpit_ws_service_idle = 15; @@ -1054,7 +1054,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..70bff73df86 100644 --- a/src/ws/cockpithandlers.c +++ b/src/ws/cockpithandlers.c @@ -263,9 +263,24 @@ 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)); + gboolean have_ssh; + /* cockpit.beiboot is part of cockpit-bridge package */ + gint status; + GError *error = NULL; + if (g_spawn_sync (NULL, + (gchar * []){ "type", "cockpit-bridge", NULL }, + NULL, + G_SPAWN_SEARCH_PATH | G_SPAWN_STDOUT_TO_DEV_NULL | G_SPAWN_STDERR_TO_DEV_NULL, + NULL, NULL, NULL, NULL, &status, &error)) + { + have_ssh = status == 0; + } + else + { + g_warning ("Failed to check for cockpit-bridge, disabling remote logins: %s", error->message); + have_ssh = FALSE; + } + page_login_to = cockpit_conf_bool ("WebService", "LoginTo", have_ssh); } require_host = is_cockpit_client || cockpit_conf_bool ("WebService", "RequireHost", FALSE); diff --git a/test/verify/check-static-login b/test/verify/check-static-login index 38bc94c8893..cced32e1c53 100755 --- a/test/verify/check-static-login +++ b/test/verify/check-static-login @@ -965,10 +965,7 @@ matchrule = ^DC=LAN,DC=COCKPIT,CN=alice$ # 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 dffd9fe7e1c..9294d4d8475 100755 --- a/test/verify/check-ws-bastion +++ b/test/verify/check-ws-bastion @@ -173,7 +173,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=cockpit.beiboot " "localhost/cockpit/ws") m.execute("until curl --fail --head -k https://localhost:9090/; do sleep 1; done") diff --git a/tools/cockpit.spec b/tools/cockpit.spec index 3484f64d7a0..0d2439ef363 100644 --- a/tools/cockpit.spec +++ b/tools/cockpit.spec @@ -84,7 +84,6 @@ BuildRequires: autoconf automake BuildRequires: make BuildRequires: python3-devel BuildRequires: gettext >= 0.21 -BuildRequires: libssh-devel >= 0.8.5 BuildRequires: openssl-devel BuildRequires: gnutls-devel >= 3.4.3 BuildRequires: zlib-devel @@ -198,7 +197,6 @@ echo '%dir %{_datadir}/cockpit/base1' >> base.list find %{buildroot}%{_datadir}/cockpit/base1 -type f -o -type l >> base.list echo '%{_sysconfdir}/cockpit/machines.d' >> base.list echo %{buildroot}%{_datadir}/polkit-1/actions/org.cockpit-project.cockpit-bridge.policy >> base.list -echo '%{_libexecdir}/cockpit-ssh' >> base.list %if %{build_pcp} echo '%dir %{_datadir}/cockpit/pcp' > pcp.list @@ -289,7 +287,6 @@ troubleshooting, interactive command-line sessions, and more. %package bridge Summary: Cockpit bridge server-side component Requires: glib-networking -Provides: cockpit-ssh = %{version}-%{release} # 233 dropped jquery.js, pages started to bundle it (commit 049e8b8dce) Conflicts: cockpit-dashboard < 233 Conflicts: cockpit-networkmanager < 233 diff --git a/tools/debian/cockpit-bridge.install b/tools/debian/cockpit-bridge.install index 2b68a23dbb5..6dc6a0cee24 100644 --- a/tools/debian/cockpit-bridge.install +++ b/tools/debian/cockpit-bridge.install @@ -1,7 +1,6 @@ etc/cockpit/machines.d usr/bin/cockpit-bridge usr/lib/cockpit/cockpit-askpass -usr/lib/cockpit/cockpit-ssh usr/lib/python* usr/share/cockpit/base1/ usr/share/man/man1/cockpit-bridge.1 diff --git a/tools/debian/control b/tools/debian/control index 4066af04b12..ae4d4594ea8 100644 --- a/tools/debian/control +++ b/tools/debian/control @@ -6,7 +6,6 @@ Build-Depends: debhelper-compat (= 13), dh-python, gettext (>= 0.19.7), gettext (>= 0.21) | appstream, - libssh-dev (>= 0.8.5), zlib1g-dev, libkrb5-dev (>= 1.11), libxslt1-dev, diff --git a/tools/make-dist b/tools/make-dist index d2edb8ef062..7c66ca8e9f2 100755 --- a/tools/make-dist +++ b/tools/make-dist @@ -47,7 +47,6 @@ else --enable-prefix-only \ --disable-pcp \ --disable-polkit \ - --disable-ssh \ > /dev/null fi