Skip to content

Commit

Permalink
Fixes DOS issue in instance list ip filter
Browse files Browse the repository at this point in the history 
Converts the ip filtering to filter the list locally based
on the network info cache instead of making an extremely expensive
call over to nova network where it attempts to retrieve a list
of every instance in the system.

Change-Id: I455f6ab4acdecacc5152b11a183027f933dc4475
Closes-bug: #1358583
  • Loading branch information
@vishvananda
vishvananda authored and Tristan Cacqueray committed Oct 28, 2014
1 parent 2aeee7a commit e98738d
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 24 deletions.
30 changes: 22 additions & 8 deletions nova/compute/api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1975,6 +1975,9 @@ def _remap_system_metadata_filter(metadata):
sort_key, sort_dir, limit=limit, marker=marker,
expected_attrs=expected_attrs)

if 'ip6' in filters or 'ip' in filters:
inst_models = self._ip_filter(inst_models, filters)

if want_objects:
return inst_models

Expand All @@ -1985,18 +1988,29 @@ def _remap_system_metadata_filter(metadata):

return instances

@staticmethod
def _ip_filter(inst_models, filters):
ipv4_f = re.compile(str(filters.get('ip')))
ipv6_f = re.compile(str(filters.get('ip6')))
result_objs = []
for instance in inst_models:
nw_info = compute_utils.get_nw_info_for_instance(instance)
for vif in nw_info:
for fixed_ip in vif.fixed_ips():
address = fixed_ip.get('address')
if not address:
continue
version = fixed_ip.get('version')
if ((version == 4 and ipv4_f.match(address)) or
(version == 6 and ipv6_f.match(address))):
result_objs.append(instance)
continue
return objects.InstanceList(objects=result_objs)

def _get_instances_by_filters(self, context, filters,
sort_key, sort_dir,
limit=None,
marker=None, expected_attrs=None):
if 'ip6' in filters or 'ip' in filters:
res = self.network_api.get_instance_uuids_by_ip_filter(context,
filters)
# NOTE(jkoelker) It is possible that we will get the same
# instance uuid twice (one for ipv4 and ipv6)
uuids = set([r['instance_uuid'] for r in res])
filters['uuid'] = uuids

fields = ['metadata', 'system_metadata', 'info_cache',
'security_groups']
if expected_attrs:
Expand Down
74 changes: 58 additions & 16 deletions nova/tests/compute/test_compute.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@
from nova.tests import matchers
from nova.tests.objects import test_flavor
from nova.tests.objects import test_migration
from nova.tests.objects import test_network
from nova import utils
from nova.virt import block_device as driver_block_device
from nova.virt import event
Expand Down Expand Up @@ -7198,6 +7197,35 @@ def _run_instance(self, params=None):
self.assertIsNone(instance['task_state'])
return instance, instance_uuid

def test_ip_filtering(self):
info = [{
'address': 'aa:bb:cc:dd:ee:ff',
'id': 1,
'network': {
'bridge': 'br0',
'id': 1,
'label': 'private',
'subnets': [{
'cidr': '192.168.0.0/24',
'ips': [{
'address': '192.168.0.10',
'type': 'fixed',
}]
}]
}
}]

info1 = objects.InstanceInfoCache(network_info=jsonutils.dumps(info))
inst1 = objects.Instance(id=1, info_cache=info1)
info[0]['network']['subnets'][0]['ips'][0]['address'] = '192.168.0.20'
info2 = objects.InstanceInfoCache(network_info=jsonutils.dumps(info))
inst2 = objects.Instance(id=2, info_cache=info2)
instances = objects.InstanceList(objects=[inst1, inst2])

instances = self.compute_api._ip_filter(instances, {'ip': '.*10'})
self.assertEqual(len(instances), 1)
self.assertEqual(instances[0].id, 1)

def test_create_with_too_little_ram(self):
# Test an instance type with too little memory.

Expand Down Expand Up @@ -7962,33 +7990,47 @@ def test_get_all_by_name_regexp(self):
db.instance_destroy(c, instance2['uuid'])
db.instance_destroy(c, instance3['uuid'])

@mock.patch('nova.db.network_get')
@mock.patch('nova.db.fixed_ips_by_virtual_interface')
def test_get_all_by_multiple_options_at_once(self, fixed_get, network_get):
def test_get_all_by_multiple_options_at_once(self):
# Test searching by multiple options at once.
c = context.get_admin_context()
network_manager = fake_network.FakeNetworkManager(self.stubs)
fixed_get.side_effect = (
network_manager.db.fixed_ips_by_virtual_interface)
network_get.return_value = (
dict(test_network.fake_network,
**network_manager.db.network_get(None, 1)))
self.stubs.Set(self.compute_api.network_api,
'get_instance_uuids_by_ip_filter',
network_manager.get_instance_uuids_by_ip_filter)

def fake_network_info(ip):
info = [{
'address': 'aa:bb:cc:dd:ee:ff',
'id': 1,
'network': {
'bridge': 'br0',
'id': 1,
'label': 'private',
'subnets': [{
'cidr': '192.168.0.0/24',
'ips': [{
'address': ip,
'type': 'fixed',
}]
}]
}
}]
return jsonutils.dumps(info)

instance1 = self._create_fake_instance({
'display_name': 'woot',
'id': 1,
'uuid': '00000000-0000-0000-0000-000000000010'})
'uuid': '00000000-0000-0000-0000-000000000010',
'info_cache': {'network_info':
fake_network_info('192.168.0.1')}})
instance2 = self._create_fake_instance({
'display_name': 'woo',
'id': 20,
'uuid': '00000000-0000-0000-0000-000000000020'})
'uuid': '00000000-0000-0000-0000-000000000020',
'info_cache': {'network_info':
fake_network_info('192.168.0.2')}})
instance3 = self._create_fake_instance({
'display_name': 'not-woot',
'id': 30,
'uuid': '00000000-0000-0000-0000-000000000030'})
'uuid': '00000000-0000-0000-0000-000000000030',
'info_cache': {'network_info':
fake_network_info('192.168.0.3')}})

# ip ends up matching 2nd octet here.. so all 3 match ip
# but 'name' only matches one
Expand Down

0 comments on commit e98738d

Please sign in to comment.