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]34eeed4290
Change-Id: I95f68be76330ff09e5eabb5ef8dd9a18f5547866 co-authored-by: Matteo Pozza Closes-Bug: #1927677 (cherry picked from commit6fbd0b758d
) (cherry picked from commit47dad4836a
) (cherry picked from commit9588cdbfd4
) (cherry picked from commit0997043f45
)
This commit is contained in:
parent
04d48527b6
commit
8906552cfc
|
@ -313,14 +313,9 @@ class NovaProxyRequestHandler(NovaProxyRequestHandlerBase,
|
||||||
if os.path.isdir(path):
|
if os.path.isdir(path):
|
||||||
parts = urlparse.urlsplit(self.path)
|
parts = urlparse.urlsplit(self.path)
|
||||||
if not parts.path.endswith('/'):
|
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
|
# Browsers interpret "Location: //uri" as an absolute URI
|
||||||
# like "http://URI"
|
# like "http://URI"
|
||||||
if new_url.startswith('//'):
|
if self.path.startswith('//'):
|
||||||
self.send_error(400,
|
self.send_error(400,
|
||||||
"URI must not start with //")
|
"URI must not start with //")
|
||||||
return None
|
return None
|
||||||
|
|
|
@ -659,6 +659,38 @@ class NovaProxyRequestHandlerBaseTestCase(test.NoDBTestCase):
|
||||||
# Verify no redirect happens and instead a 400 Bad Request is returned.
|
# Verify no redirect happens and instead a 400 Bad Request is returned.
|
||||||
self.assertIn('400 URI must not start with //', result[0].decode())
|
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):
|
class NovaWebsocketSecurityProxyTestCase(test.NoDBTestCase):
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue