fix(console): socketiod refuses a request without a signed-in session

Engine.IO accepts a connection, and holds one of socketiod's 40 threads
for it, from its first request; the console token is checked only in
Socket.IO's connect(), which a client need never send. Anyone who could
reach the panel, without an account, could open 40 websockets, answer
their pings, and no serial console would open for anybody (checked: the
41st handshake got no answer until they were closed).

The daemon's WSGI app now refuses a request whose session cookie names
no signed-in user, with 403, before Engine.IO sees it; the page always
sends that cookie. A token without its session no longer connects either,
as for novncd. Checked again: 60 such websockets are each answered 403 at
once and hold nothing.

The process tests sign their client in with a session cookie, as a
browser; python-socketio sends a custom Cookie header only with its first
request, so the client gets a cookie jar instead.

console/socketiod

@@ -31,7 +31,7 @@ from gunicorn.app.base import BaseApplication  # noqa: E402
 from django import db  # noqa: E402
 from django.conf import settings  # noqa: E402
 
-from console.tokens import authorize  # noqa: E402
+from console.tokens import authorize, session_user_id  # noqa: E402
 
 CONSOLECALLBACK = os.path.join(ROOT_PATH, "conf", "daemon", "consolecallback")
 LOG_FD = os.dup(2)  # this daemon's stderr: a worker logs there on its fd 3
@@ -202,6 +202,23 @@ def disconnect(sid, reason=None):
     stop(sid)
 
 
+def signed_in_only(app):
+    """Engine.IO holds a connection, and one of the threads, from its first
+    request; Socket.IO's connect() checks the token only after that. A request
+    without a signed-in session is refused before it takes a thread: without
+    this, anyone could hold every thread and no console would open."""
+    def wsgi(environ, start_response):
+        try:
+            signed_in = session_user_id(environ.get("HTTP_COOKIE")) is not None
+        finally:
+            db.close_old_connections()
+        if not signed_in:
+            start_response("403 Forbidden", [("Content-Type", "text/plain")])
+            return [b"Sign in to open a serial console\n"]
+        return app(environ, start_response)
+    return wsgi
+
+
 class Server(BaseApplication):
     """gunicorn with one worker (the sessions live in it); a thread per connection."""
 
@@ -217,7 +234,7 @@ class Server(BaseApplication):
         self.cfg.set("control_socket_disable", True)
 
     def load(self):
-        return socketio.WSGIApp(sio)
+        return signed_in_only(socketio.WSGIApp(sio))
 
 
 if __name__ == "__main__":

console/tests/test_live_serial.py

@@ -15,7 +15,7 @@ from vrtManager.connection import connection_manager
 from accounts.models import UserInstance
 from computes.models import Compute
 from console import tokens
-from console.tests.test_socketiod_child import consoles, console, start_socketiod, socketio
+from console.tests.test_socketiod_child import consoles, console, session_key, start_socketiod, socketio
 from instances.models import Instance
 from vrtManager.tests import livetest
 
@@ -65,22 +65,22 @@ class LiveSerialConsoleTestCase(TransactionTestCase):
 
     def admin_token(self, token):
         host, uuid, _user, _type = tokens.SIGNER.unsign(token).split(":")
-        admin, _ = get_user_model().objects.get_or_create(username="serial-live-admin", is_superuser=True)
-        return tokens.sign(host, uuid, admin.id, "pty")
+        self.admin, _ = get_user_model().objects.get_or_create(username="serial-live-admin", is_superuser=True)
+        return tokens.sign(host, uuid, self.admin.id, "pty")
 
     def test_the_console_of_a_running_vm_opens_once(self):
         for args in self.hosts:
             with self.subTest(host=args[0], type=args[3]):
                 token, dom = self.token(*args)
-                client, received = console(self.port, token)
+                client, received = console(self.port, token, session_key(self.user))
                 deadline = time.monotonic() + 30
                 while "SeaBIOS" not in "".join(received["output"]) and time.monotonic() < deadline:
                     time.sleep(0.2)
                 self.assertIn("SeaBIOS", "".join(received["output"]))
                 # one client per VM; a superuser takes a console left open over
                 with self.assertRaises(socketio.exceptions.ConnectionError):
