Merge "Disallow attach to public network"

etc/zun/policy.json

@@ -53,4 +53,6 @@
     "capsule:get_one_all_tenants": "rule:admin_api",
     "capsule:get_all": "rule:default",
     "capsule:get_all_all_tenants": "rule:admin_api",
+
+    "network:attach_external_network": "rule:context_is_admin"
 }

zun/api/controllers/v1/containers.py

@@ -39,6 +39,7 @@ from zun import objects
 
 CONF = zun.conf.CONF
 LOG = logging.getLogger(__name__)
+NETWORK_ATTACH_EXTERNAL = 'network:attach_external_network'
 
 
 def _get_container(container_id):
@@ -316,6 +317,15 @@ class ContainersController(base.Controller):
         pecan.response.status = 202
         return view.format_container(pecan.request.host_url, new_container)
 
+    def _check_external_network_attach(self, context, nets):
+        """Check if attaching to external network is permitted."""
+        if not context.can(NETWORK_ATTACH_EXTERNAL,
+                           fatal=False):
+            for net in nets:
+                if net.get('router:external') and not net.get('shared'):
+                    raise exception.ExternalNetworkAttachForbidden(
+                        network_uuid=net['network'])
+
     def _build_requested_networks(self, context, nets):
         neutron_api = neutron.NeutronAPI(context)
         requested_networks = []
@@ -323,14 +333,21 @@ class ContainersController(base.Controller):
             if net.get('port'):
                 port = neutron_api.get_neutron_port(net['port'])
                 neutron_api.ensure_neutron_port_usable(port)
+                network = neutron_api.get_neutron_network(port['network_id'])
                 requested_networks.append({'network': port['network_id'],
                                            'port': port['id'],
+                                           'router:external':
+                                               network.get('router:external'),
+                                           'shared': network.get('shared'),
                                            'v4-fixed-ip': '',
                                            'v6-fixed-ip': ''})
             elif net.get('network'):
                 network = neutron_api.get_neutron_network(net['network'])
                 requested_networks.append({'network': network['id'],
                                            'port': '',
+                                           'router:external':
+                                               network.get('router:external'),
+                                           'shared': network.get('shared'),
                                            'v4-fixed-ip': '',
                                            'v6-fixed-ip': ''})
 
@@ -343,6 +360,7 @@ class ContainersController(base.Controller):
                                        'v4-fixed-ip': '',
                                        'v6-fixed-ip': ''})
 
+        self._check_external_network_attach(context, requested_networks)
         return requested_networks
 
     def _check_security_group(self, context, security_group, container):

zun/common/context.py

@@ -14,6 +14,7 @@ import copy
 from eventlet.green import threading
 from oslo_context import context
 
+from zun.common import exception
 from zun.common import policy
 
 
@@ -104,6 +105,35 @@ class RequestContext(context.RequestContext):
 
         return context
 
+    def can(self, action, target=None, fatal=True):
+        """Verifies that the given action is valid on the target in this context.
+
+        :param action: string representing the action to be checked.
+        :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}``.
+            If None, then this default target will be considered:
+            {'project_id': self.project_id, 'user_id': self.user_id}
+        :param fatal: if False, will return False when an
+            exception.NotAuthorized occurs.
+
+        :raises zun.common.exception.NotAuthorized: if verification fails and
+            fatal is True.
+
+        :return: returns a non-False value (not necessarily "True") if
+            authorized and False if not authorized and fatal is False.
+        """
+        if target is None:
+            target = {'project_id': self.project_id,
+                      'user_id': self.user_id}
+
+        try:
+            return policy.authorize(self, action, target)
+        except exception.NotAuthorized:
+            if fatal:
+                raise
+            return False
+
 
 def make_context(*args, **kwargs):
     return RequestContext(*args, **kwargs)

zun/common/exception.py

@@ -558,3 +558,8 @@ class CapsuleNotFound(HTTPNotFound):
 
 class InvalidCapsuleTemplate(ZunException):
     message = _("Invalid capsule template: %(reason)s.")
+
+
+class ExternalNetworkAttachForbidden(NotAuthorized):
+    message = _("It is not allowed to create an interface on "
+                "external network %(network_uuid)s")

zun/common/policy.py

@@ -15,13 +15,16 @@
 
 """Policy Engine For zun."""
 
+from oslo_log import log as logging
 from oslo_policy import policy
+from oslo_utils import excutils
 
 from zun.common import exception
 import zun.conf
 
 _ENFORCER = None
 CONF = zun.conf.CONF
+LOG = logging.getLogger(__name__)
 
 
 # we can get a policy enforcer by this init.
