Skip to content

Commit

Permalink
Ensure that HTTP 400 codes are returned for invalid input
Browse files Browse the repository at this point in the history
Fixes bug 1062046

A number of additional validation functions were added. They
do basic validations.

Change-Id: I0fc207e996f11b50fdaa4d80075ae5704cba7003
  • Loading branch information
Gary Kotton committed Nov 10, 2012
1 parent 4546a18 commit 49f649c
Show file tree
Hide file tree
Showing 6 changed files with 461 additions and 41 deletions.
192 changes: 177 additions & 15 deletions quantum/api/v2/attributes.py
Expand Up @@ -35,7 +35,7 @@ def _validate_boolean(data, valid_values=None):
if data in [True, False]:
return
else:
msg = _("%s is not boolean") % data
msg = _("'%s' is not boolean") % data
LOG.debug("validate_boolean: %s", msg)
return msg

Expand All @@ -50,6 +50,19 @@ def _validate_values(data, valid_values=None):
return msg


def _validate_string(data, max_len=None):
if not isinstance(data, basestring):
msg = _("'%s' is not a valid string") % data
LOG.debug("validate_string: %s", msg)
return msg
if max_len is not None:
if len(data) > max_len:
msg = _("'%(data)s' exceeds maximum length of "
"%(max_len)s.") % locals()
LOG.debug("validate_string: %s", msg)
return msg


def _validate_range(data, valid_values=None):
min_value = valid_values[0]
max_value = valid_values[1]
Expand All @@ -68,7 +81,7 @@ def _validate_mac_address(data, valid_values=None):
netaddr.EUI(data)
return
except Exception:
msg = _("%s is not a valid MAC address") % data
msg = _("'%s' is not a valid MAC address") % data
LOG.debug("validate_mac_address: %s", msg)
return msg

Expand All @@ -78,11 +91,131 @@ def _validate_ip_address(data, valid_values=None):
netaddr.IPAddress(data)
return
except Exception:
msg = _("%s is not a valid IP address") % data
msg = _("'%s' is not a valid IP address") % data
LOG.debug("validate_ip_address: %s", msg)
return msg


def _validate_ip_pools(data, valid_values=None):
"""Validate that start and end IP addresses are present
In addition to this the IP addresses will also be validated"""
if not isinstance(data, list):
msg = _("'%s' in not a valid IP pool") % data
LOG.debug("validate_ip_pools: %s", msg)
return msg

expected_keys = set(['start', 'end'])
try:
for ip_pool in data:
if set(ip_pool.keys()) != expected_keys:
msg = _("Expected keys not found. Expected: %s "
"Provided: %s") % (expected_keys, ip_pool.keys())
LOG.debug("validate_ip_pools: %s", msg)
return msg
for k in expected_keys:
msg = _validate_ip_address(ip_pool[k])
if msg:
LOG.debug("validate_ip_pools: %s", msg)
return msg
except KeyError, e:
args = {'key_name': e.message, 'ip_pool': ip_pool}
msg = _("Invalid input. Required key: '%(key_name)s' "
"missing from %(ip_pool)s.") % args
LOG.debug("validate_ip_pools: %s", msg)
return msg
except TypeError, e:
msg = _("Invalid input. Pool %s must be a dictionary.") % ip_pool
LOG.debug("validate_ip_pools: %s", msg)
return msg
except Exception:
msg = _("'%s' in not a valid IP pool") % data
LOG.debug("validate_ip_pools: %s", msg)
return msg


