x/quark
Code Issues Proposed changes

More data validation on keys

Browse Source 
JIRA:NCP-1829

A lot of the bodies passed into quark are not properly validated. This
adds a tiny bit more validation to keys on passed bodies. Also
extensions now have proper exception handling and re-raising.

Change-Id: Ifc0a88e14a8264c8a4e353a3e16967e4c9891f05
Closes-Bug: 1595613
This commit is contained in:
Justin Hammond 2016-06-23 14:50:02 -05:00
parent 51690906a7
commit 79de4db4a7
12 changed files with 211 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
quark/api/extensions/ip_policies.py
View File

@ -16,7 +16,9 @@
from neutron.api import extensions
from neutron import manager
from neutron import wsgi
from neutron_lib import exceptions as n_exc
from oslo_log import log as logging
import webob
RESOURCE_NAME = "ip_policy"
RESOURCE_COLLECTION = "ip_policies"
@ -39,13 +41,27 @@ class IPPoliciesController(wsgi.Controller):
def create(self, request, body=None):
body = self._deserialize(request.body, request.get_content_type())
return {RESOURCE_NAME:
self._plugin.create_ip_policy(request.context, body)}
try:
return {RESOURCE_NAME:
self._plugin.create_ip_policy(request.context, body)}
except n_exc.NotFound as e:
raise webob.exc.HTTPNotFound(e)
except n_exc.Conflict as e:
raise webob.exc.HTTPConflict(e)
except n_exc.BadRequest as e:
raise webob.exc.HTTPBadRequest(e)
def update(self, request, id, body=None):
body = self._deserialize(request.body, request.get_content_type())
return {RESOURCE_NAME:
self._plugin.update_ip_policy(request.context, id, body)}
try:
return {RESOURCE_NAME:
self._plugin.update_ip_policy(request.context, id, body)}
except n_exc.NotFound as e:
raise webob.exc.HTTPNotFound(e)
except n_exc.Conflict as e:
raise webob.exc.HTTPConflict(e)
except n_exc.BadRequest as e:
raise webob.exc.HTTPBadRequest(e)
def index(self, request):
context = request.context
@ -54,12 +70,26 @@ class IPPoliciesController(wsgi.Controller):
def show(self, request, id):
context = request.context
return {RESOURCE_NAME:
self._plugin.get_ip_policy(context, id)}
try:
return {RESOURCE_NAME:
self._plugin.get_ip_policy(context, id)}
except n_exc.NotFound as e:
raise webob.exc.HTTPNotFound(e)
except n_exc.Conflict as e:
raise webob.exc.HTTPConflict(e)
except n_exc.BadRequest as e:
raise webob.exc.HTTPBadRequest(e)
def delete(self, request, id, **kwargs):
context = request.context
return self._plugin.delete_ip_policy(context, id)
try:
return self._plugin.delete_ip_policy(context, id)
except n_exc.NotFound as e:
raise webob.exc.HTTPNotFound(e)
except n_exc.Conflict as e:
raise webob.exc.HTTPConflict(e)
except n_exc.BadRequest as e:
raise webob.exc.HTTPBadRequest(e)
class Ip_policies(extensions.ExtensionDescriptor):

20
quark/api/extensions/mac_address_ranges.py
View File

@ -16,6 +16,7 @@
from neutron.api import extensions
from neutron import manager
from neutron import wsgi
from neutron_lib import exceptions as n_exc
from oslo_log import log as logging
import webob
@ -46,10 +47,16 @@ class MacAddressRangesController(wsgi.Controller):
def create(self, request, body=None):
body = self._deserialize(request.body, request.get_content_type())
if "cidr" not in body[RESOURCE_NAME]:
raise webob.exc.HTTPUnprocessableEntity()
return {"mac_address_range":
self._plugin.create_mac_address_range(request.context, body)}
try:
return {"mac_address_range":
self._plugin.create_mac_address_range(request.context,
body)}
except n_exc.NotFound as e:
raise webob.exc.HTTPNotFound(e)
except n_exc.Conflict as e:
raise webob.exc.HTTPConflict(e)
except n_exc.BadRequest as e:
raise webob.exc.HTTPBadRequest(e)
def index(self, request):
context = request.context
@ -63,7 +70,10 @@ class MacAddressRangesController(wsgi.Controller):
def delete(self, request, id, **kwargs):
context = request.context
return self._plugin.delete_mac_address_range(context, id)
try:
return self._plugin.delete_mac_address_range(context, id)
except n_exc.NotFound as e:
raise webob.exc.HTTPNotFound(e)
class Mac_address_ranges(extensions.ExtensionDescriptor):

