fix(console): a serial session follows its user's access; a superuser takes over

- The console worker learns whose session it is and checks every ten
  seconds that the user may still open the console: an owner removed or a
  user deactivated meanwhile loses it, with a line in the terminal. Access
  was checked only when the session started, which could last for days.
- One console per VM: a console left open in a forgotten tab locked
  everyone out, superusers included. A superuser's connection now takes
  the console over, and the other window is told so.

conf/daemon/consolecallback

@@ -1,5 +1,5 @@
 #!/usr/bin/env python3
-# consolecallback <instance id> - the serial console of one VM on stdin/stdout
+# consolecallback <instance id> <user id> - the serial console of one VM on stdin/stdout
 # (socketiod runs it in a pty, one per client). It opens the console through
 # the compute's connection (its stored credentials); the connection's event
 # loop thread runs the stream callbacks. The console survives guest reboots.
@@ -18,8 +18,11 @@ os.environ.setdefault("DJANGO_SETTINGS_MODULE", "webvirtcloud.settings")
 django.setup()
 
 import libvirt  # noqa: E402
+from django import db  # noqa: E402
+from django.contrib.auth import get_user_model  # noqa: E402
 
 from instances.models import Instance  # noqa: E402
+from instances.utils import can_open_console  # noqa: E402
 from operations.utils import public_error_text  # noqa: E402
 from vrtManager.connection import connection_manager  # noqa: E402
 
@@ -27,6 +30,7 @@ from vrtManager.connection import connection_manager  # noqa: E402
 # raised and told in one line instead
 libvirt.registerErrorHandler(lambda context, error: None, None)
 SEND_WAIT = 5  # seconds a paste may wait for the VM to take it
+ACCESS_CHECK = 10  # seconds between checks that the user may still use the console
 
 
 def say(text):
@@ -97,8 +101,19 @@ def from_vm(stream, events, console):
     os.write(1, data)
 
 
+def may_still_open(user_id, instance):
+    """The user still may open the console: an owner may have been removed or
+    the user deactivated since the session started."""
+    try:
+        user = get_user_model().objects.filter(pk=user_id).first()
+        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 main():
     instance = Instance.objects.select_related("compute").get(pk=int(sys.argv[1]))
+    user_id = int(sys.argv[2])
     c = instance.compute
     try:
         console = Console(connection_manager.get_connection(c.hostname, c.login, c.password, c.type), instance.uuid)
@@ -110,10 +125,16 @@ def main():
     if console.stream is None:
         say("The VM is not running: its console opens when it starts")
     libvirt.virEventAddHandle(0, libvirt.VIR_EVENT_HANDLE_READABLE, from_client, console)
+    checked = time.monotonic()
     try:
         while console.running:
             time.sleep(0.2)
             console.check()
+            if time.monotonic() - checked > ACCESS_CHECK:
+                if not may_still_open(user_id, instance):
+                    say("You may no longer open this console")
+                    sys.exit(1)
+                checked = time.monotonic()
     except libvirt.libvirtError as err:  # the host went away, the VM was undefined or migrated
         say("The console of %s closed: %s" % (instance.name, public_error_text(err)))
         sys.exit(1)

console/novncd

@@ -86,7 +86,7 @@ def get_connection_infos(token):
     from vrtManager.instance import wvmInstance
 
     try:
