From 1a12349c056b52b488591abb1671ad94a6db6526 Mon Sep 17 00:00:00 2001 From: Ahmad Hassan Date: Fri, 30 Sep 2011 15:10:33 +0100 Subject: Verify security group parameters Introduced various sanity checks before adding security group rule into the database. The checks have been implemented both in EC2 and openstack extension code. Implemented the suggestions made in first patch by Brian Fixed the unit tests in security groups Fixed pep8 issues in security group unit tests Fixes bug 869979. Change-Id: I2ac28666e90e7bdeacb7b1c2676c0719cfb9e441 --- nova/api/ec2/cloud.py | 44 ++++++++++++++++++++++----- nova/api/openstack/contrib/security_groups.py | 42 ++++++++++++++++++++----- 2 files changed, 71 insertions(+), 15 deletions(-) (limited to 'nova/api') diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 393df2870..dcf185cf1 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 @@ -727,22 +726,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 9072a34ee..bb4cd48b2 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 -- cgit