[aim-mapping] Restrict auto-ptg access

This patch restricts GET and UPDATE for auto-ptg to the admin
via policy.json RBAC enforcement mechanism. When these rules are
in effect, policy_target creation in the auto_ptg is also restricted
to only the admin. These rules can however be relaxed if required by
modifying the policy.json file as follows:

Replace:
"get_policy_target_group": "rule:admin_auto_ptg or rule:non_auto_ptg",
"update_policy_target_group": "rule:admin_auto_ptg or rule:non_auto_ptg",

with:
"get_policy_target_group": "rule:admin_or_owner or rule:shared_ptg",

This patch adds a new driver extension attribute: is_auto_ptg to
facilitate specification of rules in policy.json. This has the added
benefit of supportying the specification of a filter for auto_ptgs
when retrieving policy_target_groups.

Change-Id: I6d9e873acb2b1b3bee8d78a45527bd4d5d437eca

etc/policy.json

@@ -212,11 +212,18 @@
     "shared_scs": "field:servicechain_specs:shared=True",
     "shared_sp": "field:service_profiles:shared=True",
 
+    "auto_ptg": "field:policy_target_groups:is_auto_ptg=True",
+    "non_auto_ptg_shared": "rule:admin_or_owner or rule:shared_ptg",
+    "non_auto_ptg": "rule:non_auto_ptg_shared and not rule:auto_ptg",
+    "admin_auto_ptg_shared": "rule:admin_only or rule:shared_ptg",
+    "admin_auto_ptg": "rule:admin_auto_ptg_shared and rule:auto_ptg",
+
     "create_policy_target_group": "",
     "create_policy_target_group:shared": "rule:admin_only",
     "create_policy_target_group:service_management": "rule:admin_only",
     "create_policy_target_group:enforce_service_chains": "rule:admin_only",
-    "get_policy_target_group": "rule:admin_or_owner or rule:shared_ptg",
+    "get_policy_target_group": "rule:admin_auto_ptg or rule:non_auto_ptg",
+    "update_policy_target_group": "rule:admin_auto_ptg or rule:non_auto_ptg",
     "update_policy_target_group:shared": "rule:admin_only",
 
     "create_l2_policy": "",

gbpservice/neutron/db/grouppolicy/extensions/apic_auto_ptg_db.py

@@ -0,0 +1,37 @@
+#    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 neutron.db import model_base
+import sqlalchemy as sa
+
+
+class ApicAutoPtgDB(model_base.BASEV2):
+    __tablename__ = 'gp_apic_auto_ptg'
+    policy_target_group_id = sa.Column(
+        sa.String(36), sa.ForeignKey('gp_policy_target_groups.id',
+                                     ondelete='CASCADE'), primary_key=True)
+    is_auto_ptg = sa.Column(sa.Boolean, default=False, nullable=False)
+
+
+class ApicAutoPtgDBMixin(object):
+
+    def get_is_auto_ptg(self, session, policy_target_group_id):
+        row = (session.query(ApicAutoPtgDB).filter_by(
+               policy_target_group_id=policy_target_group_id).one())
+        return row['is_auto_ptg']
+
+    def set_is_auto_ptg(self, session, policy_target_group_id,
+                        is_auto_ptg=False):
+        with session.begin(subtransactions=True):
+            row = ApicAutoPtgDB(policy_target_group_id=policy_target_group_id,
+                                is_auto_ptg=is_auto_ptg)
+            session.add(row)

gbpservice/neutron/db/migration/alembic_migrations/versions/HEAD

@@ -1 +1 @@
-ef5a69e5bcc5
+ce662ded3ba5

gbpservice/neutron/db/migration/alembic_migrations/versions/ce662ded3ba5_is_auto_ptg.py

