Skip to content

Commit

Permalink
beiboot: Handle ssh host key prompts
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
martinpitt committed Sep 1, 2024
1 parent 4b43ebf commit d7a5835
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 18 deletions.
6 changes: 3 additions & 3 deletions containers/flatpak/test/test-browser-login-ssh.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
70 changes: 66 additions & 4 deletions pkg/static/login.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down Expand Up @@ -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:"
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
19 changes: 18 additions & 1 deletion src/cockpit/beiboot.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import importlib.resources
import logging
import os
import re
import shlex
import tempfile
from pathlib import Path
Expand Down Expand Up @@ -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()
Expand Down
5 changes: 3 additions & 2 deletions test/verify/check-client
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
55 changes: 47 additions & 8 deletions test/verify/check-static-login
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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')
Expand All @@ -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()

0 comments on commit d7a5835

Please sign in to comment.