From faceea8199a86cf1ca606d8ae1badbcf2d41aff0 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Thu, 2 Jul 2026 10:29:26 -0700 Subject: [PATCH 1/2] Fix mutating global config in websocket proxy The websocket proxy mutates the CONF host list with the Host header from the request, which would then poison future requests and/or lead to a slow resource exhaustion attack. Simply making a copy before mutation avoids the issue. Conflicts: nova/console/websocketproxy.py Generated-By: Claude Opus 4.6 Closes-Bug: #2158919 Change-Id: Ib13e479337f9b1c8b16952089d1d5f6979976b86 Signed-off-by: Dan Smith (cherry picked from commit 0612fed0e171610c916a656a8e7d2b3384c3fd47) (cherry picked from commit e4881944852aad1edcda2670b39dd19c2673af05) (cherry picked from commit 5fe20b90920d6f5a7fa4f9ffe5169907226ffbed) --- nova/console/websocketproxy.py | 2 +- .../tests/unit/console/test_websocketproxy.py | 75 +++++++++++++++++++ ...-token-origin-poison-f251ab9e3f63d6bd.yaml | 6 ++ 3 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/console-token-origin-poison-f251ab9e3f63d6bd.yaml 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/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/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). From 6fb3c34cabcf4642173b57cc6a7ef87219b2ef4f Mon Sep 17 00:00:00 2001 From: melanie witt Date: Wed, 3 Dec 2025 14:05:47 -0800 Subject: [PATCH 2/2] Make QEMU_IMG_LIMITS process limits configurable Currently the CPU time and address space process limits for qemu-img are hard-coded to 30 seconds and 1G respectively. With more recent versions of Ceph in upstream CI, we have experienced test failures that suggest 1G is no longer large enough for encrypted RBD images. In the failures the following error is raised: nova.exception.InvalidDiskInfo: Disk info file is invalid: qemu-img failed to execute on rbd:volumes/volume-c83c9b7f-0f38-4bb8-a40a-300a66080d21:id=cinder : Unexpected error while running command. Command: /opt/stack/data/venv/bin/python3.12 -m oslo_concurrency.prlimit --as=1073741824 --cpu=30 -- env LC_ALL=C LANG=C qemu-img info rbd:volumes/volume-c83c9b7f-0f38-4bb8-a40a-300a66080d21:id=cinder --force-share --output=json Exit code: -6 Stdout: '' Stderr: 'failed to allocate memory for stack: Cannot allocate memory\n' This adds config options ``images_cpu_time_limit`` and ``images_address_space_limit`` to the ``[libvirt]`` section to allow for tuning of the qemu-img process limits, similar to how Cinder and Ironic make qemu-img process limits configurable. Stable Only Changes - the defautl value is updated to 1G to maintain stable branch behavior - the ci jobs are updated to use 2G to match master - the release note is updated to reflect this. Closes-Bug: #2116852 Change-Id: I10e53de27b063b1e514e04066d0eb56a86188e9a Signed-off-by: melanie witt Signed-off-by: Sean Mooney (cherry picked from commit 7f4343198c2aec73b10ad7c01ba0df87fe41f579) (cherry picked from commit 8595b8eed4d0cabf6a7db04b0d946cf764187965) (cherry picked from commit 7a1a8a2325172a2f2a07b4447243b46e6959ed70) --- .zuul.yaml | 7 +++++++ nova/conf/libvirt.py | 6 ++++++ nova/privsep/qemu.py | 7 +++++-- nova/tests/unit/privsep/test_qemu.py | 17 +++++++++++++++++ ...qemu-img-limits-config-dd49ea73c84a7cd4.yaml | 11 +++++++++++ 5 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/qemu-img-limits-config-dd49ea73c84a7cd4.yaml 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/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/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/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 +