Policy-in-code servers rules

This adds the basic framework for registering and using default policy
rules. Rules should be defined and returned from a module in
nova/policies/, and then added to the list in nova/policies/__init__.py.

A new context.can() method has been added for policy enforcement of
registered rules. It has the same parameters as the enforce() method
currently being used.

To establish the full pattern for usage the policy checks in the servers
API module have been registered and converted to the new usage.

Now that some policy checks are registered they're being used properly
by tests. Some tests have been updated so that the instance project_id
matches the context project_id in order to pass the 'admin_or_owner'
check.

Change-Id: I71b3d1233255125cb280a000b990329f5b03fdfd
Partially-Implements: bp policy-in-code

etc/nova/policy.json

@@ -8,28 +8,7 @@
     "admin_api": "is_admin:True",
 
     "network:attach_external_network": "is_admin:True",
-    "os_compute_api:servers:detail:get_all_tenants": "is_admin:True",
-    "os_compute_api:servers:index:get_all_tenants": "is_admin:True",
-    "os_compute_api:servers:confirm_resize": "rule:admin_or_owner",
-    "os_compute_api:servers:create": "rule:admin_or_owner",
-    "os_compute_api:servers:create:attach_network": "rule:admin_or_owner",
-    "os_compute_api:servers:create:attach_volume": "rule:admin_or_owner",
-    "os_compute_api:servers:create:forced_host": "rule:admin_api",
-    "os_compute_api:servers:delete": "rule:admin_or_owner",
-    "os_compute_api:servers:update": "rule:admin_or_owner",
-    "os_compute_api:servers:detail": "rule:admin_or_owner",
-    "os_compute_api:servers:index": "rule:admin_or_owner",
-    "os_compute_api:servers:reboot": "rule:admin_or_owner",
-    "os_compute_api:servers:rebuild": "rule:admin_or_owner",
-    "os_compute_api:servers:resize": "rule:admin_or_owner",
-    "os_compute_api:servers:revert_resize": "rule:admin_or_owner",
-    "os_compute_api:servers:show": "rule:admin_or_owner",
     "os_compute_api:servers:show:host_status": "rule:admin_api",
-    "os_compute_api:servers:create_image": "rule:admin_or_owner",
-    "os_compute_api:servers:create_image:allow_volume_backed": "rule:admin_or_owner",
-    "os_compute_api:servers:start": "rule:admin_or_owner",
-    "os_compute_api:servers:stop": "rule:admin_or_owner",
-    "os_compute_api:servers:trigger_crash_dump": "rule:admin_or_owner",
     "os_compute_api:servers:migrations:force_complete": "rule:admin_api",
     "os_compute_api:servers:migrations:delete": "rule:admin_api",
     "os_compute_api:servers:discoverable": "@",

nova/api/openstack/compute/servers.py

@@ -43,6 +43,7 @@ from nova.i18n import _
 from nova.i18n import _LW
 from nova.image import glance
 from nova import objects
+from nova.policies import servers as server_policies
 from nova import utils
 
 ALIAS = 'servers'
@@ -51,7 +52,6 @@ TAG_SEARCH_FILTERS = ('tags', 'tags-any', 'not-tags', 'not-tags-any')
 CONF = nova.conf.CONF
 
 LOG = logging.getLogger(__name__)
-authorize = extensions.os_compute_authorizer(ALIAS)
 
 
 class ServersController(wsgi.Controller):
@@ -273,7 +273,7 @@ class ServersController(wsgi.Controller):
     def index(self, req):
         """Returns a list of server names and ids for a given user."""
         context = req.environ['nova.context']
-        authorize(context, action="index")
+        context.can(server_policies.get_name('index'))
         try:
             servers = self._get_servers(req, is_detail=False)
         except exception.Invalid as err:
@@ -284,7 +284,7 @@ class ServersController(wsgi.Controller):
     def detail(self, req):
         """Returns a list of server details for a given user."""
         context = req.environ['nova.context']
-        authorize(context, action="detail")
+        context.can(server_policies.get_name('detail'))
         try:
             servers = self._get_servers(req, is_detail=True)
         except exception.Invalid as err:
