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

@@ -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";
     }

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",

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

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)