diff --git a/console/socketiod b/console/socketiod index 2215ac5b..cb02db20 100755 --- a/console/socketiod +++ b/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__": diff --git a/console/tests/test_live_serial.py b/console/tests/test_live_serial.py index 74b164ea..f69be9b2 100644 --- a/console/tests/test_live_serial.py +++ b/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() diff --git a/console/tests/test_socketiod_child.py b/console/tests/test_socketiod_child.py index d2308b50..704f1d5a 100644 --- a/console/tests/test_socketiod_child.py +++ b/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")