From 2e4e9c78f8fec23ff693bd892c73d4159981780a Mon Sep 17 00:00:00 2001 From: catborise Date: Thu, 5 Nov 2026 16:39:34 +0300 Subject: [PATCH] fix(console): a VNC token works only in its user's session; refusals reach the Logs page The VNC page's token travels in the websocket URL, so nginx's access log and anyone who saw the URL had it for 120 s, and it opened the console as a bearer token. novncd now also wants the session cookie of the token's user. Its refusals were a warning without a user or an address: they now name the client address (nginx sends X-Real-IP to novncd too) and leave a Logs row "VNC console refused: reason". --- conf/nginx/webvirtcloud.conf | 1 + console/novncd | 23 ++++++++++++++++++++-- console/tests/test_views.py | 38 ++++++++++++++++++++++++++++++++++-- console/tokens.py | 18 ++++++++++++++++- 4 files changed, 75 insertions(+), 5 deletions(-) diff --git a/conf/nginx/webvirtcloud.conf b/conf/nginx/webvirtcloud.conf index 1dda01ab..bbe22fbd 100644 --- a/conf/nginx/webvirtcloud.conf +++ b/conf/nginx/webvirtcloud.conf @@ -35,6 +35,7 @@ server { location /novncd/ { proxy_pass http://wsnovncd; proxy_http_version 1.1; + proxy_set_header X-Real-IP $remote_addr; proxy_set_header Upgrade $http_upgrade; proxy_set_header Connection "upgrade"; } diff --git a/console/novncd b/console/novncd index 516b21f4..d9a9139b 100755 --- a/console/novncd +++ b/console/novncd @@ -4,6 +4,7 @@ # through an SSH tunnel or a local Unix socket. import argparse +import ipaddress import logging import os import socket @@ -22,7 +23,8 @@ django.setup() from websockify import ProxyRequestHandler, WebSocketProxy # noqa: E402 from console.sshtunnels import open_tunnel, tunnel_target # noqa: E402 -from console.tokens import ACCESS_CHECK, authorize, may_still_open # noqa: E402 +from console.tokens import ACCESS_CHECK, authorize, may_still_open, session_user_id # noqa: E402 +from logs.audit import write # noqa: E402 from vrtManager.connection import CONN_SOCKET, CONN_SSH # noqa: E402 from vrtManager.util import is_loopback # noqa: E402 from webvirtcloud.settings import WS_HOST, WS_PORT # noqa: E402 @@ -77,6 +79,14 @@ def get_connection_infos(instance): WILDCARD = (None, "", "0.0.0.0", "::") +def client_address(value): + """The client's address nginx sends in X-Real-IP, or None.""" + try: + return str(ipaddress.ip_address(value)) + except ValueError: + return None + + def end_when_refused(instance, user_id, tsock, ended): """Close the session's VNC socket, which ends it, once its user may no longer open the console (as the serial console does).""" @@ -102,11 +112,20 @@ class ConsoleClient: logging.warning("Console connection without a token in its websocket URL") return + user = None try: instance, user = authorize(token, "vnc") + # a token read from a log or a URL is no use without its user's session + if session_user_id(self.headers.get("Cookie")) != str(user.pk): + raise PermissionError("The console page was opened in another session") connhost, connuser, conntype, console_host, console_port, console_socket = get_connection_infos(instance) + except PermissionError as e: + address = client_address(self.headers.get("X-Real-IP")) + logging.warning("Console connection from %s denied: %s", address, e) + write(user.username if user else "", "-", "", f"VNC console refused: {e}", False, address) + return except Exception as e: - logging.warning("Console connection denied: %s", e) + logging.warning("Console connection failed: %s", e) return logging.debug( "Console of %s@%s (type %s): %s:%s, socket %s", diff --git a/console/tests/test_views.py b/console/tests/test_views.py index c53d2113..bbb48412 100644 --- a/console/tests/test_views.py +++ b/console/tests/test_views.py @@ -406,8 +406,14 @@ class NovncdDaemonLogicTestCase(TestCase): def setUp(self): self.get_parser = novncd_mod.get_parser self.ConsoleClient = novncd_mod.ConsoleClient - patcher = patch.object(novncd_mod, "authorize", return_value=(MagicMock(), MagicMock())) - self.authorize = patcher.start() + user = MagicMock(pk=7) + user.username = "vnc-user" + for name, value in (("authorize", (MagicMock(), user)), ("session_user_id", "7")): + patcher = patch.object(novncd_mod, name, return_value=value) + setattr(self, name, patcher.start()) + self.addCleanup(patcher.stop) + patcher = patch.object(novncd_mod.ConsoleClient, "headers", {}, create=True) + patcher.start() self.addCleanup(patcher.stop) def test_novncd_parser_defaults(self): @@ -564,6 +570,34 @@ class NovncdDaemonLogicTestCase(TestCase): # the VM access was checked for, not whichever VM has its name now self.assertEqual(mock_wvm.call_args.kwargs["uuid"], mock_instance.uuid) + def test_a_token_without_its_users_session_is_refused_and_logged(self): + from logs.models import Logs + + self.session_user_id.return_value = None # read from a log, used in another browser + handler = self.ConsoleClient() + handler.headers = {"Cookie": "sessionid=another", "X-Real-IP": "192.0.2.9"} + handler.path = "/novncd/?token=1-uuid" + handler.msg = MagicMock() + socket_factory = MagicMock() + with patch.object(novncd_mod, "get_connection_infos") as get_info, self.assertLogs(level="WARNING"): + handler._new_client(socket_factory=socket_factory) + get_info.assert_not_called() + socket_factory.assert_not_called() + self.session_user_id.assert_called_once_with("sessionid=another") + row = Logs.objects.get() + self.assertEqual((row.user, row.ip, row.success), ("vnc-user", "192.0.2.9", False)) + self.assertEqual(row.message, "VNC console refused: The console page was opened in another session") + + def test_the_session_names_its_user(self): + from django.contrib.sessions.backends.db import SessionStore + + user = get_user_model().objects.create_user("vnc-session", password="pw") + self.client.force_login(user) + cookie = f"{settings.SESSION_COOKIE_NAME}={self.client.session.session_key}; other=1" + self.assertEqual(tokens.session_user_id(cookie), str(user.pk)) + self.assertIsNone(tokens.session_user_id(f"{settings.SESSION_COOKIE_NAME}={SessionStore().session_key}")) + self.assertIsNone(tokens.session_user_id(None)) + def test_a_vnc_session_ends_when_its_user_may_no_longer_open_it(self): tsock, ended = MagicMock(), MagicMock() ended.wait.return_value = False # ACCESS_CHECK passed, the session still open diff --git a/console/tokens.py b/console/tokens.py index 503c7c2b..4b396fc5 100644 --- a/console/tokens.py +++ b/console/tokens.py @@ -1,11 +1,18 @@ """The console token: the console view signs "host:uuid:user_id:console type" for the VM whose console it shows; the console daemons (novncd for vnc, socketiod for pty) accept it for 120 s if that user may still open that VM's -console. A bearer token: whoever holds it within that time gets the console.""" +console. novncd, whose token travels in the websocket URL (and so in logs), +also wants the session cookie of the token's user (session_user_id). +socketiod's token travels in the Socket.IO handshake, not the URL.""" + +from importlib import import_module from django import db +from django.conf import settings +from django.contrib.auth import SESSION_KEY from django.contrib.auth import get_user_model from django.core.signing import BadSignature, SignatureExpired, TimestampSigner +from django.http import parse_cookie from instances.models import Instance from instances.utils import can_open_console @@ -44,3 +51,12 @@ def may_still_open(user_id, instance): return user is not None and can_open_console(user, instance) finally: db.connection.close() # a session may last for days: no idle connection meanwhile + + +def session_user_id(cookie_header): + """The id (a string) of the user signed in with the session cookie of a + request's Cookie header, or None.""" + session_key = parse_cookie(cookie_header or "").get(settings.SESSION_COOKIE_NAME) + if not session_key: + return None + return import_module(settings.SESSION_ENGINE).SessionStore(session_key).get(SESSION_KEY)