diff --git a/tests/test_devices.py b/tests/test_devices.py index 4a44b9b..810b0d1 100644 --- a/tests/test_devices.py +++ b/tests/test_devices.py @@ -266,6 +266,9 @@ class Registry(Base): self.reg.record_success(d["id"], "192.168.1.40", "n-home", 3.2) self.reg.update_address(d["id"], "192.168.1.40", label="Home") self.assertEqual(self.reg.get(d["id"])["addresses"][1]["networks"], ["n-home"]) # a label keeps what it learned + with self.assertRaises(fd.DeviceError): + self.reg.update_address(d["id"], "192.168.1.40", new_host="192.168.1.41", label="bad\nlabel") + self.assertEqual(self.reg.get(d["id"])["addresses"][1]["networks"], ["n-home"]) # rejected: unchanged self.reg.update_address(d["id"], "192.168.1.40", new_host="192.168.1.41") moved = self.reg.get(d["id"])["addresses"][1] self.assertEqual((moved["host"], moved["networks"], moved["last_ok"]), ("192.168.1.41", [], None)) diff --git a/tests/test_link.py b/tests/test_link.py index 1cdc6d3..c5a484c 100644 --- a/tests/test_link.py +++ b/tests/test_link.py @@ -118,6 +118,16 @@ class Connecting(unittest.TestCase): explain=explain) self.addCleanup(self.link.stop) + def listen6(self): + """A "different device": the same port on IPv6 loopback.""" + try: + six = socket.socket(socket.AF_INET6) + self.addCleanup(six.close) + six.bind(("::1", self.port)) + six.listen(4) + except OSError: + self.skipTest("no IPv6 loopback") + def pin(self, device_id): fd.known_hosts(device_id).parent.mkdir(parents=True, exist_ok=True) fd.known_hosts(device_id).write_text(f"frame-control-{device_id} ssh-ed25519 AAAA\n") @@ -139,13 +149,7 @@ class Connecting(unittest.TestCase): def test_falls_through_to_the_address_that_is_really_the_headset(self): # Tried in this order: a name that doesn't resolve, a different device, the headset. - try: # the "different device": the same port on IPv6 loopback - six = socket.socket(socket.AF_INET6) - self.addCleanup(six.close) - six.bind(("::1", self.port)) - six.listen(4) - except OSError: - self.skipTest("no IPv6 loopback") + self.listen6() d = self.device("nothing.invalid", "::1", "localhost") self.hosts({"::1": "wrong", "localhost": "ok"}) self.link.connect(["start"]) @@ -258,13 +262,14 @@ class Connecting(unittest.TestCase): self.assertIn("Port=1", self.routes[-1][1]) def test_test_now_checks_every_address_without_touching_the_connection(self): - d = self.device("127.0.0.1", "localhost", "nothing.invalid") + self.listen6() + d = self.device("::1", "127.0.0.1", "nothing.invalid") self.pin(d["id"]) - self.hosts({"127.0.0.1": "wrong", "localhost": "ok"}) + self.hosts({"::1": "wrong", "127.0.0.1": "ok"}) self.link.test(d["id"]) rows = {r["host"]: r for r in self.link.snapshot()["tests"][d["id"]]["rows"]} - self.assertEqual(rows["localhost"]["ssh"], "ok") - self.assertEqual(rows["127.0.0.1"]["ssh"], "wrong") + self.assertEqual(rows["127.0.0.1"]["ssh"], "ok") + self.assertEqual(rows["::1"]["ssh"], "wrong") self.assertEqual(rows["nothing.invalid"]["state"], "unresolved") self.assertEqual(self.routes, []) self.assertTrue(all("ControlPath=none" in c for c in self.calls())) @@ -432,6 +437,15 @@ class ServerConnection(unittest.TestCase): self.assertIn("stages", json.loads(line[6:])) conn.close() + def test_changes_meant_for_another_headset_are_refused(self): + conn = http.client.HTTPConnection("127.0.0.1", self.port, timeout=20) + conn.request("POST", "/api/launch", body=b'{"appid": "620"}', + headers={"X-Frame-UI": "1", "Content-Type": "application/json", "X-Frame-Device": "someoneelse"}) + r = conn.getresponse() + self.assertEqual(r.status, 409) + self.assertIn("switched headsets", json.loads(r.read())["error"]) + conn.close() + def test_guards_and_validation(self): self.assertEqual(self.request("GET", "/api/connection", key="")[0], 403) self.assertEqual(self.request("GET", "/api/devices", key="nope")[0], 403) diff --git a/ui/frame_devices.py b/ui/frame_devices.py index a78470f..e28225e 100644 --- a/ui/frame_devices.py +++ b/ui/frame_devices.py @@ -562,15 +562,20 @@ class Registry: with self.lock: d = self._find(device_id) a = self._addr(d, host) - if new_host is not None and new_host != host: + # Check everything first: a rejected edit changes nothing. + moved = new_host is not None and new_host != host + if moved: new_host = check_host(new_host) if any(x["host"] == new_host for x in d["addresses"]): raise DeviceError(f"{new_host} is already on the list") + kind = None if kind is None else check_kind(kind) + label = None if label is None else check_text(label, "label") + if moved: a.update(host=new_host, networks=[], last_ok=None, last_rtt_ms=None) # a new place: learn again if kind is not None: - a["kind"] = check_kind(kind) + a["kind"] = kind if label is not None: - a["label"] = check_text(label, "label") + a["label"] = label self.save() return copy.deepcopy(a) diff --git a/ui/frame_link.py b/ui/frame_link.py index 96c73bc..9e3b18b 100644 --- a/ui/frame_link.py +++ b/ui/frame_link.py @@ -125,6 +125,11 @@ def probe(host, port, timeout=PROBE_TIMEOUT, update=None): return last or {"state": "timeout", "detail": "No answer"} +def ssh_target(host, ip): + """Where ssh should go for an address whose probe answered from `ip`.""" + return ip if ip and re.fullmatch(r"\d{1,3}(\.\d{1,3}){3}", ip) else host + + 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", @@ -269,7 +274,9 @@ class Link: self.apply(device["alias"], self.first_route(device)) self.routed = None # the next attempt routes again with self.cond: - self.state["phase"] = "connecting" + # Say so at once: the page clears the old headset's panels when the device changes. + self.state.update(phase="connecting", device=self.public_device(device), via=None, error=None, + retry_at=None, probes=[], stages=[]) self.kicks.append("switch") self.version += 1 self.cond.notify_all() @@ -650,8 +657,7 @@ class Link: # An IPv4 address that answered is used as is, so ssh doesn't look the name up # again and try an address that didn't answer (a dead IPv6 route, say). IPv6 # answers keep the name: a link-local one needs its zone, which ssh adds itself. - ip = found.get("ip") or "" - opts = self.host_opts(device, ip if re.fullmatch(r"\d{1,3}(\.\d{1,3}){3}", ip) else a["host"]) + opts = self.host_opts(device, ssh_target(a["host"], found.get("ip"))) alias = device["alias"] with self.route_lock: if self.attempt_gen != self.gen: @@ -818,7 +824,8 @@ class Link: rows[i]["ssh"] = "checking" put() argv = [*self.mux_base[:3], "-o", "ControlPath=none", "-o", "ConnectTimeout=8", - *self.host_opts(device, a["host"]), "-o", "StrictHostKeyChecking=yes", device["alias"], "true"] + *self.host_opts(device, ssh_target(a["host"], res.get("ip"))), + "-o", "StrictHostKeyChecking=yes", device["alias"], "true"] try: r = subprocess.run(argv, capture_output=True, stdin=subprocess.DEVNULL, text=True, errors="replace", timeout=20) diff --git a/ui/index.html b/ui/index.html index 490d7cf..4fe10ae 100644 --- a/ui/index.html +++ b/ui/index.html @@ -925,8 +925,11 @@ const savesToDevice = () => !!(window.frameApp && window.frameApp.saveImages); let devGen = 0; async function api(path, body) { const gen = devGen; + // Changes name the headset the page was showing, so the server refuses one meant + // for a headset it has since switched away from. + const dev = window.connDevice ? { "X-Frame-Device": window.connDevice } : {}; const opts = body === undefined ? { headers: {"X-Frame-UI": UI_KEY} } : { - method: "POST", headers: {"Content-Type": "application/json", "X-Frame-UI": UI_KEY}, body: JSON.stringify(body) }; + method: "POST", headers: {"Content-Type": "application/json", "X-Frame-UI": UI_KEY, ...dev}, body: JSON.stringify(body) }; let r; try { r = await fetch(path, opts); } catch { @@ -1536,6 +1539,7 @@ function upload(file, mode) { const xhr = new XMLHttpRequest(); xhr.open("POST", "/api/upload"); xhr.setRequestHeader("X-Frame-UI", UI_KEY); + if (window.connDevice) xhr.setRequestHeader("X-Frame-Device", window.connDevice); xhr.setRequestHeader("X-Filename", encodeURIComponent(file.name)); xhr.setRequestHeader("X-Mode", mode); const bar = $("prog").firstElementChild; @@ -2527,6 +2531,7 @@ function onConnection(s) { if (!link.live) return; renderPill(); renderConnDlg(); const devId = s.device && s.device.id; + window.connDevice = devId || null; if (link.device !== null && devId !== link.device) { log(`Now using ${s.device.name}`, "ok"); state = null; @@ -2545,6 +2550,9 @@ function onConnection(s) { if ($(id)) $(id).innerHTML = `
Waiting for ${esc(s.device.name)}
`; }); disp.list = []; disp.port = null; + // The lists behind those panels too, so filters can't bring the old ones back. + gm.owned = null; gm.byId = new Map(); gm.results = []; gm.store = []; gm.storeQ = null; gm.shown = 0; gm.seq++; + androidApps = []; shots.list = []; $("dispSel").innerHTML = ""; $("dispSel").disabled = true; $("dispCtl").hidden = true; link.reload = true; // everything on the page was the other headset's $("battChip").hidden = true; diff --git a/ui/server.py b/ui/server.py index 37e095d..9e319c3 100755 --- a/ui/server.py +++ b/ui/server.py @@ -1468,6 +1468,12 @@ class Handler(BaseHTTPRequestHandler): if not self.local_request(): return path = urlparse(self.path).path + # A change the page made for a headset the app has since switched away from + # (its buttons were still showing): refuse it rather than do it to this one. + meant = self.headers.get("X-Frame-Device") + if LINK and meant and path != "/api/devices" and meant != LINK.active_device()["id"]: + self.send_json({"error": "Frame Control switched headsets; try again on this one"}, 409) + return try: if path == "/api/upload": with working():