From 8392c7f2656ae624877e3df539681c0a8f8b4926 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Fri, 13 Apr 2018 13:44:33 -0400 Subject: [PATCH] Add policy rule to block image-backed servers with 0 root disk flavor This adds a new policy rule which defaults to behave in a backward compatible way, but will allow operators to enforce that servers created with a zero disk flavor must also be volume-backed servers. Allowing users to upload their own images and create image-backed servers on local disk with zero root disk size flavors can be potentially hazardous if the size of the image is unexpectedly large, since it can consume the local disk (or shared storage pool). It should be noted that disabling the new policy rule will result in a non-backward compatible API behavior change and no microversion is being introduced for this because enforcement via a new microversion would not close the security gap on any previous microversions. Related compute API reference and user documentation is updated to mention the policy rule along with a release note since this is tied to a security bug, which will be backported to stable branches. Conflicts: api-ref/source/parameters.yaml doc/source/admin/flavors2.rst nova/policies/servers.py nova/tests/functional/wsgi/test_servers.py NOTE(mriedem): The api-ref/source/parameters.yaml conflict is due to If646149efb7eec8c90bf7d07c39ff4c495349941 not being in Pike. The doc/source/admin/flavors2.rst conflict is due to the doc not being in Ocata - it was migrated from the central admin-guide in Ifa0039e270e54ea2fb58ab18ce6724e5e8e061a1. The nova/policies/servers.py conflict is due to two changes in Pike: I17b6ca6e17c777ae7d337bf70ec4774ffe5187a8 and I050c4f5f19aa79a682e076cc3e47eba597f272dd. The DocumentedRuleDefault class was added to oslo.policy starting in 1.21.1 in Pike which is newer than what stable/ocata supports in global-requirements so we can't use it in this backport. The nova/tests/functional/wsgi/test_servers.py conflict is due to Ifcaaf285c8f98a1d0e8bbbc87b2f57fbce057346 and I294c54e5a22dd6e5b226a4b00e7cd116813f0704 not being in Ocata. Change-Id: Id67e1285a0522474844de130c9263e11868f67fb Closes-Bug: #1739646 (cherry picked from commit 763fd62464e9a0753e061171cc1fd826055bbc01) (cherry picked from commit 7bcd581c78bb5916bf4b52e213322e7b56283572) (cherry picked from commit 0bf75621bbd25d4ce8a3588f112cf714891556eb) --- api-ref/source/parameters.yaml | 4 +- nova/api/openstack/compute/servers.py | 3 +- nova/compute/api.py | 10 +++ nova/exception.py | 5 ++ nova/policies/servers.py | 2 + nova/tests/functional/wsgi/test_servers.py | 82 +++++++++++++++++++ nova/tests/unit/test_policy.py | 1 + ...for_zero_disk_flavor-b36a6eb4fa8b2964.yaml | 20 +++++ 8 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/bug-1739646-enforce_volume_backed_for_zero_disk_flavor-b36a6eb4fa8b2964.yaml diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index cb07c870d351..469efbd42de5 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -1863,7 +1863,9 @@ flavor_disk: deploy the instance. However, in this case filter scheduler cannot select the compute host based on the virtual image size. Therefore, 0 should only be used for volume booted instances or for testing - purposes. + purposes. Volume-backed instances can be enforced for flavors with + zero root disk via the ``os_compute_api:servers:create:zero_disk_flavor`` + policy rule. flavor_ephem_disk: in: body required: true diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 44e0bcf5a479..dfdd7fe3006c 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -653,7 +653,8 @@ class ServersController(wsgi.Controller): except exception.ConfigDriveInvalidValue: msg = _("Invalid config_drive provided.") raise exc.HTTPBadRequest(explanation=msg) - except exception.ExternalNetworkAttachForbidden as error: + except (exception.BootFromVolumeRequiredForZeroDiskFlavor, + exception.ExternalNetworkAttachForbidden) as error: raise exc.HTTPForbidden(explanation=error.format_message()) except messaging.RemoteError as err: msg = "%(err_type)s: %(err_msg)s" % {'err_type': err.exc_type, diff --git a/nova/compute/api.py b/nova/compute/api.py index 303894814380..2401f0e63a5e 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -74,6 +74,7 @@ from nova.objects import fields as fields_obj from nova.objects import keypair as keypair_obj from nova.objects import quotas as quotas_obj from nova.pci import request as pci_request +from nova.policies import servers as servers_policies import nova.policy from nova import profiler from nova import rpc @@ -682,6 +683,15 @@ class API(base.Base): if image_min_disk > dest_size: raise exception.FlavorDiskSmallerThanMinDisk( flavor_size=dest_size, image_min_disk=image_min_disk) + else: + # The user is attempting to create a server with a 0-disk + # image-backed flavor, which can lead to issues with a large + # image consuming an unexpectedly large amount of local disk + # on the compute host. Check to see if the deployment will + # allow that. + if not context.can( + servers_policies.ZERO_DISK_FLAVOR, fatal=False): + raise exception.BootFromVolumeRequiredForZeroDiskFlavor() def _get_image_defined_bdms(self, instance_type, image_meta, root_device_name): diff --git a/nova/exception.py b/nova/exception.py index 84cbcb6941f1..4d5dd844c34b 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -1346,6 +1346,11 @@ class VolumeSmallerThanMinDisk(FlavorDiskTooSmall): "size is %(image_min_disk)i bytes.") +class BootFromVolumeRequiredForZeroDiskFlavor(Forbidden): + msg_fmt = _("Only volume-backed servers are allowed for flavors with " + "zero disk.") + + class InsufficientFreeMemory(NovaException): msg_fmt = _("Insufficient free memory on compute node to start %(uuid)s.") diff --git a/nova/policies/servers.py b/nova/policies/servers.py index 3250c55fa0c1..799b5c7be40a 100644 --- a/nova/policies/servers.py +++ b/nova/policies/servers.py @@ -18,6 +18,7 @@ from nova.policies import base RULE_AOO = base.RULE_ADMIN_OR_OWNER SERVERS = 'os_compute_api:servers:%s' +ZERO_DISK_FLAVOR = SERVERS % 'create:zero_disk_flavor' rules = [ policy.RuleDefault(SERVERS % 'index', RULE_AOO), @@ -33,6 +34,7 @@ rules = [ policy.RuleDefault(SERVERS % 'create:forced_host', base.RULE_ADMIN_API), policy.RuleDefault(SERVERS % 'create:attach_volume', RULE_AOO), policy.RuleDefault(SERVERS % 'create:attach_network', RULE_AOO), + policy.RuleDefault(ZERO_DISK_FLAVOR, RULE_AOO), policy.RuleDefault(SERVERS % 'delete', RULE_AOO), policy.RuleDefault(SERVERS % 'update', RULE_AOO), policy.RuleDefault(SERVERS % 'confirm_resize', RULE_AOO), diff --git a/nova/tests/functional/wsgi/test_servers.py b/nova/tests/functional/wsgi/test_servers.py index 5f9e8c86654f..4c92b8c62cee 100644 --- a/nova/tests/functional/wsgi/test_servers.py +++ b/nova/tests/functional/wsgi/test_servers.py @@ -10,8 +10,14 @@ # License for the specific language governing permissions and limitations # under the License. +import six + +from nova.policies import base as base_policies +from nova.policies import servers as servers_policies from nova import test from nova.tests import fixtures as nova_fixtures +from nova.tests.functional.api import client as api_client +from nova.tests.functional import integrated_helpers from nova.tests.unit.image import fake as fake_image from nova.tests.unit import policy_fixture @@ -176,3 +182,79 @@ class ServersPreSchedulingTestCase(test.TestCase): def test_instance_list_from_buildrequests_old_service(self): self._test_instance_list_from_buildrequests() + + +class EnforceVolumeBackedForZeroDiskFlavorTestCase( + test.TestCase, integrated_helpers.InstanceHelperMixin): + """Tests for the os_compute_api:servers:create:zero_disk_flavor policy rule + + These tests explicitly rely on microversion 2.1. + """ + + def setUp(self): + super(EnforceVolumeBackedForZeroDiskFlavorTestCase, self).setUp() + fake_image.stub_out_image_service(self) + self.addCleanup(fake_image.FakeImageService_reset) + self.useFixture(nova_fixtures.NeutronFixture(self)) + self.policy_fixture = ( + self.useFixture(policy_fixture.RealPolicyFixture())) + api_fixture = self.useFixture(nova_fixtures.OSAPIFixture( + api_version='v2.1')) + + self.api = api_fixture.api + self.admin_api = api_fixture.admin_api + # We need a zero disk flavor for the tests in this class. + flavor_req = { + "flavor": { + "name": "zero-disk-flavor", + "ram": 1024, + "vcpus": 2, + "disk": 0 + } + } + self.zero_disk_flavor = self.admin_api.post_flavor(flavor_req) + + def test_create_image_backed_server_with_zero_disk_fails(self): + """Tests that a non-admin trying to create an image-backed server + using a flavor with 0 disk will result in a 403 error when rule + os_compute_api:servers:create:zero_disk_flavor is set to admin-only. + """ + self.policy_fixture.set_rules({ + servers_policies.ZERO_DISK_FLAVOR: base_policies.RULE_ADMIN_API}, + overwrite=False) + server_req = self._build_minimal_create_server_request( + self.api, + 'test_create_image_backed_server_with_zero_disk_fails', + fake_image.AUTO_DISK_CONFIG_ENABLED_IMAGE_UUID, + self.zero_disk_flavor['id']) + ex = self.assertRaises(api_client.OpenStackApiException, + self.api.post_server, {'server': server_req}) + self.assertIn('Only volume-backed servers are allowed for flavors ' + 'with zero disk.', six.text_type(ex)) + self.assertEqual(403, ex.response.status_code) + + def test_create_volume_backed_server_with_zero_disk_allowed(self): + """Tests that creating a volume-backed server with a zero-root + disk flavor will be allowed for admins. + """ + # For this test, we want to start conductor and the scheduler but + # we don't start compute so that scheduling fails; we don't really + # care about successfully building an active server here. + self.useFixture(nova_fixtures.PlacementFixture()) + self.useFixture(nova_fixtures.CinderFixture(self)) + self.start_service('conductor') + self.start_service('scheduler') + server_req = self._build_minimal_create_server_request( + self.api, + 'test_create_volume_backed_server_with_zero_disk_allowed', + flavor_id=self.zero_disk_flavor['id']) + server_req.pop('imageRef', None) + server_req['block_device_mapping_v2'] = [{ + 'uuid': nova_fixtures.CinderFixture.IMAGE_BACKED_VOL, + 'source_type': 'volume', + 'destination_type': 'volume', + 'boot_index': 0 + }] + server = self.admin_api.post_server({'server': server_req}) + server = self._wait_for_state_change(self.api, server, 'ERROR') + self.assertIn('No valid host', server['fault']['message']) diff --git a/nova/tests/unit/test_policy.py b/nova/tests/unit/test_policy.py index 4c3d6cb8836d..eb065f882acf 100644 --- a/nova/tests/unit/test_policy.py +++ b/nova/tests/unit/test_policy.py @@ -354,6 +354,7 @@ class RealRolePolicyTestCase(test.NoDBTestCase): "os_compute_api:servers:create", "os_compute_api:servers:create:attach_network", "os_compute_api:servers:create:attach_volume", +"os_compute_api:servers:create:zero_disk_flavor", "os_compute_api:servers:create_image", "os_compute_api:servers:delete", "os_compute_api:servers:detail", diff --git a/releasenotes/notes/bug-1739646-enforce_volume_backed_for_zero_disk_flavor-b36a6eb4fa8b2964.yaml b/releasenotes/notes/bug-1739646-enforce_volume_backed_for_zero_disk_flavor-b36a6eb4fa8b2964.yaml new file mode 100644 index 000000000000..841433e11d81 --- /dev/null +++ b/releasenotes/notes/bug-1739646-enforce_volume_backed_for_zero_disk_flavor-b36a6eb4fa8b2964.yaml @@ -0,0 +1,20 @@ +--- +security: + - | + A new policy rule, ``os_compute_api:servers:create:zero_disk_flavor``, has + been introduced which defaults to ``rule:admin_or_owner`` for backward + compatibility, but can be configured to make the compute + API enforce that server create requests using a flavor with zero root disk + must be volume-backed or fail with a ``403 HTTPForbidden`` error. + + Allowing image-backed servers with a zero root disk flavor can be + potentially hazardous if users are allowed to upload their own images, + since an instance created with a zero root disk flavor gets its size + from the image, which can be unexpectedly large and exhaust local disk + on the compute host. See https://bugs.launchpad.net/nova/+bug/1739646 for + more details. + + While this is introduced in a backward-compatible way, the default will + be changed to ``rule:admin_api`` in a subsequent release. It is advised + that you communicate this change to your users before turning on + enforcement since it will result in a compute API behavior change.