From 3f56095673f2d87485d25ba3e24b6d023a8f3aff 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 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..1e40bb9b1419 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 + function parseHostname(ssh_target) { + return ssh_target.replace(/^.*@/, '').replace(/:[0-9]+$/, ''); + } + // Hide an element (or set of elements) based on a boolean // true: element is hidden, false: element is shown function hideToggle(elements, toggle) { @@ -596,10 +601,18 @@ function debug(...args) { // value of #server-field at the time of clicking "Login" let login_machine = null; + /* set by do_hostkey_verification() for a confirmed unknown host fingerprint; + * setup_localstorage() will then write the received full known_hosts entry to the known_hosts + * database for this host */ + let login_data_host = null; + /* set if our known_host database has a non-matching host key, and we re-attempt the login + * with asking the user for confirmation */ + let ssh_host_key_change_host = null; function call_login() { login_failure(null); login_machine = id("server-field").value; + login_data_host = null; const user = trim(id("login-user-input").value); if (user === "" && !environment.is_cockpit_client) { login_failure(_("User name cannot be empty")); @@ -631,8 +644,27 @@ function debug(...args) { /* Keep information if login page was used */ localStorage.setItem('standard-login', true); + let known_hosts = ''; + if (login_machine) { + if (ssh_host_key_change_host == login_machine) { + /* We came here because logging in ran into invalid-hostkey; so try the next + * round without sending the key. do_hostkey_verification() will notice the + change and show the correct dialog. */ + debug("call_login(): previous login attempt into", login_machine, "failed due to changed key"); + } else { + // If we have a known host key, send it to ssh + const keys = get_hostkeys(login_machine); + if (keys) { + debug("call_login(): sending known_host key", keys, "for logging into", login_machine); + known_hosts = keys; + } else { + debug("call_login(): no known_hosts entry for logging into", login_machine); + } + } + } + const headers = { - Authorization: "Basic " + window.btoa(utf8(user + ":" + password)), + Authorization: "Basic " + window.btoa(utf8(user + ":" + password + '\0' + known_hosts)), "X-Superuser": superuser, }; // allow unknown remote hosts with interactive logins with "Connect to:" @@ -767,13 +799,13 @@ function debug(...args) { } function get_hostkeys(host) { - return get_known_hosts_db()[host]; + return get_known_hosts_db()[parseHostname(host)]; } function set_hostkeys(host, keys) { try { const db = get_known_hosts_db(); - db[host] = keys; + db[parseHostname(host)] = keys; localStorage.setItem("known_hosts", JSON.stringify(db)); } catch (ex) { console.warn("Can't write known_hosts database to localStorage", ex); @@ -786,6 +818,7 @@ function debug(...args) { const key_type = key.split(" ")[1]; const db_keys = get_hostkeys(key_host); + // code path for old C cockpit-ssh, which doesn't set a known_hosts file in advance (like beiboot) if (db_keys == key) { debug("do_hostkey_verification: received key matches known_hosts database, auto-accepting fingerprint", data.default); converse(data.id, data.default); @@ -824,7 +857,16 @@ function debug(...args) { function call_converse() { id("login-button").removeEventListener("click", call_converse); login_failure(null, "hostkey"); - set_hostkeys(key_host, key); + if (key.endsWith(" login-data")) { + // cockpit-beiboot sends only a placeholder, defer to login-data in setup_localstorage() + login_data_host = key_host; + debug("call_converse(): got placeholder host key (beiboot code path) for", login_data_host, + ", deferring db update"); + } else { + // cockpit-ssh already sends the actual key here + set_hostkeys(key_host, key); + debug("call_converse(): got real host key (cockpit-ssh code path) for", login_data_host); + } converse(data.id, data.default); } @@ -950,6 +992,22 @@ function debug(...args) { } else { if (window.console) console.log(xhr.statusText); + /* did the user confirm a changed SSH host key? If so, update database */ + if (ssh_host_key_change_host) { + try { + const keys = JSON.parse(xhr.responseText)["known-hosts"]; + if (keys) { + debug("send_login_request(): got updated known-hosts for changed host keys of", ssh_host_key_change_host, ":", keys); + set_hostkeys(ssh_host_key_change_host, keys); + ssh_host_key_change_host = null; + } else { + debug("send_login_request():", ssh_host_key_change_host, "changed key, but did not get an updated key from response"); + } + } catch (ex) { + console.error("Failed to parse response text as JSON:", xhr.responseText, ":", JSON.stringify(ex)); + } + } + if (xhr.statusText.startsWith("captured-stderr:")) { show_captured_stderr(decodeURIComponent(xhr.statusText.replace(/^captured-stderr:/, ''))); } else if (xhr.statusText.indexOf("authentication-not-supported") > -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 6a0bd81cb602..3200314b310c 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 from pathlib import Path @@ -166,12 +167,28 @@ 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 = 'X-Conversation - ' + 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() response = base64.b64decode(b64.encode()).decode() 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 f326d800f13a..38bc94c8893a 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()