Adapt websocketproxy tests for SimpleHTTPServer fix

In response to bug 1927677 we added a workaround to
NovaProxyRequestHandler to respond with a 400 Bad Request if an open
redirect is attempted:

  Ie36401c782f023d1d5f2623732619105dc2cfa24
  I95f68be76330ff09e5eabb5ef8dd9a18f5547866

Recently in python 3.10.6, a fix has landed in cpython to respond with
a 301 Moved Permanently to a sanitized URL that has had extra leading
'/' characters removed.

This breaks our existing unit tests which assume a 400 Bad Request as
the only expected response.

This adds handling of a 301 Moved Permanently response and asserts that
the redirect location is the expected sanitized URL. Doing this instead
of checking for a given python version will enable the tests to continue
to work if and when the cpython fix gets backported to older python
versions.

While updating the tests, the opportunity was taken to commonize the
code of two unit tests that were nearly identical.

Related-Bug: #1927677
Closes-Bug: #1986545

Change-Id: I27441d15cc6fa2ff7715ba15aa900961aadbf54a
This commit is contained in:
melanie witt 2022-08-16 06:49:53 +00:00
parent 627af67f5e
commit 15769b883e
1 changed files with 26 additions and 35 deletions

View File

@ -587,12 +587,12 @@ class NovaProxyRequestHandlerTestCase(test.NoDBTestCase):
self.wh.socket.assert_called_with('node1', 10000, connect=True)
self.wh.do_proxy.assert_called_with('<socket>')
def test_reject_open_redirect(self):
def test_reject_open_redirect(self, url='//example.com/%2F..'):
# 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',
f'GET {url} HTTP/1.1\r\n'.encode('utf-8'),
b''
]
@ -617,41 +617,32 @@ class NovaProxyRequestHandlerTestCase(test.NoDBTestCase):
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())
# NOTE: As of python 3.10.6 there is a fix for this vulnerability,
# which will cause a 301 Moved Permanently error to be returned
# instead that redirects to a sanitized version of the URL with extra
# leading '/' characters removed.
# See https://github.com/python/cpython/issues/87389 for details.
# We will consider either response to be valid for this test. This will
# also help if and when the above fix gets backported to older versions
# of python.
errmsg = result[0].decode()
expected_nova = '400 URI must not start with //'
expected_cpython = '301 Moved Permanently'
self.assertTrue(expected_nova in errmsg or expected_cpython in errmsg)
# If we detect the cpython fix, verify that the redirect location is
# now the same url but with extra leading '/' characters removed.
if expected_cpython in errmsg:
location = result[3].decode()
location = location.removeprefix('Location: ').rstrip('\r\n')
self.assertTrue(
location.startswith('/example.com/%2F..'),
msg='Redirect location is not the expected sanitized URL',
)
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)
self.test_reject_open_redirect(url='///example.com/%2F..')
@mock.patch('nova.objects.ConsoleAuthToken.validate')
def test_no_compute_rpcapi_with_invalid_token(self, mock_validate):