def _validate_fixed_ips(data, valid_values=None):
if not isinstance(data, list):
msg = _("'%s' in not a valid fixed IP") % data
LOG.debug("validate_fixed_ips: %s", msg)
return msg
ips = []
try:
for fixed_ip in data:
if 'ip_address' in fixed_ip:
msg = _validate_ip_address(fixed_ip['ip_address'])
if msg:
LOG.debug("validate_fixed_ips: %s", msg)
return msg
if 'subnet_id' in fixed_ip:
msg = _validate_regex(fixed_ip['subnet_id'], UUID_PATTERN)
if msg:
LOG.debug("validate_fixed_ips: %s", msg)
return msg
# Ensure that duplicate entries are not set - just checking IP
# suffices. Duplicate subnet_id's are legitimate.
if 'ip_address' in fixed_ip:
if fixed_ip['ip_address'] in ips:
msg = _("Duplicate entry %s") % fixed_ip
LOG.debug("validate_fixed_ips: %s", msg)
return msg
ips.append(fixed_ip['ip_address'])
except Exception:
msg = _("'%s' in not a valid fixed IP") % data
LOG.debug("validate_fixed_ips: %s", msg)
return msg


def _validate_nameservers(data, valid_values=None):
if not hasattr(data, '__iter__'):
msg = _("'%s' in not a valid nameserver") % data
LOG.debug("validate_nameservers: %s", msg)
return msg
ips = set()
for ip in data:
msg = _validate_ip_address(ip)
if msg:
# This may be a hostname
msg = _validate_regex(ip, HOSTNAME_PATTERN)
if msg:
msg = _("'%s' in not a valid nameserver") % ip
LOG.debug("validate_nameservers: %s", msg)
return msg
if ip in ips:
msg = _("Duplicate nameserver %s") % ip
LOG.debug("validate_nameservers: %s", msg)
return msg
ips.add(ip)


def _validate_hostroutes(data, valid_values=None):
if not isinstance(data, list):
msg = _("'%s' in not a valid hostroute") % data
LOG.debug("validate_hostroutes: %s", msg)
return msg
hostroutes = []
try:
for hostroute in data:
msg = _validate_subnet(hostroute['destination'])
if msg:
LOG.debug("validate_hostroutes: %s", msg)
return msg
msg = _validate_ip_address(hostroute['nexthop'])
if msg:
LOG.debug("validate_hostroutes: %s", msg)
return msg
if hostroute in hostroutes:
msg = _("Duplicate hostroute %s") % hostroute
LOG.debug("validate_hostroutes: %s", msg)
if msg:
return msg
hostroutes.append(hostroute)
except:
msg = _("'%s' in not a valid hostroute") % data
LOG.debug("validate_hostroutes: %s", msg)
return msg


def _validate_ip_address_or_none(data, valid_values=None):
if data is None:
return None
Expand All @@ -97,19 +230,21 @@ def _validate_subnet(data, valid_values=None):
except Exception:
pass

msg = _("%s is not a valid IP subnet") % data
msg = _("'%s' is not a valid IP subnet") % data
LOG.debug("validate_subnet: %s", msg)
return msg


def _validate_regex(data, valid_values=None):
match = re.match(valid_values, data)
if match:
return
else:
msg = _("%s is not valid") % data
LOG.debug("validate_regex: %s", msg)
return msg
try:
if re.match(valid_values, data):
return
except TypeError:
pass

msg = _("'%s' is not valid input") % data
LOG.debug("validate_regex: %s", msg)
return msg


def convert_to_boolean(data):
Expand All @@ -121,15 +256,23 @@ def convert_to_boolean(data):
return True
else:
return False
except ValueError, TypeError:
except (ValueError, TypeError):
if (data == "True" or data == "true"):
return True
if (data == "False" or data == "false"):
return False
msg = _("%s is not boolean") % data
msg = _("'%s' is not boolean") % data
raise q_exc.InvalidInput(error_message=msg)


def convert_to_int(data):
try:
return int(data)
except (ValueError, TypeError):
msg = _("'%s' is not a integer") % data
raise q_exc.InvalidInput(error_message=msg)


