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.

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.
  • Loading branch information
martinpitt committed Sep 20, 2024
1 parent 414003d commit 6699c33
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 20 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
94 changes: 89 additions & 5 deletions pkg/static/login.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(/(?<!:):[0-9]+$/, '')
.replace(/^\[/, '')
.replace(/\]$/, '');
}

// Hide an element (or set of elements) based on a boolean
// true: element is hidden, false: element is shown
function hideToggle(elements, toggle) {
Expand Down Expand Up @@ -596,10 +605,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 +648,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:"
Expand Down Expand Up @@ -767,13 +803,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);
Expand All @@ -786,6 +822,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);
Expand Down Expand Up @@ -824,7 +861,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);
}

Expand Down Expand Up @@ -950,6 +996,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) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
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
import time
Expand Down Expand Up @@ -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(
Expand Down
5 changes: 3 additions & 2 deletions test/verify/check-client
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
59 changes: 50 additions & 9 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", 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")
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 @@ -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")
Expand All @@ -1027,6 +1039,35 @@ Command = /usr/bin/env python3 -m cockpit.beiboot
check_session()
m.execute("echo 'admin:foobar' | chpasswd")

# 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()

0 comments on commit 6699c33

Please sign in to comment.