diff --git a/docs/devices.md b/docs/devices.md index d368f4c..6933381 100644 --- a/docs/devices.md +++ b/docs/devices.md @@ -104,7 +104,9 @@ connection is an SSH ControlMaster that every command shares; when it dies (the headset slept or left the network) the connector notices and starts again. On Windows, where OpenSSH can't share a connection, the same handshake runs once and each command then connects on its own; a command that can't reach the -headset makes the connector start again. +headset makes the connector start again. There ssh's stderr goes to a file that +the connector reads as it grows, not a pipe: Windows' OpenSSH can block writing +to a redirected pipe. Once connected, every `ssh`, `scp` and `rsync` the app runs gets `-o HostName=
-o HostKeyAlias=frame-control- @@ -121,25 +123,38 @@ lease that moved) would look like a new host there. The app keeps one known_host file per headset instead, so saving or forgetting one headset's key never touches another's: a different device answering at one of its addresses is refused, and the pill says so. A headset's first connection trusts -the key it shows, as Set Up Connection does: until a key is saved, every ssh -command gets `StrictHostKeyChecking=accept-new` (the first contact saves it, -whichever command makes it), and from then on `StrictHostKeyChecking=yes`. -`accept-new` never replaces a saved key, so it can't hide a change. +the key it shows, as Set Up Connection does: while a headset has never had an +identity here, every ssh command gets `StrictHostKeyChecking=accept-new` (the +first contact saves it, whichever command makes it), and from then on +`StrictHostKeyChecking=yes`. `accept-new` never replaces a saved key, so it +can't hide a change. The saved key's fingerprint is also recorded in +`devices.json` (`identity`). A headset counts as having had an identity if one is +recorded, if its pin file exists at all, or if a connection to it ever worked; then +a pin file that's deleted, empty, unreadable or not parseable **fails closed** +(*Checking the headset's identity* fails, "missing or can't be read") rather than +trusting whatever answers, and only **Check its key…** saves a key again. It +says when the key it reads matches the recorded fingerprint. **When the key changes** (SteamOS reinstalled or reset, a new headset at the same address, or a different device answering there), nothing is accepted by itself. The connection fails at *Checking the headset's identity*; the banner and the headset on the Devices tab say so, with **Check the new key…** (also **Check its key…** on the Devices tab at any time). That reads the key the -headset shows now, with ssh, a throwaway known_hosts file and no login -(`PreferredAuthentications=none`, so nothing of this computer's is offered to a -device that may not be the headset), and shows its SHA256 fingerprint next to +headset shows now, and nothing more: ssh writes it to a throwaway known_hosts +file during the key exchange, before any login, and is stopped as soon as it +has. Every other kind of contact is ruled out on the command line, ahead of +anything `~/.ssh/config` says: no authentication of any kind, no agent +(`IdentityAgent=none`, `ForwardAgent=no`), no X11 or port forwarding, no local +or remote command (`-N`), no terminal, and `-F none` unless the alias goes +through a jump host. It shows the key's SHA256 fingerprint next to the saved one, with the command that prints it on the headset (`ssh-keygen -lf /etc/ssh/ssh_host_ed25519_key.pub`, in Konsole on its desktop). **This is my headset: trust its new key** then saves exactly that key, replacing the old one, and reconnects. The server keeps the key it showed for 10 minutes -and saves it only if the page sends back the same fingerprint; it never saves -whatever answers next. Set Up Connection can't fix a changed key: it checks your +and saves it only if the page sends back the same fingerprint, and only while +the headset's port, user and that address are unchanged; it never saves whatever +answers next. The API's old `forget-identity`, which deleted the pin so that the +next contact saved any key unseen, is refused. Set Up Connection can't fix a changed key: it checks your own `known_hosts`, not the headset's file. **When the headset refuses this computer's key** (`Permission denied @@ -168,7 +183,7 @@ All under the usual `/api/` guards (loopback `Host`, `X-Frame-UI` header). | `GET /api/devices` | Headsets, the current network, known networks, the next free alias | | `GET /api/devices/tailscale?id=` | Tailscale peers, likely headsets first | | `GET /api/devices/mdns?id=` | Headsets found on this network | -| `POST /api/devices` | `{"action": ...}`: `use`, `update` (name, user, port), `remove`, `address-add`, `address-update`, `address-remove`, `address-move`, `test`, `forget-identity`, `identity-scan` (read the key the headset shows: `scan` with `type`, `fingerprint`, `host`, `saved`, `matches`), `identity-trust` (`fingerprint`: save the key `identity-scan` showed), `name-network`, `setup` (alias, optional host), `retry` | +| `POST /api/devices` | `{"action": ...}`: `use`, `update` (name, user, port), `remove`, `address-add`, `address-update`, `address-remove`, `address-move`, `test`, `identity-scan` (read the key the headset shows: `scan` with `type`, `fingerprint`, `host`, `saved`, `recorded`, `matches`, `recorded_match`), `identity-trust` (`fingerprint`: save the key `identity-scan` showed), `forget-identity` (refused: use the two before), `name-network`, `setup` (alias, optional host), `retry` | Every host, alias and user is checked against strict patterns before it's stored, because they end up in ssh arguments and `~/.ssh/config`; nothing goes @@ -185,7 +200,7 @@ share the format later (it still connects to one host; see ```json {"version": 1, "active": "f67f8b7e", "devices": [{"id": "f67f8b7e", "name": "Steam Frame", "alias": "frame", "user": "steamos", "port": 22, - "identity_files": ["~/.ssh/id_ed25519_frame"], + "identity_files": ["~/.ssh/id_ed25519_frame"], "identity": ["SHA256:…"], "addresses": [{"host": "frame.local", "kind": "mdns", "label": "", "networks": ["n-e0998baa61"], "last_ok": 1790593550.4, "last_rtt_ms": 0.9}]}], "networks": {"n-e0998baa61": {"name": "Home Wi-Fi", "ssid": null, "gateway": "192.168.1.1", diff --git a/tests/fakessh/ssh b/tests/fakessh/ssh index 8f721c2..89c2740 100755 --- a/tests/fakessh/ssh +++ b/tests/fakessh/ssh @@ -8,7 +8,9 @@ every call is appended to $FAKESSH_LOG as a JSON line. POSIX only. With $FAKESSH_PINS set, host keys are checked as OpenSSH does against UserKnownHostsFile under HostKeyAlias: "wrong" is then just a different key from the others' (a reinstalled headset), StrictHostKeyChecking=yes refuses a key not saved, accept-new saves a first one, and a saved key -that differs is refused. Like ssh, the first value of an option counts.""" +that differs is refused. "evil" is a device with its own key that lets anyone in without +authentication ("none") and asks for the agent: it records in $FAKESSH_DIR what it got +(agent-forwarded, command-ran). Like ssh, the first value of an option counts.""" import base64 import hashlib import json @@ -24,7 +26,7 @@ hosts = json.loads(os.environ.get("FAKESSH_HOSTS", "{}")) opts = {} i = 0 while i < len(args) and args[i].startswith("-"): - if args[i] in ("-o", "-O", "-p", "-l"): + if args[i] in ("-o", "-O", "-p", "-l", "-F"): key = args[i] val = args[i + 1] if key == "-o": @@ -37,6 +39,12 @@ while i < len(args) and args[i].startswith("-"): opts[args[i]] = True i += 1 alias = args[i] if i < len(args) else "" +command = args[i + 1:] +if opts.get("-F") != "none": + # $FAKESSH_CONFIG: what ~/.ssh/config says (JSON, lower-case option -> value), below the + # command line's options, as ssh has it; `-F none` reads no config. + for k, v in json.loads(os.environ.get("FAKESSH_CONFIG", "{}")).items(): + opts.setdefault(k, v) host = opts.get("hostname", alias).replace("%%", "%") marker = os.path.join(os.environ["FAKESSH_DIR"], "master-" + host.replace("/", "_")) say = lambda s: (sys.stderr.write(s + "\n"), sys.stderr.flush()) @@ -62,11 +70,11 @@ if what == "slow": say(f"debug1: Authenticating to {host}:22 as '{opts.get('user', 'tester')}'") if os.environ.get("FAKESSH_PINS") and "hostkeyalias" in opts and "userknownhostsfile" in opts: name, path = opts["hostkeyalias"], os.path.expanduser(opts["userknownhostsfile"]) - blob = base64.b64encode(b"fake-" + (b"other" if what == "wrong" else b"headset") + b"-key").decode() + blob = base64.b64encode(b"fake-" + {"wrong": b"other", "evil": b"evil"}.get(what, b"headset") + b"-key").decode() fp = base64.b64encode(hashlib.sha256(base64.b64decode(blob)).digest()).decode().rstrip("=") say(f"debug1: Server host key: ssh-ed25519 SHA256:{fp}") try: - saved = [ln.split() for ln in open(path).read().splitlines() if ln.strip()] + saved = [ln.split() for ln in open(path, errors="replace").read().splitlines() if ln.strip()] except OSError: saved = [] saved = [f[2] for f in saved if len(f) >= 3 and name in f[0].split(",")] @@ -90,6 +98,16 @@ if os.environ.get("FAKESSH_PINS") and "hostkeyalias" in opts and "userknownhosts else: say(f"debug1: Host '{name}' is known and matches the ED25519 host key.") say("debug1: Next authentication method: publickey") + if what == "evil": + say(f'Authenticated to {host} ([127.0.0.1]:22) using "none".') + got = os.path.join(os.environ["FAKESSH_DIR"], "evil-") + if opts.get("forwardagent", "no") != "no" and opts.get("identityagent", "") != "none": + open(got + "agent-forwarded", "w").close() + if "-N" not in opts and (command or opts.get("remotecommand", "none") != "none"): + open(got + "command-ran", "w").close() + sys.exit(0) + time.sleep(30) # -N: holds the session open + sys.exit(0) if opts.get("preferredauthentications") == "none": say(f"tester@{host}: Permission denied (publickey,password).") sys.exit(255) diff --git a/tests/test_link.py b/tests/test_link.py index a8a57fd..1809ace 100644 --- a/tests/test_link.py +++ b/tests/test_link.py @@ -291,15 +291,169 @@ class Connecting(unittest.TestCase): self.assertIn("Permanently added", r.stderr) self.assertTrue(fd.pinned(d["id"])) self.assertIn("StrictHostKeyChecking=yes", self.link.host_opts(d, "127.0.0.1")) # ...then it's checked - fd.forget_pin(d["id"]) + self.assertEqual(self.reg.get(d["id"])["identity"], [k[1] for k in fd.pinned_keys(d["id"])]) # and recorded + # The connector's first handshake, for another new headset. + self.reg.remove_device(d["id"]) + d = self.device("localhost") self.link.connect(["start"]) s = self.link.snapshot() self.assertEqual(s["phase"], "connected", s["error"]) self.assertTrue(fd.pinned(d["id"])) + self.assertTrue(self.reg.get(d["id"])["identity"]) alias, opts = self.routes[-1] self.assertIn("StrictHostKeyChecking=yes", opts) # every command after it checks strictly self.assertNotIn("StrictHostKeyChecking=accept-new", opts) + def lose_pin(self, device_id, how): + f = fd.known_hosts(device_id) + if how == "deleted": + f.unlink() + elif how == "empty": + f.write_text("") + elif how == "unreadable": + f.chmod(0) + self.addCleanup(lambda: f.exists() and f.chmod(0o600)) + elif how == "another alias": + f.write_text(f.read_text().replace(f"frame-control-{device_id}", "frame-control-someone-else")) + elif how == "not utf-8": + f.write_bytes(b"\xff\xfe" + f.read_bytes()) + + def test_a_lost_or_unusable_pin_fails_closed(self): + # Review pr85-review-r1: accept-new can't tell a new headset from one whose saved key + # went missing; real OpenSSH then trusted a replacement key. Once a headset had an + # identity, nothing is trusted again until the person checks it. + self.pins() + cases = ["deleted", "empty", "another alias", "not utf-8"] + if os.name == "posix" and os.geteuid() != 0: + cases.append("unreadable") # root reads anything + for n, how in enumerate(cases): + with self.subTest(how=how): + self.hosts({"localhost": "ok"}) + d = self.reg.add_device(f"frame-l{n}", port=self.port, hosts=[]) + self.reg.add_address(d["id"], "localhost", kind="lan") + self.link.override = None + self.reg.set_active(d["id"]) + self.link.connect(["switch"]) + self.assertEqual(self.link.snapshot()["phase"], "connected") + self.link.close_master() + d = self.reg.get(d["id"]) + self.lose_pin(d["id"], how) + self.hosts({"localhost": "wrong"}) # another device, or a reinstalled headset + r = self.command(d) + self.assertEqual(r.returncode, 255, r.stderr) + self.assertIn("Host key verification failed", r.stderr) + self.assertNotIn("Permanently added", r.stderr) + self.link.connect(["dropped"]) + s = self.link.snapshot() + self.assertEqual((s["phase"], s["error"]["stage"]), ("failed", "identity")) + self.assertEqual(s["error"]["hostkey"]["state"], "missing") + self.assertIn("missing or can't be read", s["error"]["message"]) + view = next(x for x in fl.devices_view(self.link)["devices"] if x["id"] == d["id"]) + self.assertTrue(view["identity_lost"]) + scan = fl.devices_action(self.link, {"action": "identity-scan", "id": d["id"]}, None)["scan"] + self.assertFalse(scan["matches"] or scan["recorded_match"]) # not the key it had + self.hosts({"localhost": "ok"}) + scan = fl.devices_action(self.link, {"action": "identity-scan", "id": d["id"]}, None)["scan"] + self.assertTrue(scan["recorded_match"]) # its own key, as recorded + self.assertFalse(scan["matches"]) # but not usable from the file: trusting restores it + fl.devices_action(self.link, {"action": "identity-trust", "id": d["id"], + "fingerprint": scan["fingerprint"]}, None) + self.link.connect(["switch"]) + self.assertEqual(self.link.snapshot()["phase"], "connected", self.link.snapshot()["error"]) + self.link.close_master() + + def test_a_key_check_only_reads_the_key(self): + # Review pr85-review-r1: with ForwardAgent yes in ~/.ssh/config, a device that accepts + # "none" got the agent and ran a command before anyone trusted it. + self.pins() + d = self.device("localhost") + self.hosts({"localhost": "evil"}) + os.environ["FAKESSH_CONFIG"] = json.dumps({"forwardagent": "yes", "remotecommand": "touch /tmp/x", + "permitlocalcommand": "yes", "requesttty": "force"}) + self.addCleanup(os.environ.pop, "FAKESSH_CONFIG", None) + for proxied in (False, True): + with self.subTest(proxied=proxied), \ + mock.patch.object(fl, "ssh_g", return_value=("localhost", self.port, "tester", proxied)): + t0 = time.monotonic() + scan = fl.devices_action(self.link, {"action": "identity-scan", "id": d["id"]}, None)["scan"] + self.assertLess(time.monotonic() - t0, 10) # stopped once it had the key, not left open + self.assertTrue(scan["fingerprint"].startswith("SHA256:")) + self.assertEqual([p.name for p in self.dir.iterdir() if p.name.startswith("evil-")], []) + call = [c for c in self.calls() if "PreferredAuthentications=none" in c][-1] + self.assertEqual("-F" in call and call[call.index("-F") + 1] == "none", not proxied) + self.assertIn("-N", call) + for o in ("ForwardAgent=no", "IdentityAgent=none", "ClearAllForwardings=yes", "RemoteCommand=none", + "PermitLocalCommand=no", "RequestTTY=no", "PubkeyAuthentication=no"): + self.assertIn(o, call) + self.assertLess(call.index("StrictHostKeyChecking=accept-new"), call.index("HostKeyAlias=frame-control-" + + d["id"])) + self.assertFalse(fd.pinned(d["id"])) + self.assertEqual(sorted(p.name for p in fd.known_hosts(d["id"]).parent.iterdir()), []) # no scan files + + def test_a_key_read_before_the_address_or_port_changed_isnt_trusted(self): + self.pins() + d = self.device("localhost") + self.hosts({"localhost": "ok"}) + for change in ({"action": "update", "id": d["id"], "port": self.port + 1}, + {"action": "address-remove", "id": d["id"], "host": "localhost"}): + with self.subTest(change=change["action"]): + scan = fl.devices_action(self.link, {"action": "identity-scan", "id": d["id"]}, None)["scan"] + fl.devices_action(self.link, change, None) + with self.assertRaises(fd.DeviceError): + fl.devices_action(self.link, {"action": "identity-trust", "id": d["id"], + "fingerprint": scan["fingerprint"]}, None) + self.assertFalse(fd.pinned(d["id"])) + self.reg.update_device(d["id"], port=self.port) + if not self.reg.get(d["id"])["addresses"]: + self.reg.add_address(d["id"], "localhost", kind="lan") + + def test_forgetting_an_identity_is_refused(self): + self.pins() + d = self.device("localhost") + self.hosts({"localhost": "ok"}) + self.link.connect(["start"]) + saved = fd.known_hosts(d["id"]).read_text() + with self.assertRaises(fd.DeviceError) as caught: + fl.devices_action(self.link, {"action": "forget-identity", "id": d["id"]}, None) + self.assertIn("identity-scan", str(caught.exception)) + self.assertEqual(fd.known_hosts(d["id"]).read_text(), saved) + + def test_without_a_shared_connection_ssh_writes_to_a_file_not_a_pipe(self): + # Windows: no ControlMaster, and its OpenSSH can block writing to a stderr pipe. + self.pins() + tmp = self.dir / "tmp" + tmp.mkdir() + link = fl.Link(self.reg, env_alias=None, mux_base=["ssh", "-o", "BatchMode=yes"], control=None, + apply=lambda *a: None, explain=explain) + self.addCleanup(link.stop) + real = subprocess.Popen + seen = [] + + def popen(argv, **kw): + if "-v" in argv: # the handshake (run_ssh calls go through Popen too) + seen.append(kw.get("stderr")) + return real(argv, **kw) + with mock.patch.object(tempfile, "tempdir", str(tmp)), mock.patch.object(fl.subprocess, "Popen", popen): + d = self.device("localhost") + self.hosts({"localhost": "ok"}) + link.connect(["start"]) + s = link.snapshot() + self.assertEqual(s["phase"], "connected", s["error"]) + self.assertEqual([st["state"] for st in s["stages"]], ["done"] * 5) + self.assertIn("saved this headset's identity", next(st for st in s["stages"] if st["id"] == "identity")["detail"]) + self.reg.update_device(d["id"], user="nobody") + self.hosts({"localhost": "denied"}) + link.connect(["switch"]) + s = link.snapshot() + self.assertEqual((s["phase"], s["error"]["stage"], s["error"].get("auth")), ("failed", "login", "denied")) + self.assertTrue(seen and all(isinstance(x, int) for x in seen)) # a file descriptor, never PIPE + self.assertNotIn(subprocess.PIPE, seen) + for _ in range(50): # the reader removes its file once ssh has exited + if not list(tmp.iterdir()): + break + time.sleep(0.05) + self.assertEqual(list(tmp.iterdir()), []) + def test_a_changed_key_is_shown_never_accepted_and_trusted_on_request(self): # 0.4.2 on Linux and Windows: "WARNING: REMOTE HOST IDENTIFICATION HAS CHANGED!", with # "Run Set Up Connection again" as the advice, which can't replace the saved key. @@ -609,7 +763,7 @@ class Connecting(unittest.TestCase): {"action": "update", "id": d["id"], "port": 2222}, {"action": "address-remove", "id": d["id"], "host": "localhost"}, {"action": "address-update", "id": d["id"], "host": "localhost", "newHost": "127.0.0.1"}, - {"action": "forget-identity", "id": d["id"]}): + {"action": "identity-trust", "id": d["id"], "fingerprint": "SHA256:x"}): with self.assertRaises(fd.DeviceError, msg=body): fl.devices_action(self.link, body, open_setup=None, busy=lambda: 1) # Renaming, or changing another headset, is fine. diff --git a/ui/frame_devices.py b/ui/frame_devices.py index 1929fe9..69d89b9 100644 --- a/ui/frame_devices.py +++ b/ui/frame_devices.py @@ -437,6 +437,7 @@ def _write_pin(device_id, lines): fd_, tmp = tempfile.mkstemp(prefix=".seed-", dir=str(pin_dir())) with os.fdopen(fd_, "w", encoding="utf-8") as fh: fh.write("\n".join(dict.fromkeys(lines)) + "\n") + frame_host.make_private(tmp) # Windows: this user only, not the folder's inherited ACL os.replace(tmp, target) # whole file at once: ssh never sees half of it @@ -487,9 +488,26 @@ def scan_file(device_id): headset's key to, without touching the saved one. `~` as in known_hosts_opt.""" name = f".scan-{device_id}-{secrets.token_hex(4)}" path = pin_dir() / name + path.touch(**({} if frame_host.WINDOWS else {"mode": 0o600})) + frame_host.make_private(path) return path, (str(path) if os.environ.get("FRAME_CONTROL_SSH_DIR") else f"~/.ssh/{PIN_DIR}/{name}") +def identity_established(device): + """Whether this headset ever had an SSH identity here: one recorded in devices.json, a + pin file at all (even empty, unreadable or not parseable), or a connection that worked + (which saved one). Then a missing or unusable pin fails closed, needing Check its key; + only a headset with none of these may save the first key it meets (accept-new).""" + if device.get("identity"): + return True + try: + if os.path.lexists(known_hosts(device["id"])): + return True + except OSError: + return True + return any(a.get("last_ok") for a in device.get("addresses") or ()) + + def forget_pin(device_id): """Drop a headset's saved key (the headset was removed, or the API's forget-identity). The next connection trusts whatever key the headset shows, as a first connection does; @@ -777,6 +795,21 @@ class Registry: a["last_rtt_ms"] = rtt_ms self.save() + def record_identity(self, device_id, fingerprints): + """Remember the fingerprints of the key saved for a headset, so that losing the pin + file later can't turn into trusting whatever answers (identity_established).""" + fingerprints = [f for f in fingerprints if isinstance(f, str) and f.startswith("SHA256:")][:8] + if not fingerprints: + return + with self._changing(): + try: + d = self._find(device_id) + except DeviceError: + return + if d.get("identity") != fingerprints: + d["identity"] = fingerprints + self.save() + def undismiss(self, alias): """Set Up Connection is about to run for this alias: import its block again.""" with self._changing(): @@ -860,15 +893,15 @@ class Registry: d["addresses"].append(dict(new_address(host), label="From Set Up Connection")) d["config_host"] = host changed = True - if seed and host: - seed_pin(d["id"], [host], b["port"]) + if seed and host and seed_pin(d["id"], [host], b["port"]): + d["identity"] = [f for _, f in pinned_keys(d["id"])][:8] or d.get("identity") elif host and host != d.get("config_host"): # Set Up Connection ran again and found the headset somewhere new. d["config_host"] = host if not any(a["host"] == host for a in d["addresses"]): d["addresses"].insert(0, dict(new_address(host), label="From Set Up Connection")) - if seed: - seed_pin(d["id"], [host], b["port"]) + if seed and seed_pin(d["id"], [host], b["port"]) and not d.get("identity"): + d["identity"] = [f for _, f in pinned_keys(d["id"])][:8] or None changed = True if d.get("config_login") != [user, b["port"]]: # Set Up Connection (or an edit) changed who to log in as, or the port. diff --git a/ui/frame_link.py b/ui/frame_link.py index 2217888..2543327 100644 --- a/ui/frame_link.py +++ b/ui/frame_link.py @@ -28,11 +28,13 @@ Python stdlib only. Runs on this computer, never on the Frame. import copy import hashlib import ipaddress +import os import queue import re import socket import subprocess import sys +import tempfile import threading import time @@ -258,6 +260,29 @@ def ssh_target(host, ip): return host +def _unlink(path): + try: + os.unlink(path) + except OSError: + pass + + +def _scanned_key(path, name): + """The first whole, valid known_hosts line for `name` that a key check wrote, or None. + Only lines ssh has finished (ending in a newline): a half-written key mustn't count.""" + try: + with open(path, encoding="utf-8") as fh: + text = fh.read() + except (OSError, UnicodeDecodeError): + return None + for line in text.splitlines(True): + if line.endswith("\n"): + entry = frame_devices.key_entry(line.strip(), name) + if entry: + return entry + return None + + def probe_raw(host, port, result): """ssh's own wording for a failed probe, so the server's UNREACHABLE table explains it.""" return {"unresolved": f"ssh: Could not resolve hostname {host}: not found", @@ -508,12 +533,13 @@ class Link: if not host: # a headset with no addresses: reach nothing, not whatever ~/.ssh/config says return ["-o", "HostName=no-address.invalid"] # Whatever ~/.ssh/config says for Host *, every command checks the headset's pinned key: - # StrictHostKeyChecking=yes. Until a key is saved (a headset just added, or one whose - # file is gone), accept-new: the first contact saves it, as Set Up Connection and the - # connector's first handshake do, rather than every command failing with "No host key - # is known" until the connector gets there. accept-new still refuses a key that differs - # from a saved one, so it never replaces a pin; only Trust a new key does that. - strict = "yes" if frame_devices.pinned(device["id"]) else "accept-new" + # StrictHostKeyChecking=yes. Only a headset that never had an identity here (just + # added, never connected) gets accept-new: the first contact saves its key, as Set Up + # Connection and the connector's first handshake do, rather than every command failing + # with "No host key is known" until the connector gets there. Once it had one, a pin + # file that's gone, empty or unreadable fails closed (identity_established): accept-new + # can't tell that from a new headset and would trust whatever answers. + strict = self.strictness(device) if strict == "accept-new": try: frame_devices.pin_dir() # ssh won't create the folder its known_hosts file goes in @@ -524,6 +550,15 @@ class Link: "-o", f"UserKnownHostsFile={frame_devices.known_hosts_opt(device['id'])}", "-o", "HashKnownHosts=no", "-o", f"User={device['user']}", "-o", f"Port={device['port']}"] + def strictness(self, device): + """yes, or accept-new for a headset that never had an identity. A key saved since (by + any command's first contact) is recorded, so losing its file later fails closed.""" + if not frame_devices.identity_established(device): + return "accept-new" + if not device.get("identity") and frame_devices.pinned(device["id"]): + self.reg.record_identity(device["id"], [f for _, f in frame_devices.pinned_keys(device["id"])]) + return "yes" + def public_device(self, d): return {k: d.get(k) for k in ("id", "name", "alias", "user", "port", "transient")} @@ -880,6 +915,7 @@ class Link: if device.get("transient"): return self.reg.record_success(device["id"], host, net.get("id"), rtt) + self.reg.record_identity(device["id"], [f for _, f in frame_devices.pinned_keys(device["id"])]) # Set Up Connection may have changed the block while this attempt ran: take that # in (and reconnect) rather than writing this attempt's older settings over it. self.watch_config() @@ -952,15 +988,9 @@ class Link: return "ok" extra = [] if not device.get("transient"): - if frame_devices.pinned(device["id"]): - extra = ["-o", "StrictHostKeyChecking=yes"] - else: - # First connection since this headset was added: trust what it shows - # (as Set Up Connection does), and pin it from now on. ssh won't create - # the folder its known_hosts file goes in. - extra = ["-o", "StrictHostKeyChecking=accept-new"] - pins = frame_devices.known_hosts(device["id"]).parent - pins.mkdir(**({} if frame_host.WINDOWS else {"mode": 0o700}), parents=True, exist_ok=True) + # A headset's first connection trusts what it shows (as Set Up Connection does) and + # pins it; any later one checks the pin strictly (see host_opts). + extra = ["-o", f"StrictHostKeyChecking={self.strictness(device)}"] if self.control: # No ConnectTimeout: with it, OpenSSH's master takes ~5s to open its socket. # `extra` first: ssh takes the first value of an option, and it may say accept-new. @@ -968,15 +998,29 @@ class Link: "-o", "ServerAliveCountMax=2", "-N", alias] else: argv = [*self.mux_base, *extra, *opts, "-v", "-o", "ConnectTimeout=10", alias, "true"] + lines = queue.Queue() + collecting = [True] + errfile = None try: - proc = subprocess.Popen(argv, stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, - stderr=subprocess.PIPE, **frame_host.DETACHED) + if self.control: + proc = subprocess.Popen(argv, stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, + stderr=subprocess.PIPE, **frame_host.DETACHED) + else: + # No master (Windows): ssh runs `true` and exits. Its stderr goes to a file read + # as it grows, not a pipe: Windows' OpenSSH can block writing to a redirected + # pipe (see frame_host.run_ssh), and the handshake would never finish. + fd_, errfile = tempfile.mkstemp(prefix="frame-ssh-") + try: + proc = subprocess.Popen(argv, stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, + stderr=fd_, **frame_host.DETACHED) + finally: + os.close(fd_) except OSError as e: + if errfile: + _unlink(errfile) self.fail("ssh", f"Couldn't run ssh: {e}", str(e)) return "stop" self.pending = proc # so stop() can end it mid-handshake - lines = queue.Queue() - collecting = [True] def read(): for raw in iter(proc.stderr.readline, b""): @@ -984,7 +1028,31 @@ class Link: lines.put(raw.decode("utf-8", "replace").rstrip()) lines.put(None) proc.stderr.close() - threading.Thread(target=read, daemon=True).start() + + def tail(): + buf = b"" + try: + with open(errfile, "rb") as fh: + while True: + chunk = fh.read() + if not chunk: + if proc.poll() is None: + time.sleep(0.05) + continue + chunk = fh.read() # whatever it wrote just before exiting + if not chunk: + break + buf += chunk + *done, buf = buf.split(b"\n") + for raw in done: + lines.put(raw.decode("utf-8", "replace").rstrip()) + except OSError: + pass + if buf: + lines.put(buf.decode("utf-8", "replace").rstrip()) + lines.put(None) + _unlink(errfile) + threading.Thread(target=read if errfile is None else tail, daemon=True).start() step, said, authed = "ssh", [], False deadline = time.monotonic() + HANDSHAKE_TIMEOUT @@ -1065,6 +1133,8 @@ class Link: self.master = proc if "StrictHostKeyChecking=accept-new" in extra and frame_devices.pinned(device["id"]): # The first connection saved the key: from now on every command checks it strictly. + frame_host.make_private(frame_devices.known_hosts(device["id"])) # ssh wrote it + self.reg.record_identity(device["id"], [f for _, f in frame_devices.pinned_keys(device["id"])]) with self.route_lock: if self.attempt_gen == self.gen: self.opts = self.host_opts(device, ssh_target(a["host"], found.get("ip"))) @@ -1087,10 +1157,17 @@ class Link: # type any more: either way not the identity saved. Never accepted here: the page # shows the new key's fingerprint and the person decides (Trust a new key). name = (device or {}).get("name") or alias - self.fail("identity", f"This address answered with a different SSH identity from the one saved for {name}. " - "If SteamOS was reinstalled or reset, or this is a new headset, check its new key and trust it " - "on the Devices tab. If not, a different device may be answering here.", text, - hostkey={"state": "changed", "type": shown[0] if shown else None, + lost = bool(device) and not frame_devices.pinned(device["id"]) and \ + frame_devices.identity_established(device) + if lost: + message = (f"The SSH identity saved for {name} is missing or can't be read, so Frame Control won't " + "trust whatever answers. Check its key and trust it on the Devices tab.") + else: + message = (f"This address answered with a different SSH identity from the one saved for {name}. " + "If SteamOS was reinstalled or reset, or this is a new headset, check its new key and trust " + "it on the Devices tab. If not, a different device may be answering here.") + self.fail("identity", message, text, + hostkey={"state": "missing" if lost else "changed", "type": shown[0] if shown else None, "fingerprint": shown[1] if shown else None, "host": host}) return "next" # A refused key is the same at every address: stop. (Judged by ssh's own words, not @@ -1173,12 +1250,27 @@ class Link: self.devices_changed() # ---- Trust a new key ---- + # Every way a key check could do more than read the key: no login of any kind, no agent + # (nor forwarding it), no X11, port or tunnel forwarding, no local or remote command, no + # terminal, no shared connection, no learning keys. Given first, so ssh takes these over + # anything ~/.ssh/config (or host_opts) says. -N: no command, even if a device accepts "none". + SCAN_OPTS = ["BatchMode=yes", "ControlMaster=no", "ControlPath=none", "ConnectTimeout=8", + "StrictHostKeyChecking=accept-new", "UpdateHostKeys=no", "HashKnownHosts=no", + "PreferredAuthentications=none", "PubkeyAuthentication=no", "PasswordAuthentication=no", + "KbdInteractiveAuthentication=no", "HostbasedAuthentication=no", "IdentityAgent=none", + "ForwardAgent=no", "ForwardX11=no", "ForwardX11Trusted=no", "ClearAllForwardings=yes", + "Tunnel=no", "PermitLocalCommand=no", "RemoteCommand=none", "RequestTTY=no"] + SCAN_TIMEOUT = 20 + def scan_identity(self, device_id): - """Ask the headset for its host key without trusting it: ssh with a throwaway - known_hosts file and no login (PreferredAuthentications=none, so nothing of ours is - offered to a device that may not be the headset). The address that showed a changed - key goes first, then the rest in order. -> {"type", "fingerprint", "host", "saved", - "matches"}; the key is kept (SCAN_TTL) for trust_identity, which pins exactly it.""" + """Read the headset's host key without trusting it or logging in. ssh writes it to a + throwaway known_hosts file during the key exchange, before any authentication, and is + stopped as soon as it has (or when it gives up); SCAN_OPTS and -N rule out anything else. + `-F none` too, unless ~/.ssh/config sends the alias through a jump host (then it's + needed for that, and SCAN_OPTS still win). The address that showed the changed key goes + first, then the rest in order. -> {"type", "fingerprint", "host", "saved", "recorded", + "matches", "recorded_match"}; the key is kept (SCAN_TTL) for trust_identity, which pins + exactly it.""" device = self.reg.get(device_id) if not device["addresses"]: raise frame_devices.DeviceError("This headset has no addresses to ask") @@ -1190,47 +1282,71 @@ class Link: if first in hosts: hosts.remove(first) hosts.insert(0, first) - name = frame_devices.host_key_alias(device_id) + no_config = [] if ssh_g(device["alias"])[3] else ["-F", "none"] reasons = [] for host in hosts: - path, opt = frame_devices.scan_file(device_id) - # Ours first: ssh takes an option's first value, and host_opts says yes and the saved file. - argv = [*self.mux_base[:3], "-o", "ControlPath=none", "-o", "ConnectTimeout=8", - "-o", "StrictHostKeyChecking=accept-new", "-o", f"UserKnownHostsFile={opt}", - "-o", "UpdateHostKeys=no", - "-o", "PreferredAuthentications=none", - *self.host_opts(device, host), device["alias"], "true"] - try: - r = frame_host.run_ssh(argv, capture_output=True, stdin=subprocess.DEVNULL, text=True, - errors="replace", timeout=25) - err_text = r.stderr or "" - try: - lines = path.read_text(encoding="utf-8").splitlines() - except (OSError, UnicodeDecodeError): - lines = [] - except (OSError, subprocess.TimeoutExpired): - err_text, lines = f"Timed out talking to {device['alias']}", [] - finally: - try: - path.unlink() - except OSError: - pass - key = next((e for e in (frame_devices.key_entry(line, name) for line in lines) if e), None) + key, err_text = self._scan_one(device, host, no_config) if key: saved = frame_devices.pinned_keys(device_id) - scan = {"type": key[0], "blob": key[1], "fingerprint": key[2], "host": host, "at": now()} + recorded = [f for f in device.get("identity") or [] if isinstance(f, str)] + scan = {"type": key[0], "blob": key[1], "fingerprint": key[2], "host": host, + "port": device["port"], "user": device["user"], "at": now()} with self.cond: self.scans[device_id] = scan return {"type": key[0], "fingerprint": key[2], "host": host, - "saved": [{"type": t, "fingerprint": f} for t, f in saved], - "matches": any(f == key[2] for _, f in saved)} + "saved": [{"type": t, "fingerprint": f} for t, f in saved], "recorded": recorded, + "matches": any(f == key[2] for _, f in saved), "recorded_match": key[2] in recorded} text = err_text.strip() reasons.append(f"{host}: " + (self.explain(text) or (text.splitlines() or ["no answer"])[-1])) raise frame_devices.DeviceError("Couldn't read the headset's key. " + "; ".join(reasons)) + def _scan_one(self, device, host, no_config): + """-> ((type, blob, fingerprint) or None, what ssh said).""" + name = frame_devices.host_key_alias(device["id"]) + path, opt = frame_devices.scan_file(device["id"]) + opts = [a for o in [*self.SCAN_OPTS, f"UserKnownHostsFile={opt}"] for a in ("-o", o)] + argv = [self.mux_base[0], *no_config, *opts, *self.host_opts(device, host), "-N", device["alias"]] + fd_, errfile = tempfile.mkstemp(prefix="frame-scan-") # a file, not a pipe (see frame_host.run_ssh) + key, proc = None, None + try: + try: + proc = subprocess.Popen(argv, stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, stderr=fd_, + **frame_host.DETACHED) + finally: + os.close(fd_) + deadline = time.monotonic() + self.SCAN_TIMEOUT + while key is None: + ended = proc.poll() is not None + key = _scanned_key(path, name) + if key or ended or self.stopped or time.monotonic() > deadline: + break + time.sleep(0.05) + except OSError as e: + return None, str(e) + finally: + if proc and proc.poll() is None: + proc.kill() # it has the key (or gave up): nothing more to do with that device + if proc: + try: + proc.wait(5) + except subprocess.TimeoutExpired: + pass + try: + with open(errfile, "rb") as fh: + said = fh.read().decode("utf-8", "replace") + except OSError: + said = "" + _unlink(errfile) + _unlink(path) + if key is None and not said.strip() and proc and proc.returncode is not None and proc.returncode < 0: + said = f"Timed out talking to {device['alias']}" + return key, said + def trust_identity(self, device_id, fingerprint): """Pin the key scan_identity read, and only if `fingerprint` is the one it showed (what - the person saw and confirmed), replacing whatever was saved. -> the scan.""" + the person saw and confirmed), replacing whatever was saved; and only while the headset + is still reached as it was then (same port and user, the address still one of its own). + -> the scan.""" with self.cond: scan = self.scans.get(device_id) if not scan or now() - scan["at"] > SCAN_TTL: @@ -1239,8 +1355,13 @@ class Link: if not isinstance(fingerprint, str) or fingerprint != scan["fingerprint"]: raise frame_devices.DeviceError("That isn't the key the headset showed. Check its key again") self.scans.pop(device_id) - self.reg.get(device_id) # still a headset here + d = self.reg.get(device_id) # still a headset here + if (d["port"], d["user"]) != (scan["port"], scan["user"]) or \ + scan["host"] not in [a["host"] for a in d["addresses"]]: + raise frame_devices.DeviceError("The headset's address, port or user changed since its key was read. " + "Check its key again") frame_devices.pin_key(device_id, scan["type"], scan["blob"]) + self.reg.record_identity(device_id, [scan["fingerprint"]]) return scan @@ -1262,6 +1383,8 @@ def devices_view(link): view["active"] = d["id"] == active["id"] view["pinned"] = frame_devices.pinned(d["id"]) view["keys"] = [{"type": t, "fingerprint": f} for t, f in frame_devices.pinned_keys(d["id"])] + # It had an identity, but the saved key is gone or unusable: nothing is trusted until checked. + view["identity_lost"] = not view["pinned"] and frame_devices.identity_established(d) view["addresses"] = [dict(a, network_names=[names.get(n, "an unnamed network") for n in a["networks"]]) for a in d["addresses"]] devices.append(view) @@ -1294,7 +1417,7 @@ def devices_action(link, body, open_setup, busy=lambda: 0): is_active = did == active["id"] moves = action == "use" or (action == "retry" and link.alive()) or (is_active and ( # (a retry while connected would cut the install's connection) - action in ("remove", "address-remove", "forget-identity", "identity-trust") + action in ("remove", "address-remove", "identity-trust") or (action == "update" and login_change(reg, did, body)) or (action == "address-update" and body.get("newHost") not in (None, body.get("host"))))) if moves and busy(): @@ -1374,11 +1497,12 @@ def devices_action(link, body, open_setup, busy=lambda: 0): threading.Thread(target=link.test, args=(did,), daemon=True).start() msg = f"Testing {len(d['addresses'])} address{'es' * (len(d['addresses']) != 1)}" elif action == "forget-identity": - d = reg.get(did) - frame_devices.forget_pin(did) - if is_active: - link.kick("switch") - msg = f"Forgot {d['name']}'s SSH identity; the next connection saves the one it shows" + # Dropping the pin used to let the next contact save whatever key answered, unseen. + # A new key is now always shown first: identity-scan, then identity-trust. + reg.get(did) + raise frame_devices.DeviceError("Forgetting a headset's identity is no longer supported: use Check its key " + "on the Devices tab (identity-scan, then identity-trust), which shows the " + "new key before saving it") elif action == "identity-scan": d = reg.get(did) scan = link.scan_identity(did) diff --git a/ui/index.html b/ui/index.html index 9b40209..50d92b9 100644 --- a/ui/index.html +++ b/ui/index.html @@ -4800,7 +4800,8 @@ function renderDetail() { `).join(""); const err = d.active && link.s && link.s.phase === "failed" ? link.s.error : null; - const problem = err && err.hostkey ? `