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)