summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean Mooney <work@seanmooney.info>2021-08-23 15:37:48 +0100
committerElod Illes <elod.illes@est.tech>2021-10-08 09:38:47 +0000
commit8906552cfc2525a44251d4cf313ece61e57251eb (patch)
treec11c09383809505676b6926431c56df448ffae9f
parent04d48527b62a35d912f93bc75613a6cca606df66 (diff)
downloadnova-8906552cfc2525a44251d4cf313ece61e57251eb.tar.gz
address open redirect with 3 forward slashes
Ie36401c782f023d1d5f2623732619105dc2cfa24 was intended to address OSSA-2021-002 (CVE-2021-3654) however after its release it was discovered that the fix only worked for urls with 2 leading slashes or more then 4. This change adresses the missing edgecase for 3 leading slashes and also maintian support for rejecting 2+. Conflicts: nova/console/websocketproxy.py nova/tests/unit/console/test_websocketproxy.py NOTE(melwitt): The conflict and difference in websocketproxy.py from the cherry picked change: HTTPStatus.BAD_REQUEST => 400 is due to the fact that HTTPStatus does not exist in Python 2.7. The conflict in test_websocketproxy.py is because change I23ac1cc79482d0fabb359486a4b934463854cae5 (Allow TLS ciphers/protocols to be configurable for console proxies) is not in Train. The difference in test_websocketproxy.py from the cherry picked change is due to a difference in internal implementation [1] in Python < 3.6. See change I546d376869a992601b443fb95acf1034da2a8f36 for reference. [1] https://github.com/python/cpython/commit/34eeed42901666fce099947f93dfdfc05411f286 Change-Id: I95f68be76330ff09e5eabb5ef8dd9a18f5547866 co-authored-by: Matteo Pozza Closes-Bug: #1927677 (cherry picked from commit 6fbd0b758dcac71323f3be179b1a9d1c17a4acc5) (cherry picked from commit 47dad4836a26292e9d34e516e1525ecf00be127c) (cherry picked from commit 9588cdbfd4649ea53d60303f2d10c5d62a070a07) (cherry picked from commit 0997043f459ac616b594363b5b253bd0ae6ed9eb)
-rw-r--r--nova/console/websocketproxy.py7
-rw-r--r--nova/tests/unit/console/test_websocketproxy.py32
2 files changed, 33 insertions, 6 deletions
diff --git a/nova/console/websocketproxy.py b/nova/console/websocketproxy.py
index 87d755ec66..7641a7cc08 100644
--- a/nova/console/websocketproxy.py
+++ b/nova/console/websocketproxy.py
@@ -313,14 +313,9 @@ class NovaProxyRequestHandler(NovaProxyRequestHandlerBase,
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(400,
"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 45917c2f41..27677bbf78 100644
--- a/nova/tests/unit/console/test_websocketproxy.py
+++ b/nova/tests/unit/console/test_websocketproxy.py
@@ -659,6 +659,38 @@ class NovaProxyRequestHandlerBaseTestCase(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''
+ ]
+
+ 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.
+ handler = websocketproxy.NovaProxyRequestHandler(
+ mock_req, client_addr, mock_server)
+
+ # Collect the response data to verify at the end. The
+ # SimpleHTTPRequestHandler writes the response data to a 'wfile'
+ # attribute.
+ output = io.BytesIO()
+ handler.wfile = output
+ # Process the mock_req again to do the capture.
+ handler.do_GET()
+ output.seek(0)
+ result = output.readlines()
+
+ # Verify no redirect happens and instead a 400 Bad Request is returned.
+ self.assertIn('400 URI must not start with //', result[0].decode())
+
class NovaWebsocketSecurityProxyTestCase(test.NoDBTestCase):