@@ -383,9 +383,9 @@ class ServersController(wsgi.Controller):
         elevated = None
         if all_tenants:
             if is_detail:
-                authorize(context, action="detail:get_all_tenants")
+                context.can(server_policies.get_name('detail:get_all_tenants'))
             else:
-                authorize(context, action="index:get_all_tenants")
+                context.can(server_policies.get_name('index:get_all_tenants'))
             elevated = context.elevated()
         else:
             if context.project_id:
@@ -538,7 +538,7 @@ class ServersController(wsgi.Controller):
     def show(self, req, id):
         """Returns server details by server id."""
         context = req.environ['nova.context']
-        authorize(context, action="show")
+        context.can(server_policies.get_name('show'))
         instance = self._get_server(context, req, id, is_detail=True)
         return self._view_builder.show(req, instance)
 
@@ -585,7 +585,7 @@ class ServersController(wsgi.Controller):
             'project_id': context.project_id,
             'user_id': context.user_id,
             'availability_zone': availability_zone}
-        authorize(context, target, 'create')
+        context.can(server_policies.get_name('create'), target)
 
         # TODO(Shao He, Feng) move this policy check to os-availabilty-zone
         # extension after refactor it.
@@ -596,13 +596,14 @@ class ServersController(wsgi.Controller):
         except exception.InvalidInput as err:
             raise exc.HTTPBadRequest(explanation=six.text_type(err))
         if host or node:
-            authorize(context, {}, 'create:forced_host')
+            context.can(server_policies.get_name('create:forced_host'), {})
 
         block_device_mapping = create_kwargs.get("block_device_mapping")
         # TODO(Shao He, Feng) move this policy check to os-block-device-mapping
         # extension after refactor it.
         if block_device_mapping:
-            authorize(context, target, 'create:attach_volume')
+            context.can(server_policies.get_name('create:attach_volume'),
+                        target)
 
         image_uuid = self._image_from_req_data(server_dict, create_kwargs)
 
@@ -626,7 +627,8 @@ class ServersController(wsgi.Controller):
                 requested_networks)
 
         if requested_networks and len(requested_networks):
-            authorize(context, target, 'create:attach_network')
+            context.can(server_policies.get_name('create:attach_network'),
+                        target)
 
         try:
             flavor_id = self._flavor_id_from_req_data(body)
@@ -801,7 +803,7 @@ class ServersController(wsgi.Controller):
         resize_schema['properties']['resize']['properties'].update(schema)
 
     def _delete(self, context, req, instance_uuid):
-        authorize(context, action='delete')
+        context.can(server_policies.get_name('delete'))
         instance = self._get_server(context, req, instance_uuid)
         if CONF.reclaim_instance_interval:
             try:
@@ -823,7 +825,7 @@ class ServersController(wsgi.Controller):
 
         ctxt = req.environ['nova.context']
         update_dict = {}
