<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/openstack/nova.git/nova/tests/unit/console/test_websocketproxy.py, branch master</title>
<subtitle>opendev.org: openstack/nova.git
</subtitle>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openstack/nova.git/'/>
<entry>
<title>Remove use of removeprefix</title>
<updated>2022-12-20T16:12:12+00:00</updated>
<author>
<name>Stephen Finucane</name>
<email>sfinucan@redhat.com</email>
</author>
<published>2022-12-15T00:09:04+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openstack/nova.git/commit/?id=3ccf82ef9e2c87a1d33a0dda8929c05e80844087'/>
<id>3ccf82ef9e2c87a1d33a0dda8929c05e80844087</id>
<content type='text'>
This is not supported on Python 3.8 [1]. I have no idea why this was not
failing CI.

[1] https://docs.python.org/3.9/library/stdtypes.html#str.removeprefix

Change-Id: I225e9ced0f75c415b1d2fee05440291e3d8635c0
Signed-off-by: Stephen Finucane &lt;sfinucan@redhat.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This is not supported on Python 3.8 [1]. I have no idea why this was not
failing CI.

[1] https://docs.python.org/3.9/library/stdtypes.html#str.removeprefix

Change-Id: I225e9ced0f75c415b1d2fee05440291e3d8635c0
Signed-off-by: Stephen Finucane &lt;sfinucan@redhat.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Adapt websocketproxy tests for SimpleHTTPServer fix</title>
<updated>2022-08-17T00:08:57+00:00</updated>
<author>
<name>melanie witt</name>
<email>melwittt@gmail.com</email>
</author>
<published>2022-08-16T06:49:53+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openstack/nova.git/commit/?id=15769b883ed4a86d62b141ea30d3f1590565d8e0'/>
<id>15769b883ed4a86d62b141ea30d3f1590565d8e0</id>
<content type='text'>
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
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
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
</pre>
</div>
</content>
</entry>
<entry>
<title>Use unittest.mock instead of third party mock</title>
<updated>2022-08-01T15:46:26+00:00</updated>
<author>
<name>Stephen Finucane</name>
<email>stephenfin@redhat.com</email>
</author>
<published>2020-03-24T15:12:07+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openstack/nova.git/commit/?id=89ef050b8c049b9a6f0e2c70408fc93c826c55e0'/>
<id>89ef050b8c049b9a6f0e2c70408fc93c826c55e0</id>
<content type='text'>
Now that we no longer support py27, we can use the standard library
unittest.mock module instead of the third party mock lib. Most of this
is autogenerated, as described below, but there is one manual change
necessary:

nova/tests/functional/regressions/test_bug_1781286.py
  We need to avoid using 'fixtures.MockPatch' since fixtures is using
  'mock' (the library) under the hood and a call to 'mock.patch.stop'
  found in that test will now "stop" mocks from the wrong library. We
  have discussed making this configurable but the option proposed isn't
  that pretty [1] so this is better.

The remainder was auto-generated with the following (hacky) script, with
one or two manual tweaks after the fact:

  import glob

  for path in glob.glob('nova/tests/**/*.py', recursive=True):
      with open(path) as fh:
          lines = fh.readlines()
      if 'import mock\n' not in lines:
          continue
      import_group_found = False
      create_first_party_group = False
      for num, line in enumerate(lines):
          line = line.strip()
          if line.startswith('import ') or line.startswith('from '):
              tokens = line.split()
              for lib in (
                  'ddt', 'six', 'webob', 'fixtures', 'testtools'
                  'neutron', 'cinder', 'ironic', 'keystone', 'oslo',
              ):
                  if lib in tokens[1]:
                      create_first_party_group = True
                      break
              if create_first_party_group:
                  break
              import_group_found = True
          if not import_group_found:
              continue
          if line.startswith('import ') or line.startswith('from '):
              tokens = line.split()
              if tokens[1] &gt; 'unittest':
                  break
              elif tokens[1] == 'unittest' and (
                  len(tokens) == 2 or tokens[4] &gt; 'mock'
              ):
                  break
          elif not line:
              break
      if create_first_party_group:
          lines.insert(num, 'from unittest import mock\n\n')
      else:
          lines.insert(num, 'from unittest import mock\n')
      del lines[lines.index('import mock\n')]
      with open(path, 'w+') as fh:
          fh.writelines(lines)

