diff --git a/containers/flatpak/test/test-browser-login-ssh.js b/containers/flatpak/test/test-browser-login-ssh.js index 38e1129a97f4..32e5c57eb05a 100644 --- a/containers/flatpak/test/test-browser-login-ssh.js +++ b/containers/flatpak/test/test-browser-login-ssh.js @@ -12,9 +12,9 @@ async function test() { document.getElementById("server-field").value = "%HOST%"; ph_mouse("#login-button", "click"); - // unknown host key - await assert_conversation("authenticity of host"); - document.getElementById("conversation-input").value = "yes"; + // accept unknown host key + await ph_wait_present("#hostkey-message-1"); + await ph_wait_in_text("#hostkey-message-1", "%HOST%"); ph_mouse("#login-button", "click"); await ph_wait_present("#conversation-prompt"); diff --git a/doc/authentication.md b/doc/authentication.md index 9f91d7c0bd1d..aba8b5be1173 100644 --- a/doc/authentication.md +++ b/doc/authentication.md @@ -99,14 +99,15 @@ Cockpit also supports logging directly into remote machines. The remote machine connect to is provided by using a application name that begins with `cockpit+=`. The default command used for this is cockpit-ssh. -The section `SSH-Login` defines the options for all ssh commands. The section +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. - * `host` The default host to log into. Defaults to 127.0.0.1. + * `host` The default host to log into. Defaults to 127.0.0.1. That host's key + will not be checked/validated. * `connectToUnknownHosts`. By default cockpit will refuse to connect to any machines that - are not already present in ssh's global `known_hosts` file (usually - `/etc/ssh/ssh_known_hosts`). Set this to `true` is to allow those connections - to proceed. + are not already present in ssh's global `known_hosts` file (usually + `/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 @@ -159,7 +160,7 @@ Actions Setting an action can modify the behavior for an auth scheme. Currently two actions are supported. - * **remote-login-ssh** Use the `SSH-Login` section instead. + * **remote-login-ssh** Use the `Ssh-Login` section instead. * **none** Disable this auth scheme. To configure an action add the `action` option. For example to disable basic authentication, diff --git a/pkg/static/login.js b/pkg/static/login.js index 997755bfa5db..200935ea9471 100644 --- a/pkg/static/login.js +++ b/pkg/static/login.js @@ -94,6 +94,15 @@ function debug(...args) { return document.getElementById(name); } + // strip off "user@", "*:port", and IPv6 brackets from login target (but keep two :: intact for IPv6) + function parseHostname(ssh_target) { + return ssh_target + .replace(/^.*@/, '') + .replace(/(? -1) { @@ -959,7 +1026,17 @@ function debug(...args) { } else if (xhr.statusText.indexOf("unknown-host") > -1) { host_failure(_("Refusing to connect. Host is unknown")); } else if (xhr.statusText.indexOf("invalid-hostkey") > -1) { - host_failure(_("Refusing to connect. Hostkey does not match")); + /* ssh/ferny/beiboot immediately fail in this case, it's not a conversation; + * ask the user for confirmation and try again */ + if (ssh_host_key_change_host === null) { + debug("send_login_request(): invalid-hostkey, trying again to let the user confirm"); + ssh_host_key_change_host = login_machine; + call_login(); + } else { + // but only once, to avoid loops; this is also the code path for cockpit-ssh + debug("send_login_request(): invalid-hostkey, and already retried, giving up"); + host_failure(_("Refusing to connect. Hostkey does not match")); + } } else if (is_conversation) { login_failure(_("Authentication failed")); } else { @@ -1038,6 +1115,18 @@ function debug(...args) { localStorage.setItem(application + 'login-data', str); /* Backwards compatibility for packages that aren't application prefixed */ localStorage.setItem('login-data', str); + + /* When confirming a host key with cockpit-beiboot, login-data contains the known_hosts pubkey; + * update our database */ + if (login_data_host) { + const hostkey = response["login-data"]["known-hosts"]; + if (hostkey) { + console.debug("setup_localstorage(): updating known_hosts database for deferred host key for", login_data_host, ":", hostkey); + set_hostkeys(login_data_host, hostkey); + } else { + console.error("login.js internal error: setup_localstorage() received a pending login-data host, but login-data does not contain known-hosts"); + } + } } /* URL Root is set by cockpit ws and shouldn't be prefixed diff --git a/src/cockpit/beiboot.py b/src/cockpit/beiboot.py index e4117437a4ae..8b577c2a881a 100644 --- a/src/cockpit/beiboot.py +++ b/src/cockpit/beiboot.py @@ -21,7 +21,10 @@ import importlib.resources import logging import os +import re import shlex +import tempfile +import time from pathlib import Path from typing import Dict, Iterable, Optional, Sequence @@ -35,7 +38,7 @@ from cockpit.jsonutil import JsonObject, get_str from cockpit.packages import Packages, PackagesLoader, patch_libexecdir from cockpit.peer import Peer -from cockpit.protocol import CockpitProblem +from cockpit.protocol import CockpitProblem, CockpitProtocolError from cockpit.router import Router, RoutingRule from cockpit.transports import StdioTransport @@ -129,10 +132,25 @@ class AuthorizeResponder(ferny.AskpassHandler): commands = ('ferny.askpass', 'cockpit.report-exists') router: Router - def __init__(self, router: Router): + def __init__(self, router: Router, basic_password: Optional[str]): self.router = router + self.basic_password = basic_password + self.have_basic_password = basic_password is not None async def do_askpass(self, messages: str, prompt: str, hint: str) -> Optional[str]: + logger.debug("AuthorizeResponder: prompt %r, messages %r, hint %r", prompt, messages, hint) + + if self.have_basic_password and 'password:' in prompt.lower(): + # with our NumberOfPasswordPrompts=1 ssh should never actually ask us more than once; assert that + if self.basic_password is None: + raise CockpitProtocolError( + f"ssh asked for password a second time, but we already sent it; prompt: {messages}") + + logger.debug("AuthorizeResponder: sending Basic auth password for prompt %r", prompt) + reply = self.basic_password + self.basic_password = None + return reply + if hint == 'none': # We have three problems here: # @@ -150,14 +168,35 @@ async def do_askpass(self, messages: str, prompt: str, hint: str) -> Optional[st # Let's avoid all of that by just showing nothing. return None - challenge = 'X-Conversation - ' + base64.b64encode(prompt.encode()).decode() + # FIXME: is this a host key prompt? This should be handled more elegantly, + # see https://github.com/cockpit-project/cockpit/pull/19668 + fp_match = re.search(r'\n(\w+) key fingerprint is ([^.]+)\.', prompt) + # let ssh resolve aliases, don't use our original "destination" + host_match = re.search(r"authenticity of host '([^ ]+) ", prompt) + args = {} + if fp_match and host_match: + # login.js do_hostkey_verification() expects host-key to be "hostname keytype key" + # we don't have access to the full key yet (that will be sent later as `login-data` challenge response), + # so just send a placeholder + args['host-key'] = f'{host_match.group(1)} {fp_match.group(1)} login-data' + # very oddly named, login.js do_hostkey_verification() expects the fingerprint here for user confirmation + args['default'] = fp_match.group(2) + + challenge_id = f'{os.getpid()}-{time.time()}' + challenge_prefix = f'X-Conversation {challenge_id}' + challenge = challenge_prefix + ' ' + base64.b64encode(prompt.encode()).decode() response = await self.router.request_authorization(challenge, + timeout=None, messages=messages, prompt=prompt, hint=hint, - echo=False) + echo=False, + **args) - b64 = response.removeprefix('X-Conversation -').strip() + if not response.startswith(challenge_prefix): + raise CockpitProtocolError( + f"AuthorizeResponder: response {response} does not match challenge {challenge_prefix}") + b64 = response.removeprefix(challenge_prefix).strip() response = base64.b64decode(b64.encode()).decode() logger.debug('Returning a %d chars response', len(response)) return response @@ -178,8 +217,11 @@ def python_interpreter(comment: str) -> tuple[Sequence[str], Sequence[str]]: def via_ssh(cmd: Sequence[str], dest: str, ssh_askpass: Path, *ssh_opts: str) -> tuple[Sequence[str], Sequence[str]]: host, _, port = dest.rpartition(':') - # catch cases like `host:123` but not cases like `[2001:abcd::1] - if port.isdigit(): + # catch cases like `host:123` but not cases like `[2001:abcd::1]` or `::1` + if port.isdigit() and not host.endswith(':'): + # strip off [] IPv6 brackets + if host.startswith('[') and host.endswith(']'): + host = host[1:-1] destination = ['-p', port, host] else: destination = [dest] @@ -210,6 +252,8 @@ class SshPeer(Peer): def __init__(self, router: Router, destination: str, args: argparse.Namespace): self.destination = destination self.always = args.always + self.tmpdir = tempfile.TemporaryDirectory() + self.known_hosts_file = Path(self.tmpdir.name) / 'user-known-hosts' super().__init__(router) async def do_connect_transport(self) -> None: @@ -234,17 +278,21 @@ async def connect_from_flatpak(self) -> None: async def connect_from_bastion_host(self) -> None: basic_password = None - username_opts = [] + known_hosts = None + # right now we open a new ssh connection for each auth attempt + args = ['-o', 'NumberOfPasswordPrompts=1'] # do we have user/password (Basic auth) from the login page? auth = await self.router.request_authorization_object("*") response = get_str(auth, 'response') if response.startswith('Basic '): - user, basic_password = base64.b64decode(response.split(' ', 1)[1]).decode().split(':', 1) + decoded = base64.b64decode(response[6:]).decode() + user_password, _, known_hosts = decoded.partition('\0') + user, _, basic_password = user_password.partition(':') if user: # this can be empty, i.e. auth is just ":" logger.debug("got username %s and password from Basic auth", user) - username_opts = ['-l', user] + args += ['-l', user] # We want to run a python interpreter somewhere... cmd, env = python_interpreter('cockpit-bridge') @@ -256,12 +304,16 @@ async def connect_from_bastion_host(self) -> None: if not ssh_askpass.exists(): logger.error("Could not find cockpit-askpass helper at %r", askpass) - cmd, env = via_ssh(cmd, self.destination, ssh_askpass, *username_opts) + if known_hosts is not None: + self.known_hosts_file.write_text(known_hosts) + args += ['-o', f'UserKnownHostsfile={self.known_hosts_file!s}'] + cmd, env = via_ssh(cmd, self.destination, ssh_askpass, *args) + await self.boot(cmd, env, basic_password) async def boot(self, cmd: Sequence[str], env: Sequence[str], basic_password: 'str | None' = None) -> None: beiboot_helper = BridgeBeibootHelper(self) - agent = ferny.InteractionAgent([AuthorizeResponder(self.router), beiboot_helper]) + agent = ferny.InteractionAgent([AuthorizeResponder(self.router, basic_password), beiboot_helper]) logger.debug("Launching command: cmd=%s env=%s", cmd, env) transport = await self.spawn(cmd, env, stderr=agent, start_new_session=True) @@ -330,6 +382,13 @@ async def run(args) -> None: try: message = dict(await bridge.ssh_peer.start()) + if bridge.ssh_peer.known_hosts_file.exists(): + bridge.write_control( + command='authorize', challenge='x-login-data', cookie='-', login_data={ + 'known-hosts': bridge.ssh_peer.known_hosts_file.read_text() + } + ) + # See comment in do_init() above: we tell cockpit-ws that we support # this and then handle it ourselves when we get the init message. capabilities = message.setdefault('capabilities', {}) @@ -359,7 +418,12 @@ async def run(args) -> None: problem = 'unknown-host' else: problem = 'internal-error' - bridge.write_control(command='init', problem=problem, message=str(error)) + # if the user confirmed a new SSH host key before the error, tell the UI + if bridge.ssh_peer.known_hosts_file.exists(): + bridge.write_control(command='init', problem=problem, message=str(error), + known_hosts=bridge.ssh_peer.known_hosts_file.read_text()) + else: + bridge.write_control(command='init', problem=problem, message=str(error)) return except CockpitProblem as exc: logger.debug("CockpitProblem: %s", exc) diff --git a/test/browser/run-test.sh b/test/browser/run-test.sh index f862ca344cc1..b1f6f8eb963d 100644 --- a/test/browser/run-test.sh +++ b/test/browser/run-test.sh @@ -91,6 +91,7 @@ if [ "$PLAN" = "main" ]; then TestLogin.testFailingWebsocketSafari TestLogin.testFailingWebsocketSafariNoCA TestLogin.testLogging + TestLogin.testLoginSshBeiboot TestLogin.testRaw TestLogin.testServer TestLogin.testUnsupportedBrowser diff --git a/test/verify/check-client b/test/verify/check-client index 80c0c8b6febe..aa8a81600b73 100755 --- a/test/verify/check-client +++ b/test/verify/check-client @@ -58,6 +58,17 @@ LoginTo = true [Ssh-Login] Command = /usr/bin/env python3 -m cockpit.beiboot """) + # use the connect_from_flatpak() code path of beiboot.py + self.m_client.write("/.flatpak-info", "test mock") + self.m_client.write("/usr/local/bin/flatpak-spawn", """#!/bin/sh -eux +if [ "$1" = "--host" ]; then shift; continue; fi +while [ "${1#--env=}" != "$1" ]; do + export "${1#--env=}" + shift + continue +done +exec "$@" +""", perm="0755") def check_login(self, expected_user): b = self.browser @@ -107,8 +118,9 @@ Command = /usr/bin/env python3 -m cockpit.beiboot b.wait_not_visible("#recent-hosts-list") b.set_val("#server-field", "10.111.113.2") b.click("#login-button") - b.wait_in_text("#conversation-group", "authenticity of host '10.111.113.2") - b.set_val("#conversation-input", "yes") + # accept unknown host key + b.wait_in_text("#hostkey-message-1", "10.111.113.2") + b.wait_in_text("#hostkey-fingerprint", "SHA256:") b.click("#login-button") b.wait_text("#conversation-prompt", "admin@10.111.113.2's password: ") b.set_val("#conversation-input", "foobar") diff --git a/test/verify/check-static-login b/test/verify/check-static-login index a83552d6bc4f..a5b6e9883e84 100755 --- a/test/verify/check-static-login +++ b/test/verify/check-static-login @@ -954,6 +954,124 @@ 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): + 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): + b.open("/") + b.set_val('#login-user-input', user) + b.set_val('#login-password-input', password) + b.click("#show-other-login-options") + b.set_val("#server-field", server or my_ip) + b.click("#login-button") + if expect_hostkey: + b.wait_in_text("#hostkey-message-1", server or my_ip) + b.wait_in_text("#hostkey-fingerprint", "SHA256:") + b.click("#login-button") + + def check_no_processes(): + m.execute(f"while pgrep -af '[s]sh .* {my_ip}' >&2; do sleep 1; done") + m.execute("while pgrep -af '[c]ockpit.beiboot' >&2; do sleep 1; done") + + def check_session(server=None): + b.wait_visible('#content') + b.enter_page('/system') + b.wait_visible('.system-information') + m.execute(f"pgrep -af '[s]sh .* -l admin .*{server or my_ip}'") + m.execute(f"pgrep -af '[c]ockpit.beiboot.*{server or my_ip}'") + b.logout() + check_no_processes() + + def check_store_hostkey(expected: str) -> None: + db_hostkey = b.eval_js(f'JSON.parse(window.localStorage.getItem("known_hosts"))["{my_ip}"]') + if m.image.startswith('debian') or m.image.startswith('ubuntu'): + # these OSes encrypt host keys, so don't compare them + db_hostkey = ' '.join(db_hostkey.split()[1:]) + expected = ' '.join(expected.split()[1:]) + self.assertEqual(db_hostkey, expected) + + # successful login through SSH + try_login("admin", "foobar", expect_hostkey=True) + check_session() + # wrote full host key into session storage + # some OpenSSH versions add a comment here, filter that out + real_hostkey = m.execute(f"ssh-keyscan -t ssh-ed25519 {my_ip} | grep -v ^#") + check_store_hostkey(real_hostkey) + + # host key is now known, but wrong password + try_login("admin", "wrong") + b.wait_text("#login-error-message", "Wrong user name or password") + check_no_processes() + # goes back to normal login form + b.wait_visible('#login-user-input') + + # colliding usernames; user names in "Connect to:" are *not* supported, + # but pin down the behaviour + try_login("admin", "foobar", server=f"other@{my_ip}") + check_session() + + # IPv6 + try_login("admin", "foobar", server="::1", expect_hostkey=True) + check_session(server="::1") + try_login("admin", "foobar", server="[::1]:22") + check_session(server="::1") + + # empty password + self.write_file("/etc/ssh/sshd_config.d/01-empty-password.conf", "PermitEmptyPasswords yes", + post_restore_action=self.restart_sshd) + m.execute(self.restart_sshd) + m.execute("passwd -d admin") + try_login("admin", "") + check_session() + m.execute("echo 'admin:foobar' | chpasswd") + if m.image == 'arch': + # HACK: PermitEmptyPasswords somehow breaks regular password auth + m.execute("rm /etc/ssh/sshd_config.d/01-empty-password.conf") + m.execute(self.restart_sshd) + + # non-matching host key + bad_key = "172.27.0.15 ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAINKEIUL5s7ebg5Y6JdYNq4+mAaaaaaP1VRBDUiVdHT3R" + b.eval_js(f"""window.localStorage.setItem('known_hosts', '{{"{my_ip}": "{bad_key}"}}')""") + + # first try with wrong credentials + try_login("admin", "wrong") + b.wait_text("#hostkey-title", f"{my_ip} key changed") + b.wait_in_text("#hostkey-fingerprint", "SHA256:") + b.click("#login-button.pf-m-danger") + b.wait_text("#login-error-message", "Authentication failed") + check_no_processes() + # new host key is accepted and updated in db + # confirmation replaces (not amends) known key + check_store_hostkey(real_hostkey) + + # now with correct credentials, does not ask for host key any more + try_login("admin", "foobar") + check_session() + check_store_hostkey(real_hostkey) + + # good credentials with bad key + b.eval_js(f"""window.localStorage.setItem('known_hosts', '{{"{my_ip}": "{bad_key}"}}')""") + try_login("admin", "foobar") + b.wait_text("#hostkey-title", f"{my_ip} key changed") + b.wait_in_text("#hostkey-fingerprint", "SHA256:") + b.click("#login-button.pf-m-danger") + check_session() + check_store_hostkey(real_hostkey) + if __name__ == '__main__': testlib.test_main()