From d7a583556f5d5e74ae810386c2e4c19803c42d12 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. We want to keep using/storing full host keys 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 reently 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" side we need to jump through a few hoops: * For a previously unknown host, we only get the full host key after a successful login, as part of the GET /login's `login-data` response. So defer updating our localStorage's `known_hosts` database until then. * 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. * For a changed host key where the login fails due to wrong credentials, the login page never receives the new host key whose fingerprint the user just confirmed. So we can't remember that confirmation, and the next time the user has to re-confirm the changed key. #20954 provides some required plumbing for that. In the meantime, this seems acceptable as long as cockpit-beiboot isn't the default bastion host implementation yet. 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 | 70 +++++++++++++++++-- src/cockpit/beiboot.py | 19 ++++- test/verify/check-client | 5 +- test/verify/check-static-login | 55 ++++++++++++--- 5 files changed, 137 insertions(+), 18 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 f4ebfcbf949d..54adc40f4649 100644 --- a/pkg/static/login.js +++ b/pkg/static/login.js @@ -596,10 +596,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 +639,28 @@ 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; strip off user name and port from server field + const hostname = login_machine.replace(/^.*@/, '').replace(/:[0-9]+$/, ''); + const keys = get_known_hosts_db()[hostname]; + if (keys) { + debug("call_login(): sending known_host key", keys, "for logging into", hostname); + known_hosts = '\0' + keys; + } else { + debug("call_login(): no known_hosts entry for logging into", hostname); + } + } + } + const headers = { - Authorization: "Basic " + window.btoa(utf8(user + ":" + password)), + Authorization: "Basic " + window.btoa(utf8(user + ":" + password + known_hosts)), "X-Superuser": superuser, }; // allow unknown remote hosts with interactive logins with "Connect to:" @@ -780,6 +808,7 @@ function debug(...args) { const key_host = key.split(" ")[0]; const key_type = key.split(" ")[1]; + // code path for old C cockpit-ssh, which doesn't set a known_hosts file in advance (like beiboot) if (key_db[key_host] == key) { debug("do_hostkey_verification: received key matches known_hosts database, auto-accepting fingerprint", data.default); converse(data.id, data.default); @@ -818,8 +847,17 @@ 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); + 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 + key_db[key_host] = key; + set_known_hosts_db(key_db); + debug("call_converse(): got real host key (cockpit-ssh code path) for", login_data_host); + } converse(data.id, data.default); } @@ -958,7 +996,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 { @@ -1037,6 +1085,20 @@ 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) { + const key_db = get_known_hosts_db(); + key_db[login_data_host] = hostkey; + console.debug("setup_localstorage(): updating known_hosts database for deferred host key for", login_data_host, ":", hostkey); + set_known_hosts_db(key_db); + } 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 1b157a118619..dee004f616d1 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 80c0c8b6febe..7c6ed443434a 100755 --- a/test/verify/check-client +++ b/test/verify/check-client @@ -107,8 +107,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 72397acd4fcc..5e2a23c9c637 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,33 @@ 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() + # quirk: login page does not receive and thus not store the full key with a failed login + # see https://github.com/cockpit-project/cockpit/pull/20954 for how this could eventually be fixed + # so the store keeps the old (bad) key + check_store_hostkey(bad_key) + + # now with correct credentials + try_login("admin", "foobar") + # due to the above quirk, need to re-confirm changed host key + 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() + + # confirmation replaces (not amends) known key + check_store_hostkey(real_hostkey) + if __name__ == '__main__': testlib.test_main()