Note that we cannot remove mock from our requirements files yet due to
importing pypowervm unit test code in nova unit tests. This library
still uses the mock lib, and since we are importing test code and that
lib (correctly) only declares mock in its test-requirements.txt, mock
would not otherwise be installed and would cause errors while loading
nova unit test code.

[1] https://github.com/testing-cabal/fixtures/pull/49

Change-Id: Id5b04cf2f6ca24af8e366d23f15cf0e5cac8e1cc
Signed-off-by: Stephen Finucane &lt;stephenfin@redhat.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Now that we no longer support py27, we can use the standard library
unittest.mock module instead of the third party mock lib. Most of this
is autogenerated, as described below, but there is one manual change
necessary:

nova/tests/functional/regressions/test_bug_1781286.py
  We need to avoid using 'fixtures.MockPatch' since fixtures is using
  'mock' (the library) under the hood and a call to 'mock.patch.stop'
  found in that test will now "stop" mocks from the wrong library. We
  have discussed making this configurable but the option proposed isn't
  that pretty [1] so this is better.

The remainder was auto-generated with the following (hacky) script, with
one or two manual tweaks after the fact:

  import glob

  for path in glob.glob('nova/tests/**/*.py', recursive=True):
      with open(path) as fh:
          lines = fh.readlines()
      if 'import mock\n' not in lines:
          continue
      import_group_found = False
      create_first_party_group = False
      for num, line in enumerate(lines):
          line = line.strip()
          if line.startswith('import ') or line.startswith('from '):
              tokens = line.split()
              for lib in (
                  'ddt', 'six', 'webob', 'fixtures', 'testtools'
                  'neutron', 'cinder', 'ironic', 'keystone', 'oslo',
              ):
                  if lib in tokens[1]:
                      create_first_party_group = True
                      break
              if create_first_party_group:
                  break
              import_group_found = True
          if not import_group_found:
              continue
          if line.startswith('import ') or line.startswith('from '):
              tokens = line.split()
              if tokens[1] &gt; 'unittest':
                  break
              elif tokens[1] == 'unittest' and (
                  len(tokens) == 2 or tokens[4] &gt; 'mock'
              ):
                  break
          elif not line:
              break
      if create_first_party_group:
          lines.insert(num, 'from unittest import mock\n\n')
      else:
          lines.insert(num, 'from unittest import mock\n')
      del lines[lines.index('import mock\n')]
      with open(path, 'w+') as fh:
          fh.writelines(lines)

Note that we cannot remove mock from our requirements files yet due to
importing pypowervm unit test code in nova unit tests. This library
still uses the mock lib, and since we are importing test code and that
lib (correctly) only declares mock in its test-requirements.txt, mock
would not otherwise be installed and would cause errors while loading
nova unit test code.

[1] https://github.com/testing-cabal/fixtures/pull/49

Change-Id: Id5b04cf2f6ca24af8e366d23f15cf0e5cac8e1cc
Signed-off-by: Stephen Finucane &lt;stephenfin@redhat.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Fix duplicates keys</title>
<updated>2022-05-26T10:13:13+00:00</updated>
<author>
<name>Rajesh Tailor</name>
<email>ratailor@redhat.com</email>
</author>
<published>2022-05-26T10:13:13+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openstack/nova.git/commit/?id=a240cf8d09f2d9d811f6d26ff726963b04ee1fb7'/>
<id>a240cf8d09f2d9d811f6d26ff726963b04ee1fb7</id>
<content type='text'>
This change fixes duplicate dictionary keys in unit tests.

Change-Id: Ie969d994a3f2f67580e4d6debc5c9dc9ffc13600
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This change fixes duplicate dictionary keys in unit tests.