15
quark/api/extensions/routes.py
View File

@ -49,13 +49,14 @@ class RoutesController(wsgi.Controller):
def create(self, request, body=None):
body = self._deserialize(request.body, request.get_content_type())
keys = ["subnet_id", "gateway", "cidr"]
for k in keys:
if k not in body[RESOURCE_NAME]:
raise webob.exc.HTTPUnprocessableEntity()
return {"route":
self._plugin.create_route(request.context, body)}
try:
return {"route": self._plugin.create_route(request.context, body)}
except n_exc.NotFound as e:
raise webob.exc.HTTPNotFound(e)
except n_exc.Conflict as e:
raise webob.exc.HTTPConflict(e)
except n_exc.BadRequest as e:
raise webob.exc.HTTPBadRequest(e)
def index(self, request):
context = request.context

27
quark/api/extensions/segment_allocation_ranges.py
View File

@ -16,7 +16,9 @@
from neutron.api import extensions
from neutron import manager
from neutron import wsgi
from neutron_lib import exceptions as n_exc
from oslo_log import log as logging
import webob
RESOURCE_NAME = 'segment_allocation_range'
RESOURCE_COLLECTION = RESOURCE_NAME + "s"
@ -40,9 +42,16 @@ class SegmentAllocationRangesController(wsgi.Controller):
def create(self, request, body=None):
body = self._deserialize(request.body, request.get_content_type())
return {"segment_allocation_range":
self._plugin.create_segment_allocation_range(
request.context, body)}
try:
return {"segment_allocation_range":
self._plugin.create_segment_allocation_range(
request.context, body)}
except n_exc.NotFound as e:
raise webob.exc.HTTPNotFound(e)
except n_exc.Conflict as e:
raise webob.exc.HTTPConflict(e)
except n_exc.BadRequest as e:
raise webob.exc.HTTPBadRequest(e)
def index(self, request):
context = request.context
@ -52,12 +61,18 @@ class SegmentAllocationRangesController(wsgi.Controller):
def show(self, request, id):
context = request.context
return {"segment_allocation_range":
self._plugin.get_segment_allocation_range(context, id)}
try:
return {"segment_allocation_range":
self._plugin.get_segment_allocation_range(context, id)}
except n_exc.NotFound as e:
raise webob.exc.HTTPNotFound(e)
def delete(self, request, id, **kwargs):
context = request.context
return self._plugin.delete_segment_allocation_range(context, id)
try:
return self._plugin.delete_segment_allocation_range(context, id)
except n_exc.NotFound as e:
raise webob.exc.HTTPNotFound(e)
class Segment_allocation_ranges(extensions.ExtensionDescriptor):

43
quark/plugin.py
View File

