Reject open redirection in the console proxy

Our console proxies (novnc, serial, spice) run in a websockify server
whose request handler inherits from the python standard
SimpleHTTPRequestHandler. There is a known issue [1] in the
SimpleHTTPRequestHandler which allows open redirects by way of URLs
in the following format:

  http://vncproxy.my.domain.com//example.com/%2F..

which if visited, will redirect a user to example.com.

We can intercept a request and reject requests that pass a redirection
URL beginning with "//" by implementing the
SimpleHTTPRequestHandler.send_head() method containing the
vulnerability to reject such requests with a 400 Bad Request.

This code is copied from a patch suggested in one of the issue comments
[2].

Closes-Bug: #1927677

[1] https://bugs.python.org/issue32084
[2] https://bugs.python.org/issue32084#msg306545

Conflicts:
    nova/console/websocketproxy.py
    nova/tests/unit/console/test_websocketproxy.py

NOTE(melwitt): The conflicts are because the following changes are not
in Victoria:

  Ib2c406327fef2fb4868d8050fc476a7d17706e23 (Remove six.moves)
  I58b0382c86d4ef798572edb63d311e0e3e6937bb (Refactor and rename
    test_tcp_rst_no_compute_rpcapi)

Change-Id: Ie36401c782f023d1d5f2623732619105dc2cfa24
(cherry picked from commit 781612b)
(cherry picked from commit 4709256)

Author: melwitt

nova/console/websocketproxy.py

@@ -19,6 +19,8 @@
 '''
 
 import copy
+from http import HTTPStatus
+import os
 import socket
 import sys
 
@@ -281,6 +283,27 @@ def new_websocket_client(self):
     def socket(self, *args, **kwargs):
         return websockifyserver.WebSockifyServer.socket(*args, **kwargs)
 
+    def send_head(self):
+        # This code is copied from this example patch:
+        # https://bugs.python.org/issue32084#msg306545
+        path = self.translate_path(self.path)
+        if os.path.isdir(path):
+            parts = urlparse.urlsplit(self.path)
+            if not parts.path.endswith('/'):
+                # redirect browser - doing basically what apache does
+                new_parts = (parts[0], parts[1], parts[2] + '/',
+                              parts[3], parts[4])
+                new_url = urlparse.urlunsplit(new_parts)
+
+                # Browsers interpret "Location: //uri" as an absolute URI
+                # like "http://URI"
+                if new_url.startswith('//'):
+                    self.send_error(HTTPStatus.BAD_REQUEST,
+                                    "URI must not start with //")
+                    return None
+
+        return super(NovaProxyRequestHandler, self).send_head()
+
 
 class NovaWebSocketProxy(websockify.WebSocketProxy):
     def __init__(self, *args, **kwargs):

nova/tests/unit/console/test_websocketproxy.py

@@ -626,6 +626,40 @@ def test_tcp_rst_no_compute_rpcapi(self):
         self.wh.server.top_new_client(conn, address)
         self.assertIsNone(self.wh._compute_rpcapi)
 
+    def test_reject_open_redirect(self):
+        # This will test the behavior when an attempt is made to cause an open
+        # redirect. It should be rejected.
+        mock_req = mock.MagicMock()
+        mock_req.makefile().readline.side_effect = [
+            b'GET //example.com/%2F.. HTTP/1.1\r\n',
+            b''
+        ]
+
+        # Collect the response data to verify at the end. The
+        # SimpleHTTPRequestHandler writes the response data by calling the
+        # request socket sendall() method.
+        self.data = b''
+
+        def fake_sendall(data):
+            self.data += data
+
+        mock_req.sendall.side_effect = fake_sendall
+
+        client_addr = ('8.8.8.8', 54321)
+        mock_server = mock.MagicMock()
+        # This specifies that the server will be able to handle requests other
+        # than only websockets.
+        mock_server.only_upgrade = False
+
+        # Constructing a handler will process the mock_req request passed in.
+        websocketproxy.NovaProxyRequestHandler(
+            mock_req, client_addr, mock_server)
+
+        # Verify no redirect happens and instead a 400 Bad Request is returned.
+        self.data = self.data.decode()
+        self.assertIn('Error code: 400', self.data)
+        self.assertIn('Message: URI must not start with //', self.data)
+
     @mock.patch('websockify.websocketproxy.select_ssl_version')
     def test_ssl_min_version_is_not_set(self, mock_select_ssl):
         websocketproxy.NovaWebSocketProxy()

releasenotes/notes/console-proxy-reject-open-redirect-4ac0a7895acca7eb.yaml

@@ -0,0 +1,19 @@
+---
+security:
+  - |
+    A vulnerability in the console proxies (novnc, serial, spice) that allowed
+    open redirection has been `patched`_. The novnc, serial, and spice console
+    proxies are implemented as websockify servers and the request handler
+    inherits from the python standard SimpleHTTPRequestHandler. There is a
+    `known issue`_ in the SimpleHTTPRequestHandler which allows open redirects
+    by way of URLs in the following format::
+
+      http://vncproxy.my.domain.com//example.com/%2F..
+
+    which if visited, will redirect a user to example.com.
+
+    The novnc, serial, and spice console proxies will now reject requests that
+    pass a redirection URL beginning with "//" with a 400 Bad Request.
+
+    .. _patched: https://bugs.launchpad.net/nova/+bug/1927677
+    .. _known issue: https://bugs.python.org/issue32084