-                    console(self.port, token)
-                admin_client, admin_received = console(self.port, self.admin_token(token))
+                    console(self.port, token, session_key(self.user))
+                admin_client, admin_received = console(self.port, self.admin_token(token), session_key(self.admin))
                 wait_for(lambda: received["taken_over"])
                 self.assertEqual(received["taken_over"], ["serial-live-admin"])
                 client.disconnect()

console/tests/test_socketiod_child.py

@@ -13,9 +13,13 @@ import time
 import unittest
 from pathlib import Path
 
+from urllib.error import HTTPError
+from urllib.request import Request, urlopen
+
+from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.db import connection
-from django.test import TransactionTestCase
+from django.test import Client, TransactionTestCase
 
 from accounts.models import UserInstance
 from computes.models import Compute
@@ -23,6 +27,7 @@ from console import tokens
 from instances.models import Instance
 
 try:
+    import requests
     import socketio
 except ImportError:  # pragma: no cover
     socketio = None
@@ -84,9 +89,19 @@ def start_socketiod(test):
     test.fail("socketiod did not listen within 20 s")
 
 
-def console(port, token):
-    """A connected client and what it receives until the server disconnects it."""
-    client = socketio.Client(reconnection=False)
+def session_key(user):
+    """The session cookie value of user signed in, as the console page's browser has it."""
+    browser = Client()
+    browser.force_login(user)
+    return browser.cookies[settings.SESSION_COOKIE_NAME].value
+
+
+def console(port, token, session_key):
+    """A connected client, signed in with session_key as a browser would be,
+    and what it receives until the server disconnects it."""
+    browser = requests.Session()  # sends the cookie with each request, not only the first
+    browser.cookies.set(settings.SESSION_COOKIE_NAME, session_key)
+    client = socketio.Client(reconnection=False, http_session=browser)
     received = {"output": [], "taken_over": [], "gone": threading.Event()}
     client.on("pty_output", lambda data: received["output"].append(data["output"]))
     client.on("taken_over", lambda data: received["taken_over"].append(data["by"]))
@@ -103,6 +118,7 @@ class SocketiodProcessTestCase(TransactionTestCase):
     def setUp(self):
         compute = Compute.objects.create(name="local", hostname="localhost", login="", password="", type=4)
         user = get_user_model().objects.create_user("serial-owner", password="pw")
+        self.session_key = session_key(user)
         self.tokens = {}
         for n, name in enumerate(("vm-a", "vm-b"), 1):
             vm = Instance.objects.create(compute=compute, name=name, uuid=f"22222222-0000-0000-0000-00000000000{n}")
@@ -110,8 +126,8 @@ class SocketiodProcessTestCase(TransactionTestCase):
             self.tokens[name] = tokens.sign(compute.id, vm.uuid, user.id, "pty")
         self.server, self.port = start_socketiod(self)
 
-    def console(self, token):
-        return console(self.port, token)
+    def console(self, token, session_key=None):
+        return console(self.port, token, self.session_key if session_key is None else session_key)
 
     def test_each_client_gets_its_own_console(self):
         sessions = {name: self.console(token) for name, token in self.tokens.items()}
@@ -137,3 +153,15 @@ class SocketiodProcessTestCase(TransactionTestCase):
             with self.subTest(token=token), self.assertRaises(socketio.exceptions.ConnectionError):
                 self.console(token)
         self.assertEqual(consoles(self.server.pid), [])
+
+    def test_a_request_without_a_session_is_refused_before_it_takes_a_thread(self):
+        # Engine.IO would hold the connection (and a thread) before connect() checks the token
+        handshake = "http://127.0.0.1:%d/socket.io/?EIO=4&transport=polling" % self.port
+        name = settings.SESSION_COOKIE_NAME
+        for cookie in ("", f"{name}=not-a-session"):
+            with self.subTest(cookie=cookie), self.assertRaises(HTTPError) as refusal:
+                urlopen(Request(handshake, headers={"Cookie": cookie}), timeout=10)
+            self.assertEqual(refusal.exception.code, 403)
+        self.assertEqual(urlopen(Request(handshake, headers={"Cookie": f"{name}={self.session_key}"}), timeout=10).status, 200)
+        with self.assertRaises(socketio.exceptions.ConnectionError):  # a valid token is no use without the session
+            self.console(self.tokens["vm-a"], session_key="not-a-session")