diff options
author | Jenkins <jenkins@review.openstack.org> | 2011-12-23 13:46:32 +0000 |
---|---|---|
committer | Gerrit Code Review <review@openstack.org> | 2011-12-23 13:46:32 +0000 |
commit | 03c36d0524e0442a3481f8d089e65e06b49d6fc4 (patch) | |
tree | 2542471a3529964e77e7de83d54ed5b287d43085 | |
parent | fedbce7d7139ca0c9fcd2de8103f2f9f7c1f0058 (diff) | |
parent | bcf241259246179035f20649f947b99b21d7978a (diff) | |
download | nova-03c36d0524e0442a3481f8d089e65e06b49d6fc4.tar.gz |
Merge "Verify security group parameters" into stable/diablo
-rw-r--r-- | nova/api/ec2/cloud.py | 44 | ||||
-rw-r--r-- | nova/api/openstack/contrib/security_groups.py | 42 | ||||
-rw-r--r-- | nova/exception.py | 2 | ||||
-rw-r--r-- | nova/tests/test_api.py | 48 | ||||
-rw-r--r-- | nova/utils.py | 21 |
5 files changed, 140 insertions, 17 deletions
diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 79d603c072..64fce0c2b5 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -23,7 +23,6 @@ datastore. """ import base64 -import netaddr import os import re import shutil @@ -692,22 +691,53 @@ class CloudController(object): elif cidr_ip: # If this fails, it throws an exception. This is what we want. cidr_ip = urllib.unquote(cidr_ip).decode() - netaddr.IPNetwork(cidr_ip) + + if not utils.is_valid_cidr(cidr_ip): + # Raise exception for non-valid address + raise exception.InvalidCidr(cidr=cidr_ip) + values['cidr'] = cidr_ip else: values['cidr'] = '0.0.0.0/0' if ip_protocol and from_port and to_port: - from_port = int(from_port) - to_port = int(to_port) + ip_protocol = str(ip_protocol) + try: + # Verify integer conversions + from_port = int(from_port) + to_port = int(to_port) + except ValueError: + if ip_protocol.upper() == 'ICMP': + raise exception.InvalidInput(reason="Type and" + " Code must be integers for ICMP protocol type") + else: + raise exception.InvalidInput(reason="To and From ports " + "must be integers") if ip_protocol.upper() not in ['TCP', 'UDP', 'ICMP']: raise exception.InvalidIpProtocol(protocol=ip_protocol) - if ((min(from_port, to_port) < -1) or - (max(from_port, to_port) > 65535)): + + # Verify that from_port must always be less than + # or equal to to_port + if from_port > to_port: + raise exception.InvalidPortRange(from_port=from_port, + to_port=to_port, msg="Former value cannot" + " be greater than the later") + + # Verify valid TCP, UDP port ranges + if (ip_protocol.upper() in ['TCP', 'UDP'] and + (from_port < 1 or to_port > 65535)): + raise exception.InvalidPortRange(from_port=from_port, + to_port=to_port, msg="Valid TCP ports should" + " be between 1-65535") + + # Verify ICMP type and code + if (ip_protocol.upper() == "ICMP" and + (from_port < -1 or to_port > 255)): raise exception.InvalidPortRange(from_port=from_port, - to_port=to_port) + to_port=to_port, msg="For ICMP, the" + " type:code must be valid") values['protocol'] = ip_protocol values['from_port'] = from_port diff --git a/nova/api/openstack/contrib/security_groups.py b/nova/api/openstack/contrib/security_groups.py index 1fd64f3b8e..e8f1f2ca68 100644 --- a/nova/api/openstack/contrib/security_groups.py +++ b/nova/api/openstack/contrib/security_groups.py @@ -15,7 +15,6 @@ """The security groups extension.""" -import netaddr import urllib from webob import exc import webob @@ -26,6 +25,7 @@ from nova import exception from nova import flags from nova import log as logging from nova import rpc +from nova import utils from nova.api.openstack import common from nova.api.openstack import extensions from nova.api.openstack import wsgi @@ -270,28 +270,54 @@ class SecurityGroupRulesController(SecurityGroupController): # If this fails, it throws an exception. This is what we want. try: cidr = urllib.unquote(cidr).decode() - netaddr.IPNetwork(cidr) except Exception: raise exception.InvalidCidr(cidr=cidr) + + if not utils.is_valid_cidr(cidr): + # Raise exception for non-valid address + raise exception.InvalidCidr(cidr=cidr) + values['cidr'] = cidr else: values['cidr'] = '0.0.0.0/0' if ip_protocol and from_port and to_port: + ip_protocol = str(ip_protocol) try: from_port = int(from_port) to_port = int(to_port) except ValueError: - raise exception.InvalidPortRange(from_port=from_port, - to_port=to_port) - ip_protocol = str(ip_protocol) + if ip_protocol.upper() == 'ICMP': + raise exception.InvalidInput(reason="Type and" + " Code must be integers for ICMP protocol type") + else: + raise exception.InvalidInput(reason="To and From ports " + "must be integers") + if ip_protocol.upper() not in ['TCP', 'UDP', 'ICMP']: raise exception.InvalidIpProtocol(protocol=ip_protocol) - if ((min(from_port, to_port) < -1) or - (max(from_port, to_port) > 65535)): + + # Verify that from_port must always be less than + # or equal to to_port + if from_port > to_port: + raise exception.InvalidPortRange(from_port=from_port, + to_port=to_port, msg="Former value cannot" + " be greater than the later") + + # Verify valid TCP, UDP port ranges + if (ip_protocol.upper() in ['TCP', 'UDP'] and + (from_port < 1 or to_port > 65535)): + raise exception.InvalidPortRange(from_port=from_port, + to_port=to_port, msg="Valid TCP ports should" + " be between 1-65535") + + # Verify ICMP type and code + if (ip_protocol.upper() == "ICMP" and + (from_port < -1 or to_port > 255)): raise exception.InvalidPortRange(from_port=from_port, - to_port=to_port) + to_port=to_port, msg="For ICMP, the" + " type:code must be valid") values['protocol'] = ip_protocol values['from_port'] = from_port diff --git a/nova/exception.py b/nova/exception.py index 7ccf12a55d..c2a98eb709 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -210,7 +210,7 @@ class InvalidVolumeType(Invalid): class InvalidPortRange(Invalid): - message = _("Invalid port range %(from_port)s:%(to_port)s.") + message = _("Invalid port range %(from_port)s:%(to_port)s. %(msg)s") class InvalidIpProtocol(Invalid): diff --git a/nova/tests/test_api.py b/nova/tests/test_api.py index e9f1145dde..2d3d4b6049 100644 --- a/nova/tests/test_api.py +++ b/nova/tests/test_api.py @@ -386,6 +386,50 @@ class ApiEc2TestCase(test.TestCase): group.connection = self.ec2 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') + # Invalid CIDR address + self.assertRaises(Exception, + group.authorize, 'tcp', 80, 81, '0.0.0.0/0444') + # Missing ports + self.assertRaises(Exception, + group.authorize, 'tcp', '0.0.0.0/0') + # from port cannot be greater than to port + self.assertRaises(Exception, + group.authorize, 'tcp', 100, 1, '0.0.0.0/0') + # For tcp, negative values are not allowed + self.assertRaises(Exception, + group.authorize, 'tcp', -1, 1, '0.0.0.0/0') + # For tcp, valid port range 1-65535 + self.assertRaises(Exception, + group.authorize, 'tcp', 1, 65599, '0.0.0.0/0') + # For icmp, only -1:-1 is allowed for type:code + self.assertRaises(Exception, + group.authorize, 'icmp', -1, 0, '0.0.0.0/0') + # Non valid type:code + self.assertRaises(Exception, + group.authorize, 'icmp', 0, 3, '0.0.0.0/0') + # Invalid Cidr for ICMP type + self.assertRaises(Exception, + group.authorize, 'icmp', -1, -1, '0.0.444.0/4') + # Invalid protocol + self.assertRaises(Exception, + group.authorize, 'xyz', 1, 14, '0.0.0.0/0') + # Invalid port + self.assertRaises(Exception, + group.authorize, 'tcp', " ", "81", '0.0.0.0/0') + # Invalid icmp port + self.assertRaises(Exception, + group.authorize, 'icmp', " ", "81", '0.0.0.0/0') + # Invalid CIDR Address + self.assertRaises(Exception, + group.authorize, 'icmp', -1, -1, '0.0.0.0') + # Invalid CIDR Address + self.assertRaises(Exception, + group.authorize, 'icmp', -1, -1, '0.0.0.0/') + # Invalid Cidr ports + self.assertRaises(Exception, + group.authorize, 'icmp', 1, 256, '0.0.0.0/0') self.expect_http() self.mox.ReplayAll() @@ -394,7 +438,7 @@ class ApiEc2TestCase(test.TestCase): group = [grp for grp in rv if grp.name == security_group_name][0] - self.assertEquals(len(group.rules), 1) + self.assertEquals(len(group.rules), 3) 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) @@ -405,6 +449,8 @@ class ApiEc2TestCase(test.TestCase): group.connection = self.ec2 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') self.expect_http() self.mox.ReplayAll() diff --git a/nova/utils.py b/nova/utils.py index 4648f5573b..696e0603dc 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -37,6 +37,7 @@ import time import types import uuid import pyclbr +import netaddr from xml.sax import saxutils from eventlet import event @@ -864,6 +865,26 @@ def is_valid_ipv4(address): return True +def is_valid_cidr(address): + """Check if the provided ipv4 or ipv6 address is a valid + CIDR address or not""" + try: + # Validate the correct CIDR Address + netaddr.IPNetwork(address) + except netaddr.core.AddrFormatError: + return False + + # Prior validation partially verify /xx part + # Verify it here + ip_segment = address.split('/') + + if (len(ip_segment) <= 1 or + ip_segment[1] == ''): + return False + + return True + + def monkey_patch(): """ If the Flags.monkey_patch set as True, this functuion patches a decorator |