def convert_kvp_str_to_list(data):
"""Convert a value of the form 'key=value' to ['key', 'value'].
Expand Down Expand Up @@ -160,6 +303,8 @@ def convert_kvp_list_to_dict(kvp_list):
kvp_map[key].add(value)
return dict((x, list(y)) for x, y in kvp_map.iteritems())

HOSTNAME_PATTERN = ("(?=^.{1,254}$)(^(?:(?!\d+\.|-)[a-zA-Z0-9_\-]"
"{1,63}(?<!-)\.?)+(?:[a-zA-Z]{2,})$)")

HEX_ELEM = '[0-9A-Fa-f]'
UUID_PATTERN = '-'.join([HEX_ELEM + '{8}', HEX_ELEM + '{4}',
Expand All @@ -172,12 +317,17 @@ def convert_kvp_list_to_dict(kvp_list):
# Dictionary that maintains a list of validation functions
validators = {'type:boolean': _validate_boolean,
'type:values': _validate_values,
'type:string': _validate_string,
'type:range': _validate_range,
'type:mac_address': _validate_mac_address,
'type:fixed_ips': _validate_fixed_ips,
'type:ip_address': _validate_ip_address,
'type:ip_address_or_none': _validate_ip_address_or_none,
'type:subnet': _validate_subnet,
'type:regex': _validate_regex}
'type:regex': _validate_regex,
'type:ip_pools': _validate_ip_pools,
'type:hostroutes': _validate_hostroutes,
'type:nameservers': _validate_nameservers}

# Note: a default of ATTR_NOT_SPECIFIED indicates that an
# attribute is not required, but will be generated by the plugin
Expand Down Expand Up @@ -208,6 +358,7 @@ def convert_kvp_list_to_dict(kvp_list):
'validate': {'type:regex': UUID_PATTERN},
'is_visible': True},
'name': {'allow_post': True, 'allow_put': True,
'validate': {'type:string': None},
'default': '', 'is_visible': True},
'subnets': {'allow_post': False, 'allow_put': False,
'default': [],
Expand All @@ -220,6 +371,7 @@ def convert_kvp_list_to_dict(kvp_list):
'status': {'allow_post': False, 'allow_put': False,
'is_visible': True},
'tenant_id': {'allow_post': True, 'allow_put': False,
'validate': {'type:string': None},
'required_by_policy': True,
'is_visible': True},
SHARED: {'allow_post': True,
Expand All @@ -236,6 +388,7 @@ def convert_kvp_list_to_dict(kvp_list):
'validate': {'type:regex': UUID_PATTERN},
'is_visible': True},
'name': {'allow_post': True, 'allow_put': True, 'default': '',
'validate': {'type:string': None},
'is_visible': True},
'network_id': {'allow_post': True, 'allow_put': False,
'required_by_policy': True,
Expand All @@ -254,15 +407,19 @@ def convert_kvp_list_to_dict(kvp_list):
'fixed_ips': {'allow_post': True, 'allow_put': True,
'default': ATTR_NOT_SPECIFIED,
'convert_list_to': convert_kvp_list_to_dict,
'validate': {'type:fixed_ips': None},
'enforce_policy': True,
'is_visible': True},
'device_id': {'allow_post': True, 'allow_put': True,
'validate': {'type:string': None},
'default': '',
'is_visible': True},
'device_owner': {'allow_post': True, 'allow_put': True,
'validate': {'type:string': None},
'default': '',
'is_visible': True},
'tenant_id': {'allow_post': True, 'allow_put': False,
'validate': {'type:string': None},
'required_by_policy': True,
'is_visible': True},
'status': {'allow_post': False, 'allow_put': False,
Expand All @@ -273,9 +430,10 @@ def convert_kvp_list_to_dict(kvp_list):
'validate': {'type:regex': UUID_PATTERN},
'is_visible': True},
'name': {'allow_post': True, 'allow_put': True, 'default': '',
'validate': {'type:string': None},
'is_visible': True},
'ip_version': {'allow_post': True, 'allow_put': False,
'convert_to': int,
'convert_to': convert_to_int,
'validate': {'type:values': [4, 6]},
'is_visible': True},
'network_id': {'allow_post': True, 'allow_put': False,
Expand All @@ -292,14 +450,18 @@ def convert_kvp_list_to_dict(kvp_list):
#TODO(salvatore-orlando): Enable PUT on allocation_pools
'allocation_pools': {'allow_post': True, 'allow_put': False,
'default': ATTR_NOT_SPECIFIED,
'validate': {'type:ip_pools': None},
'is_visible': True},
'dns_nameservers': {'allow_post': True, 'allow_put': True,
'default': ATTR_NOT_SPECIFIED,
'validate': {'type:nameservers': None},
'is_visible': True},
'host_routes': {'allow_post': True, 'allow_put': True,
'default': ATTR_NOT_SPECIFIED,
'validate': {'type:hostroutes': None},
'is_visible': True},
'tenant_id': {'allow_post': True, 'allow_put': False,
'validate': {'type:string': None},
'required_by_policy': True,
'is_visible': True},
'enable_dhcp': {'allow_post': True, 'allow_put': True,
Expand Down
6 changes: 5 additions & 1 deletion quantum/api/v2/base.py
Expand Up @@ -16,6 +16,7 @@
import logging
import socket

import netaddr
import webob.exc

from quantum.api.v2 import attributes
Expand Down Expand Up @@ -44,7 +45,10 @@
exceptions.HostRoutesExhausted: webob.exc.HTTPBadRequest,
exceptions.DNSNameServersExhausted: webob.exc.HTTPBadRequest,
# Some plugins enforce policies as well
exceptions.PolicyNotAuthorized: webob.exc.HTTPForbidden
exceptions.PolicyNotAuthorized: webob.exc.HTTPForbidden,
netaddr.AddrFormatError: webob.exc.HTTPBadRequest,
AttributeError: webob.exc.HTTPBadRequest,
ValueError: webob.exc.HTTPBadRequest,
}

QUOTAS = quota.QUOTAS
Expand Down
5 changes: 4 additions & 1 deletion quantum/api/v2/resource.py
Expand Up @@ -18,6 +18,7 @@
"""
import logging