@ -21,9 +21,9 @@ from neutron.extensions import securitygroup as sg_ext
from neutron import neutron_plugin_base_v2
from neutron.quota import resource as qres
from neutron.quota import resource_registry as qres_reg
from neutron_lib import exceptions as n_exc
from oslo_config import cfg
from oslo_log import log as logging
import webob.exc
from quark.api import extensions
from quark import ip_availability
@ -137,18 +137,25 @@ class Plugin(neutron_plugin_base_v2.NeutronPluginBaseV2,
def __init__(self):
LOG.info("Starting quark plugin")
def _fix_missing_tenant_id(self, context, resource):
def _fix_missing_tenant_id(self, context, body, key):
"""Will add the tenant_id to the context from body.
It is assumed that the body must have a tenant_id because neutron
core could never have gotten here otherwise.
"""
if not body:
raise n_exc.BadRequest(resource=key,
msg="Body malformed")
resource = body.get(key)
if not resource:
raise n_exc.BadRequest(resource=key,
msg="Body malformed")
if context.tenant_id is None:
context.tenant_id = resource.get("tenant_id")
if context.tenant_id is None:
msg = _("Running without keystone AuthN requires "
"that tenant_id is specified")
raise webob.exc.HTTPBadRequest(msg)
raise n_exc.BadRequest(resource=key, msg=msg)
@sessioned
def get_mac_address_range(self, context, id, fields=None):
@ -160,7 +167,7 @@ class Plugin(neutron_plugin_base_v2.NeutronPluginBaseV2,
@sessioned
def create_mac_address_range(self, context, mac_range):
self._fix_missing_tenant_id(context, mac_range["mac_address_range"])
self._fix_missing_tenant_id(context, mac_range, "mac_address_range")
return mac_address_ranges.create_mac_address_range(context, mac_range)
@sessioned
@ -169,13 +176,13 @@ class Plugin(neutron_plugin_base_v2.NeutronPluginBaseV2,
@sessioned
def create_security_group(self, context, security_group):
self._fix_missing_tenant_id(context, security_group["security_group"])
self._fix_missing_tenant_id(context, security_group, "security_group")
return security_groups.create_security_group(context, security_group)
@sessioned
def create_security_group_rule(self, context, security_group_rule):
self._fix_missing_tenant_id(context,
security_group_rule["security_group_rule"])
self._fix_missing_tenant_id(
context, security_group_rule, "security_group_rule")
return security_groups.create_security_group_rule(context,
security_group_rule)
@ -218,7 +225,7 @@ class Plugin(neutron_plugin_base_v2.NeutronPluginBaseV2,
@sessioned
def create_ip_policy(self, context, ip_policy):
self._fix_missing_tenant_id(context, ip_policy["ip_policy"])
self._fix_missing_tenant_id(context, ip_policy, "ip_policy")
return ip_policies.create_ip_policy(context, ip_policy)
@sessioned
@ -247,7 +254,7 @@ class Plugin(neutron_plugin_base_v2.NeutronPluginBaseV2,
@sessioned
def create_ip_address(self, context, ip_address):
self._fix_missing_tenant_id(context, ip_address["ip_address"])
self._fix_missing_tenant_id(context, ip_address, "ip_address")
return ip_addresses.create_ip_address(context, ip_address)
@sessioned
@ -260,7 +267,7 @@ class Plugin(neutron_plugin_base_v2.NeutronPluginBaseV2,
@sessioned
def create_port(self, context, port):
self._fix_missing_tenant_id(context, port["port"])
self._fix_missing_tenant_id(context, port, "port")
return ports.create_port(context, port)
@sessioned
@ -320,7 +327,7 @@ class Plugin(neutron_plugin_base_v2.NeutronPluginBaseV2,
@sessioned
def create_route(self, context, route):
self._fix_missing_tenant_id(context, route["route"])
self._fix_missing_tenant_id(context, route, "route")
return routes.create_route(context, route)
@sessioned
@ -329,7 +336,7 @@ class Plugin(neutron_plugin_base_v2.NeutronPluginBaseV2,
@sessioned
def create_subnet(self, context, subnet):
self._fix_missing_tenant_id(context, subnet["subnet"])
self._fix_missing_tenant_id(context, subnet, "subnet")
return subnets.create_subnet(context, subnet)
@sessioned
@ -360,7 +367,7 @@ class Plugin(neutron_plugin_base_v2.NeutronPluginBaseV2,
@sessioned
def create_network(self, context, network):
self._fix_missing_tenant_id(context, network["network"])
self._fix_missing_tenant_id(context, network, "network")
return networks.create_network(context, network)
@sessioned
@ -418,7 +425,7 @@ class Plugin(neutron_plugin_base_v2.NeutronPluginBaseV2,
@sessioned
def create_floatingip(self, context, floatingip):
self._fix_missing_tenant_id(context, floatingip["floatingip"])
self._fix_missing_tenant_id(context, floatingip, "floatingip")
return floating_ips.create_floatingip(context,
floatingip["floatingip"])
@ -466,7 +473,7 @@ class Plugin(neutron_plugin_base_v2.NeutronPluginBaseV2,
@sessioned
def create_segment_allocation_range(self, context, sa_range):
self._fix_missing_tenant_id(
context, sa_range["segment_allocation_range"])
context, sa_range, "segment_allocation_range")
return segment_allocation_ranges.create_segment_allocation_range(
context, sa_range)
@ -476,7 +483,7 @@ class Plugin(neutron_plugin_base_v2.NeutronPluginBaseV2,
context, id)
def create_scalingip(self, context, scalingip):
self._fix_missing_tenant_id(context, scalingip["scalingip"])
self._fix_missing_tenant_id(context, scalingip, "scalingip")
return floating_ips.create_scalingip(context, scalingip["scalingip"])
@sessioned
@ -511,12 +518,12 @@ class Plugin(neutron_plugin_base_v2.NeutronPluginBaseV2,
@sessioned
def create_job(self, context, job):
self._fix_missing_tenant_id(context, job['job'])
self._fix_missing_tenant_id(context, job, 'job')
return jobs.create_job(context, job)
@sessioned
def update_job(self, context, id, job):
self._fix_missing_tenant_id(context, job['job'])
self._fix_missing_tenant_id(context, job, 'job')
return jobs.update_job(context, id, job)
@sessioned

13
quark/plugin_modules/mac_address_ranges.py
View File

@ -90,8 +90,17 @@ def create_mac_address_range(context, mac_range):
if not context.is_admin:
raise n_exc.NotAuthorized()
cidr = mac_range["mac_address_range"]["cidr"]
do_not_use = mac_range["mac_address_range"].get("do_not_use", "0")
if not mac_range or "mac_address_range" not in mac_range:
raise n_exc.BadRequest(resource="mac_address_range",
msg="Malformed body")
rng = mac_range.get("mac_address_range")
cidr = rng.get("cidr")
if not cidr:
raise n_exc.BadRequest(resource="mac_address_range",
msg="Missing cidr")
do_not_use = rng.get("do_not_use", "0")
cidr, first_address, last_address = _to_mac_range(cidr)
with context.session.begin():
new_range = db_api.mac_address_range_create(

6
quark/plugin_modules/routes.py
View File

@ -48,7 +48,11 @@ def get_routes(context):
def create_route(context, route):
LOG.info("create_route for tenant %s" % context.tenant_id)
route = route["route"]
if not route:
raise n_exc.BadRequest(resource="routes", msg="Malformed body")
route = route.get("route")
if not route:
raise n_exc.BadRequest(resource="routes", msg="Malformed body")
for key in ["gateway", "cidr", "subnet_id"]:
if key not in route:
raise n_exc.BadRequest(resource="routes",

10
quark/tests/functional/plugin_modules/test_floating_ips.py
View File

@ -2,6 +2,7 @@ import json
import mock
import netaddr
from neutron import context
from neutron_lib import exceptions as n_exc
from oslo_config import cfg
from quark.db import api as db_api
@ -147,6 +148,15 @@ class TestFloatingIPs(BaseFloatingIPTest):
self.assertEqual(flip['floating_ip_address'],
get_flip['floating_ip_address'])
def test_create_raises_missing_key(self):
flip_req = dict(
floating_network_id=self.floating_network['id'],
port_id=self.user_port1['id']
)
flip_req = {'floatingipz': flip_req}
with self.assertRaises(n_exc.BadRequest):
self.plugin.create_floatingip(self.context, flip_req)
def test_update_floating_ip(self):
floating_ip = dict(
floating_network_id=self.floating_network.id,

21
quark/tests/functional/plugin_modules/test_routes.py
View File

@ -16,6 +16,7 @@
import contextlib
import mock
from neutron.common import exceptions as n_exc_ext
from neutron_lib import exceptions as n_exc
from oslo_config import cfg
from quark import exceptions as q_exc
@ -65,6 +66,26 @@ class QuarkCreateRoutes(BaseFunctionalTest):
for key in create_route.keys():
self.assertEqual(new_route[key], create_route[key])
def test_create_route_with_garbage_bad_request(self):
cidr = "192.168.0.0/24"
ip_policy = dict(exclude=["192.168.0.1/32"])
ip_policy = {"ip_policy": ip_policy}
network = dict(name="public", tenant_id="fake", network_plugin="BASE")
network = {"network": network}
subnet = dict(ip_version=4, next_auto_assign_ip=2,
cidr=cidr, ip_policy=None,
tenant_id="fake")
subnet = {"subnet": subnet}
create_route = dict(cidr="172.16.0.0/24", gateway="172.16.0.1")
with self._stubs(network, subnet, ip_policy) as (net, sub, ipp):
self.assertIsNotNone(net)
self.assertIsNotNone(sub)
self.assertIsNotNone(ipp)
create_route["subnet_id"] = sub["id"]
with self.assertRaises(n_exc.BadRequest):
routes_api.create_route(
self.context, dict(routes=create_route))
def test_create_route_gateway_conflict_raises(self):
cidr = "192.168.0.0/24"
ip_policy = dict(exclude=["192.168.0.0/32", "192.168.0.255/32"])

3
quark/tests/functional/plugin_modules/test_shared_ips.py
View File

@ -22,7 +22,6 @@ from neutron import context
from neutron_lib import exceptions as n_exc
from oslo_config import cfg
from oslo_log import log as logging
from webob import exc as w_exc
from quark.db import api as db_api
from quark.db import ip_types
@ -341,7 +340,7 @@ class QuarkSharedIPs(BaseFunctionalTest):
network_id=net['id'],
version=4)}
api = plugin.Plugin()
with self.assertRaises(w_exc.HTTPBadRequest):
with self.assertRaises(n_exc.BadRequest):
api.create_ip_address(no_tenant_context, shared_ip)
def test_create_shared_ips_fails_with_plural_body(self):

2
quark/tests/functional/test_ip.py
View File

@ -1,4 +1,4 @@
# Copyright (c) 2013 OpenStack Foundation
# Copyright (c) 2013 Rackspace Hosting Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.

56
quark/tests/functional/test_plugin.py Normal file
View File

@ -0,0 +1,56 @@
# Copyright (c) 2016 Rackspace Hosting Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
# implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from neutron import context
from neutron_lib import exceptions as n_exc
from quark import plugin
from quark.tests.functional.base import BaseFunctionalTest
class QuarkPluginNegativeTest(BaseFunctionalTest):
def setUp(self):
super(QuarkPluginNegativeTest, self).setUp()
self.plugin = plugin.Plugin()
self.blank_context = context.Context(None, None, is_admin=True)
self.with_tenant_id = {"thing": {"tenant_id": "stuff"}}
self.without_tenant_id = {"thing": {"attr": "stuff"}}
class QuarkPluginTenantlessNegativeTests(QuarkPluginNegativeTest):
def test_tenant_check_no_raise(self):
ret = self.plugin._fix_missing_tenant_id(
self.blank_context, self.with_tenant_id, "thing")
self.assertEqual(None, ret)
def test_tenant_check_raises_if_no_tenant(self):
with self.assertRaises(n_exc.BadRequest):
self.plugin._fix_missing_tenant_id(
self.blank_context, self.without_tenant_id, "thing")
def test_tenant_check_no_raise_if_tenant_in_context(self):
self.plugin._fix_missing_tenant_id(
self.context, self.without_tenant_id, "thing")
def test_tenant_check_raises_missing_body(self):
with self.assertRaises(n_exc.BadRequest):
self.plugin._fix_missing_tenant_id(
self.blank_context, {}, "thing")
with self.assertRaises(n_exc.BadRequest):
self.plugin._fix_missing_tenant_id(
self.blank_context, None, "thing")