@@ -93,6 +96,46 @@ def enforce(context, rule=None, target=None,
                             do_raise=do_raise, exc=exc, *args, **kwargs)
 
 
+def authorize(context, action, target, do_raise=True, exc=None):
+    """Verifies that the action is valid on the target in this context.
+
+       :param context: zun context
+       :param action: string representing the action to be checked
+           this should be colon separated for clarity.
+           i.e. ``network:attach_external_network``
+       :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:`authorize` (both
+                   positional and keyword arguments) will be passed to
+                   the exception class. If not specified,
+                   :class:`PolicyNotAuthorized` will be used.
+
+       :raises zun.common.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.
+    """
+    credentials = context.to_policy_values()
+    if not exc:
+        exc = exception.PolicyNotAuthorized
+    try:
+        result = _ENFORCER.enforce(action, target, credentials,
+                                   do_raise=do_raise, exc=exc, action=action)
+    except Exception:
+        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 user is admin according to policy setting.
 

zun/tests/unit/api/controllers/v1/test_containers.py

@@ -577,6 +577,7 @@ class TestContainerController(api_base.FunctionalTest):
         self.assertEqual(1, len(requested_networks))
         self.assertEqual(fake_network['id'], requested_networks[0]['network'])
 
+    @patch('zun.network.neutron.NeutronAPI.get_neutron_network')
     @patch('zun.network.neutron.NeutronAPI.get_neutron_port')
     @patch('zun.network.neutron.NeutronAPI.ensure_neutron_port_usable')
     @patch('zun.compute.api.API.container_show')
@@ -585,10 +586,13 @@ class TestContainerController(api_base.FunctionalTest):
     @patch('zun.compute.api.API.image_search')
     def test_create_container_with_requested_neutron_port(
             self, mock_search, mock_container_delete, mock_container_create,
-            mock_container_show, mock_ensure_port_usable, mock_get_port):
+            mock_container_show, mock_ensure_port_usable, mock_get_port,
+            mock_get_network):
         mock_container_create.side_effect = lambda x, y, z, v: y
         fake_port = {'network_id': 'foo', 'id': 'bar'}
+        fake_private_network = {'router:external': False, 'shared': False}
         mock_get_port.return_value = fake_port
+        mock_get_network.return_value = fake_private_network
         # Create a container with a command
         params = ('{"name": "MyDocker", "image": "ubuntu",'
                   '"command": "env", "memory": "512",'
@@ -636,6 +640,44 @@ class TestContainerController(api_base.FunctionalTest):
         self.assertEqual(0, len(c))
         self.assertTrue(mock_container_create.called)
 
+    @patch('zun.compute.api.API.container_create')
+    @patch('zun.common.context.RequestContext.can')
+    @patch('zun.network.neutron.NeutronAPI.get_neutron_network')
+    @patch('zun.network.neutron.NeutronAPI.ensure_neutron_port_usable')
+    @patch('zun.compute.api.API.image_search')
+    def test_create_container_with_public_network(
+            self, mock_search, mock_ensure_port_usable, mock_get_network,
+            mock_authorize, mock_container_create):
+        fake_public_network = {'id': 'fakepubnetid',
+                               'router:external': True,
+                               'shared': False}
+        mock_get_network.return_value = fake_public_network
+        # Create a container with a command
+        params = ('{"name": "MyDocker", "image": "ubuntu",'
+                  '"command": "env", "memory": "512",'
+                  '"environment": {"key1": "val1", "key2": "val2"},'
+                  '"nets": [{"network": "testpublicnet"}]}')
+        headers = {'OpenStack-API-Version': CURRENT_VERSION}
+        response = self.app.post('/v1/containers/',
+                                 params=params, headers=headers,
+                                 content_type='application/json')
+        fake_admin_authorize = True
+        mock_authorize.return_value = fake_admin_authorize
+        self.assertEqual(202, response.status_int)
+
+        fake_not_admin_authorize = False
+        mock_authorize.return_value = fake_not_admin_authorize
+        response = self.app.post('/v1/containers/',
+                                 params=params, headers=headers,
+                                 content_type='application/json',
+                                 expect_errors=True)
+        self.assertEqual(403, response.status_int)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(
+            "It is not allowed to create an interface on external network %s" %
+            fake_public_network['id'], response.json['errors'][0]['detail'])
+        self.assertTrue(mock_container_create.not_called)
+
     @patch('zun.network.neutron.NeutronAPI.get_available_network')
     @patch('zun.compute.api.API.container_show')
     @patch('zun.compute.api.API.container_create')