import netaddr
import webob
import webob.dec
import webob.exc
Expand Down Expand Up @@ -93,7 +94,9 @@ def resource(request):
method = getattr(controller, action)

result = method(request=request, **args)
except exceptions.QuantumException as e:
except (ValueError, AttributeError,
exceptions.QuantumException,
netaddr.AddrFormatError) as e:
LOG.exception('%s failed' % action)
body = serializer({'QuantumError': str(e)})
kwargs = {'body': body, 'content_type': content_type}
Expand Down
8 changes: 4 additions & 4 deletions quantum/db/db_base_plugin_v2.py
Expand Up @@ -956,8 +956,8 @@ def _validate_subnet(self, s):
s['gateway_ip'] != attributes.ATTR_NOT_SPECIFIED):
self._validate_ip_version(ip_ver, s['gateway_ip'], 'gateway_ip')

if 'dns_nameservers' in s and \
s['dns_nameservers'] != attributes.ATTR_NOT_SPECIFIED:
if ('dns_nameservers' in s and
s['dns_nameservers'] != attributes.ATTR_NOT_SPECIFIED):
if len(s['dns_nameservers']) > cfg.CONF.max_dns_nameservers:
raise q_exc.DNSNameServersExhausted(
subnet_id=id,
Expand All @@ -970,8 +970,8 @@ def _validate_subnet(self, s):
error_message=("error parsing dns address %s" % dns))
self._validate_ip_version(ip_ver, dns, 'dns_nameserver')

if 'host_routes' in s and \
s['host_routes'] != attributes.ATTR_NOT_SPECIFIED:
if ('host_routes' in s and
s['host_routes'] != attributes.ATTR_NOT_SPECIFIED):
if len(s['host_routes']) > cfg.CONF.max_subnet_host_routes:
raise q_exc.HostRoutesExhausted(
subnet_id=id,
Expand Down

0 comments on commit 49f649c

Please sign in to comment.