diff --git a/Authors b/Authors index 31ced6e8e31..6e8fecf2704 100644 --- a/Authors +++ b/Authors @@ -68,6 +68,7 @@ François Charlier Gabe Westmaas Gary Kotton Gaurav Gupta +Greg Althaus Hengqing Hu Hisaharu Ishii Hisaki Ohara diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index a95a0324fb7..8e187d61ad9 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -463,7 +463,7 @@ def _format_security_group(self, context, group): else: for protocol, min_port, max_port in (('icmp', -1, -1), ('tcp', 1, 65535), - ('udp', 1, 65536)): + ('udp', 1, 65535)): r['ipProtocol'] = protocol r['fromPort'] = min_port r['toPort'] = max_port @@ -559,15 +559,16 @@ def _rule_dict_last_step(self, context, to_port=None, from_port=None, # 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: + if (ip_proto_upper == 'ICMP' and + from_port is None and to_port is None): from_port = -1 to_port = -1 - elif (ip_proto_upper in ['TCP', 'UDP'] and not from_port - and not to_port): + elif (ip_proto_upper in ['TCP', 'UDP'] and from_port is None + and to_port is None): from_port = 1 to_port = 65535 - if ip_protocol and from_port and to_port: + if ip_protocol and from_port is not None and to_port is not None: ip_protocol = str(ip_protocol) try: @@ -587,7 +588,8 @@ def _rule_dict_last_step(self, context, to_port=None, from_port=None, # Verify that from_port must always be less than # or equal to to_port - if from_port > to_port: + if (ip_protocol.upper() in ['TCP', 'UDP'] and + (from_port > to_port)): raise exception.InvalidPortRange(from_port=from_port, to_port=to_port, msg="Former value cannot" " be greater than the later") @@ -601,7 +603,8 @@ def _rule_dict_last_step(self, context, to_port=None, from_port=None, # Verify ICMP type and code if (ip_protocol.upper() == "ICMP" and - (from_port < -1 or to_port > 255)): + (from_port < -1 or from_port > 255 or + to_port < -1 or to_port > 255)): raise exception.InvalidPortRange(from_port=from_port, to_port=to_port, msg="For ICMP, the" " type:code must be valid") diff --git a/nova/api/openstack/compute/contrib/security_groups.py b/nova/api/openstack/compute/contrib/security_groups.py index bc4551ec75f..3c7a0dcbe4f 100644 --- a/nova/api/openstack/compute/contrib/security_groups.py +++ b/nova/api/openstack/compute/contrib/security_groups.py @@ -440,15 +440,16 @@ def _rule_args_to_dict(self, context, to_port=None, from_port=None, # 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: + if (ip_proto_upper == 'ICMP' and + from_port is None and to_port is None): from_port = -1 to_port = -1 - elif (ip_proto_upper in ['TCP', 'UDP'] and not from_port - and not to_port): + elif (ip_proto_upper in ['TCP', 'UDP'] and from_port is None + and to_port is None): from_port = 1 to_port = 65535 - if ip_protocol and from_port and to_port: + if ip_protocol and from_port is not None and to_port is not None: ip_protocol = str(ip_protocol) try: @@ -467,7 +468,8 @@ def _rule_args_to_dict(self, context, to_port=None, from_port=None, # Verify that from_port must always be less than # or equal to to_port - if from_port > to_port: + if (ip_protocol.upper() in ['TCP', 'UDP'] and + from_port > to_port): raise exception.InvalidPortRange(from_port=from_port, to_port=to_port, msg="Former value cannot" " be greater than the later") @@ -481,7 +483,8 @@ def _rule_args_to_dict(self, context, to_port=None, from_port=None, # Verify ICMP type and code if (ip_protocol.upper() == "ICMP" and - (from_port < -1 or to_port > 255)): + (from_port < -1 or from_port > 255 or + to_port < -1 or to_port > 255)): raise exception.InvalidPortRange(from_port=from_port, to_port=to_port, msg="For ICMP, the" " type:code must be valid") diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py index fc6c656fe13..3c02cf29f70 100644 --- a/nova/tests/api/ec2/test_cloud.py +++ b/nova/tests/api/ec2/test_cloud.py @@ -382,7 +382,7 @@ def test_describe_security_group_ingress_groups(self): 'userId': u'someuser'}], 'ipProtocol': 'udp', 'ipRanges': [], - 'toPort': 65536}, + 'toPort': 65535}, {'fromPort': 80, 'groups': [{'groupName': u'othergroup2', 'userId': u'someuser'}], 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 f1d86c0e645..a9422cd26d3 100644 --- a/nova/tests/api/openstack/compute/contrib/test_security_groups.py +++ b/nova/tests/api/openstack/compute/contrib/test_security_groups.py @@ -826,6 +826,42 @@ 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_create_with_ports(self, id_val, proto, from_port, to_port): + rule = { + 'ip_protocol': proto, 'from_port': from_port, 'to_port': to_port, + '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': from_port, + 'group': {'tenant_id': '123', 'name': 'test'}, + 'ip_protocol': proto, 'to_port': to_port, 'parent_group_id': 2, + 'ip_range': {}, 'id': id_val + } + self.assertTrue(security_group_rule['ip_protocol'] == proto) + self.assertTrue(security_group_rule['id'] == id_val) + self.assertTrue(security_group_rule['from_port'] == from_port) + self.assertTrue(security_group_rule['to_port'] == to_port) + self.assertTrue(security_group_rule == expected_rule) + + def test_create_with_ports_icmp(self): + self._test_create_with_ports(1, 'icmp', 0, 1) + self._test_create_with_ports(2, 'icmp', 0, 0) + self._test_create_with_ports(3, 'icmp', 1, 0) + + def test_create_with_ports_tcp(self): + self._test_create_with_ports(1, 'tcp', 1, 1) + self._test_create_with_ports(2, 'tcp', 1, 65535) + self._test_create_with_ports(3, 'tcp', 65535, 65535) + + def test_create_with_ports_udp(self): + self._test_create_with_ports(1, 'udp', 1, 1) + self._test_create_with_ports(2, 'udp', 1, 65535) + self._test_create_with_ports(3, 'udp', 65535, 65535) + def test_delete(self): rule = security_group_rule_template(id=10) diff --git a/nova/tests/test_api.py b/nova/tests/test_api.py index 5735a7dc2ee..baaee98577c 100644 --- a/nova/tests/test_api.py +++ b/nova/tests/test_api.py @@ -394,6 +394,11 @@ def test_authorize_revoke_security_group_cidr(self): group.authorize('tcp', 80, 81, '0.0.0.0/0') group.authorize('icmp', -1, -1, '0.0.0.0/0') group.authorize('udp', 80, 81, '0.0.0.0/0') + group.authorize('tcp', 1, 65535, '0.0.0.0/0') + group.authorize('udp', 1, 65535, '0.0.0.0/0') + group.authorize('icmp', 1, 0, '0.0.0.0/0') + group.authorize('icmp', 0, 1, '0.0.0.0/0') + group.authorize('icmp', 0, 0, '0.0.0.0/0') def _assert(message, *args): try: @@ -414,10 +419,6 @@ def _assert(message, *args): _assert('Invalid port range', 'tcp', -1, 1, '0.0.0.0/0') # For tcp, valid port range 1-65535 _assert('Invalid port range', 'tcp', 1, 65599, '0.0.0.0/0') - # For icmp, only -1:-1 is allowed for type:code - _assert('An unknown error has occurred', 'icmp', -1, 0, '0.0.0.0/0') - # Non valid type:code - _assert('An unknown error has occurred', 'icmp', 0, 3, '0.0.0.0/0') # Invalid Cidr for ICMP type _assert('Invalid CIDR', 'icmp', -1, -1, '0.0.444.0/4') # Invalid protocol @@ -441,7 +442,7 @@ def _assert(message, *args): group = [grp for grp in rv if grp.name == security_group_name][0] - self.assertEquals(len(group.rules), 3) + self.assertEquals(len(group.rules), 8) self.assertEquals(int(group.rules[0].from_port), 80) self.assertEquals(int(group.rules[0].to_port), 81) self.assertEquals(len(group.rules[0].grants), 1) @@ -454,6 +455,11 @@ def _assert(message, *args): group.revoke('tcp', 80, 81, '0.0.0.0/0') group.revoke('icmp', -1, -1, '0.0.0.0/0') group.revoke('udp', 80, 81, '0.0.0.0/0') + group.revoke('tcp', 1, 65535, '0.0.0.0/0') + group.revoke('udp', 1, 65535, '0.0.0.0/0') + group.revoke('icmp', 1, 0, '0.0.0.0/0') + group.revoke('icmp', 0, 1, '0.0.0.0/0') + group.revoke('icmp', 0, 0, '0.0.0.0/0') self.expect_http() self.mox.ReplayAll()