summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJenkins <jenkins@review.openstack.org>2011-12-23 13:46:32 +0000
committerGerrit Code Review <review@openstack.org>2011-12-23 13:46:32 +0000
commit03c36d0524e0442a3481f8d089e65e06b49d6fc4 (patch)
tree2542471a3529964e77e7de83d54ed5b287d43085
parentfedbce7d7139ca0c9fcd2de8103f2f9f7c1f0058 (diff)
parentbcf241259246179035f20649f947b99b21d7978a (diff)
downloadnova-03c36d0524e0442a3481f8d089e65e06b49d6fc4.tar.gz
Merge "Verify security group parameters" into stable/diablo
-rw-r--r--nova/api/ec2/cloud.py44
-rw-r--r--nova/api/openstack/contrib/security_groups.py42
-rw-r--r--nova/exception.py2
-rw-r--r--nova/tests/test_api.py48
-rw-r--r--nova/utils.py21
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