-        instance = authorize(token)
+        instance, _user = authorize(token)
         conn = wvmInstance(
             instance.compute.hostname,
             instance.compute.login,

console/socketiod

@@ -49,26 +49,35 @@ def connect(sid, environ, auth):
     if not settings.SERIAL_CONSOLE_ENABLED:
         raise socketio.exceptions.ConnectionRefusedError("The serial console is turned off on this server")
     try:
-        instance = authorize((auth or {}).get("token"))
+        instance, user = authorize((auth or {}).get("token"))
     except PermissionError:
         raise socketio.exceptions.ConnectionRefusedError("The console link has expired: reload the page")
     finally:
         db.close_old_connections()
+    taken = None
     with sessions_lock:
-        if any(s["vm"] == instance.pk for s in sessions.values()):
+        open_sid = next((s for s, session in sessions.items() if session["vm"] == instance.pk), None)
+        if open_sid is not None and not user.is_superuser:
             raise socketio.exceptions.ConnectionRefusedError("The console of this VM is open in another window")
-        if len(sessions) >= MAX_SESSIONS:
+        if open_sid is None and len(sessions) >= MAX_SESSIONS:
             raise socketio.exceptions.ConnectionRefusedError("Too many consoles are open on this server: try again later")
+        if open_sid is not None:  # a superuser takes over a console left open
+            taken = open_sid, sessions.pop(open_sid)
         pid, fd = pty.fork()
         if pid == 0:
             try:
                 # pty.fork's master is inheritable: without this each worker would
                 # hold the other sessions' consoles open
                 os.closerange(3, os.sysconf("SC_OPEN_MAX"))
-                os.execv(sys.executable, [sys.executable, CONSOLECALLBACK, str(instance.pk)])
+                os.execv(sys.executable, [sys.executable, CONSOLECALLBACK, str(instance.pk), str(user.pk)])
             finally:
                 os._exit(1)
         sessions[sid] = session = {"pid": pid, "fd": fd, "vm": instance.pk, "lock": threading.Lock(), "open": True}
+    if taken:
+        old_sid, old_session = taken
+        sio.emit("pty_output", {"output": "\r\n[An administrator opened this console]\r\n"}, to=old_sid)
+        end(old_session)
+        sio.disconnect(old_sid)
     sio.start_background_task(forward, sid, session)
 
 
@@ -102,8 +111,12 @@ def stop(sid):
     """End the sid's worker; once, whoever calls it first."""
     with sessions_lock:
         session = sessions.pop(sid, None)
-    if session is None:
-        return
+    if session is not None:
+        end(session)
+
+
+def end(session):
+    """End a session taken out of sessions."""
     try:
         os.kill(session["pid"], signal.SIGKILL)  # first: a write blocked on its terminal ends
     except ProcessLookupError:

console/tests/test_consolecallback.py

@@ -7,6 +7,13 @@ from pathlib import Path
 from unittest import TestCase
 from unittest.mock import MagicMock, patch
 
+from django.contrib.auth import get_user_model
+from django.test import TestCase as DjangoTestCase
+
+from accounts.models import UserInstance
+from computes.models import Compute
+from instances.models import Instance
+
 CONSOLECALLBACK = Path(__file__).resolve().parent.parent.parent / "conf" / "daemon" / "consolecallback"
 
 
@@ -57,3 +64,19 @@ class ConsoleWorkerTests(TestCase):
             self.cb.from_client(None, 0, None, self.console)
         self.assertFalse(self.console.running)
         self.console.stream.send.assert_not_called()
+
+
+class AccessTests(DjangoTestCase):
+    """The worker ends a session whose user may no longer open the console."""
+
+    def test_an_owner_removed_meanwhile_loses_the_console(self):
+        cb = load()
+        compute = Compute.objects.create(name="c", hostname="192.0.2.1", login="u", password="p", type=1)
+        vm = Instance.objects.create(compute=compute, name="vm-a", uuid="11111111-0000-0000-0000-000000000009")
+        owner = get_user_model().objects.create_user("owner", password="pw")
+        ownership = UserInstance.objects.create(user=owner, instance=vm)
+        with patch.object(cb.db.connection, "close"):  # the test's transaction keeps its connection
+            self.assertTrue(cb.may_still_open(owner.pk, vm))
+            ownership.delete()
+            self.assertFalse(cb.may_still_open(owner.pk, vm))
+            self.assertFalse(cb.may_still_open(999999, vm))

console/tests/test_live_serial.py

@@ -23,6 +23,12 @@ from instances.tests import livetest
 P = livetest.PREFIX
 
 
+def wait_for(condition, seconds=30):
+    deadline = time.monotonic() + seconds
+    while not condition() and time.monotonic() < deadline:
+        time.sleep(0.2)
+
+
 def serial_vm(conn, name):
     """A paused VM without graphics whose BIOS writes to the serial console once resumed."""
     dom = conn.defineXML(
@@ -57,6 +63,11 @@ class LiveSerialConsoleTestCase(TransactionTestCase):
         UserInstance.objects.create(user=self.user, instance=vm)
         return tokens.SIGNER.sign(f"{compute.id}:{vm.uuid}:{self.user.id}"), dom
 
+    def admin_token(self, token):
+        host, uuid, _user = tokens.SIGNER.unsign(token).split(":")
+        admin, _ = get_user_model().objects.get_or_create(username="serial-live-admin", is_superuser=True)
+        return tokens.SIGNER.sign(f"{host}:{uuid}:{admin.id}")
+
     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]):
