Skip to content

Commit

Permalink
ws: Use cockpit-beiboot for direct remote connections
Browse files Browse the repository at this point in the history
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
  • Loading branch information
martinpitt committed Sep 27, 2024
1 parent a3ba462 commit 29b482b
Show file tree
Hide file tree
Showing 16 changed files with 56 additions and 48 deletions.
10 changes: 5 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,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__':
Expand Down
9 changes: 3 additions & 6 deletions doc/authentication.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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:

Expand Down Expand Up @@ -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.
2 changes: 1 addition & 1 deletion doc/man/cockpit.conf.xml
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ ForwardedForHeader = X-Forwarded-For
on the login screen is visible and allows logging into another server. When set to
<literal>false</literal>, direct remote logins are disallowed. If this option is not specified
then it will be automatically detected based on whether the
<command>cockpit-ssh</command> process is available or not.</para>
<command>cockpit-bridge</command> and <command>ssh</command> programs are available.</para>

<para>If cockpit-ws is exposed to the public internet, and also has access to a private
internal network, it is recommended to explicitly set <literal>LoginTo=false</literal>. This prevents
Expand Down
2 changes: 1 addition & 1 deletion doc/urls.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
6 changes: 4 additions & 2 deletions src/ws/cockpitauth.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
26 changes: 23 additions & 3 deletions src/ws/cockpithandlers.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/ws/cockpitws.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 0 additions & 3 deletions src/ws/test-server.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion test/browser/run-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ if [ "$PLAN" = "main" ]; then
TestLogin.testFailingWebsocketSafari
TestLogin.testFailingWebsocketSafariNoCA
TestLogin.testLogging
TestLogin.testLoginSshBeiboot
TestLogin.testSSH
TestLogin.testRaw
TestLogin.testServer
TestLogin.testUnsupportedBrowser
Expand Down
3 changes: 1 addition & 2 deletions test/common/testlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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"
]

Expand Down
1 change: 1 addition & 0 deletions test/verify/check-connection
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
5 changes: 1 addition & 4 deletions test/verify/check-loopback
Original file line number Diff line number Diff line change
Expand Up @@ -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.*")
Expand Down
2 changes: 1 addition & 1 deletion test/verify/check-shell-multi-machine
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 0 additions & 4 deletions test/verify/check-shell-multi-machine-key
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
26 changes: 13 additions & 13 deletions test/verify/check-static-login
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down Expand Up @@ -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")
Expand All @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -954,21 +959,16 @@ matchrule = <SUBJECT>^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):
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 @@ -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")

Expand Down

0 comments on commit 29b482b

Please sign in to comment.