Change-Id: Ie969d994a3f2f67580e4d6debc5c9dc9ffc13600
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge "Reduce mocking in test_reject_open_redirect for compat"</title>
<updated>2021-08-30T17:16:15+00:00</updated>
<author>
<name>Zuul</name>
<email>zuul@review.opendev.org</email>
</author>
<published>2021-08-30T17:16:15+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openstack/nova.git/commit/?id=b4886738bb6e3fdc5ef321838f0b3292e401669f'/>
<id>b4886738bb6e3fdc5ef321838f0b3292e401669f</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>address open redirect with 3 forward slashes</title>
<updated>2021-08-23T14:51:06+00:00</updated>
<author>
<name>Sean Mooney</name>
<email>work@seanmooney.info</email>
</author>
<published>2021-08-23T14:37:48+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openstack/nova.git/commit/?id=6fbd0b758dcac71323f3be179b1a9d1c17a4acc5'/>
<id>6fbd0b758dcac71323f3be179b1a9d1c17a4acc5</id>
<content type='text'>
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+.

Change-Id: I95f68be76330ff09e5eabb5ef8dd9a18f5547866
co-authored-by: Matteo Pozza
Closes-Bug: #1927677
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
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+.

Change-Id: I95f68be76330ff09e5eabb5ef8dd9a18f5547866
co-authored-by: Matteo Pozza
Closes-Bug: #1927677
</pre>
</div>
</content>
</entry>
<entry>
<title>Reduce mocking in test_reject_open_redirect for compat</title>
<updated>2021-07-31T00:35:38+00:00</updated>
<author>
<name>melanie witt</name>
<email>melwittt@gmail.com</email>
</author>
<published>2021-07-31T00:30:16+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openstack/nova.git/commit/?id=214cabe6848a1fdb4f5941d994c6cc11107fc4af'/>
<id>214cabe6848a1fdb4f5941d994c6cc11107fc4af</id>
<content type='text'>
This is a followup for change Ie36401c782f023d1d5f2623732619105dc2cfa24
to reduce mocking in the unit test coverage for it.

While backporting the bug fix, it was found to be incompatible with
earlier versions of Python &lt; 3.6 due to a difference in internal
implementation [1].

This reduces the mocking in the unit test to be more agnostic to the
internals of the StreamRequestHandler (ancestor of
SimpleHTTPRequestHandler) and work across Python versions &gt;= 2.7.

Related-Bug: #1927677

[1] https://github.com/python/cpython/commit/34eeed42901666fce099947f93dfdfc05411f286

Change-Id: I546d376869a992601b443fb95acf1034da2a8f36
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This is a followup for change Ie36401c782f023d1d5f2623732619105dc2cfa24
to reduce mocking in the unit test coverage for it.

While backporting the bug fix, it was found to be incompatible with
earlier versions of Python &lt; 3.6 due to a difference in internal
implementation [1].

This reduces the mocking in the unit test to be more agnostic to the
internals of the StreamRequestHandler (ancestor of
SimpleHTTPRequestHandler) and work across Python versions &gt;= 2.7.

Related-Bug: #1927677

[1] https://github.com/python/cpython/commit/34eeed42901666fce099947f93dfdfc05411f286

Change-Id: I546d376869a992601b443fb95acf1034da2a8f36
</pre>
</div>
</content>
</entry>
<entry>
<title>Reject open redirection in the console proxy</title>
<updated>2021-05-14T15:26:00+00:00</updated>
<author>
<name>melanie witt</name>
<email>melwittt@gmail.com</email>
</author>
<published>2021-05-13T05:43:42+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openstack/nova.git/commit/?id=781612b33282ed298f742c85dab58a075c8b793e'/>
<id>781612b33282ed298f742c85dab58a075c8b793e</id>
<content type='text'>
Our console proxies (novnc, serial, spice) run in a websockify server
whose request handler inherits from the python standard
SimpleHTTPRequestHandler. There is a known issue [1] in the
SimpleHTTPRequestHandler which allows open redirects by way of URLs
in the following format:

  http://vncproxy.my.domain.com//example.com/%2F..

which if visited, will redirect a user to example.com.

We can intercept a request and reject requests that pass a redirection
URL beginning with "//" by implementing the
SimpleHTTPRequestHandler.send_head() method containing the
vulnerability to reject such requests with a 400 Bad Request.

This code is copied from a patch suggested in one of the issue comments
[2].

Closes-Bug: #1927677

[1] https://bugs.python.org/issue32084
[2] https://bugs.python.org/issue32084#msg306545

