Skip to content

Commit

Permalink
Merge "address open redirect with 3 forward slashes"
Browse files Browse the repository at this point in the history
  • Loading branch information
Zuul authored and openstack-gerrit committed Aug 24, 2021
2 parents 2d2fb2d + 6fbd0b7 commit c37a465
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 6 deletions.
7 changes: 1 addition & 6 deletions nova/console/websocketproxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,14 +280,9 @@ def send_head(self):
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
Expand Down
34 changes: 34 additions & 0 deletions nova/tests/unit/console/test_websocketproxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,40 @@ def fake_sendall(data):
self.assertIn('Error code: 400', self.data)
self.assertIn('Message: URI must not start with //', self.data)

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('nova.objects.ConsoleAuthToken.validate')
def test_no_compute_rpcapi_with_invalid_token(self, mock_validate):
"""Tests that we don't create a ComputeAPI object until we actually
Expand Down

0 comments on commit c37a465

Please sign in to comment.