From 9606c80402f6db20d62b689c58aa8f024183628a Mon Sep 17 00:00:00 2001 From: Mohammed Naser Date: Tue, 2 Apr 2019 11:34:58 -0400 Subject: [PATCH] Add 'path' query parameter to console access url Starting in noVNC v1.1.0, the token query parameter is no longer forwarded via cookie [1]. We must instead use the 'path' query parameter to pass the token through to the websocketproxy [2]. This means that if someone deploys noVNC v1.1.0, VNC consoles will break in nova because the code is relying on the cookie functionality that v1.1.0 removed. This modifies the ConsoleAuthToken.access_url property to include the 'path' query parameter as part of the returned access_url that the client will use to call the console proxy service. This change is backward compatible with noVNC < v1.1.0. The 'path' query parameter is a long supported feature in noVNC. Co-Authored-By: melanie witt Closes-Bug: #1822676 [1] https://github.com/novnc/noVNC/commit/51f9f0098d306bbc67cc8e02ae547921b6f6585c [2] https://github.com/novnc/noVNC/pull/1220 Change-Id: I2ddf0f4d768b698e980594dd67206464a9cea37b --- .../get-rdp-console-post-resp.json | 4 +-- .../get-serial-console-post-resp.json | 4 +-- .../get-spice-console-post-resp.json | 2 +- .../get-vnc-console-post-resp.json | 2 +- .../v2.6/create-vnc-console-resp.json | 2 +- .../SCH_5009_V00_NUAC-VNC_OpenStack.svg | 4 +-- doc/source/admin/remote-console-access.rst | 2 +- .../compute/rest_api_version_history.rst | 2 +- nova/objects/console_auth_token.py | 5 ++- .../get-rdp-console-post-resp.json.tpl | 2 +- .../get-serial-console-post-resp.json.tpl | 2 +- .../get-spice-console-post-resp.json.tpl | 2 +- .../get-vnc-console-post-resp.json.tpl | 2 +- .../test_console_auth_tokens.py | 4 ++- .../tests/unit/console/test_websocketproxy.py | 36 ++++++++++--------- .../unit/objects/test_console_auth_token.py | 7 ++-- .../support-novnc-1.1.0-ce677fe3381b2a11.yaml | 7 ++++ 17 files changed, 53 insertions(+), 36 deletions(-) create mode 100644 releasenotes/notes/support-novnc-1.1.0-ce677fe3381b2a11.yaml diff --git a/doc/api_samples/os-remote-consoles/get-rdp-console-post-resp.json b/doc/api_samples/os-remote-consoles/get-rdp-console-post-resp.json index 09f3ca3d8cc4..6cf16af42c8a 100644 --- a/doc/api_samples/os-remote-consoles/get-rdp-console-post-resp.json +++ b/doc/api_samples/os-remote-consoles/get-rdp-console-post-resp.json @@ -1,6 +1,6 @@ { "console": { "type": "rdp-html5", - "url": "http://127.0.0.1:6083/?token=191996c3-7b0f-42f3-95a7-f1839f2da6ed" + "url": "http://127.0.0.1:6083/?path=%3Ftoken%3D21efbb20-b84c-4d1f-807d-4e14f6884b7f" } -} +} \ No newline at end of file diff --git a/doc/api_samples/os-remote-consoles/get-serial-console-post-resp.json b/doc/api_samples/os-remote-consoles/get-serial-console-post-resp.json index 990c9653cca0..f144653fafe0 100644 --- a/doc/api_samples/os-remote-consoles/get-serial-console-post-resp.json +++ b/doc/api_samples/os-remote-consoles/get-serial-console-post-resp.json @@ -1,6 +1,6 @@ { "console": { "type": "serial", - "url":"ws://127.0.0.1:6083/?token=f9906a48-b71e-4f18-baca-c987da3ebdb3" + "url": "ws://127.0.0.1:6083/?path=%3Ftoken%3D6ac46b4c-2705-4d8b-baa3-1b6f1b0c7dd3" } -} +} \ No newline at end of file diff --git a/doc/api_samples/os-remote-consoles/get-spice-console-post-resp.json b/doc/api_samples/os-remote-consoles/get-spice-console-post-resp.json index f0e09f47db99..5d2b2cfecd65 100644 --- a/doc/api_samples/os-remote-consoles/get-spice-console-post-resp.json +++ b/doc/api_samples/os-remote-consoles/get-spice-console-post-resp.json @@ -1,6 +1,6 @@ { "console": { "type": "spice-html5", - "url": "http://127.0.0.1:6082/spice_auto.html?token=a30e5d08-6a20-4043-958f-0852440c6af4" + "url": "http://127.0.0.1:6082/spice_auto.html?path=%3Ftoken%3Da7bd9607-421c-44b9-8689-18e87ada2f78" } } \ No newline at end of file diff --git a/doc/api_samples/os-remote-consoles/get-vnc-console-post-resp.json b/doc/api_samples/os-remote-consoles/get-vnc-console-post-resp.json index fe15b779335b..faa6ce302254 100644 --- a/doc/api_samples/os-remote-consoles/get-vnc-console-post-resp.json +++ b/doc/api_samples/os-remote-consoles/get-vnc-console-post-resp.json @@ -1,6 +1,6 @@ { "console": { "type": "novnc", - "url": "http://127.0.0.1:6080/vnc_auto.html?token=191996c3-7b0f-42f3-95a7-f1839f2da6ed" + "url": "http://127.0.0.1:6080/vnc_auto.html?path=%3Ftoken%3Ddaae261f-474d-4cae-8f6a-1865278ed8c9" } } \ No newline at end of file diff --git a/doc/api_samples/os-remote-consoles/v2.6/create-vnc-console-resp.json b/doc/api_samples/os-remote-consoles/v2.6/create-vnc-console-resp.json index b427a6902229..12eade5a2bae 100644 --- a/doc/api_samples/os-remote-consoles/v2.6/create-vnc-console-resp.json +++ b/doc/api_samples/os-remote-consoles/v2.6/create-vnc-console-resp.json @@ -2,6 +2,6 @@ "remote_console": { "protocol": "vnc", "type": "novnc", - "url": "http://example.com:6080/vnc_auto.html?token=b60bcfc3-5fd4-4d21-986c-e83379107819" + "url": "http://example.com:6080/vnc_auto.html?path=%3Ftoken%3Db60bcfc3-5fd4-4d21-986c-e83379107819" } } diff --git a/doc/source/admin/figures/SCH_5009_V00_NUAC-VNC_OpenStack.svg b/doc/source/admin/figures/SCH_5009_V00_NUAC-VNC_OpenStack.svg index 563dea780b00..f29341188293 100644 --- a/doc/source/admin/figures/SCH_5009_V00_NUAC-VNC_OpenStack.svg +++ b/doc/source/admin/figures/SCH_5009_V00_NUAC-VNC_OpenStack.svg @@ -467,12 +467,12 @@ Sheet.53 - Browses the url returned Http://novncip:port/?token=xyz + Browses the url returned Http://novncip:port/?path=%3Ftoken%3Dxyz Browses the url returnedHttp://novncip:port/?token=xyz + x="4" dy="1.2em" class="st13">Http://novncip:port/?path=%3Ftoken%3Dxyz Sheet.28 diff --git a/doc/source/admin/remote-console-access.rst b/doc/source/admin/remote-console-access.rst index eac4fd43ebda..b44050c900c7 100644 --- a/doc/source/admin/remote-console-access.rst +++ b/doc/source/admin/remote-console-access.rst @@ -30,7 +30,7 @@ nova provides services to handle this proxying. Consider a noVNC-based VNC console connection for example: #. A user connects to the API and gets an ``access_url`` such as, - ``http://ip:port/?token=xyz``. + ``http://ip:port/?path=%3Ftoken%3Dxyz``. #. The user pastes the URL in a browser or uses it as a client parameter. diff --git a/nova/api/openstack/compute/rest_api_version_history.rst b/nova/api/openstack/compute/rest_api_version_history.rst index 112a3ee061b5..9f2105e51b61 100644 --- a/nova/api/openstack/compute/rest_api_version_history.rst +++ b/nova/api/openstack/compute/rest_api_version_history.rst @@ -84,7 +84,7 @@ Example response:: "remote_console": { "protocol": "vnc", "type": "novnc", - "url": "http://example.com:6080/vnc_auto.html?token=XYZ" + "url": "http://example.com:6080/vnc_auto.html?path=%3Ftoken%3DXYZ" } } diff --git a/nova/objects/console_auth_token.py b/nova/objects/console_auth_token.py index e6e5a2e58c8d..0e243129ad4b 100644 --- a/nova/objects/console_auth_token.py +++ b/nova/objects/console_auth_token.py @@ -18,6 +18,7 @@ from oslo_log import log as logging from oslo_utils import strutils from oslo_utils import timeutils from oslo_utils import uuidutils +import six.moves.urllib.parse as urlparse from nova.db import api as db from nova import exception @@ -60,7 +61,9 @@ class ConsoleAuthToken(base.NovaTimestampObject, base.NovaObject): specific to this authorization. """ if self.obj_attr_is_set('id'): - return '%s?token=%s' % (self.access_url_base, self.token) + qparams = {'path': '?token=%s' % self.token} + return '%s?%s' % (self.access_url_base, + urlparse.urlencode(qparams)) @staticmethod def _from_db_object(context, obj, db_obj): diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-rdp-console-post-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-rdp-console-post-resp.json.tpl index c3955d6ac0ec..cbe172ef8620 100644 --- a/nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-rdp-console-post-resp.json.tpl +++ b/nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-rdp-console-post-resp.json.tpl @@ -1,6 +1,6 @@ { "console": { "type": "rdp-html5", - "url": "http://127.0.0.1:6083/?token=%(uuid)s" + "url": "http://127.0.0.1:6083/?path=%%3Ftoken%%3D%(uuid)s" } } diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-serial-console-post-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-serial-console-post-resp.json.tpl index 721ce2b2ea8c..384938463d92 100644 --- a/nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-serial-console-post-resp.json.tpl +++ b/nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-serial-console-post-resp.json.tpl @@ -1,6 +1,6 @@ { "console": { "type": "serial", - "url": "ws://127.0.0.1:6083/?token=%(uuid)s" + "url": "ws://127.0.0.1:6083/?path=%%3Ftoken%%3D%(uuid)s" } } diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-spice-console-post-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-spice-console-post-resp.json.tpl index 65b72a866ffe..4a53a05f39ec 100644 --- a/nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-spice-console-post-resp.json.tpl +++ b/nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-spice-console-post-resp.json.tpl @@ -1,6 +1,6 @@ { "console": { "type": "spice-html5", - "url": "http://127.0.0.1:6082/spice_auto.html?token=%(uuid)s" + "url": "http://127.0.0.1:6082/spice_auto.html?path=%%3Ftoken%%3D%(uuid)s" } } diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-vnc-console-post-resp.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-vnc-console-post-resp.json.tpl index 2eeee7c54373..b0b119cf2205 100644 --- a/nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-vnc-console-post-resp.json.tpl +++ b/nova/tests/functional/api_sample_tests/api_samples/os-remote-consoles/get-vnc-console-post-resp.json.tpl @@ -1,6 +1,6 @@ { "console": { "type": "novnc", - "url": "http://127.0.0.1:6080/vnc_auto.html?token=%(uuid)s" + "url": "http://127.0.0.1:6080/vnc_auto.html?path=%%3Ftoken%%3D%(uuid)s" } } diff --git a/nova/tests/functional/api_sample_tests/test_console_auth_tokens.py b/nova/tests/functional/api_sample_tests/test_console_auth_tokens.py index fcd79d54b6f7..b41b21b1c059 100644 --- a/nova/tests/functional/api_sample_tests/test_console_auth_tokens.py +++ b/nova/tests/functional/api_sample_tests/test_console_auth_tokens.py @@ -15,6 +15,7 @@ import re from oslo_serialization import jsonutils +import six.moves.urllib.parse as urlparse from nova.tests.functional.api_sample_tests import test_servers @@ -32,7 +33,8 @@ class ConsoleAuthTokensSampleJsonTests(test_servers.ServersSampleBase): {'action': 'os-getRDPConsole'}) url = self._get_console_url(response.content) - return re.match('.+?token=([^&]+)', url).groups()[0] + path = urlparse.urlencode({'path': '?token='}) + return re.match('.+?%s([^&]+)' % path, url).groups()[0] def test_get_console_connect_info(self): self.flags(enabled=True, group='rdp') diff --git a/nova/tests/unit/console/test_websocketproxy.py b/nova/tests/unit/console/test_websocketproxy.py index 750aa9d5339a..9faa5e02291c 100644 --- a/nova/tests/unit/console/test_websocketproxy.py +++ b/nova/tests/unit/console/test_websocketproxy.py @@ -19,6 +19,7 @@ import socket import mock from oslo_utils.fixture import uuidsentinel as uuids +import six.moves.urllib.parse as urlparse import nova.conf from nova.console.securityproxy import base @@ -47,6 +48,7 @@ class NovaProxyRequestHandlerDBTestCase(test.TestCase): self.wh.msg = mock.MagicMock() self.wh.do_proxy = mock.MagicMock() self.wh.headers = mock.MagicMock() + self.path = urlparse.urlencode({'path': '?token=123-456-789'}) def _fake_console_db(self, **updates): console_db = copy.deepcopy(fake_ca.fake_token_dict) @@ -96,7 +98,7 @@ class NovaProxyRequestHandlerDBTestCase(test.TestCase): tsock.recv.return_value = "HTTP/1.1 200 OK\r\n\r\n" self.wh.socket.return_value = tsock - self.wh.path = "http://127.0.0.1/?token=123-456-789" + self.wh.path = "http://127.0.0.1/?%s" % self.path self.wh.headers = self.fake_header if instance_not_found: @@ -143,6 +145,8 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase): self.wh.msg = mock.MagicMock() self.wh.do_proxy = mock.MagicMock() self.wh.headers = mock.MagicMock() + self.path = urlparse.urlencode({'path': '?token=123-456-789'}) + self.path_invalid = urlparse.urlencode({'path': '?token=XXX'}) fake_header = { 'cookie': 'token="123-456-789"', @@ -208,7 +212,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase): 'access_url': 'https://example.net:6080' } self.wh.socket.return_value = '' - self.wh.path = "http://127.0.0.1/?token=123-456-789" + self.wh.path = "http://127.0.0.1/?%s" % self.path self.wh.headers = self.fake_header self.wh.new_websocket_client() @@ -241,7 +245,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase): validate.return_value = objects.ConsoleAuthToken(**params) self.wh.socket.return_value = '' - self.wh.path = "http://127.0.0.1/?token=123-456-789" + self.wh.path = "http://127.0.0.1/?%s" % self.path self.wh.headers = self.fake_header self.wh.new_websocket_client() @@ -267,7 +271,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase): validate.return_value = objects.ConsoleAuthToken(**params) self.wh.socket.return_value = '' - self.wh.path = "http://127.0.0.1/?token=123-456-789" + self.wh.path = "http://127.0.0.1/?%s" % self.path self.wh.headers = self.fake_header self.wh.new_websocket_client() @@ -292,7 +296,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase): validate.return_value = objects.ConsoleAuthToken(**params) self.wh.socket.return_value = '' - self.wh.path = "http://[2001:db8::1]/?token=123-456-789" + self.wh.path = "http://[2001:db8::1]/?%s" % self.path self.wh.headers = self.fake_header_ipv6 self.wh.new_websocket_client() @@ -305,7 +309,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase): def test_new_websocket_client_token_invalid(self, validate): validate.side_effect = exception.InvalidToken(token='XXX') - self.wh.path = "http://127.0.0.1/?token=XXX" + self.wh.path = "http://127.0.0.1/?%s" % self.path_invalid self.wh.headers = self.fake_header_bad_token self.assertRaises(exception.InvalidToken, @@ -333,7 +337,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase): tsock.recv.return_value = "HTTP/1.1 200 OK\r\n\r\n" self.wh.socket.return_value = tsock - self.wh.path = "http://127.0.0.1/?token=123-456-789" + self.wh.path = "http://127.0.0.1/?%s" % self.path self.wh.headers = self.fake_header self.wh.new_websocket_client() @@ -366,7 +370,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase): tsock.recv.return_value = "HTTP/1.1 500 Internal Server Error\r\n\r\n" self.wh.socket.return_value = tsock - self.wh.path = "http://127.0.0.1/?token=123-456-789" + self.wh.path = "http://127.0.0.1/?%s" % self.path self.wh.headers = self.fake_header self.assertRaises(exception.InvalidConnectionInfo, @@ -398,7 +402,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase): HTTP_RESP] self.wh.socket.return_value = tsock - self.wh.path = "http://127.0.0.1/?token=123-456-789" + self.wh.path = "http://127.0.0.1/?%s" % self.path self.wh.headers = self.fake_header self.wh.new_websocket_client() @@ -428,7 +432,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase): validate.return_value = objects.ConsoleAuthToken(**params) self.wh.socket.return_value = '' - self.wh.path = "http://127.0.0.1/?token=123-456-789" + self.wh.path = "http://127.0.0.1/?%s" % self.path self.wh.headers = self.fake_header self.wh.new_websocket_client() @@ -448,7 +452,7 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase): 'console_type': 'novnc' } self.wh.socket.return_value = '' - self.wh.path = "ws://127.0.0.1/?token=123-456-789" + self.wh.path = "ws://127.0.0.1/?%s" % self.path self.wh.headers = self.fake_header self.assertRaises(exception.NovaException, @@ -457,10 +461,9 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase): @mock.patch('socket.getfqdn') def test_address_string_doesnt_do_reverse_dns_lookup(self, getfqdn): request_mock = mock.MagicMock() - request_mock.makefile().readline.side_effect = [ - b'GET /vnc.html?token=123-456-789 HTTP/1.1\r\n', - b'' - ] + msg = 'GET /vnc.html?%s HTTP/1.1\r\n' % self.path + request_mock.makefile().readline.side_effect = [msg.encode('utf-8'), + b''] server_mock = mock.MagicMock() client_address = ('8.8.8.8', 54321) @@ -714,7 +717,8 @@ class NovaWebsocketSecurityProxyTestCase(test.NoDBTestCase): with mock.patch('websockify.ProxyRequestHandler'): self.wh = websocketproxy.NovaProxyRequestHandler() self.wh.server = self.server - self.wh.path = "http://127.0.0.1/?token=123-456-789" + path = urlparse.urlencode({'path': '?token=123-456-789'}) + self.wh.path = "http://127.0.0.1/?%s" % path self.wh.socket = mock.MagicMock() self.wh.msg = mock.MagicMock() self.wh.do_proxy = mock.MagicMock() diff --git a/nova/tests/unit/objects/test_console_auth_token.py b/nova/tests/unit/objects/test_console_auth_token.py index bbc33086b0d3..9138c6c305ab 100644 --- a/nova/tests/unit/objects/test_console_auth_token.py +++ b/nova/tests/unit/objects/test_console_auth_token.py @@ -19,6 +19,7 @@ import mock from oslo_db.exception import DBDuplicateEntry from oslo_utils.fixture import uuidsentinel from oslo_utils import timeutils +import six.moves.urllib.parse as urlparse from nova import exception from nova.objects import console_auth_token as token_obj @@ -70,9 +71,9 @@ class _TestConsoleAuthToken(object): self.compare_obj(obj, expected) url = obj.access_url - expected_url = '%s?token=%s' % ( - fakes.fake_token_dict['access_url_base'], - fakes.fake_token) + path = urlparse.urlencode({'path': '?token=%s' % fakes.fake_token}) + expected_url = '%s?%s' % ( + fakes.fake_token_dict['access_url_base'], path) self.assertEqual(expected_url, url) @mock.patch('nova.db.api.console_auth_token_create') diff --git a/releasenotes/notes/support-novnc-1.1.0-ce677fe3381b2a11.yaml b/releasenotes/notes/support-novnc-1.1.0-ce677fe3381b2a11.yaml new file mode 100644 index 000000000000..5b47d478dfd9 --- /dev/null +++ b/releasenotes/notes/support-novnc-1.1.0-ce677fe3381b2a11.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Add support for noVNC >= v1.1.0 for VNC consoles. Prior to this fix, VNC + console token validation always failed regardless of actual token validity + with noVNC >= v1.1.0. See + https://bugs.launchpad.net/nova/+bug/1822676 for more details.