Merge "address open redirect with 3 forward slashes" into stable/victoria22.3.0

2 files changed, 35 insertions, 6 deletions

diff --git a/nova/console/websocketproxy.py b/nova/console/websocketproxy.py
index 1410d64260..8512ac62ad 100644
--- a/nova/console/websocketproxy.py
+++ b/nova/console/websocketproxy.py
@@ -290,14 +290,9 @@ class NovaProxyRequestHandler(websockify.ProxyRequestHandler):
         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('//'):
+                if self.path.startswith('//'):
                     self.send_error(HTTPStatus.BAD_REQUEST,
                                     "URI must not start with //")
                     return None
diff --git a/nova/tests/unit/console/test_websocketproxy.py b/nova/tests/unit/console/test_websocketproxy.py
index 11561f4c53..92883134a2 100644
--- a/nova/tests/unit/console/test_websocketproxy.py
+++ b/nova/tests/unit/console/test_websocketproxy.py
@@ -659,6 +659,40 @@ class NovaProxyRequestHandlerTestCase(test.NoDBTestCase):
         # Verify no redirect happens and instead a 400 Bad Request is returned.
         self.assertIn('400 URI must not start with //', result[0].decode())
 
+    def test_reject_open_redirect_3_slashes(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()