mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-08 11:16:47 +00:00
fix(console): a VNC console connects to the VM its page was opened for
- novncd takes the console token only from its websocket URL. The page also put the token in a cookie, one for the whole browser, and novncd preferred the cookie: with two consoles open, a tab that (re)connected after another console was opened got that other VM. - novncd opens the VM by its UUID, the VM the access check was made for, not by its name, which another VM on the host may carry now.
This commit is contained in:
catborise
parent
8d7f28482f
commit
cbd6317e55
3 changed files with 24 additions and 76 deletions
|
|
@ -18,8 +18,7 @@ django.setup()
|
|||
import re
|
||||
import socket
|
||||
|
||||
# from six.moves import http_cookies as Cookie
|
||||
from http import cookies as Cookie
|
||||
from urllib.parse import parse_qs, urlparse
|
||||
import argparse
|
||||
from console.sshtunnels import SSHTunnels
|
||||
from vrtManager.connection import CONN_SOCKET, CONN_SSH
|
||||
|
|
@ -94,6 +93,7 @@ def get_connection_infos(token):
|
|||
instance.compute.password,
|
||||
instance.compute.type,
|
||||
instance.name,
|
||||
uuid=instance.uuid, # the VM access was checked for, whatever its name now
|
||||
)
|
||||
if instance.compute.hostname.count(":"):
|
||||
connhost = instance.compute.hostname.split(":")[0]
|
||||
|
|
@ -122,40 +122,11 @@ def get_connection_infos(token):
|
|||
|
||||
class CompatibilityMixIn(object):
|
||||
def _new_client(self, daemon, socket_factory):
|
||||
# NoVNC uses it's own convention that forward token
|
||||
# from the request to a cookie header, we should check
|
||||
# also for this behavior
|
||||
token = None
|
||||
hcookie = self.headers.get("cookie")
|
||||
|
||||
if hcookie:
|
||||
cookie = Cookie.SimpleCookie()
|
||||
for hcookie_part in hcookie.split(";"):
|
||||
hcookie_part = hcookie_part.lstrip()
|
||||
try:
|
||||
cookie.load(hcookie_part)
|
||||
except Cookie.CookieError:
|
||||
# NOTE(stgleb): Do not print out cookie content
|
||||
# for security reasons.
|
||||
self.msg("Found malformed cookie")
|
||||
else:
|
||||
if "token" in cookie:
|
||||
token = cookie["token"].value
|
||||
|
||||
# Fallback to query parameter if token not found in cookie
|
||||
if not token and hasattr(self, "path"):
|
||||
from urllib.parse import parse_qs, urlparse
|
||||
|
||||
try:
|
||||
parsed_url = urlparse(self.path)
|
||||
query_params = parse_qs(parsed_url.query)
|
||||
if "token" in query_params:
|
||||
token = query_params["token"][0]
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# The page puts its own console's token on its websocket URL: a cookie
|
||||
# is shared by the browser's tabs and would name the last console opened
|
||||
token = parse_qs(urlparse(self.path).query).get("token", [None])[0]
|
||||
if not token:
|
||||
self.msg("No console token provided in cookie or query parameters")
|
||||
self.msg("No console token in the websocket URL")
|
||||
return
|
||||
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -85,7 +85,7 @@ class ConsoleViewsTestCase(TestCase):
|
|||
self.assertEqual(response.status_code, 200)
|
||||
self.assertTemplateUsed(response, "console-vnc-lite.html")
|
||||
signer = TimestampSigner(salt="console.novnc")
|
||||
unsigned = signer.unsign(response.cookies["token"].value, max_age=120)
|
||||
unsigned = signer.unsign(response.context["token"], max_age=120)
|
||||
self.assertEqual(
|
||||
unsigned, f"{self.compute.id}:{self.instance.uuid}:{self.admin_user.id}"
|
||||
)
|
||||
|
|
@ -250,19 +250,13 @@ class ConsoleViewsTestCase(TestCase):
|
|||
self.assertContains(response, "const host = 'testserver';")
|
||||
|
||||
@patch("console.views.wvmInstance")
|
||||
def test_console_token_cookie_flags(self, mock_wvm):
|
||||
# The token cookie is only read by novncd, never by JS.
|
||||
mock_conn = MagicMock()
|
||||
mock_conn.get_console_type.return_value = "vnc"
|
||||
mock_conn.get_console_websocket_port.return_value = None
|
||||
mock_wvm.return_value = mock_conn
|
||||
|
||||
def test_the_page_sets_no_token_cookie(self, mock_wvm):
|
||||
# one cookie for every tab would name the last console opened
|
||||
mock_wvm.return_value.get_console_type.return_value = "vnc"
|
||||
mock_wvm.return_value.get_console_websocket_port.return_value = None
|
||||
self.client.force_login(self.admin_user)
|
||||
response = self.client.get(reverse("console") + f"?token={self.token}")
|
||||
|
||||
cookie = response.cookies["token"]
|
||||
self.assertTrue(cookie["httponly"])
|
||||
self.assertEqual(cookie["samesite"], "Lax")
|
||||
self.assertNotIn("token", response.cookies)
|
||||
|
||||
@patch("console.views.wvmInstance")
|
||||
def test_console_view_instances_perm_does_not_open_console(self, mock_wvm):
|
||||
|
|
@ -510,29 +504,17 @@ class NovncdDaemonLogicTestCase(TestCase):
|
|||
self.assertEqual(opts.cert, "/path/to/cert.pem")
|
||||
|
||||
@patch.object(novncd_mod, "get_connection_infos")
|
||||
def test_compatibility_mixin_token_from_cookie(self, mock_get_info):
|
||||
mock_get_info.return_value = (
|
||||
"host1",
|
||||
22,
|
||||
"root",
|
||||
1,
|
||||
"192.168.1.50",
|
||||
5900,
|
||||
None,
|
||||
)
|
||||
|
||||
def test_a_cookie_does_not_choose_the_console(self, mock_get_info):
|
||||
# another tab's console in a cookie: the URL's own token is used
|
||||
mock_get_info.return_value = ("host1", 22, "root", 1, "192.168.1.50", 5900, None)
|
||||
handler = self.CompatibilityMixIn()
|
||||
handler.headers = {"cookie": "other=123; token=1-uuid-from-cookie; test=456"}
|
||||
handler.path = "/novncd/"
|
||||
handler.headers = {"cookie": "token=1-uuid-of-another-tab"}
|
||||
handler.path = "/novncd/?token=1-uuid-of-this-tab"
|
||||
handler.msg = MagicMock()
|
||||
handler.do_proxy = MagicMock()
|
||||
handler.verbose = False
|
||||
|
||||
socket_factory = MagicMock()
|
||||
handler._new_client(daemon=False, socket_factory=socket_factory)
|
||||
|
||||
mock_get_info.assert_called_once_with("1-uuid-from-cookie")
|
||||
socket_factory.assert_called_once_with("host1", 5900, connect=True)
|
||||
handler._new_client(daemon=False, socket_factory=MagicMock())
|
||||
mock_get_info.assert_called_once_with("1-uuid-of-this-tab")
|
||||
|
||||
@patch.object(novncd_mod, "get_connection_infos")
|
||||
def test_compatibility_mixin_token_from_query_params(self, mock_get_info):
|
||||
|
|
@ -564,8 +546,8 @@ class NovncdDaemonLogicTestCase(TestCase):
|
|||
# libvirt on the panel's host, the VM's console listening on 127.0.0.1
|
||||
mock_get_info.return_value = ("localhost", None, "", novncd_mod.CONN_SOCKET, "127.0.0.1", 5902, None)
|
||||
handler = self.CompatibilityMixIn()
|
||||
handler.headers = {"cookie": "token=1-uuid"}
|
||||
handler.path = "/novncd/"
|
||||
handler.headers = {}
|
||||
handler.path = "/novncd/?token=1-uuid"
|
||||
handler.msg = MagicMock()
|
||||
handler.do_proxy = MagicMock()
|
||||
handler.verbose = False
|
||||
|
|
@ -586,9 +568,7 @@ class NovncdDaemonLogicTestCase(TestCase):
|
|||
|
||||
mock_get_info.assert_not_called()
|
||||
socket_factory.assert_not_called()
|
||||
handler.msg.assert_called_with(
|
||||
"No console token provided in cookie or query parameters"
|
||||
)
|
||||
handler.msg.assert_called_with("No console token in the websocket URL")
|
||||
|
||||
@patch("console.tokens.authorize")
|
||||
@patch("vrtManager.instance.wvmInstance")
|
||||
|
|
@ -615,6 +595,8 @@ class NovncdDaemonLogicTestCase(TestCase):
|
|||
infos = novncd_mod.get_connection_infos(token)
|
||||
self.assertEqual(infos[0], "127.0.0.1")
|
||||
self.assertEqual(infos[5], 5900)
|
||||
# the VM access was checked for, not whichever VM has its name now
|
||||
self.assertEqual(mock_wvm.call_args.kwargs["uuid"], mock_instance.uuid)
|
||||
|
||||
@patch("sys.argv", ["novncd"])
|
||||
@override_settings(DEBUG=False)
|
||||
|
|
|
|||
|
|
@ -150,9 +150,4 @@ def console(request):
|
|||
"Console type '%(type)s' is not supported. Please switch the VM graphics to VNC."
|
||||
) % {"type": console_type}
|
||||
response = render(request, "console-vnc-lite.html", locals())
|
||||
|
||||
# Only novncd reads this cookie; JavaScript gets the token from the page.
|
||||
response.set_cookie(
|
||||
"token", token, httponly=True, samesite="Lax", secure=request.is_secure()
|
||||
)
|
||||
return response
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue