From 985bd7390d0df59019732c1bd52c5642deee4465 Mon Sep 17 00:00:00 2001 From: David Lyle Date: Wed, 9 Oct 2013 15:12:54 -0600 Subject: [PATCH] adding policy checks for cinder Adding cinder policy rules file for policy checks. Implementing rule checks as well. Some cinder API calls actually hit nova, so adding those calls as well. Also a couple of improvements to the Horizon policy engine. First, now providing the token scope project_id and user_id as targets by default, unless otherwise specified. Most service policy rules check on or both of these. Second, checking to see if rule exists, before attempting enforcement. If the rule does not exist, using the default rule for that service. This now matches what the service policy engines do. Implements: blueprint block-rbac Change-Id: Ifef08b8975280f4e621ba8eebec9d405e1e870a2 --- openstack_dashboard/conf/cinder_policy.json | 58 +++++++++++++++++++ openstack_dashboard/conf/nova_policy.json | 2 + .../dashboards/admin/volumes/tables.py | 2 + .../volume_snapshots/tables.py | 10 ++++ .../dashboards/project/volumes/tables.py | 32 +++++++++- openstack_dashboard/policy.py | 29 +++++++++- openstack_dashboard/settings.py | 3 +- openstack_dashboard/test/tests/policy.py | 24 ++++++-- 8 files changed, 153 insertions(+), 7 deletions(-) create mode 100644 openstack_dashboard/conf/cinder_policy.json diff --git a/openstack_dashboard/conf/cinder_policy.json b/openstack_dashboard/conf/cinder_policy.json new file mode 100644 index 0000000000..a068af0ac4 --- /dev/null +++ b/openstack_dashboard/conf/cinder_policy.json @@ -0,0 +1,58 @@ +{ + "context_is_admin": [["role:admin"]], + "admin_or_owner": [["is_admin:True"], ["project_id:%(project_id)s"]], + "default": [["rule:admin_or_owner"]], + + "admin_api": [["is_admin:True"]], + + "volume:create": [], + "volume:delete": [["rule:default"]], + "volume:get_all": [], + "volume:get_volume_metadata": [], + "volume:get_volume_admin_metadata": [["rule:admin_api"]], + "volume:delete_volume_admin_metadata": [["rule:admin_api"]], + "volume:update_volume_admin_metadata": [["rule:admin_api"]], + "volume:create_snapshot": [["rule:default"]], + "volume:delete_snapshot": [["rule:default"]], + "volume:get_snapshot": [], + "volume:get_all_snapshots": [], + "volume:extend": [], + + "volume_extension:types_manage": [["rule:admin_api"]], + "volume_extension:types_extra_specs": [["rule:admin_api"]], + "volume_extension:volume_type_encryption": [["rule:admin_api"]], + "volume_extension:volume_encryption_metadata": [["rule:admin_api"]], + "volume_extension:extended_snapshot_attributes": [], + "volume_extension:volume_image_metadata": [], + + "volume_extension:quotas:show": [], + "volume_extension:quotas:update": [["rule:admin_api"]], + "volume_extension:quota_classes": [], + + "volume_extension:volume_admin_actions:reset_status": [["rule:admin_api"]], + "volume_extension:snapshot_admin_actions:reset_status": [["rule:admin_api"]], + "volume_extension:volume_admin_actions:force_delete": [["rule:admin_api"]], + "volume_extension:snapshot_admin_actions:force_delete": [["rule:admin_api"]], + "volume_extension:volume_admin_actions:migrate_volume": [["rule:admin_api"]], + "volume_extension:volume_admin_actions:migrate_volume_completion": [["rule:admin_api"]], + + "volume_extension:volume_host_attribute": [["rule:admin_api"]], + "volume_extension:volume_tenant_attribute": [["rule:admin_api"]], + "volume_extension:volume_mig_status_attribute": [["rule:admin_api"]], + "volume_extension:hosts": [["rule:admin_api"]], + "volume_extension:services": [["rule:admin_api"]], + "volume:services": [["rule:admin_api"]], + + "volume:create_transfer": [], + "volume:accept_transfer": [], + "volume:delete_transfer": [], + "volume:get_all_transfers": [], + + "backup:create" : [], + "backup:delete": [], + "backup:get": [], + "backup:get_all": [], + "backup:restore": [], + + "snapshot_extension:snapshot_actions:update_snapshot_status": [] +} diff --git a/openstack_dashboard/conf/nova_policy.json b/openstack_dashboard/conf/nova_policy.json index 239ba1ae19..b1d7e6706e 100644 --- a/openstack_dashboard/conf/nova_policy.json +++ b/openstack_dashboard/conf/nova_policy.json @@ -12,6 +12,8 @@ "compute:get_all": "", "compute:get_all_tenants": "", "compute:unlock_override": "rule:admin_api", + "compute:attach_volume" : "rule:default", + "compute:detach_volume" : "rule:default", "compute:shelve": "", "compute:shelve_offload": "", diff --git a/openstack_dashboard/dashboards/admin/volumes/tables.py b/openstack_dashboard/dashboards/admin/volumes/tables.py index 8518fe26fa..b704501f1c 100644 --- a/openstack_dashboard/dashboards/admin/volumes/tables.py +++ b/openstack_dashboard/dashboards/admin/volumes/tables.py @@ -23,11 +23,13 @@ class CreateVolumeType(tables.LinkAction): verbose_name = _("Create Volume Type") url = "horizon:admin:volumes:create_type" classes = ("ajax-modal", "btn-create") + policy_rules = (("volume", "volume_extension:types_manage"),) class DeleteVolumeType(tables.DeleteAction): data_type_singular = _("Volume Type") data_type_plural = _("Volume Types") + policy_rules = (("volume", "volume_extension:types_manage"),) def delete(self, request, obj_id): cinder.volume_type_delete(request, obj_id) diff --git a/openstack_dashboard/dashboards/project/images_and_snapshots/volume_snapshots/tables.py b/openstack_dashboard/dashboards/project/images_and_snapshots/volume_snapshots/tables.py index bd9c34fdfd..f294e7b11a 100644 --- a/openstack_dashboard/dashboards/project/images_and_snapshots/volume_snapshots/tables.py +++ b/openstack_dashboard/dashboards/project/images_and_snapshots/volume_snapshots/tables.py @@ -33,6 +33,15 @@ class DeleteVolumeSnapshot(tables.DeleteAction): data_type_singular = _("Volume Snapshot") data_type_plural = _("Volume Snapshots") action_past = _("Scheduled deletion of %(data_type)s") + policy_rules = (("volume", "volume:delete_snapshot"),) + + def get_policy_target(self, request, datum=None): + project_id = None + if datum: + project_id = getattr(datum, + "os-extended-snapshot-attributes:project_id", + None) + return {"project_id": project_id} def delete(self, request, obj_id): api.cinder.volume_snapshot_delete(request, obj_id) @@ -43,6 +52,7 @@ class CreateVolumeFromSnapshot(tables.LinkAction): verbose_name = _("Create Volume") url = "horizon:project:volumes:create" classes = ("ajax-modal", "btn-camera") + policy_rules = (("volume", "volume:create"),) def get_link_url(self, datum): base_url = reverse(self.url) diff --git a/openstack_dashboard/dashboards/project/volumes/tables.py b/openstack_dashboard/dashboards/project/volumes/tables.py index 4798ea82e0..268bd865a7 100644 --- a/openstack_dashboard/dashboards/project/volumes/tables.py +++ b/openstack_dashboard/dashboards/project/volumes/tables.py @@ -28,6 +28,7 @@ from horizon import tables from openstack_dashboard import api from openstack_dashboard.api import cinder +from openstack_dashboard import policy from openstack_dashboard.usage import quotas @@ -38,6 +39,13 @@ class DeleteVolume(tables.DeleteAction): data_type_singular = _("Volume") data_type_plural = _("Volumes") action_past = _("Scheduled deletion of %(data_type)s") + policy_rules = (("volume", "volume:delete"),) + + def get_policy_target(self, request, datum=None): + project_id = None + if datum: + project_id = getattr(datum, "os-vol-tenant-attr:tenant_id", None) + return {"project_id": project_id} def delete(self, request, obj_id): obj = self.table.get_object_by_id(obj_id) @@ -61,6 +69,7 @@ class CreateVolume(tables.LinkAction): verbose_name = _("Create Volume") url = "horizon:project:volumes:create" classes = ("ajax-modal", "btn-create") + policy_rules = (("volume", "volume:create"),) def allowed(self, request, volume=None): usages = quotas.tenant_quota_usages(request) @@ -84,7 +93,20 @@ class EditAttachments(tables.LinkAction): classes = ("ajax-modal", "btn-edit") def allowed(self, request, volume=None): - return volume.status in ("available", "in-use") + if volume: + project_id = getattr(volume, "os-vol-tenant-attr:tenant_id", None) + attach_allowed = \ + policy.check((("compute", "compute:attach_volume"),), + request, + {"project_id": project_id}) + detach_allowed = \ + policy.check((("compute", "compute:detach_volume"),), + request, + {"project_id": project_id}) + + if attach_allowed or detach_allowed: + return volume.status in ("available", "in-use") + return False class CreateSnapshot(tables.LinkAction): @@ -92,6 +114,13 @@ class CreateSnapshot(tables.LinkAction): verbose_name = _("Create Snapshot") url = "horizon:project:volumes:create_snapshot" classes = ("ajax-modal", "btn-camera") + policy_rules = (("volume", "volume:create_snapshot"),) + + def get_policy_target(self, request, datum=None): + project_id = None + if datum: + project_id = getattr(datum, "os-vol-tenant-attr:tenant_id", None) + return {"project_id": project_id} def allowed(self, request, volume=None): return volume.status in ("available", "in-use") @@ -219,6 +248,7 @@ class DetachVolume(tables.BatchAction): data_type_singular = _("Volume") data_type_plural = _("Volumes") classes = ('btn-danger', 'btn-detach') + policy_rules = (("compute", "compute:detach_volume"),) def action(self, request, obj_id): attachment = self.table.get_object_by_id(obj_id) diff --git a/openstack_dashboard/policy.py b/openstack_dashboard/policy.py index beef87e601..e8c5d67976 100644 --- a/openstack_dashboard/policy.py +++ b/openstack_dashboard/policy.py @@ -97,6 +97,23 @@ def check(actions, request, target={}): :returns: boolean if the user has permission or not for the actions. """ user = auth_utils.get_user(request) + + # Several service policy engines default to a project id check for + # ownership. Since the user is already scoped to a project, if a + # different project id has not been specified use the currently scoped + # project's id. + # + # The reason is the operator can edit the local copies of the service + # policy file. If a rule is removed, then the default rule is used. We + # don't want to block all actions because the operator did not fully + # understand the implication of editing the policy file. Additionally, + # the service APIs will correct us if we are too permissive. + if 'project_id' not in target: + target['project_id'] = user.project_id + # same for user_id + if 'user_id' not in target: + target['user_id'] = user.id + credentials = _user_to_credentials(request, user) enforcer = _get_enforcer() @@ -106,7 +123,17 @@ def check(actions, request, target={}): if scope in enforcer: # if any check fails return failure if not enforcer[scope].enforce(action, target, credentials): - return False + # to match service implementations, if a rule is not found, + # use the default rule for that service policy + # + # waiting to make the check because the first call to + # enforce loads the rules + if action not in enforcer[scope].rules: + if not enforcer[scope].enforce('default', + target, credentials): + return False + else: + return False # if no policy for scope, allow action, underlying API will # ultimately block the action if not permitted, treat as though # allowed diff --git a/openstack_dashboard/settings.py b/openstack_dashboard/settings.py index 86f0da0193..bf2bec706e 100644 --- a/openstack_dashboard/settings.py +++ b/openstack_dashboard/settings.py @@ -208,7 +208,8 @@ POLICY_FILES_PATH = os.path.join(ROOT_PATH, "conf") # Map of local copy of service policy files POLICY_FILES = { 'identity': 'keystone_policy.json', - 'compute': 'nova_policy.json' + 'compute': 'nova_policy.json', + 'volume': 'cinder_policy.json' } SECRET_KEY = None diff --git a/openstack_dashboard/test/tests/policy.py b/openstack_dashboard/test/tests/policy.py index dca4836c5d..58e93b1e20 100644 --- a/openstack_dashboard/test/tests/policy.py +++ b/openstack_dashboard/test/tests/policy.py @@ -38,9 +38,17 @@ class PolicyTestCase(test.TestCase): request=self.request) self.assertFalse(value) - def test_check_nova_admin_required_false(self): + def test_check_identity_rule_not_found_false(self): policy.reset() - value = policy.check((("compute", "admin__or_owner"),), + value = policy.check((("identity", "i_dont_exist"),), + request=self.request) + # this should fail because the default check for + # identity is admin_required + self.assertFalse(value) + + def test_check_nova_context_is_admin_false(self): + policy.reset() + value = policy.check((("compute", "context_is_admin"),), request=self.request) self.assertFalse(value) @@ -65,6 +73,14 @@ class PolicyTestCaseAdmin(test.BaseAdminViewTests): request=self.request) self.assertTrue(value) + def test_check_identity_rule_not_found_true(self): + policy.reset() + value = policy.check((("identity", "i_dont_exist"),), + request=self.request) + # this should succeed because the default check for + # identity is admin_required + self.assertTrue(value) + def test_compound_check_true(self): policy.reset() value = policy.check((("identity", "admin_required"), @@ -72,8 +88,8 @@ class PolicyTestCaseAdmin(test.BaseAdminViewTests): request=self.request) self.assertTrue(value) - def test_check_nova_admin_required_true(self): + def test_check_nova_context_is_admin_true(self): policy.reset() - value = policy.check((("compute", "admin__or_owner"),), + value = policy.check((("compute", "context_is_admin"),), request=self.request) self.assertTrue(value)