Skip to content

Commit

Permalink
ssh: Replace cockpit-ssh with cockpit.beiboot
Browse files Browse the repository at this point in the history
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
  • Loading branch information
martinpitt committed Sep 2, 2024
1 parent 1bdf790 commit c34aa52
Show file tree
Hide file tree
Showing 14 changed files with 24 additions and 117 deletions.
1 change: 0 additions & 1 deletion Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 0 additions & 15 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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], ,
Expand Down Expand Up @@ -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}
Expand Down
1 change: 0 additions & 1 deletion containers/flatpak/prepare
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
8 changes: 3 additions & 5 deletions containers/ws/cockpit-auth-ssh-key
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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__':
Expand Down
1 change: 0 additions & 1 deletion src/bridge/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
78 changes: 0 additions & 78 deletions src/ssh/Makefile-ssh.am

This file was deleted.

3 changes: 1 addition & 2 deletions src/ws/cockpitauth.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
21 changes: 18 additions & 3 deletions src/ws/cockpithandlers.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 1 addition & 4 deletions test/verify/check-static-login
Original file line number Diff line number Diff line change
Expand Up @@ -965,10 +965,7 @@ matchrule = <SUBJECT>^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):
Expand Down
2 changes: 1 addition & 1 deletion test/verify/check-ws-bastion
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
3 changes: 0 additions & 3 deletions tools/cockpit.spec
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion tools/debian/cockpit-bridge.install
Original file line number Diff line number Diff line change
@@ -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
Expand Down
1 change: 0 additions & 1 deletion tools/debian/control
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion tools/make-dist
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ else
--enable-prefix-only \
--disable-pcp \
--disable-polkit \
--disable-ssh \
> /dev/null
fi

Expand Down

0 comments on commit c34aa52

Please sign in to comment.