diff --git a/.zuul.yaml b/.zuul.yaml index c8995fb7cfc..901c98f50d2 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -280,6 +280,10 @@ devstack_services: openstack-cli-server: true devstack_local_conf: + post-config: + $NOVA_CPU_CONF: + libvirt: + images_address_space_limit: 2 test-config: $TEMPEST_CONFIG: compute-feature-enabled: @@ -722,6 +726,9 @@ images_rbd_glance_store_name: robust workarounds: never_download_image_if_on_rbd: True + $NOVA_CPU_CONF: + libvirt: + images_address_space_limit: 2 $GLANCE_API_CONF: DEFAULT: enabled_backends: "cheap:file, robust:rbd, web:http" diff --git a/nova/conf/libvirt.py b/nova/conf/libvirt.py index b77986b915b..0c2fcc169fe 100644 --- a/nova/conf/libvirt.py +++ b/nova/conf/libvirt.py @@ -1042,6 +1042,12 @@ * Qemu >= 1.5 (raw format) * Qemu >= 1.6 (qcow2 format) """), + cfg.IntOpt('images_cpu_time_limit', + default=30, + help='CPU time process limit in seconds for qemu-img'), + cfg.IntOpt('images_address_space_limit', + default=1, + help='Address space process limit in gigabytes for qemu-img'), ] libvirt_lvm_opts = [ diff --git a/nova/console/websocketproxy.py b/nova/console/websocketproxy.py index f71b9e1ebbe..c2b4e7f0494 100644 --- a/nova/console/websocketproxy.py +++ b/nova/console/websocketproxy.py @@ -203,7 +203,7 @@ def new_websocket_client(self): expected_origin_hostname = e.split(']')[0][1:] else: expected_origin_hostname = e.split(':')[0] - expected_origin_hostnames = CONF.console.allowed_origins + expected_origin_hostnames = list(CONF.console.allowed_origins) expected_origin_hostnames.append(expected_origin_hostname) origin_url = self.headers.get('Origin') # missing origin header indicates non-browser client which is OK diff --git a/nova/privsep/qemu.py b/nova/privsep/qemu.py index 40cdd387333..36f463703ba 100644 --- a/nova/privsep/qemu.py +++ b/nova/privsep/qemu.py @@ -25,15 +25,18 @@ from oslo_log import log as logging from oslo_utils import units +import nova.conf from nova import exception from nova.i18n import _ import nova.privsep.utils LOG = logging.getLogger(__name__) +CONF = nova.conf.CONF + QEMU_IMG_LIMITS = processutils.ProcessLimits( - cpu_time=30, - address_space=1 * units.Gi) + cpu_time=CONF.libvirt.images_cpu_time_limit, + address_space=CONF.libvirt.images_address_space_limit * units.Gi) class EncryptionOptions(ty.TypedDict): diff --git a/nova/tests/unit/console/test_websocketproxy.py b/nova/tests/unit/console/test_websocketproxy.py index 088ed8e64d2..8c3660bab24 100644 --- a/nova/tests/unit/console/test_websocketproxy.py +++ b/nova/tests/unit/console/test_websocketproxy.py @@ -658,6 +658,81 @@ def test_reject_open_redirect(self, url='//example.com/%2F..'): def test_reject_open_redirect_3_slashes(self): self.test_reject_open_redirect(url='///example.com/%2F..') + @mock.patch('nova.console.websocketproxy.NovaProxyRequestHandler.' + '_check_console_port') + @mock.patch('nova.objects.ConsoleAuthToken.validate') + def test_host_header_does_not_poison_allowed_origins( + self, validate, check_port): + """Verify that the Host header from one request does not persist in + CONF.console.allowed_origins and affect subsequent origin checks. + + Regression test for bug 2158919. + """ + params = { + 'id': 1, + 'token': '123-456-789', + 'instance_uuid': uuids.instance, + 'host': 'node1', + 'port': '10000', + 'console_type': 'novnc', + 'access_url_base': 'https://example.net:6080' + } + validate.return_value = objects.ConsoleAuthToken(**params) + + self.wh.socket.return_value = '' + self.wh.path = "http://127.0.0.1/?token=123-456-789" + self.wh.headers = self.fake_header + + original_conf_origins = list(CONF.console.allowed_origins) + + self.wh.new_websocket_client() + + self.assertEqual(original_conf_origins, + CONF.console.allowed_origins) + + @mock.patch('nova.console.websocketproxy.NovaProxyRequestHandler.' + '_check_console_port') + @mock.patch('nova.objects.ConsoleAuthToken.validate') + def test_previous_host_does_not_bypass_origin_check( + self, validate, check_port): + """Verify that a Host header from a prior request cannot be used to + bypass the origin check on a subsequent request. + + Regression test for bug 2158919. + """ + params = { + 'id': 1, + 'token': '123-456-789', + 'instance_uuid': uuids.instance, + 'host': 'node1', + 'port': '10000', + 'console_type': 'novnc', + 'access_url_base': 'https://example.net:6080' + } + validate.return_value = objects.ConsoleAuthToken(**params) + + self.wh.socket.return_value = '' + self.wh.path = "http://127.0.0.1/?token=123-456-789" + + # First request: Host header introduces evil.com + self.wh.headers = { + 'cookie': 'token="123-456-789"', + 'Origin': 'https://evil.com:6080', + 'Host': 'evil.com:6080', + } + self.wh.new_websocket_client() + + # Second request: Origin is evil.com but Host is legitimate. + # This must be rejected — evil.com should not have been persisted + # into the allow-list by the first request. + self.wh.headers = { + 'cookie': 'token="123-456-789"', + 'Origin': 'https://evil.com:6080', + 'Host': 'example.net:6080', + } + self.assertRaises(exception.ValidationError, + self.wh.new_websocket_client) + @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 diff --git a/nova/tests/unit/privsep/test_qemu.py b/nova/tests/unit/privsep/test_qemu.py index e0554637e07..a1d720e8702 100644 --- a/nova/tests/unit/privsep/test_qemu.py +++ b/nova/tests/unit/privsep/test_qemu.py @@ -13,9 +13,11 @@ # License for the specific language governing permissions and limitations # under the License. +import importlib from unittest import mock import ddt +from oslo_utils import units import nova.privsep.qemu from nova import test @@ -203,9 +205,24 @@ def _test_qemu_img_info(self, method, mock_isdir, mock_execute): # Assert that the expected command is used mock_execute.assert_called_once_with( *expected_cmd, prlimit=nova.privsep.qemu.QEMU_IMG_LIMITS) + return mock_execute.call_args def test_privileged_qemu_img_info(self): self._test_qemu_img_info(nova.privsep.qemu.privileged_qemu_img_info) def test_unprivileged_qemu_img_info(self): self._test_qemu_img_info(nova.privsep.qemu.unprivileged_qemu_img_info) + + def test_qemu_img_info_limits_config(self): + self.flags(images_cpu_time_limit=60, group='libvirt') + self.flags(images_address_space_limit=3, group='libvirt') + # Reload the nova.privsep.qemu module after setting the conf options + # because QEMU_IMG_LIMITS is global. + importlib.reload(nova.privsep.qemu) + # Save the call args of execute() to assert. + call_args = self._test_qemu_img_info( + nova.privsep.qemu.unprivileged_qemu_img_info) + # Verify that execute() was called with the configured values. + self.assertEqual(60, call_args.kwargs['prlimit'].cpu_time) + self.assertEqual(3 * units.Gi, + call_args.kwargs['prlimit'].address_space) diff --git a/releasenotes/notes/console-token-origin-poison-f251ab9e3f63d6bd.yaml b/releasenotes/notes/console-token-origin-poison-f251ab9e3f63d6bd.yaml new file mode 100644 index 00000000000..8f8c3df972f --- /dev/null +++ b/releasenotes/notes/console-token-origin-poison-f251ab9e3f63d6bd.yaml @@ -0,0 +1,6 @@ +--- +security: + - | + Bug #2158919 is fixed, which involved an authenticated user able to poison + the server-side allowed origins list (and potentially exhaust memory by + extending it until failure). diff --git a/releasenotes/notes/qemu-img-limits-config-dd49ea73c84a7cd4.yaml b/releasenotes/notes/qemu-img-limits-config-dd49ea73c84a7cd4.yaml new file mode 100644 index 00000000000..22fa27c71ad --- /dev/null +++ b/releasenotes/notes/qemu-img-limits-config-dd49ea73c84a7cd4.yaml @@ -0,0 +1,11 @@ +features: + - | + New configuration options ``[libvirt]images_cpu_time_limit`` and + ``[libvirt]images_address_space_limit`` have been added to enable tuning of + process limits for qemu-img. The default for + ``[libvirt]images_address_space_limit`` is unchanged from the hard-coded 1G + limit in order to maintain existing behavior. If you are using newer versions + of Ceph this should be increased. For more details, see bug `#2116852`_. + + .. _#2116852: https://bugs.launchpad.net/nova/+bug/2116852 +