From afce95b08773630e4d9f16e86a22e03f3bb0bc21 Mon Sep 17 00:00:00 2001 From: Martin Pitt Date: Thu, 5 Oct 2023 10:25:05 +0200 Subject: [PATCH] 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 | 90 +++++++++++++++++-- src/cockpit/beiboot.py | 19 +++- test/verify/check-client | 5 +- test/verify/check-static-login | 57 ++++++++++-- 5 files changed, 158 insertions(+), 19 deletions(-) diff --git a/containers/flatpak/test/test-browser-login-ssh.js b/containers/flatpak/test/test-browser-login-ssh.js index 38e1129a97f..32e5c57eb05 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 65530cae3a3..48906ad9174 100644 --- a/pkg/static/login.js +++ b/pkg/static/login.js @@ -94,6 +94,11 @@ function debug(...args) { return document.getElementById(name); } + // strip off "user@" and "*:port" from login target (but keep two :: intact for IPv6) + function parseHostname(ssh_target) { + return ssh_target.replace(/^.*@/, '').replace(/(? -1) { @@ -964,7 +1022,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 +1111,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 ae047ad3608..f114fc645ca 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 704514c04ef..aa8a81600b7 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 f326d800f13..38bc94c8893 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", 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') @@ -1012,6 +1024,35 @@ Command = /usr/bin/env python3 -m cockpit.beiboot try_login("admin", "foobar", server=f"other@{my_ip}") check_session() + # 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()