@@ -0,0 +1,45 @@
+#    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.
+
+"""is_auto_ptg
+
+Revision ID: ce662ded3ba5
+Revises: ef5a69e5bcc5
+Create Date: 2016-12-15 16:13:23.836874
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'ce662ded3ba5'
+down_revision = 'ef5a69e5bcc5'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+
+    op.create_table(
+        'gp_apic_auto_ptg',
+        sa.Column('policy_target_group_id', sa.String(length=36),
+                  nullable=False),
+        sa.Column('is_auto_ptg', sa.Boolean,
+                  server_default=sa.sql.false(), nullable=False),
+        sa.ForeignKeyConstraint(
+            ['policy_target_group_id'], ['gp_policy_target_groups.id'],
+            name='gp_apic_auto_ptg_fk_ptgid', ondelete='CASCADE'),
+        sa.PrimaryKeyConstraint('policy_target_group_id')
+    )
+
+
+def downgrade():
+    pass

gbpservice/neutron/extensions/cisco_apic_gbp.py

@@ -30,6 +30,10 @@ EXTENDED_ATTRIBUTES_2_0 = {
         'intra_ptg_allow': {
             'allow_post': True, 'allow_put': True, 'default': True,
             'convert_to': attr.convert_to_boolean, 'is_visible': True},
+        'is_auto_ptg': {
+            'allow_post': False, 'allow_put': False,
+            'convert_to': attr.convert_to_boolean, 'is_visible': True,
+            'enforce_policy': True},
     },
     gp.POLICY_RULES: {
         cisco_apic.DIST_NAMES: {

gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping.py

@@ -23,6 +23,7 @@ from neutron.common import constants as n_constants
 from neutron.common import exceptions as n_exc
 from neutron import context as n_context
 from neutron import manager
+from neutron import policy
 from oslo_concurrency import lockutils
 from oslo_config import cfg
 from oslo_log import helpers as log
@@ -616,11 +617,12 @@ class AIMMappingDriver(nrd.CommonNeutronBase, aim_rpc.AIMMappingRPCMixin):
 
     @log.log_method_call
     def create_policy_target_precommit(self, context):
+        ptg = self._get_policy_target_group(
+            context._plugin_context, context.current['policy_target_group_id'],
+            clean_session=False)
+        policy.enforce(context._plugin_context, 'get_policy_target_group',
+                       ptg, pluralized='policy_target_groups')
         if not context.current['port_id']:
-            ptg = self._db_plugin(
-                context._plugin).get_policy_target_group(
-                    context._plugin_context,
-                    context.current['policy_target_group_id'])
             subnets = self._get_subnets(
                 context._plugin_context, {'id': ptg['subnets']},
                 clean_session=False)

gbpservice/neutron/services/grouppolicy/drivers/extensions/aim_mapping_extension_driver.py

@@ -15,7 +15,9 @@ from neutron import manager as n_manager
 from oslo_log import log as logging
 
 from gbpservice.neutron.db.grouppolicy.extensions import (
-    apic_intra_ptg_db as db)
+    apic_auto_ptg_db as auto_ptg_db)
+from gbpservice.neutron.db.grouppolicy.extensions import (
+    apic_intra_ptg_db as intra_ptg_db)
 from gbpservice.neutron.db.grouppolicy import group_policy_db as gp_db
 from gbpservice.neutron.extensions import cisco_apic_gbp
 from gbpservice.neutron.extensions import group_policy as gpolicy
@@ -26,7 +28,8 @@ LOG = logging.getLogger(__name__)
 
 
 class AIMExtensionDriver(api.ExtensionDriver,
-                         db.ApicIntraPtgDBMixin):
+                         intra_ptg_db.ApicIntraPtgDBMixin,
+                         auto_ptg_db.ApicAutoPtgDBMixin):
     _supported_extension_alias = cisco_apic_gbp.ALIAS
     _extension_dict = cisco_apic_gbp.EXTENDED_ATTRIBUTES_2_0
 
@@ -71,12 +74,19 @@ class AIMExtensionDriver(api.ExtensionDriver,
 
     def process_create_policy_target_group(self, session, data, result):
         self._set_intra_ptg_allow(session, data, result)
+        result['is_auto_ptg'] = bool(
+            gpolicy.AUTO_PTG_REGEX.match(result['id']))
+        self.set_is_auto_ptg(
+            session, policy_target_group_id=result['id'],
+            is_auto_ptg=result['is_auto_ptg'])
 
     def process_update_policy_target_group(self, session, data, result):
         self._set_intra_ptg_allow(session, data, result)
 
     def extend_policy_target_group_dict(self, session, result):
         self._extend_ptg_dict_with_intra_ptg_allow(session, result)
+        result['is_auto_ptg'] = self.get_is_auto_ptg(
+            session, policy_target_group_id=result['id'])
         self._pd.extend_policy_target_group_dict(session, result)
 
     def extend_policy_rule_dict(self, session, result):

gbpservice/neutron/services/grouppolicy/policy_driver_manager.py

@@ -15,6 +15,7 @@ from neutron._i18n import _LI
 from neutron.common import exceptions as n_exc
 from oslo_config import cfg
 from oslo_log import log
+from oslo_policy import policy as oslo_policy
 from oslo_utils import excutils
 import stevedore
 
@@ -121,7 +122,8 @@ class PolicyDriverManager(stevedore.named.NamedExtensionManager):
         for driver in drivers:
             try:
                 getattr(driver.obj, method_name)(context)
-            except (gp_exc.GroupPolicyException, n_exc.NeutronException):
+            except (gp_exc.GroupPolicyException, n_exc.NeutronException,
+                    oslo_policy.PolicyNotAuthorized):
                 with excutils.save_and_reraise_exception():
                     LOG.exception(
                         _LE("Policy driver '%(name)s' failed in %(method)s"),

gbpservice/neutron/tests/etc/test-policy.json

@@ -20,10 +20,16 @@
     "shared_scn": "field:servicechain_nodes:shared=True",
     "shared_scs": "field:servicechain_specs:shared=True",
     "shared_sp": "field:service_profiles:shared=True",
+    "auto_ptg": "field:policy_target_groups:is_auto_ptg=True",
+    "non_auto_ptg_shared": "rule:admin_or_owner or rule:shared_ptg",
+    "non_auto_ptg": "rule:non_auto_ptg_shared and not rule:auto_ptg",
+    "admin_auto_ptg_shared": "rule:admin_only or rule:shared_ptg",
+    "admin_auto_ptg": "rule:admin_auto_ptg_shared and rule:auto_ptg",
 
     "create_policy_target_group": "",
     "create_policy_target_group:enforce_service_chains": "rule:admin_only",
-    "get_policy_target_group": "rule:admin_or_owner or rule:shared_ptg",
+    "get_policy_target_group": "rule:admin_auto_ptg or rule:non_auto_ptg",
+    "update_policy_target_group": "rule:admin_auto_ptg or rule:non_auto_ptg",
     "create_policy_target_group:service_management": "rule:admin_only",
 
     "create_l2_policy": "",

gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_mapping_driver.py

@@ -458,8 +458,14 @@ class AIMBaseTestCase(test_nr_base.CommonNeutronBaseTestCase,
         self._validate_router_interface_created()
 
         ptg_id = ptg['id']
-        ptg_show = self.show_policy_target_group(
-            ptg_id, expected_res_status=200)['policy_target_group']
+        if ptg['id'].startswith(aimd.AUTO_PTG_PREFIX):
+            # the test policy.json restricts auto-ptg access to admin
+            ptg_show = self.show_policy_target_group(
+                ptg_id, is_admin_context=True,
+                expected_res_status=200)['policy_target_group']
+        else:
+            ptg_show = self.show_policy_target_group(
+                ptg_id, expected_res_status=200)['policy_target_group']
         aim_epg_name = self.driver.apic_epg_name_for_policy_target_group(
             self._neutron_context.session, ptg_id)
         aim_tenant_name = str(self.name_mapper.tenant(
@@ -1142,8 +1148,10 @@ class TestL2PolicyWithAutoPTG(TestL2PolicyBase):
         self.assertEqual(aimd.AUTO_PTG_NAME_PREFIX % l2p_id, str(ptg['name']))
         self.assertEqual(shared, ptg['shared'])
         prs_lists = self._get_provided_consumed_prs_lists(shared)
+        # the test policy.json restricts auto-ptg access to admin
         ptg = self.update_policy_target_group(
-            ptg['id'], expected_res_status=webob.exc.HTTPOk.code,
+            ptg['id'], is_admin_context=True,
+            expected_res_status=webob.exc.HTTPOk.code,
             name='new name', description='something-else',
             provided_policy_rule_sets={prs_lists['provided']['id']:
                                        'scope'},
@@ -1151,8 +1159,9 @@ class TestL2PolicyWithAutoPTG(TestL2PolicyBase):
                                        'scope'})['policy_target_group']
         self._test_policy_target_group_aim_mappings(
             ptg, prs_lists, l2p)
+        # the test policy.json restricts auto-ptg access to admin
         self.update_policy_target_group(
-            ptg['id'], shared=(not shared),
+            ptg['id'], is_admin_context=True, shared=(not shared),
             expected_res_status=webob.exc.HTTPBadRequest.code)
         # Auto PTG cannot be deleted by user
         res = self.delete_policy_target_group(
@@ -1167,6 +1176,7 @@ class TestL2PolicyWithAutoPTG(TestL2PolicyBase):
         aim_epg_display_name = aim_epg.display_name
         ptg = self.update_policy_target_group(
             ptg['id'], expected_res_status=webob.exc.HTTPOk.code,
+            is_admin_context=True,
             name='new name', description='something-else',
             provided_policy_rule_sets={},
             consumed_policy_rule_sets={})['policy_target_group']
@@ -1245,9 +1255,11 @@ class TestL2PolicyWithAutoPTG(TestL2PolicyBase):
         self._test_epg_policy_enforcement_attr(ptg)
 
         auto_ptg_id = self.driver._get_auto_ptg_id(ptg['l2_policy_id'])
-        self.show_policy_target_group(
-            auto_ptg_id, expected_res_status=200)['policy_target_group']
-        self._test_epg_policy_enforcement_attr(ptg)
+        # the test policy.json restricts auto-ptg access to admin
+        auto_ptg = self.show_policy_target_group(
+            auto_ptg_id, is_admin_context=True,
+            expected_res_status=200)['policy_target_group']
+        self._test_epg_policy_enforcement_attr(auto_ptg)
 
         self.delete_policy_target_group(ptg_id, expected_res_status=204)
         self.show_policy_target_group(ptg_id, expected_res_status=404)
@@ -1259,6 +1271,45 @@ class TestL2PolicyWithAutoPTG(TestL2PolicyBase):
         self.assertEqual([], self._plugin.get_subnetpools(self._context))
         self.assertEqual([], self._l3_plugin.get_routers(self._context))
 
+    def test_auto_ptg_rbac(self):
+        ptg = self.create_policy_target_group()['policy_target_group']
+        # non-admin can create pt on non-auto-ptg
+        self.create_policy_target(policy_target_group_id=ptg['id'],
+                                  expected_res_status=201)
+        # admin can create pt on non-auto-ptg
+        self.create_policy_target(policy_target_group_id=ptg['id'],
+                                  is_admin_context=True,
+                                  expected_res_status=201)
+        # non-admin can retrieve and update non-auto-ptg
+        self.show_policy_target_group(ptg['id'], expected_res_status=200)
+        self.update_policy_target_group(
+            ptg['id'], expected_res_status=200, name='new_name')
+        # admin can retrieve and update non-auto-ptg
+        self.show_policy_target_group(ptg['id'], is_admin_context=True,
+                                      expected_res_status=200)
+        self.update_policy_target_group(
+            ptg['id'], is_admin_context=True, expected_res_status=200,
+            name='new_name')
+
+        auto_ptg_id = self.driver._get_auto_ptg_id(ptg['l2_policy_id'])
+        # non-admin cannot retrieve or update auto-ptg
+        self.show_policy_target_group(auto_ptg_id, expected_res_status=404)
+        self.update_policy_target_group(
+            auto_ptg_id, expected_res_status=403, name='new_name')
+        # admin can retrieve and update auto-ptg
+        self.show_policy_target_group(auto_ptg_id, is_admin_context=True,
+                                      expected_res_status=200)
+        self.update_policy_target_group(
+            auto_ptg_id, is_admin_context=True, expected_res_status=200,
+            name='new_name')
+        # admin can create pt on auto-ptg
+        self.create_policy_target(
+            policy_target_group_id=auto_ptg_id, is_admin_context=True,
+            expected_res_status=201)
+        # non-admin cannot create pt on auto-ptg
+        self.create_policy_target(policy_target_group_id=auto_ptg_id,
+                                  expected_res_status=403)
+
 
 class TestL2PolicyRollback(TestL2PolicyBase):