From dbb7dd03fea68120ef5ac9bbb1b3f184e3f2eacc Mon Sep 17 00:00:00 2001
From: Andrew Laski <andrew.laski@rackspace.com>
Date: Wed, 9 Apr 2014 09:27:44 -0400
Subject: [PATCH] Add RBAC policy for ec2 API security groups calls

The revoke_security_group_ingress, revoke_security_group_ingress, and
delete_security_group calls in the ec2 API were not restricted by policy
checks.  This prevented a deployer from restricting their usage via
roles or other checks.  Checks have been added for these calls.

Based on commit d4056f8723cc6cefb28ff6e5a7c0df5ea77f82ef but modified
for the backport.

Closes-Bug: #1290537
Change-Id: I4bf681bedd68ed2216b429d34db735823e0a6189
---
 nova/api/ec2/cloud.py            | 10 ++++++++
 nova/tests/api/ec2/test_cloud.py | 44 ++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index 94ff160bc588..36c2f12af230 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -30,6 +30,7 @@ from oslo.config import cfg
 from nova.api.ec2 import ec2utils
 from nova.api.ec2 import inst_state
 from nova.api.metadata import password
+from nova.api.openstack import extensions
 from nova.api import validator
 from nova import availability_zones
 from nova import block_device
@@ -85,6 +86,9 @@ LOG = logging.getLogger(__name__)
 
 QUOTAS = quota.QUOTAS
 
+security_group_authorizer = extensions.extension_authorizer('compute',
+                                                            'security_groups')
+
 
 def validate_ec2_id(val):
     if not validator.validate_str()(val):
@@ -631,6 +635,8 @@ class CloudController(object):
         security_group = self.security_group_api.get(context, group_name,
                                                      group_id)
 
+        security_group_authorizer(context, security_group)
+
         prevalues = kwargs.get('ip_permissions', [kwargs])
 
         rule_ids = []
@@ -665,6 +671,8 @@ class CloudController(object):
         security_group = self.security_group_api.get(context, group_name,
                                                      group_id)
 
+        security_group_authorizer(context, security_group)
+
         prevalues = kwargs.get('ip_permissions', [kwargs])
         postvalues = []
         for values in prevalues:
@@ -737,6 +745,8 @@ class CloudController(object):
         security_group = self.security_group_api.get(context, group_name,
                                                      group_id)
 
+        security_group_authorizer(context, security_group)
+
         self.security_group_api.destroy(context, security_group)
 
         return True
diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py
index 269a7383d6bc..b28d194a8b9d 100644
--- a/nova/tests/api/ec2/test_cloud.py
+++ b/nova/tests/api/ec2/test_cloud.py
@@ -23,6 +23,7 @@ import copy
 import datetime
 import functools
 import iso8601
+import mock
 import os
 import string
 import tempfile
@@ -47,6 +48,7 @@ from nova.image import s3
 from nova.network import api as network_api
 from nova.network import neutronv2
 from nova.openstack.common import log as logging
+from nova.openstack.common import policy as common_policy
 from nova.openstack.common import timeutils
 from nova import test
 from nova.tests.api.openstack.compute.contrib import (
@@ -471,6 +473,34 @@ class CloudTestCase(test.TestCase):
         delete = self.cloud.delete_security_group
         self.assertRaises(exception.MissingParameter, delete, self.context)
 
+    def test_delete_security_group_policy_not_allowed(self):
+        rules = common_policy.Rules(
+                {'compute_extension:security_groups':
+                    common_policy.parse_rule('project_id:%(project_id)s')})
+        common_policy.set_rules(rules)
+
+        with mock.patch.object(self.cloud.security_group_api,
+                'get') as get:
+            get.return_value = {'project_id': 'invalid'}
+
+            self.assertRaises(exception.PolicyNotAuthorized,
+                    self.cloud.delete_security_group, self.context,
+                    'fake-name', 'fake-id')
+
+    def test_authorize_security_group_ingress_policy_not_allowed(self):
+        rules = common_policy.Rules(
+                {'compute_extension:security_groups':
+                    common_policy.parse_rule('project_id:%(project_id)s')})
+        common_policy.set_rules(rules)
+
+        with mock.patch.object(self.cloud.security_group_api,
+                'get') as get:
+            get.return_value = {'project_id': 'invalid'}
+
+            self.assertRaises(exception.PolicyNotAuthorized,
+                    self.cloud.authorize_security_group_ingress, self.context,
+                    'fake-name', 'fake-id')
+
     def test_authorize_security_group_ingress(self):
         kwargs = {'project_id': self.context.project_id, 'name': 'test'}
         sec = db.security_group_create(self.context, kwargs)
@@ -575,6 +605,20 @@ class CloudTestCase(test.TestCase):
         db.security_group_destroy(self.context, sec2['id'])
         db.security_group_destroy(self.context, sec1['id'])
 
+    def test_revoke_security_group_ingress_policy_not_allowed(self):
+        rules = common_policy.Rules(
+                {'compute_extension:security_groups':
+                    common_policy.parse_rule('project_id:%(project_id)s')})
+        common_policy.set_rules(rules)
+
+        with mock.patch.object(self.cloud.security_group_api,
+                'get') as get:
+            get.return_value = {'project_id': 'invalid'}
+
+            self.assertRaises(exception.PolicyNotAuthorized,
+                    self.cloud.revoke_security_group_ingress, self.context,
+                    'fake-name', 'fake-id')
+
     def test_revoke_security_group_ingress(self):
         kwargs = {'project_id': self.context.project_id, 'name': 'test'}
         sec = db.security_group_create(self.context, kwargs)