diff --git a/.zuul.yaml b/.zuul.yaml index 3000c196b22..b3926585328 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -326,6 +326,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: @@ -787,6 +791,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/cmd/baseproxy.py b/nova/cmd/baseproxy.py index 9c8d298bce5..3f3604473e5 100644 --- a/nova/cmd/baseproxy.py +++ b/nova/cmd/baseproxy.py @@ -81,7 +81,7 @@ def proxy(host, port, security_proxy=None): ssl_minimum_version=CONF.console.ssl_minimum_version, daemon=CONF.daemon, record=CONF.record, - traffic=not CONF.daemon, + traffic=not CONF.daemon and CONF.debug, web=CONF.web, file_only=True, RequestHandlerClass=websocketproxy.NovaProxyRequestHandler, diff --git a/nova/compute/api.py b/nova/compute/api.py index ca6d9e58a64..6db75882369 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2270,6 +2270,10 @@ def create( msg = _('The requested availability zone is not available') raise exception.InvalidRequest(msg) + if scheduler_hints: + scheduler_hints = {k: v for k, v in scheduler_hints.items() + if not k.startswith('_nova')} + filter_properties = scheduler_utils.build_filter_properties( scheduler_hints, forced_host, forced_node, flavor) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index ce6b98ddcd9..8c9614d334d 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1151,10 +1151,14 @@ def _validate_vtpm_configuration(self, instances): if self.driver.capabilities.get('supports_vtpm', False): return - for instance in instances: + total_instances = len(instances) + for i, instance in enumerate(instances): if instance.deleted: continue + LOG.debug('Checking vTPM constraint for instance %d/%d: %s', + i + 1, total_instances, instance.uuid) + # NOTE(stephenfin): We don't have an attribute on the instance to # check for this, so we need to inspect the flavor/image metadata if hardware.get_vtpm_constraint( @@ -1781,10 +1785,13 @@ def init_host(self, service_ref): # startup before we start mucking with instances we think are # ours. self._check_for_host_rename(nodes_by_uuid) - + expected_attrs = ['info_cache', 'metadata', 'system_metadata', + 'numa_topology', 'flavor'] + LOG.debug('Loading instances for host %s with expected_attrs: %s', + self.host, ', '.join(expected_attrs)) instances = objects.InstanceList.get_by_host( - context, self.host, - expected_attrs=['info_cache', 'metadata', 'numa_topology']) + context, self.host, expected_attrs=expected_attrs) + LOG.debug('Loaded %d instances for host', len(instances)) self.init_virt_events() diff --git a/nova/conf/libvirt.py b/nova/conf/libvirt.py index d65cdccbe5f..cadb19d9e21 100644 --- a/nova/conf/libvirt.py +++ b/nova/conf/libvirt.py @@ -1078,6 +1078,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 f1334b9dbb7..d70fc896f9f 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/cmd/test_baseproxy.py b/nova/tests/unit/cmd/test_baseproxy.py index 25f3905f246..5d56413c849 100644 --- a/nova/tests/unit/cmd/test_baseproxy.py +++ b/nova/tests/unit/cmd/test_baseproxy.py @@ -71,7 +71,7 @@ def test_proxy(self, mock_select_ssl_version, mock_start, mock_init, listen_host='0.0.0.0', listen_port='6080', source_is_ipv6=False, cert='self.pem', key=None, ssl_only=False, ssl_ciphers=None, ssl_minimum_version='default', daemon=False, record=None, - security_proxy=None, traffic=True, + security_proxy=None, traffic=False, web='/usr/share/spice-html5', file_only=True, RequestHandlerClass=websocketproxy.NovaProxyRequestHandler) mock_start.assert_called_once_with() @@ -98,6 +98,31 @@ def test_proxy_ssl_settings(self, mock_start, mock_init, mock_exists): listen_host='0.0.0.0', listen_port='6080', source_is_ipv6=False, cert='self.pem', key=None, ssl_only=False, ssl_ciphers='ALL:!aNULL', ssl_minimum_version='tlsv1_3', - daemon=False, record=None, security_proxy=None, traffic=True, + daemon=False, record=None, security_proxy=None, traffic=False, web='/usr/share/spice-html5', file_only=True, RequestHandlerClass=websocketproxy.NovaProxyRequestHandler) + + @mock.patch('os.path.exists', return_value=True) + @mock.patch.object(logging, 'setup') + @mock.patch.object(gmr.TextGuruMeditation, 'setup_autorun') + @mock.patch('nova.console.websocketproxy.NovaWebSocketProxy.__init__', + return_value=None) + @mock.patch('nova.console.websocketproxy.NovaWebSocketProxy.start_server') + @mock.patch('websockify.websocketproxy.select_ssl_version', + return_value=None) + def test_proxy_with_traffic_logging( + self, mock_select_ssl_version, mock_start, mock_init, mock_gmr, + mock_log, mock_exists, + ): + self.flags(debug=True) + baseproxy.proxy('0.0.0.0', '6080') + mock_log.assert_called_once_with(baseproxy.CONF, 'nova') + mock_gmr.assert_called_once_with(version, conf=baseproxy.CONF) + mock_init.assert_called_once_with( + listen_host='0.0.0.0', listen_port='6080', source_is_ipv6=False, + cert='self.pem', key=None, ssl_only=False, ssl_ciphers=None, + ssl_minimum_version='default', daemon=False, record=None, + security_proxy=None, traffic=True, + web='/usr/share/spice-html5', file_only=True, + RequestHandlerClass=websocketproxy.NovaProxyRequestHandler) + mock_start.assert_called_once_with() diff --git a/nova/tests/unit/compute/test_api.py b/nova/tests/unit/compute/test_api.py index f68b5d774b2..4dd95f797b4 100644 --- a/nova/tests/unit/compute/test_api.py +++ b/nova/tests/unit/compute/test_api.py @@ -219,6 +219,24 @@ def _obj_to_list_obj(self, list_obj, obj): list_obj.obj_reset_changes() return list_obj + @mock.patch('nova.scheduler.utils.build_filter_properties') + def test_create_strips_internal_scheduler_hints(self, + mock_build_filter): + mock_build_filter.side_effect = ( + test.TestingException('stop early')) + flavor = self._create_flavor() + self.assertRaises( + test.TestingException, + self.compute_api.create, + self.context, flavor, 'image_id', + scheduler_hints={ + '_nova_check_type': 'rebuild', + '_nova_future': 'something', + 'group': 'valid-group-uuid', + }) + actual_hints = mock_build_filter.call_args[0][0] + self.assertEqual({'group': 'valid-group-uuid'}, actual_hints) + @mock.patch( 'nova.network.neutron.API.is_remote_managed_port', new=mock.Mock(return_value=False), diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 4b6980aff82..4772283b80a 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -1131,7 +1131,8 @@ def _do_mock_calls(mock_update_scheduler, mock_inst_init, mock_init_host.assert_called_once_with(host=our_host) mock_host_get.assert_called_once_with(self.context, our_host, - expected_attrs=['info_cache', 'metadata', 'numa_topology']) + expected_attrs=['info_cache', 'metadata', 'system_metadata', + 'numa_topology', 'flavor']) mock_update_scheduler.assert_called_once_with( self.context, inst_list) @@ -1307,7 +1308,8 @@ def test_init_host_with_evacuated_instance(self, mock_save, mock_mig_get, mock_init_host.assert_called_once_with(host=our_host) mock_host_get.assert_called_once_with(self.context, our_host, - expected_attrs=['info_cache', 'metadata', 'numa_topology']) + expected_attrs=['info_cache', 'metadata', 'system_metadata', + 'numa_topology', 'flavor']) mock_init_virt.assert_called_once_with() mock_temp_mut.assert_called_once_with(self.context, read_deleted='yes') mock_get_inst.assert_called_once_with(self.context) 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/bug-2141981-5d5d3559c3b0a838.yaml b/releasenotes/notes/bug-2141981-5d5d3559c3b0a838.yaml new file mode 100644 index 00000000000..0aa55fa99f2 --- /dev/null +++ b/releasenotes/notes/bug-2141981-5d5d3559c3b0a838.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Fixed slow nova-compute startup caused by lazy-loading instance + attributes. During init_host(), the _validate_vtpm_configuration() + method accesses instance.flavor and instance.image_meta, which + triggered 2*N sequential database queries for N instances. These + attributes are now pre-loaded in the initial InstanceList query, + reducing startup time significantly for hosts with many instances. 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 +