@@ -68,12 +79,17 @@ class LiveSerialConsoleTestCase(TransactionTestCase):
                 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
+                # 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))
+                wait_for(lambda: "An administrator opened this console" in "".join(received["output"]))
+                self.assertIn("An administrator opened this console", "".join(received["output"]))
                 client.disconnect()
-                deadline = time.monotonic() + 5
-                while consoles(self.server.pid) and time.monotonic() < deadline:
-                    time.sleep(0.05)
-                self.assertEqual(consoles(self.server.pid), [])
+                # the VM stops: the terminal says so
                 dom.destroy()
+                wait_for(lambda: "not running" in "".join(admin_received["output"]))
+                self.assertIn("The VM is not running", "".join(admin_received["output"]))
+                admin_client.disconnect()
+                wait_for(lambda: not consoles(self.server.pid))
+                self.assertEqual(consoles(self.server.pid), [])

console/tests/test_socketiod.py

@@ -40,7 +40,7 @@ class Base(TestCase):
 
 class TokenTestCase(Base):
     def test_a_token_opens_its_vm_for_a_user_who_may_open_it(self):
-        self.assertEqual(tokens.authorize(self.token()), self.vm)
+        self.assertEqual(tokens.authorize(self.token()), (self.vm, self.user))
 
     def test_refused_tokens(self):
         stranger = get_user_model().objects.create_user("stranger", password="pw")
@@ -142,6 +142,18 @@ class SocketiodTestCase(Base):
         write.assert_not_called()
         self.assertIsNone(session_input)
 
+    @override_settings(SERIAL_CONSOLE_ENABLED=True)
+    def test_a_superuser_takes_over_a_console_left_open(self):
+        admin = get_user_model().objects.create_superuser("root", "root@example.com", "pw")
+        self.d.connect("owner-tab", {}, {"token": self.token()})
+        old = self.d.sessions["owner-tab"]
+        with patch.object(self.d.sio, "emit") as emit, patch.object(self.d.sio, "disconnect") as disconnect:
+            self.assertIsNone(self.d.connect("admin-tab", {}, {"token": self.token(admin)}))
+        self.assertEqual(list(self.d.sessions), ["admin-tab"])
+        self.assertFalse(old["open"])
+        self.assertIn("An administrator opened this console", emit.call_args.args[1]["output"])
+        disconnect.assert_called_once_with("owner-tab")
+
     @override_settings(SERIAL_CONSOLE_ENABLED=True)
     def test_two_connections_at_once_start_one_console(self):
         import threading
@@ -157,7 +169,7 @@ class SocketiodTestCase(Base):
 
         threads = [threading.Thread(target=connect, args=(n,)) for n in range(8)]
         # the threads have their own database connections, outside this test's transaction
-        with patch.object(self.d, "authorize", return_value=self.vm):
+        with patch.object(self.d, "authorize", return_value=(self.vm, self.user)):
             for t in threads:
                 t.start()
             for t in threads:

console/tests/test_views.py

@@ -606,7 +606,7 @@ class NovncdDaemonLogicTestCase(TestCase):
         mock_instance.compute.password = ""
         mock_instance.compute.type = 1
         mock_instance.name = "test-vm"
-        mock_inst_get.return_value = mock_instance
+        mock_inst_get.return_value = (mock_instance, user)
 
         mock_conn = MagicMock()
         mock_conn.get_console_listener_addr.return_value = "127.0.0.1"

console/tokens.py

@@ -14,7 +14,8 @@ MAX_AGE = 120
 
 
 def authorize(token):
-    """The Instance the token opens; PermissionError otherwise."""
+    """The Instance the token opens and the user it was given to;
+    PermissionError otherwise."""
     try:
         host, uuid, user_id = SIGNER.unsign(token, max_age=MAX_AGE).split(":")
         host, user_id = int(host), int(user_id)
@@ -24,4 +25,4 @@ def authorize(token):
     user = get_user_model().objects.filter(id=user_id, is_active=True).first()
     if instance is None or user is None or not can_open_console(user, instance):
         raise PermissionError("No access to this console")
-    return instance
+    return instance, user