Change-Id: Ie36401c782f023d1d5f2623732619105dc2cfa24
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Our console proxies (novnc, serial, spice) run in a websockify server
whose request handler inherits from the python standard
SimpleHTTPRequestHandler. There is a known issue [1] in the
SimpleHTTPRequestHandler which allows open redirects by way of URLs
in the following format:

  http://vncproxy.my.domain.com//example.com/%2F..

which if visited, will redirect a user to example.com.

We can intercept a request and reject requests that pass a redirection
URL beginning with "//" by implementing the
SimpleHTTPRequestHandler.send_head() method containing the
vulnerability to reject such requests with a 400 Bad Request.

This code is copied from a patch suggested in one of the issue comments
[2].

Closes-Bug: #1927677

[1] https://bugs.python.org/issue32084
[2] https://bugs.python.org/issue32084#msg306545

Change-Id: Ie36401c782f023d1d5f2623732619105dc2cfa24
</pre>
</div>
</content>
</entry>
<entry>
<title>Remove references to 'sys.version_info'</title>
<updated>2021-04-21T11:02:27+00:00</updated>
<author>
<name>Stephen Finucane</name>
<email>stephenfin@redhat.com</email>
</author>
<published>2020-10-05T16:49:08+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openstack/nova.git/commit/?id=bab3c8f3cba87b75d271308ef35dada18afaa03b'/>
<id>bab3c8f3cba87b75d271308ef35dada18afaa03b</id>
<content type='text'>
We support Python 3.6 as a minimum now, making these checks no-ops.

Change-Id: I5ca2439c948687022f8d88df978bc7ee77199fcc
Signed-off-by: Stephen Finucane &lt;stephenfin@redhat.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
We support Python 3.6 as a minimum now, making these checks no-ops.

Change-Id: I5ca2439c948687022f8d88df978bc7ee77199fcc
Signed-off-by: Stephen Finucane &lt;stephenfin@redhat.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Refactor and rename test_tcp_rst_no_compute_rpcapi</title>
<updated>2021-02-03T23:15:06+00:00</updated>
<author>
<name>melanie witt</name>
<email>melwittt@gmail.com</email>
</author>
<published>2020-07-16T00:55:25+00:00</published>
<link rel='alternate' type='text/html' href='http://trove.baserock.org/cgit/delta/openstack/nova.git/commit/?id=f6bacd3fde9d4f0de638dc2bb3ba2ac2520df874'/>
<id>f6bacd3fde9d4f0de638dc2bb3ba2ac2520df874</id>
<content type='text'>
The NovaProxyRequestHandlerTestCase.test_tcp_rst_no_compute_rpcapi test
fails with mock==4.0.2 because mock is calling the
NovaProxyRequestHandler class's compute_rpcapi @property while creating
mock autospecs. The test asserts that the @property is not called when
a TCP RST is received, so mock calling the @property itself causes the
test to fail erroneously.

This is also the case in the built-in unittest.mock in python 3.8 [1].

We can refactor (and rename) this unit test to more concisely test the
desired behavior when TCP RST or otherwise unvalidated requests are
handled by the console proxy request handler. This has the benefit of
(1) making the test work with mock==4.0.2 and python 3.8 and (2)
removing unnecessary dependency and potentially incorrect assumptions
about the internal details of websockify.

Closes-Bug: #1887735

[1] https://bugs.python.org/issue41768

Change-Id: I58b0382c86d4ef798572edb63d311e0e3e6937bb
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The NovaProxyRequestHandlerTestCase.test_tcp_rst_no_compute_rpcapi test
fails with mock==4.0.2 because mock is calling the
NovaProxyRequestHandler class's compute_rpcapi @property while creating
mock autospecs. The test asserts that the @property is not called when
a TCP RST is received, so mock calling the @property itself causes the
test to fail erroneously.

This is also the case in the built-in unittest.mock in python 3.8 [1].

We can refactor (and rename) this unit test to more concisely test the
desired behavior when TCP RST or otherwise unvalidated requests are
handled by the console proxy request handler. This has the benefit of
(1) making the test work with mock==4.0.2 and python 3.8 and (2)
removing unnecessary dependency and potentially incorrect assumptions
about the internal details of websockify.

Closes-Bug: #1887735

[1] https://bugs.python.org/issue41768

Change-Id: I58b0382c86d4ef798572edb63d311e0e3e6937bb
</pre>
</div>
</content>
</entry>
</feed>
