From 48afe00ab2a7ff98a310f4df482d9b555fc3ec0d Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Tue, 3 Sep 2024 07:27:11 +0200 Subject: [PATCH 1/9] doc: Clean up Ssh-Login in authentication.md Consistently spell the section "Ssh-Login". This makes it easier to grep. Document the current behaviour that the specified `host` will not have its host key checked. Fix indentation of the `connectToUnknownHosts` paragraph. --- doc/authentication.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) 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, From 38140e6fd8680c52a15484add058a7e4f0290ddb Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 2 Sep 2024 13:32:23 +0200 Subject: [PATCH 2/9] login: Refactor host key database API We only ever need to get or update the keys for a particular host. So replace the current direct database manipulation with `{get,set}_hostkeys()`. This will come in handy in the next commits when we need to update the host keys from multiple places. --- pkg/static/login.js | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/pkg/static/login.js b/pkg/static/login.js index 997755bfa5db..65530cae3a31 100644 --- a/pkg/static/login.js +++ b/pkg/static/login.js @@ -766,8 +766,14 @@ function debug(...args) { } } - function set_known_hosts_db(db) { + function get_hostkeys(host) { + return get_known_hosts_db()[host]; + } + + function set_hostkeys(host, keys) { try { + const db = get_known_hosts_db(); + db[host] = keys; localStorage.setItem("known_hosts", JSON.stringify(db)); } catch (ex) { console.warn("Can't write known_hosts database to localStorage", ex); @@ -775,20 +781,20 @@ function debug(...args) { } function do_hostkey_verification(data) { - const key_db = get_known_hosts_db(); const key = data["host-key"]; const key_host = key.split(" ")[0]; const key_type = key.split(" ")[1]; + const db_keys = get_hostkeys(key_host); - if (key_db[key_host] == key) { + if (db_keys == key) { debug("do_hostkey_verification: received key matches known_hosts database, auto-accepting fingerprint", data.default); converse(data.id, data.default); return; } - if (key_db[key_host]) { + if (db_keys) { debug("do_hostkey_verification: received key fingerprint", data.default, "for host", key_host, - "does not match key in known_hosts database:", key_db[key_host], "; treating as changed"); + "does not match key in known_hosts database:", db_keys, "; treating as changed"); id("hostkey-title").textContent = format(_("$0 key changed"), login_machine); show("#hostkey-warning-group"); id("hostkey-message-1").textContent = ""; @@ -818,8 +824,7 @@ function debug(...args) { function call_converse() { id("login-button").removeEventListener("click", call_converse); login_failure(null, "hostkey"); - key_db[key_host] = key; - set_known_hosts_db(key_db); + set_hostkeys(key_host, key); converse(data.id, data.default); } @@ -828,7 +833,7 @@ function debug(...args) { show_form("hostkey"); show("#get-out-link"); - if (key_db[key_host]) { + if (db_keys) { id("login-button").classList.add("pf-m-danger"); id("login-button").classList.remove("pf-m-primary"); } From e3d5b0e33b92e1d1a00c6479e8320868f9b853bb Mon Sep 17 00:00:00 2001 From: Allison Karlitskaya Date: Tue, 20 Aug 2024 10:19:02 +0200 Subject: [PATCH 3/9] beiboot: support for temporary UserKnownHostsFile In the bastion host case, create a temporary UserKnownHostsFile for ssh and populate it with data from the authorization (if provided). On successful login, send the updated key data back to the user via the long-disused `x-login-data` mechanism. That'll get it added to the JSON blob that is sent as the reply to the `GET /login` request. On failed login, send the key data as part of the Cockpit protocol error, in a `known-hosts` attribute. Co-Authored-By: Martin Pitt --- src/cockpit/beiboot.py | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/src/cockpit/beiboot.py b/src/cockpit/beiboot.py index e4117437a4ae..5b2d61f7772a 100644 --- a/src/cockpit/beiboot.py +++ b/src/cockpit/beiboot.py @@ -22,6 +22,7 @@ import logging import os import shlex +import tempfile from pathlib import Path from typing import Dict, Iterable, Optional, Sequence @@ -210,6 +211,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 +237,20 @@ async def connect_from_flatpak(self) -> None: async def connect_from_bastion_host(self) -> None: basic_password = None - username_opts = [] + known_hosts = None + args = [] # 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,7 +262,11 @@ 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: @@ -330,6 +340,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 +376,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) From 2c15eb84929acaecbefa61f5729b77c56483d932 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Tue, 3 Sep 2024 08:48:26 +0200 Subject: [PATCH 4/9] test: Make TestClient use beiboot's "flatpak" code path Mock enough of the flatpak environment (/.flatpak-info and `flatpak-spawn` wrapper) to make beiboot.py take its `connect_from_flatpak()` code path. This makes the test more realistic. --- test/verify/check-client | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/verify/check-client b/test/verify/check-client index 80c0c8b6febe..704514c04ef9 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 From b59c5704ab870417e55029bb42b413dd52453f50 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 2 Sep 2024 15:18:53 +0200 Subject: [PATCH 5/9] beiboot: Don't retry bad password three times In bastion mode, we previously used the default 3 password prompts, and sending the same password every time. This just unnecessarily trigges `faillock` and takes too long. Do the same as in "remote channel" bridge mode (`SshPeer` in remote.py). This can be dropped if/when we rework the whole machinery to not restart ssh from scratch with each new authentication attempt, and just follow the conversation. --- src/cockpit/beiboot.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cockpit/beiboot.py b/src/cockpit/beiboot.py index 5b2d61f7772a..f3ec30dff4c2 100644 --- a/src/cockpit/beiboot.py +++ b/src/cockpit/beiboot.py @@ -238,7 +238,8 @@ async def connect_from_flatpak(self) -> None: async def connect_from_bastion_host(self) -> None: basic_password = None known_hosts = None - args = [] + # 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("*") From 0005471799e44723323033f306766154eaa55755 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Tue, 3 Sep 2024 12:50:50 +0200 Subject: [PATCH 6/9] beiboot: Use distinct X-Conversation nonces Using a constant `-` for the authorization challenge is problematic: The login page load always starts with an initial generic auth challenge, which gets cleaned up (in `cockpit_session_reset()`) after some time. When the second challenge (for e.g. an unknown host key) then uses the same `-` challenge, ws fails with "Invalid conversation token: -". Avoid this by using an *actual* nonce: Combine the pid and time to a value that is reasonably unique to avoid collisions. Also add an assertion that the returned challenge is the one that we actually sent. --- src/cockpit/beiboot.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/cockpit/beiboot.py b/src/cockpit/beiboot.py index f3ec30dff4c2..50dbb063e0a4 100644 --- a/src/cockpit/beiboot.py +++ b/src/cockpit/beiboot.py @@ -23,6 +23,7 @@ import os import shlex import tempfile +import time from pathlib import Path from typing import Dict, Iterable, Optional, Sequence @@ -36,7 +37,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 @@ -151,14 +152,19 @@ 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() + 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, messages=messages, prompt=prompt, hint=hint, echo=False) - 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 From 0b9bc9063d3e10577143b095e5c818e487652962 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Thu, 19 Sep 2024 14:39:16 +0200 Subject: [PATCH 7/9] bridge: Fix beiboot's IPv6 address parsing Teach `via_ssh()` to correctly treat IPv6 addresses like `::1`, i.e. don't consider a double :: as a port separator. For IPv6 addresses with a port, the usual syntax is `[A::B:C]:22`, but `ssh` does not recognize the brackets. Strip them off after separating the port. (Both covered by `TestLogin.testServer`) --- src/cockpit/beiboot.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/cockpit/beiboot.py b/src/cockpit/beiboot.py index 50dbb063e0a4..aecfce150128 100644 --- a/src/cockpit/beiboot.py +++ b/src/cockpit/beiboot.py @@ -185,8 +185,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] From 339a89363ad36cac9b7fd71e5bfa7080c77a303f Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Mon, 25 Sep 2023 14:58:20 +0200 Subject: [PATCH 8/9] beiboot: Send initial authorize request In order to use cockpit.beiboot as cockpit-ssh replacement from the bastion host (not Client mode) login page, it needs to consider the given username and password. cockpit-ssh sends an initial `authorize` message for that and checks for "Basic" auth. If that fails, it aborts immediately with `authentication-failed`. Implement the same in cockpit.beiboot. Note: The UI does not currently get along with multiple password attempts. Once we drop cockpit-ssh, we should fix the UI and cockpit.beiboot to behave like the flatpak, keep the initial SSH running, and just answer the "try again" prompts. Cover this in a new `TestLogin.testLoginSshBeiboot`. Once we generally replace cockpit-ssh with cockpit.beiboot, this will get absorbed by TestLogin and TestMultiMachine* and can be dropped again. --- src/cockpit/beiboot.py | 19 ++++++++- test/browser/run-test.sh | 1 + test/verify/check-static-login | 77 ++++++++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 2 deletions(-) diff --git a/src/cockpit/beiboot.py b/src/cockpit/beiboot.py index aecfce150128..87cd9aa82189 100644 --- a/src/cockpit/beiboot.py +++ b/src/cockpit/beiboot.py @@ -131,10 +131,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: # @@ -281,7 +296,7 @@ async def connect_from_bastion_host(self) -> None: 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) 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-static-login b/test/verify/check-static-login index a83552d6bc4f..dcd5000a5bbe 100755 --- a/test/verify/check-static-login +++ b/test/verify/check-static-login @@ -954,6 +954,83 @@ 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): + 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") + # ack unknown host key; FIXME: this should be a proper authorize message, not a prompt + b.wait_in_text("#conversation-prompt", "authenticity of host") + b.set_val("#conversation-input", "yes") + 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() + + # successful login through SSH + try_login("admin", "foobar") + check_session() + + # wrong password + try_login("admin", "wrong") + b.wait_in_text("#login-error-message", "Authentication failed") + 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") + 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) + if __name__ == '__main__': testlib.test_main() From db8d88c48c7eb8adbb6e56ae8dd3ffee086eac2a Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Thu, 5 Oct 2023 10:25:05 +0200 Subject: [PATCH 9/9] beiboot: Handle ssh host key prompts Stop treating host key prompts as generic conversation messages. We want the UI to handle them properly, with some verbiage/buttons and the UI/recipe for confirming/validating host keys, instead of letting the user type "yes". The login page recognizes these through the presence of the `host-key` authorize field. We can't use ferny's builtin `do_hostkey()` responder for this, as that requires `ferny.Session(handle_host_key=True)`, and that API is not flexible enough to handle our ssh command modifications and the extra beiboot_helper handler. This needs some bigger redesign, see #19668. So just recognize and parse SSH's host key prompts, and rely on our integration tests to spot breakage in future distro releases. While cockpit-ssh gave us the full host key right away during conversation, ssh (and thus ferny/beiboot) don't work that way: During conversation we only get the fingerprint. So for the sending direction, utilize the recently introduced temporary UserKnownHostsFile feature of beiboot, and send the known_hosts entry for the given host as part of the `Authorization:` header. For the receiving direction we need to handle three cases: * For a previously unknown host and a successful login, we only get the full host key as part of the GET /login's `login-data` response. So defer updating our localStorage's `known_hosts` database during the conversation until that happens. * For a failed login (like wrong password) after already confirming the key change, get the host key from the protocol error message. * ssh (and thus ferny/beiboot) don't report changed host keys as part of conversation, but immediatley abort. So remember that state in `ssh_host_key_change_host`, and re-attempt the login without sending known_hosts data. Our usual logic in `do_hostkey_verification()` will notice the change and present the correct dialog. Adjust TestLogin.testLoginSshBeiboot to only expect the host key on the first login attempt. Add testing of changed host key behaviour. --- .../flatpak/test/test-browser-login-ssh.js | 6 +- pkg/static/login.js | 94 ++++++++++++++++++- src/cockpit/beiboot.py | 19 +++- test/verify/check-client | 5 +- test/verify/check-static-login | 59 ++++++++++-- 5 files changed, 163 insertions(+), 20 deletions(-) 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/pkg/static/login.js b/pkg/static/login.js index 65530cae3a31..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) { @@ -964,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 { @@ -1043,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 87cd9aa82189..8b577c2a881a 100644 --- a/src/cockpit/beiboot.py +++ b/src/cockpit/beiboot.py @@ -21,6 +21,7 @@ import importlib.resources import logging import os +import re import shlex import tempfile import time @@ -167,14 +168,30 @@ 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 + # 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) if not response.startswith(challenge_prefix): raise CockpitProtocolError( diff --git a/test/verify/check-client b/test/verify/check-client index 704514c04ef9..aa8a81600b73 100755 --- a/test/verify/check-client +++ b/test/verify/check-client @@ -118,8 +118,9 @@ exec "$@" 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 dcd5000a5bbe..a5b6e9883e84 100755 --- a/test/verify/check-static-login +++ b/test/verify/check-static-login @@ -971,17 +971,17 @@ Command = /usr/bin/env python3 -m cockpit.beiboot """, append=True) m.start_cockpit() - def try_login(user, password, server=None): + 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") - # ack unknown host key; FIXME: this should be a proper authorize message, not a prompt - b.wait_in_text("#conversation-prompt", "authenticity of host") - b.set_val("#conversation-input", "yes") - 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") @@ -996,13 +996,25 @@ Command = /usr/bin/env python3 -m cockpit.beiboot 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") + 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) - # wrong password + # host key is now known, but wrong password try_login("admin", "wrong") - b.wait_in_text("#login-error-message", "Authentication failed") + 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') @@ -1013,7 +1025,7 @@ Command = /usr/bin/env python3 -m cockpit.beiboot check_session() # IPv6 - try_login("admin", "foobar", server="::1") + try_login("admin", "foobar", server="::1", expect_hostkey=True) check_session(server="::1") try_login("admin", "foobar", server="[::1]:22") check_session(server="::1") @@ -1031,6 +1043,35 @@ Command = /usr/bin/env python3 -m cockpit.beiboot 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()