-        authorize(ctxt, action='update')
+        ctxt.can(server_policies.get_name('update'))
 
         if 'name' in body['server']:
             update_dict['display_name'] = common.normalize_name(
@@ -857,7 +859,7 @@ class ServersController(wsgi.Controller):
     @wsgi.action('confirmResize')
     def _action_confirm_resize(self, req, id, body):
         context = req.environ['nova.context']
-        authorize(context, action='confirm_resize')
+        context.can(server_policies.get_name('confirm_resize'))
         instance = self._get_server(context, req, id)
         try:
             self.compute_api.confirm_resize(context, instance)
@@ -877,7 +879,7 @@ class ServersController(wsgi.Controller):
     @wsgi.action('revertResize')
     def _action_revert_resize(self, req, id, body):
         context = req.environ['nova.context']
-        authorize(context, action='revert_resize')
+        context.can(server_policies.get_name('revert_resize'))
         instance = self._get_server(context, req, id)
         try:
             self.compute_api.revert_resize(context, instance)
@@ -903,7 +905,7 @@ class ServersController(wsgi.Controller):
 
         reboot_type = body['reboot']['type'].upper()
         context = req.environ['nova.context']
-        authorize(context, action='reboot')
+        context.can(server_policies.get_name('reboot'))
         instance = self._get_server(context, req, id)
 
         try:
@@ -917,7 +919,7 @@ class ServersController(wsgi.Controller):
     def _resize(self, req, instance_id, flavor_id, **kwargs):
         """Begin the resize process with given instance/flavor."""
         context = req.environ["nova.context"]
-        authorize(context, action='resize')
+        context.can(server_policies.get_name('resize'))
         instance = self._get_server(context, req, instance_id)
 
         try:
@@ -1023,7 +1025,7 @@ class ServersController(wsgi.Controller):
         password = self._get_server_admin_password(rebuild_dict)
 
         context = req.environ['nova.context']
-        authorize(context, action='rebuild')
+        context.can(server_policies.get_name('rebuild'))
         instance = self._get_server(context, req, id)
 
         attr_map = {
@@ -1098,7 +1100,7 @@ class ServersController(wsgi.Controller):
     def _action_create_image(self, req, id, body):
         """Snapshot a server instance."""
         context = req.environ['nova.context']
-        authorize(context, action='create_image')
+        context.can(server_policies.get_name('create_image'))
 
         entity = body["createImage"]
         image_name = common.normalize_name(entity["name"])
@@ -1114,7 +1116,8 @@ class ServersController(wsgi.Controller):
         try:
             if compute_utils.is_volume_backed_instance(context, instance,
                                                           bdms):
-                authorize(context, action="create_image:allow_volume_backed")
+                context.can(server_policies.get_name(
+                    'create_image:allow_volume_backed'))
                 image = self.compute_api.snapshot_volume_backed(
                                                        context,
                                                        instance,
@@ -1175,7 +1178,7 @@ class ServersController(wsgi.Controller):
         """Start an instance."""
         context = req.environ['nova.context']
         instance = self._get_instance(context, id)
-        authorize(context, instance, 'start')
+        context.can(server_policies.get_name('start'), instance)
         LOG.debug('start instance', instance=instance)
         try:
             self.compute_api.start(context, instance)
@@ -1194,7 +1197,7 @@ class ServersController(wsgi.Controller):
         """Stop an instance."""
         context = req.environ['nova.context']
         instance = self._get_instance(context, id)
-        authorize(context, instance, 'stop')
+        context.can(server_policies.get_name('stop'), instance)
         LOG.debug('stop instance', instance=instance)
         try:
             self.compute_api.stop(context, instance)
@@ -1215,7 +1218,7 @@ class ServersController(wsgi.Controller):
         """Trigger crash dump in an instance"""
         context = req.environ['nova.context']
         instance = self._get_instance(context, id)
-        authorize(context, instance, 'trigger_crash_dump')
+        context.can(server_policies.get_name('trigger_crash_dump'), instance)
         try:
             self.compute_api.trigger_crash_dump(context, instance)
         except exception.InstanceInvalidState as state_error:

nova/context.py

@@ -218,6 +218,12 @@ class RequestContext(context.RequestContext):
 
         return context
 
+    def can(self, rule, target=None):
+        if target is None:
+            target = {'project_id': self.project_id,
+                      'user_id': self.user_id}
+        return policy.authorize(self, rule, target)
+
     def __str__(self):
         return "<Context %s>" % self.to_dict()
 

nova/policies/__init__.py

@@ -0,0 +1,24 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+
+import itertools
+
+from nova.policies import base
+from nova.policies import servers
+
+
+def list_rules():
+    return itertools.chain(
+        base.list_rules(),
+        servers.list_rules()
+    )

nova/policies/base.py

@@ -0,0 +1,24 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from oslo_policy import policy
+
+rules = [
+    policy.RuleDefault('context_is_admin', 'role:admin'),
+    policy.RuleDefault('admin_or_owner',
+                       'is_admin:True or project_id:%(project_id)s'),
+    policy.RuleDefault('admin_api', 'is_admin:True'),
+]
+
+
+def list_rules():
+    return rules

nova/policies/servers.py

@@ -0,0 +1,53 @@
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+
+from oslo_policy import policy
+
+
+RULE_AOO = 'rule:admin_or_owner'
+
+
+def get_name(action=None):
+    name = 'os_compute_api:servers'
+    if action:
+        name = name + ':%s' % action
+    return name
+
+rules = [
+    policy.RuleDefault(get_name('index'), RULE_AOO),
+    policy.RuleDefault(get_name('detail'), RULE_AOO),
+    policy.RuleDefault(get_name('detail:get_all_tenants'), RULE_AOO),
+    policy.RuleDefault(get_name('index:get_all_tenants'), RULE_AOO),
+    policy.RuleDefault(get_name('show'), RULE_AOO),
+    policy.RuleDefault(get_name('create'), RULE_AOO),
+    policy.RuleDefault(get_name('create:forced_host'), RULE_AOO),
+    policy.RuleDefault(get_name('create:attach_volume'), RULE_AOO),
+    policy.RuleDefault(get_name('create:attach_network'), RULE_AOO),
+    policy.RuleDefault(get_name('delete'), RULE_AOO),
+    policy.RuleDefault(get_name('update'), RULE_AOO),
+    policy.RuleDefault(get_name('confirm_resize'), RULE_AOO),
+    policy.RuleDefault(get_name('revert_resize'), RULE_AOO),
+    policy.RuleDefault(get_name('reboot'), RULE_AOO),
+    policy.RuleDefault(get_name('resize'), RULE_AOO),
+    policy.RuleDefault(get_name('rebuild'), RULE_AOO),
+    policy.RuleDefault(get_name('create_image'), RULE_AOO),
+    policy.RuleDefault(get_name('create_image:allow_volume_backed'),
+                       RULE_AOO),
+    policy.RuleDefault(get_name('start'), RULE_AOO),
+    policy.RuleDefault(get_name('stop'), RULE_AOO),
+    policy.RuleDefault(get_name('trigger_crash_dump'), RULE_AOO),
+]
+
+
+def list_rules():
+    return rules

nova/policy.py

@@ -22,6 +22,8 @@ from oslo_policy import policy
 from oslo_utils import excutils
 
 from nova import exception
+from nova.i18n import _LE
+from nova import policies
 
 
 CONF = cfg.CONF
@@ -55,6 +57,7 @@ def init(policy_file=None, rules=None, default_rule=None, use_conf=True):
                                     rules=rules,
                                     default_rule=default_rule,
                                     use_conf=use_conf)
+        register_rules(_ENFORCER)
 
 
 def set_rules(rules, overwrite=True, use_conf=False):
@@ -70,6 +73,8 @@ def set_rules(rules, overwrite=True, use_conf=False):
     _ENFORCER.set_rules(rules, overwrite, use_conf)
 
 
+# TODO(alaski): All users of this method should move over to authorize() as
+# policies are registered and ultimately this should be removed.
 def enforce(context, action, target, do_raise=True, exc=None):
     """Verifies that the action is valid on the target in this context.
 
@@ -108,6 +113,53 @@ def enforce(context, action, target, do_raise=True, exc=None):
     return result
 
 
+def authorize(context, action, target, do_raise=True, exc=None):
+    """Verifies that the action is valid on the target in this context.
+
+       :param context: nova context
+       :param action: string representing the action to be checked
+           this should be colon separated for clarity.
+           i.e. ``compute:create_instance``,
+           ``compute:attach_volume``,
+           ``volume:attach_volume``
+       :param target: dictionary representing the object of the action
+           for object creation this should be a dictionary representing the
+           location of the object e.g. ``{'project_id': context.project_id}``
+       :param do_raise: if True (the default), raises PolicyNotAuthorized;
+           if False, returns False
+       :param exc: Class of the exception to raise if the check fails.
+                   Any remaining arguments passed to :meth:`enforce` (both
+                   positional and keyword arguments) will be passed to
+                   the exception class. If not specified,
+                   :class:`PolicyNotAuthorized` will be used.
+
+       :raises nova.exception.PolicyNotAuthorized: if verification fails
+           and do_raise is True. Or if 'exc' is specified it will raise an
+           exception of that type.
+
+       :return: returns a non-False value (not necessarily "True") if
+           authorized, and the exact value False if not authorized and
+           do_raise is False.
+    """
+    init()
+    credentials = context.to_dict()
+    if not exc:
+        exc = exception.PolicyNotAuthorized
+    try:
+        result = _ENFORCER.authorize(action, target, credentials,
+                                     do_raise=do_raise, exc=exc, action=action)
+    except policy.PolicyNotRegistered:
+        with excutils.save_and_reraise_exception():
+            LOG.exception(_LE('Policy not registered'))
+    except Exception:
+        credentials.pop('auth_token', None)
+        with excutils.save_and_reraise_exception():
+            LOG.debug('Policy check for %(action)s failed with credentials '
+                      '%(credentials)s',
+                      {'action': action, 'credentials': credentials})
+    return result
+
+
 def check_is_admin(context):
     """Whether or not roles contains 'admin' role according to policy setting.
 
@@ -140,3 +192,7 @@ class IsAdminCheck(policy.Check):
 def get_rules():
     if _ENFORCER:
         return _ENFORCER.rules
+
+
+def register_rules(enforcer):
+    enforcer.register_defaults(policies.list_rules())

nova/tests/unit/api/openstack/compute/test_serversV21.py

@@ -1614,7 +1614,8 @@ class ServersControllerRebuildInstanceTest(ControllerTest):
             if uuid == 'test_inst':
                 raise webob.exc.HTTPNotFound(explanation='fakeout')
             return fakes.stub_instance_obj(None,
-                                           vm_state=vm_states.ACTIVE)
+                                           vm_state=vm_states.ACTIVE,
+                                           project_id='fake')
 
         self.useFixture(
             fixtures.MonkeyPatch('nova.api.openstack.compute.servers.'
@@ -2101,7 +2102,8 @@ class ServersControllerTriggerCrashDumpTest(ControllerTest):
         super(ServersControllerTriggerCrashDumpTest, self).setUp()
 
         self.instance = fakes.stub_instance_obj(None,
-                                                vm_state=vm_states.ACTIVE)
+                                                vm_state=vm_states.ACTIVE,
+                                                project_id='fake')
 
         def fake_get(ctrl, ctxt, uuid):
             if uuid != FAKE_UUID:

nova/tests/unit/api/openstack/fakes.py

@@ -323,6 +323,8 @@ def get_fake_uuid(token=0):
 
 def fake_instance_get(**kwargs):
     def _return_server(context, uuid, columns_to_join=None, use_slave=False):
+        if 'project_id' not in kwargs:
+            kwargs['project_id'] = 'fake'
         return stub_instance(1, **kwargs)
     return _return_server
 

nova/tests/unit/fake_policy.py

@@ -21,28 +21,7 @@ policy_data = """
 
     "context_is_admin": "role:admin or role:administrator",
 
-    "os_compute_api:servers:confirm_resize": "",
-    "os_compute_api:servers:create": "",
-    "os_compute_api:servers:create:attach_network": "",
-    "os_compute_api:servers:create:attach_volume": "",
-    "os_compute_api:servers:create:forced_host": "",
-    "os_compute_api:servers:delete": "",
-    "os_compute_api:servers:detail": "",
-    "os_compute_api:servers:detail:get_all_tenants": "",
-    "os_compute_api:servers:index": "",
-    "os_compute_api:servers:index:get_all_tenants": "",
-    "os_compute_api:servers:reboot": "",
-    "os_compute_api:servers:rebuild": "",
-    "os_compute_api:servers:resize": "",
-    "os_compute_api:servers:revert_resize": "",
-    "os_compute_api:servers:show": "",
     "os_compute_api:servers:show:host_status": "",
-    "os_compute_api:servers:create_image": "",
-    "os_compute_api:servers:create_image:allow_volume_backed": "",
-    "os_compute_api:servers:update": "",
-    "os_compute_api:servers:start": "",
-    "os_compute_api:servers:stop": "",
-    "os_compute_api:servers:trigger_crash_dump": "",
     "os_compute_api:servers:migrations:delete": "rule:admin_api",
     "os_compute_api:servers:migrations:force_complete": "",
     "os_compute_api:servers:migrations:index": "rule:admin_api",