From ed4a4e9fdfdee82e1f2783c1b93b7cece85862cc Mon Sep 17 00:00:00 2001 From: rangoDJ <18576423+rangoDJ@users.noreply.github.com> Date: Wed, 7 Oct 2026 16:36:55 -0400 Subject: [PATCH] Reuse a masked password only for the host it was saved for A profile sent back with its password or SSH key still masked had the stored secret restored by profile id alone, with no check that the host, port or protocol were unchanged. Pointing a saved profile at another machine and connecting sent the stored credentials there, although the API never shows them. Masked secrets are now refused for a different target; the password has to be entered again. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 ++ root/app/backend/main.py | 22 +++++++++++-- tests/test_main.py | 69 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 2 deletions(-) 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 177554a..914379c 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, @@ -384,11 +384,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() 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 "") @@ -421,7 +437,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 a832a68..f16c2c9 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -114,3 +114,72 @@ def test_port_bounds(): def test_port_defaults_to_none_for_the_protocol_default(): assert main.ConnectionProfile(host="1.2.3.4").port is None + + +# ----------------- 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})