diff --git a/console/novncd b/console/novncd index c11aca7b..758a92d4 100755 --- a/console/novncd +++ b/console/novncd @@ -86,52 +86,35 @@ except ImportError: def get_connection_infos(token): - from django.conf import settings from django.contrib.auth import get_user_model from django.core.signing import BadSignature, SignatureExpired, TimestampSigner from instances.models import Instance from instances.utils import can_open_console from vrtManager.instance import wvmInstance - host = None - uuid = None - user_id = None - + # The console view signs exactly "host:uuid:user_id". Anything else + # (unsigned, expired, or without the user part) is rejected, because the + # per-user access check below depends on user_id. signer = TimestampSigner(salt="console.novnc") try: unsigned = signer.unsign(token, max_age=120) - parts = unsigned.split(":") - if len(parts) >= 2: - host = int(parts[0]) - uuid = parts[1] - if len(parts) >= 3: - user_id = int(parts[2]) + host, uuid, user_id = unsigned.split(":") + host = int(host) + user_id = int(user_id) except (BadSignature, SignatureExpired, ValueError, TypeError): - if "test" in sys.argv: - try: - temptoken = token.split("-", 1) - host = int(temptoken[0]) - uuid = temptoken[1] - except Exception: - raise - else: - raise PermissionError("Invalid or expired console token") - - if host is None or uuid is None: - raise ValueError("Invalid console token format") + raise PermissionError("Invalid or expired console token") try: instance = Instance.objects.get(compute_id=host, uuid=uuid) - if user_id is not None: - User = get_user_model() - try: - user = User.objects.get(id=user_id) - except User.DoesNotExist: - raise PermissionError("User not found") - if not user.is_active: - raise PermissionError("User account is inactive or disabled") - if not can_open_console(user, instance): - raise PermissionError("User no longer has access to this instance") + User = get_user_model() + try: + user = User.objects.get(id=user_id) + except User.DoesNotExist: + raise PermissionError("User not found") + if not user.is_active: + raise PermissionError("User account is inactive or disabled") + if not can_open_console(user, instance): + raise PermissionError("User no longer has access to this instance") conn = wvmInstance( instance.compute.hostname, diff --git a/console/tests.py b/console/tests.py index b006ffe6..84cf95d9 100644 --- a/console/tests.py +++ b/console/tests.py @@ -525,6 +525,38 @@ class NovncdDaemonLogicTestCase(TestCase): with self.assertRaises(PermissionError): novncd_mod.get_connection_infos(token) + def test_get_connection_infos_rejects_signed_token_without_user(self): + # The console view always signs host:uuid:user_id; a token without the + # user part would skip the per-user access check entirely. + compute = Compute.objects.create( + name="novnc-2part-compute", hostname="127.0.0.1", login="root", password="", type=1 + ) + instance = Instance.objects.create( + compute=compute, + name="novnc-2part-vm", + uuid="66666666-6666-6666-6666-666666666666", + ) + signer = TimestampSigner(salt="console.novnc") + token = signer.sign(f"{compute.id}:{instance.uuid}") + + with self.assertRaises(PermissionError): + novncd_mod.get_connection_infos(token) + + def test_get_connection_infos_rejects_unsigned_token_even_under_test_runner(self): + # sys.argv contains "test" while this suite runs; production code must + # not change behaviour based on that. + compute = Compute.objects.create( + name="novnc-unsigned-compute", hostname="127.0.0.1", login="root", password="", type=1 + ) + instance = Instance.objects.create( + compute=compute, + name="novnc-unsigned-vm", + uuid="88888888-8888-8888-8888-888888888888", + ) + + with self.assertRaises(PermissionError): + novncd_mod.get_connection_infos(f"{compute.id}-{instance.uuid}") + def test_get_connection_infos_view_instances_user_is_not_enough(self): User = get_user_model() viewer = User.objects.create_user(username="novnc_viewer", password="password")