diff --git a/README.md b/README.md index 6b89a4d..422b2f6 100644 --- a/README.md +++ b/README.md @@ -217,6 +217,8 @@ Pick the protocol at the top of the connection form. Each session runs fullscree Passwords and SSH keys are stored in `/config/profiles.json` (readable only by the container user) and are never passed on a command line. SSH keys and VNC password files are written to a private temporary directory for the length of the session and deleted afterwards. +The WebUI never shows a saved password or key again. It is only reused for the host, port and protocol it was saved with: after changing any of those on a saved profile, enter the password (or SSH key) again. + ### Disconnecting idle sessions A session runs inside the container, not the browser, so closing the tab leaves it running: Windows stays logged in, and a Windows PC can't be used locally while it is. Set `IDLE_DISCONNECT_MINUTES` to end the session once no dashboard has been open for that long: diff --git a/root/app/backend/main.py b/root/app/backend/main.py index 90b8347..81c9b96 100644 --- a/root/app/backend/main.py +++ b/root/app/backend/main.py @@ -39,7 +39,7 @@ verify_state_cookie, clear_state_cookie ) -from session_manager import session_manager +from session_manager import DEFAULT_PORTS, session_manager from log_buffer import BufferLogHandler, FileTail, LogBuffer from file_manager import ( list_files, @@ -395,11 +395,27 @@ def restore_masked_secrets(data: Dict[str, Any], stored: Optional[Dict[str, Any] if data.get(secret) == PASSWORD_MASK: data[secret] = stored.get(secret, "") if stored else "" +def same_target(data: Dict[str, Any], stored: Dict[str, Any]) -> bool: + """Whether `data` connects to the same machine as the stored profile.""" + def target(p: Dict[str, Any]): + protocol = p.get("protocol") or "rdp" + return (protocol, str(p.get("host") or "").lower(), p.get("port") or DEFAULT_PORTS.get(protocol)) + return target(data) == target(stored) + +def check_masked_secrets_target(data: Dict[str, Any], stored: Optional[Dict[str, Any]]): + """A masked secret is only ever reused for the machine it was saved for. Otherwise + changing a profile's host would send its stored password to any machine, without + the password ever being shown.""" + if stored and any(data.get(s) == PASSWORD_MASK for s in SECRET_FIELDS) and not same_target(data, stored): + raise HTTPException(status_code=400, + detail="The host, port or protocol changed: enter the password (or SSH key) again") + @app.post("/api/profiles") def save_profile(profile: ConnectionProfile, user: dict = Depends(get_current_user)): with _profiles_lock: profiles = load_profiles(for_update=True) existing = find_profile(profiles, profile.id) + check_masked_secrets_target(profile.model_dump(), existing) for secret in SECRET_FIELDS: if getattr(profile, secret) == PASSWORD_MASK: setattr(profile, secret, existing.get(secret, "") if existing else "") @@ -432,7 +448,9 @@ def connect_session(req: ConnectRequest, user: dict = Depends(get_current_user)) config_dict = req.custom.model_dump() # Form was loaded from a saved profile and its secrets left untouched if any(config_dict.get(s) == PASSWORD_MASK for s in SECRET_FIELDS): - restore_masked_secrets(config_dict, find_profile(load_profiles(), req.custom.id)) + stored = find_profile(load_profiles(), req.custom.id) + check_masked_secrets_target(config_dict, stored) + restore_masked_secrets(config_dict, stored) else: raise HTTPException(status_code=400, detail="Missing connection parameters") diff --git a/tests/test_main.py b/tests/test_main.py index 0bdfd2c..e20ff45 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -161,3 +161,72 @@ def test_a_missing_file_is_created_on_save(damaged): path.unlink() assert client.post("/api/profiles", json={"protocol": "rdp", "host": "10.0.0.9"}).status_code == 200 assert [p["host"] for p in main.load_profiles()] == ["10.0.0.9"] + + +# ----------------- Masked secrets stay with their host ----------------- + +@pytest.fixture +def api(tmp_path, monkeypatch): + """The API in no-auth mode with an empty profile store; connects are recorded, not run.""" + from fastapi.testclient import TestClient + monkeypatch.setattr(main, "PROFILES_FILE", tmp_path / "profiles.json") + monkeypatch.setattr(main, "AUTH_MODE", "none") + import auth + monkeypatch.setattr(auth, "AUTH_MODE", "none") + connects = [] + monkeypatch.setattr(main.session_manager, "connect", + lambda config: connects.append(config) or {"success": True, "message": "ok"}) + client = TestClient(main.app) + profile_id = client.post("/api/profiles", json={ + "protocol": "rdp", "host": "10.0.0.5", "username": "kodi", "password": "real-pw", + }).json()["id"] + return client, profile_id, connects + + +def masked(profile_id, **changes): + return {"id": profile_id, "protocol": "rdp", "host": "10.0.0.5", "username": "kodi", + "password": main.PASSWORD_MASK, **changes} + + +def test_connect_reuses_the_password_for_the_same_host(api): + client, profile_id, connects = api + res = client.post("/api/session/connect", json={"custom": masked(profile_id, resolution="1920x1080")}) + assert res.status_code == 200 + assert connects[-1]["password"] == "real-pw" + + +@pytest.mark.parametrize("changes", [ + {"host": "attacker.example"}, + {"host": "10.0.0.6"}, + {"port": 3390}, + {"protocol": "vnc"}, +]) +def test_connect_refuses_a_masked_password_for_another_target(api, changes): + """Otherwise any dashboard user could send a saved password to their own machine.""" + client, profile_id, connects = api + res = client.post("/api/session/connect", json={"custom": masked(profile_id, **changes)}) + assert res.status_code == 400 + assert connects == [] + + +def test_saving_a_new_host_with_the_masked_password_is_refused(api): + client, profile_id, _ = api + res = client.post("/api/profiles", json=masked(profile_id, host="attacker.example")) + assert res.status_code == 400 + stored = main.find_profile(main.load_profiles(), profile_id) + assert stored["host"] == "10.0.0.5" and stored["password"] == "real-pw" + + +def test_a_new_host_with_a_new_password_is_saved(api): + client, profile_id, _ = api + res = client.post("/api/profiles", json=masked(profile_id, host="10.0.0.9", password="new-pw")) + assert res.status_code == 200 + stored = main.find_profile(main.load_profiles(), profile_id) + assert stored["host"] == "10.0.0.9" and stored["password"] == "new-pw" + + +def test_host_case_and_the_default_port_count_as_the_same_target(api): + client, profile_id, connects = api + res = client.post("/api/session/connect", json={"custom": masked(profile_id, host="10.0.0.5", port=3389)}) + assert res.status_code == 200 + assert main.same_target({"host": "PC.local"}, {"host": "pc.local", "port": 3389})