Merge "Fix issues with security group auths without ports."

4 files changed, 114 insertions, 0 deletions

diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py
index f4886551d..a93124e7a 100644
--- a/nova/api/ec2/cloud.py
+++ b/nova/api/ec2/cloud.py
@@ -555,6 +555,18 @@ class CloudController(object):
         else:
             values['cidr'] = '0.0.0.0/0'
 
+        if source_security_group_name:
+            # Open everything if an explicit port range or type/code are not
+            # specified, but only if a source group was specified.
+            ip_proto_upper = ip_protocol.upper() if ip_protocol else ''
+            if ip_proto_upper == 'ICMP' and not from_port and not to_port:
+                from_port = -1
+                to_port = -1
+            elif (ip_proto_upper in ['TCP', 'UDP'] and not from_port
+                  and not to_port):
+                from_port = 1
+                to_port = 65535
+
         if ip_protocol and from_port and to_port:
 
             ip_protocol = str(ip_protocol)
diff --git a/nova/api/openstack/compute/contrib/security_groups.py b/nova/api/openstack/compute/contrib/security_groups.py
index 9a4cdc7e8..bc4551ec7 100644
--- a/nova/api/openstack/compute/contrib/security_groups.py
+++ b/nova/api/openstack/compute/contrib/security_groups.py
@@ -436,6 +436,18 @@ class SecurityGroupRulesController(SecurityGroupControllerBase):
         else:
             values['cidr'] = '0.0.0.0/0'
 
+        if group_id:
+            # Open everything if an explicit port range or type/code are not
+            # specified, but only if a source group was specified.
+            ip_proto_upper = ip_protocol.upper() if ip_protocol else ''
+            if ip_proto_upper == 'ICMP' and not from_port and not to_port:
+                from_port = -1
+                to_port = -1
+            elif (ip_proto_upper in ['TCP', 'UDP'] and not from_port
+                  and not to_port):
+                from_port = 1
+                to_port = 65535
+
         if ip_protocol and from_port and to_port:
 
             ip_protocol = str(ip_protocol)
diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py
index 9cf777e7a..d8ff478b8 100644
--- a/nova/tests/api/ec2/test_cloud.py
+++ b/nova/tests/api/ec2/test_cloud.py
@@ -435,6 +435,60 @@ class CloudTestCase(test.TestCase):
         self.assertRaises(exception.EC2APIError, authz, self.context,
                           group_name=sec['name'], **kwargs)
 
+    def _test_authorize_security_group_no_ports_with_source_group(self, proto):
+        kwargs = {'project_id': self.context.project_id, 'name': 'test'}
+        sec = db.security_group_create(self.context, kwargs)
+
+        authz = self.cloud.authorize_security_group_ingress
+        auth_kwargs = {'ip_protocol': proto,
+                       'groups': {'1': {'user_id': self.context.user_id,
+                                        'group_name': u'test'}}}
+        self.assertTrue(authz(self.context, group_name=sec['name'],
+                        **auth_kwargs))
+
+        describe = self.cloud.describe_security_groups
+        groups = describe(self.context, group_name=['test'])
+        self.assertEquals(len(groups['securityGroupInfo']), 1)
+        actual_rules = groups['securityGroupInfo'][0]['ipPermissions']
+        expected_rules = [{'groups': [{'groupName': 'test',
+                                       'userId': self.context.user_id}],
+                           'ipProtocol': proto,
+                           'ipRanges': []}]
+        if proto == 'icmp':
+            expected_rules[0]['fromPort'] = -1
+            expected_rules[0]['toPort'] = -1
+        else:
+            expected_rules[0]['fromPort'] = 1
+            expected_rules[0]['toPort'] = 65535
+        self.assertTrue(expected_rules == actual_rules)
+        describe = self.cloud.describe_security_groups
+        groups = describe(self.context, group_name=['test'])
+
+        db.security_group_destroy(self.context, sec['id'])
+
+    def _test_authorize_security_group_no_ports_no_source_group(self, proto):
+        kwargs = {'project_id': self.context.project_id, 'name': 'test'}
+        sec = db.security_group_create(self.context, kwargs)
+
+        authz = self.cloud.authorize_security_group_ingress
+        auth_kwargs = {'ip_protocol': proto}
+        self.assertRaises(exception.EC2APIError, authz, self.context,
+                          group_name=sec['name'], **auth_kwargs)
+
+        db.security_group_destroy(self.context, sec['id'])
+
+    def test_authorize_security_group_no_ports_icmp(self):
+        self._test_authorize_security_group_no_ports_with_source_group('icmp')
+        self._test_authorize_security_group_no_ports_no_source_group('icmp')
+
+    def test_authorize_security_group_no_ports_tcp(self):
+        self._test_authorize_security_group_no_ports_with_source_group('tcp')
+        self._test_authorize_security_group_no_ports_no_source_group('tcp')
+
+    def test_authorize_security_group_no_ports_udp(self):
+        self._test_authorize_security_group_no_ports_with_source_group('udp')
+        self._test_authorize_security_group_no_ports_no_source_group('udp')
+
     def test_revoke_security_group_ingress_missing_group_name_or_id(self):
         kwargs = {'to_port': '999', 'from_port': '999', 'ip_protocol': 'tcp'}
         revoke = self.cloud.revoke_security_group_ingress
diff --git a/nova/tests/api/openstack/compute/contrib/test_security_groups.py b/nova/tests/api/openstack/compute/contrib/test_security_groups.py
index 782eb409b..f1d86c0e6 100644
--- a/nova/tests/api/openstack/compute/contrib/test_security_groups.py
+++ b/nova/tests/api/openstack/compute/contrib/test_security_groups.py
@@ -790,6 +790,42 @@ class TestSecurityGroupRules(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
                           req, {'security_group_rule': rule})
 
+    def _test_create_with_no_ports_and_no_group(self, proto):
+        rule = {'ip_protocol': proto, 'parent_group_id': '2'}
+
+        req = fakes.HTTPRequest.blank('/v2/fake/os-security-group-rules')
+        self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
+                          req, {'security_group_rule': rule})
+
+    def  _test_create_with_no_ports(self, proto):
+        rule = {'ip_protocol': proto, 'parent_group_id': '2', 'group_id': '1'}
+
+        req = fakes.HTTPRequest.blank('/v2/fake/os-security-group-rules')
+        res_dict = self.controller.create(req, {'security_group_rule': rule})
+
+        security_group_rule = res_dict['security_group_rule']
+        expected_rule = {
+            'from_port': 1, 'group': {'tenant_id': '123', 'name': 'test'},
+            'ip_protocol': proto, 'to_port': 65535, 'parent_group_id': 2,
+            'ip_range': {}, 'id': 1
+        }
+        if proto == 'icmp':
+            expected_rule['to_port'] = -1
+            expected_rule['from_port'] = -1
+        self.assertTrue(security_group_rule == expected_rule)
+
+    def test_create_with_no_ports_icmp(self):
+        self._test_create_with_no_ports_and_no_group('icmp')
+        self._test_create_with_no_ports('icmp')
+
+    def test_create_with_no_ports_tcp(self):
+        self._test_create_with_no_ports_and_no_group('tcp')
+        self._test_create_with_no_ports('tcp')
+
+    def test_create_with_no_ports_udp(self):
+        self._test_create_with_no_ports_and_no_group('udp')
+        self._test_create_with_no_ports('udp')
+
     def test_delete(self):
         rule = security_group_rule_template(id=10)