From 728f2b215e0b9f12ea256acb1e39f7b93d918e6f Mon Sep 17 00:00:00 2001 From: Ghanshyam Date: Thu, 30 Jan 2020 18:18:39 -0600 Subject: [PATCH] Fix os-attach-interfaces policy to be admin_or_owner os-attach-interfaces APi policy is default to admin_or_owner[1] but API is allowed for everyone. We can see the test trying with other project context can access the API - https://review.opendev.org/#/c/705126/1 This is because API does not pass the server project_id in policy target[2] and if no target is passed then, policy.py add the default targets which is nothing but context.project_id (allow for everyone who try to access)[3] This commit fix this policy by passing the server's project_id in policy target. [1] https://github.com/openstack/nova/blob/c16315165ce307c605cf4b608b2df3aa06f46982/nova/policies/attach_interfaces.py#L28 [2] https://github.com/openstack/nova/blob/c16315165ce307c605cf4b608b2df3aa06f46982/nova/api/openstack/compute/attach_interfaces.py#L70 [3] https://github.com/openstack/nova/blob/c16315165ce307c605cf4b608b2df3aa06f46982/nova/policy.py#L191 Closes-bug: #1861464 Change-Id: I1e2247884169e6ba3e5302be4323428c67ce7a10 --- .../openstack/compute/attach_interfaces.py | 32 ++++++++++------- .../compute/test_attach_interfaces.py | 36 +++++++++++++------ 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/nova/api/openstack/compute/attach_interfaces.py b/nova/api/openstack/compute/attach_interfaces.py index 7b9ae41501..ff53d92e6b 100644 --- a/nova/api/openstack/compute/attach_interfaces.py +++ b/nova/api/openstack/compute/attach_interfaces.py @@ -67,9 +67,10 @@ class InterfaceAttachmentController(wsgi.Controller): def index(self, req, server_id): """Returns the list of interface attachments for a given instance.""" context = req.environ['nova.context'] - context.can(ai_policies.BASE_POLICY_NAME) - instance = common.get_instance(self.compute_api, context, server_id) + context.can(ai_policies.BASE_POLICY_NAME, + target={'project_id': instance.project_id}) + search_opts = {'device_id': instance.uuid} try: @@ -108,13 +109,11 @@ class InterfaceAttachmentController(wsgi.Controller): def show(self, req, server_id, id): """Return data about the given interface attachment.""" context = req.environ['nova.context'] - context.can(ai_policies.BASE_POLICY_NAME) + instance = common.get_instance(self.compute_api, context, server_id) + context.can(ai_policies.BASE_POLICY_NAME, + target={'project_id': instance.project_id}) port_id = id - # NOTE(mriedem): We need to verify the instance actually exists from - # the server_id even though we're not using the instance for anything, - # just the port id. - common.get_instance(self.compute_api, context, server_id) try: port_info = self.network_api.show_port(context, port_id) @@ -139,8 +138,12 @@ class InterfaceAttachmentController(wsgi.Controller): def create(self, req, server_id, body): """Attach an interface to an instance.""" context = req.environ['nova.context'] - context.can(ai_policies.BASE_POLICY_NAME) - context.can(ai_policies.POLICY_ROOT % 'create') + instance = common.get_instance(self.compute_api, context, server_id) + + context.can(ai_policies.BASE_POLICY_NAME, + target={'project_id': instance.project_id}) + context.can(ai_policies.POLICY_ROOT % 'create', + target={'project_id': instance.project_id}) network_id = None port_id = None @@ -163,7 +166,6 @@ class InterfaceAttachmentController(wsgi.Controller): msg = _("Must input network_id when request IP address") raise exc.HTTPBadRequest(explanation=msg) - instance = common.get_instance(self.compute_api, context, server_id) try: vif = self.compute_api.attach_interface(context, instance, network_id, port_id, req_ip, tag=tag) @@ -199,12 +201,16 @@ class InterfaceAttachmentController(wsgi.Controller): def delete(self, req, server_id, id): """Detach an interface from an instance.""" context = req.environ['nova.context'] - context.can(ai_policies.BASE_POLICY_NAME) - context.can(ai_policies.POLICY_ROOT % 'delete') - port_id = id instance = common.get_instance(self.compute_api, context, server_id, expected_attrs=['device_metadata']) + + context.can(ai_policies.BASE_POLICY_NAME, + target={'project_id': instance.project_id}) + context.can(ai_policies.POLICY_ROOT % 'delete', + target={'project_id': instance.project_id}) + port_id = id + try: self.compute_api.detach_interface(context, instance, port_id=port_id) diff --git a/nova/tests/unit/api/openstack/compute/test_attach_interfaces.py b/nova/tests/unit/api/openstack/compute/test_attach_interfaces.py index c1d6167982..caf9fcbede 100644 --- a/nova/tests/unit/api/openstack/compute/test_attach_interfaces.py +++ b/nova/tests/unit/api/openstack/compute/test_attach_interfaces.py @@ -25,6 +25,7 @@ from nova import exception from nova import objects from nova import test from nova.tests.unit.api.openstack import fakes +from nova.tests.unit import fake_instance from nova.tests.unit import fake_network_cache_model @@ -109,8 +110,10 @@ def fake_detach_interface(self, context, instance, port_id): raise exception.PortNotFound(port_id=port_id) -def fake_get_instance(self, *args, **kwargs): - return objects.Instance(uuid=FAKE_UUID1) +def fake_get_instance(self, context, instance_id, expected_attrs=None, + cell_down_support=False): + return fake_instance.fake_instance_obj( + context, id=1, uuid=instance_id, project_id=context.project_id) class InterfaceAttachTestsV21(test.NoDBTestCase): @@ -178,8 +181,9 @@ class InterfaceAttachTestsV21(test.NoDBTestCase): def test_delete(self): self.stub_out('nova.compute.api.API.detach_interface', fake_detach_interface) - - inst = objects.Instance(uuid=FAKE_UUID1) + req_context = self.req.environ['nova.context'] + inst = objects.Instance(uuid=FAKE_UUID1, + project_id=req_context.project_id) with mock.patch.object(common, 'get_instance', return_value=inst) as mock_get_instance: result = self.attachments.delete(self.req, FAKE_UUID1, @@ -360,7 +364,9 @@ class InterfaceAttachTestsV21(test.NoDBTestCase): def test_attach_interface_fixed_ip_already_in_use(self, attach_mock, get_mock): - fake_instance = objects.Instance(uuid=FAKE_UUID1) + req_context = self.req.environ['nova.context'] + fake_instance = objects.Instance(uuid=FAKE_UUID1, + project_id=req_context.project_id) get_mock.return_value = fake_instance attach_mock.side_effect = exception.FixedIpAlreadyInUse( address='10.0.2.2', instance_uuid=FAKE_UUID1) @@ -380,7 +386,9 @@ class InterfaceAttachTestsV21(test.NoDBTestCase): def test_attach_interface_port_in_use(self, attach_mock, get_mock): - fake_instance = objects.Instance(uuid=FAKE_UUID1) + req_context = self.req.environ['nova.context'] + fake_instance = objects.Instance(uuid=FAKE_UUID1, + project_id=req_context.project_id) get_mock.return_value = fake_instance attach_mock.side_effect = exception.PortInUse( port_id=FAKE_PORT_ID1) @@ -400,7 +408,9 @@ class InterfaceAttachTestsV21(test.NoDBTestCase): def test_attach_interface_port_not_usable(self, attach_mock, get_mock): - fake_instance = objects.Instance(uuid=FAKE_UUID1) + req_context = self.req.environ['nova.context'] + fake_instance = objects.Instance(uuid=FAKE_UUID1, + project_id=req_context.project_id) get_mock.return_value = fake_instance attach_mock.side_effect = exception.PortNotUsable( port_id=FAKE_PORT_ID1, @@ -419,8 +429,9 @@ class InterfaceAttachTestsV21(test.NoDBTestCase): @mock.patch.object(compute_api.API, 'get') @mock.patch.object(compute_api.API, 'attach_interface') def test_attach_interface_failed_no_network(self, attach_mock, get_mock): + req_context = self.req.environ['nova.context'] fake_instance = objects.Instance(uuid=FAKE_UUID1, - project_id=FAKE_UUID2) + project_id=req_context.project_id) get_mock.return_value = fake_instance attach_mock.side_effect = ( exception.InterfaceAttachFailedNoNetwork(project_id=FAKE_UUID2)) @@ -438,7 +449,9 @@ class InterfaceAttachTestsV21(test.NoDBTestCase): def test_attach_interface_no_more_fixed_ips(self, attach_mock, get_mock): - fake_instance = objects.Instance(uuid=FAKE_UUID1) + req_context = self.req.environ['nova.context'] + fake_instance = objects.Instance(uuid=FAKE_UUID1, + project_id=req_context.project_id) get_mock.return_value = fake_instance attach_mock.side_effect = exception.NoMoreFixedIps( net=FAKE_NET_ID1) @@ -457,8 +470,9 @@ class InterfaceAttachTestsV21(test.NoDBTestCase): @mock.patch.object(compute_api.API, 'attach_interface') def test_attach_interface_failed_securitygroup_cannot_be_applied( self, attach_mock, get_mock): + req_context = self.req.environ['nova.context'] fake_instance = objects.Instance(uuid=FAKE_UUID1, - project_id=FAKE_UUID2) + project_id=req_context.project_id) get_mock.return_value = fake_instance attach_mock.side_effect = ( exception.SecurityGroupCannotBeApplied()) @@ -580,6 +594,8 @@ class AttachInterfacesPolicyEnforcementv21(test.NoDBTestCase): self.rule_name = "os_compute_api:os-attach-interfaces" self.policy.set_rules({self.rule_name: "project:non_fake"}) + self.stub_out('nova.compute.api.API.get', fake_get_instance) + def test_index_attach_interfaces_policy_failed(self): exc = self.assertRaises( exception.PolicyNotAuthorized,