x/vmware-nsx
Code Issues Proposed changes

NSX-V| Fix FWaaS rules order

Browse Source 
Reorder the FW rules on the edge, so that internal & MD proxy traffic
will always be allowed, but other traffic will go through the FWaaS rules.

In additon support the case of firewall policy with no rules,
and do not add the firewall rules if the router has no external gateway.

Change-Id: Ia4afad53a4b68f87947eec9d0d25007128b174e9
This commit is contained in:
Adit Sarfaty 2017-04-19 15:00:20 +03:00
parent 63968c4962
commit e04cef8c17
7 changed files with 162 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
vmware_nsx/plugins/nsx_v/drivers/shared_router_driver.py
View File

@ -297,6 +297,7 @@ class RouterSharedDriver(router_driver.RouterBaseDriver):
# Fake fw rule to open subnets firewall flows and static routes
# relative flows
fake_subnet_fw_rule = {
'name': 'Subnet Rule',
'action': 'allow',
'enabled': True,
'source_ip_address': subnet_cidrs,
@ -307,6 +308,7 @@ class RouterSharedDriver(router_driver.RouterBaseDriver):
if dnat_cidrs:
# Fake fw rule to open dnat firewall flows
fake_dnat_fw_rule = {
'name': 'DNAT Rule',
'action': 'allow',
'enabled': True,
'destination_ip_address': dnat_cidrs}

43
vmware_nsx/plugins/nsx_v/plugin.py
View File

@ -3318,6 +3318,7 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
router['id'])
if subnet_cidrs:
no_snat_fw_rules.append({
'name': 'No SNAT Rule',
'action': 'allow',
'enabled': True,
'source_vnic_groups': ["external"],
@ -3496,41 +3497,57 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
self._update_edge_router(context, router_id)
def _update_subnets_and_dnat_firewall(self, context, router,
router_id=None, allow_external=True):
router_id=None):
fw_rules = []
if not router_id:
router_id = router['id']
# Add FW rule to open subnets firewall flows and static routes
# relative flows
subnet_cidrs = self._find_router_subnets_cidrs(context, router['id'])
routes = self._get_extra_routes_by_router_id(context, router_id)
subnet_cidrs.extend([route['destination'] for route in routes])
if subnet_cidrs:
# Fake fw rule to open subnets firewall flows and static routes
# relative flows
fake_subnet_fw_rule = {
subnet_fw_rule = {
'name': 'Subnet Rule',
'action': 'allow',
'enabled': True,
'source_ip_address': subnet_cidrs,
'destination_ip_address': subnet_cidrs}
fw_rules.append(fake_subnet_fw_rule)
_, dnat_rules = self._get_nat_rules(context, router)
fw_rules.append(subnet_fw_rule)
# If metadata service is enabled, block access to inter-edge network
if self.metadata_proxy_handler:
fw_rules += nsx_v_md_proxy.get_router_fw_rules()
# Add fw rules if FWaaS is enabled
allow_external = True
if (self.fwaas_callbacks.should_apply_firewall_to_router(
context, router, router_id)):
fw_rules.extend(self.fwaas_callbacks.get_fwaas_rules_for_router(
context, router['id']))
# If we have a firewall we shouldn't add the default
# allow-external rule
allow_external = False
# Add FW rule to open dnat firewall flows
_, dnat_rules = self._get_nat_rules(context, router)
dnat_cidrs = [rule['dst'] for rule in dnat_rules]
if dnat_cidrs:
# Fake fw rule to open dnat firewall flows
fake_dnat_fw_rule = {
dnat_fw_rule = {
'name': 'DNAT Rule',
'action': 'allow',
'enabled': True,
'destination_ip_address': dnat_cidrs}
fw_rules.append(fake_dnat_fw_rule)
fw_rules.append(dnat_fw_rule)
# Add no-snat rules
nosnat_fw_rules = self._get_nosnat_subnets_fw_rules(
context, router)
fw_rules.extend(nosnat_fw_rules)
# Get the load balancer rules in case they are refreshed
# (relevant only for older LB that are still on the router edge)
edge_id = self._get_edge_id_by_rtr_id(context, router_id)
lb_rules = nsxv_db.get_nsxv_lbaas_loadbalancer_binding_by_edge(
context.session, edge_id)
@ -3546,13 +3563,9 @@ class NsxVPluginV2(addr_pair_db.AllowedAddressPairsMixin,
}
fw_rules.append(lb_fw_rule)
# Add fw rules if FWaaS is enabled
fw_rules.extend(self.fwaas_callbacks.get_fwaas_rules_for_router(
context, router, router_id))
fake_fw = {'firewall_rule_list': fw_rules}
fw = {'firewall_rule_list': fw_rules}
try:
edge_utils.update_firewall(self.nsx_v, context, router_id, fake_fw,
edge_utils.update_firewall(self.nsx_v, context, router_id, fw,
allow_external=allow_external)
except vsh_exc.ResourceNotFound:
LOG.error("Failed to update firewall for router %s",

13
vmware_nsx/plugins/nsx_v/vshield/edge_firewall_driver.py
View File

@ -30,6 +30,7 @@ VSE_FWAAS_REJECT = "reject"
FWAAS_ALLOW = "allow"
FWAAS_DENY = "deny"
FWAAS_REJECT = "reject"
FWAAS_ALLOW_EXT_RULE_NAME = 'Allow To External'
class EdgeFirewallDriver(object):
@ -191,11 +192,12 @@ class EdgeFirewallDriver(object):
ruleTag = 1
vcns_rules = []
if allow_external:
vcns_rules.append(
{'action': "accept",
'enabled': True,
'destination': {'vnicGroupId': ["external"]},
'ruleTag': ruleTag})
vcns_rules.append({'name': FWAAS_ALLOW_EXT_RULE_NAME,
'action': "accept",
'enabled': True,
'destination': {'vnicGroupId': ["external"]},
'ruleTag': ruleTag})
ruleTag += 1
for rule in firewall['firewall_rule_list']:
tag = rule.get('ruleTag', ruleTag)
vcns_rule = self._convert_firewall_rule(rule, tag)
@ -387,7 +389,6 @@ class EdgeFirewallDriver(object):
LOG.exception("Failed to update firewall "
"with edge_id: %s", edge_id)
vcns_fw_config = self._get_firewall(edge_id)
nsxv_db.cleanup_nsxv_edge_firewallrule_binding(
context.session, edge_id)

83
vmware_nsx/services/fwaas/nsx_v/edge_fwaas_driver.py
View File

@ -24,6 +24,7 @@ from neutron_fwaas.services.firewall.drivers import fwaas_base
from vmware_nsx.common import locking
from vmware_nsx.plugins.nsx_v.vshield.common import (
exceptions as vcns_exc)
from vmware_nsx.plugins.nsx_v.vshield import edge_firewall_driver
from vmware_nsx.plugins.nsx_v.vshield import edge_utils
from vmware_nsx.plugins.nsx_v.vshield import vcns_driver
@ -44,12 +45,35 @@ class EdgeFwaasDriver(fwaas_base.FwaasDriverBase):
super(EdgeFwaasDriver, self).__init__()
self._nsxv = vcns_driver.VcnsDriver(None)
def should_apply_firewall_to_router(self, router_data):
"""Return True if the firewall rules should be added the router
Return False in those cases:
- shared router (not supported)
- router without an external gateway
"""
if not router_data.get('external_gateway_info'):
LOG.info("Cannot apply firewall to router %s with no gateway",
router_data['id'])
return False
if (not router_data.get('distributed') and
router_data.get('router_type') == 'shared'):
LOG.info("Cannot apply firewall to shared router %s",
router_data['id'])
return False
return True
def _get_routers_edges(self, context, apply_list):
# Get edges for all the routers in the apply list.
# note that shared routers are currently not supported
edge_manager = self.edge_manager
edges = []
for router_info in apply_list:
# No FWaaS rules needed if there is no external gateway
if not self.should_apply_firewall_to_router(router_info.router):
continue
lookup_id = None
router_id = router_info.router_id
if router_info.router.get('distributed'):
@ -57,10 +81,6 @@ class EdgeFwaasDriver(fwaas_base.FwaasDriverBase):
# we need the plr edge id
lookup_id = edge_manager.get_plr_by_tlr_id(
context, router_id)
elif router_info.router.get('router_type') == 'shared':
# Shared router (currently not supported)
LOG.info("Cannot apply firewall to shared router %s",
router_id)
else:
# Exclusive router
lookup_id = router_id
@ -94,6 +114,18 @@ class EdgeFwaasDriver(fwaas_base.FwaasDriverBase):
return translated_rules
def _is_allow_external_rule(self, rule):
rule_name = rule.get('name', '')
if rule_name == edge_firewall_driver.FWAAS_ALLOW_EXT_RULE_NAME:
return True
# For older routers, the allow-external rule didn't have a name
# TODO(asarfaty): delete this in the future
if (not rule_name and
rule.get('action') == edge_firewall_driver.FWAAS_ALLOW and
rule.get('destination_vnic_groups', []) == ['external']):
return True
return False
def _get_other_backend_rules(self, context, edge_id):
"""Get a list of current backend rules from other applications
@ -107,36 +139,42 @@ class EdgeFwaasDriver(fwaas_base.FwaasDriverBase):
# Need to create a new one
backend_rules = []
# remove old FWaaS rules from the rules list
relevant_rules = []
# remove old FWaaS rules from the rules list.
# also delete the allow-external rule, if it is there.
# If necessary - we will add it again later
other_rules = []
for rule_item in backend_rules:
rule = rule_item['firewall_rule']
if not rule.get('name', '').startswith(RULE_NAME_PREFIX):
relevant_rules.append(rule)
rule_name = rule.get('name', '')
if (not rule_name.startswith(RULE_NAME_PREFIX) and
not self._is_allow_external_rule(rule)):
other_rules.append(rule)
return relevant_rules
return other_rules
def _set_rules_on_edge(self, context, edge_id, fw_id, translated_rules):
def _set_rules_on_edge(self, context, edge_id, fw_id, translated_rules,
allow_external=False):
"""delete old FWaaS rules from the Edge, and add new ones
Note that the edge might have other FW rules like NAT or LBaas
that should remain there.
allow_external is usually False because it shouldn't exist with a
firewall. It should only be True when the firewall is being deleted.
"""
# Get the existing backend rules which do not belong to FWaaS
backend_rules = self._get_other_backend_rules(context, edge_id)
# add new FWaaS rules at the end by their original order
backend_rules.extend(translated_rules)
# update the backend
# allow_external is False because it was already added
try:
with locking.LockManager.get_lock(str(edge_id)):
self._nsxv.update_firewall(
edge_id,
{'firewall_rule_list': backend_rules},
context,
allow_external=False)
allow_external=allow_external)
except Exception as e:
# catch known library exceptions and raise Fwaas generic exception
LOG.error("Failed to update backend firewall %(fw)s: "
@ -174,29 +212,34 @@ class EdgeFwaasDriver(fwaas_base.FwaasDriverBase):
"""Remove previous policy and apply the new policy."""
self._create_or_update_firewall(agent_mode, apply_list, firewall)
def _delete_firewall_or_set_default_policy(self, apply_list, firewall):
def _delete_firewall_or_set_default_policy(self, apply_list, firewall,
allow_external):
context = n_context.get_admin_context()
router_edges = self._get_routers_edges(context, apply_list)
if router_edges:
for edge_id in router_edges:
self._set_rules_on_edge(context, edge_id, firewall['id'], [])
self._set_rules_on_edge(context, edge_id, firewall['id'], [],
allow_external=allow_external)
@log_helpers.log_method_call
def delete_firewall(self, agent_mode, apply_list, firewall):
"""Delete firewall.
Removes rules created by this instance from the backend firewall.
Removes rules created by this instance from the backend firewall
And add the default allow-external rule.
"""
self._delete_firewall_or_set_default_policy(apply_list, firewall)
self._delete_firewall_or_set_default_policy(apply_list, firewall,
allow_external=True)
@log_helpers.log_method_call
def apply_default_policy(self, agent_mode, apply_list, firewall):
"""Apply the default policy (deny all).
The backend firewall always has this policy as default, so we only
need to delete the current rules.
The backend firewall always has this policy (=deny all) as default,
so we only need to delete the current rules.
"""
self._delete_firewall_or_set_default_policy(apply_list, firewall)
self._delete_firewall_or_set_default_policy(apply_list, firewall,
allow_external=False)
def get_firewall_translated_rules(self, firewall):
if firewall['admin_state_up']:

37
vmware_nsx/services/fwaas/nsx_v/fwaas_callbacks.py
View File

@ -14,6 +14,7 @@
# under the License.
from oslo_config import cfg
from oslo_log import log as logging
from neutron.agent.l3 import router_info
from neutron.common import config as neutron_config # noqa
@ -25,6 +26,8 @@ from neutron_fwaas.services.firewall.agents.l3reference \
from neutron_lib import context as n_context
from neutron_lib.plugins import directory
LOG = logging.getLogger(__name__)
class NsxvFwaasCallbacks(firewall_l3_agent.L3WithFWaaS):
"""NSX-V RPC callbacks for Firewall As A Service - V1."""
@ -70,31 +73,37 @@ class NsxvFwaasCallbacks(firewall_l3_agent.L3WithFWaaS):
return [self._router_dict_to_obj(ri) for ri in tenant_routers
if ri['id'] in router_ids]
def get_fwaas_rules_for_router(self, context, router, router_id):
"""Return the list of (translated) fwaas rules for this router."""
def should_apply_firewall_to_router(self, context, router, router_id):
"""Return True if the FWaaS rules should be added to this router."""
if not self.fwaas_enabled:
return []
ctx_elevated = context.elevated()
return False
# get all the relevant router info
# ("router" does not have all the fields)
ctx_elevated = context.elevated()
router_data = self.core_plugin.get_router(ctx_elevated, router['id'])
if not router_data:
return []
LOG.error("Couldn't read router %s data", router['id'])
return False
# Check if the FWaaS driver supports this router
if not self.fwaas_driver.should_apply_firewall_to_router(router_data):
return False
if router_data.get('distributed'):
# in case of distributed router router['id'] is the id of the
# neutron router
# in case of a distributed-router:
# router['id'] is the id of the neutron router (=tlr)
# and router_id is the plr/tlr (the one that is being updated)
if router_id == router['id']:
# Do not add firewall rules on the tlr router.
return []
if router_data.get('router_type') == 'shared':
# Currently there is no FWaaS support for shared routers
return []
return False
# Exclusive router or PLR
fw_id = self._get_router_firewall_id(ctx_elevated, router['id'])
return True
def get_fwaas_rules_for_router(self, context, router_id):
"""Return the list of (translated) FWaaS rules for this router."""
ctx_elevated = context.elevated()
fw_id = self._get_router_firewall_id(ctx_elevated, router_id)
if fw_id:
return self._get_fw_applicable_rules(ctx_elevated, fw_id)
return []

30
vmware_nsx/tests/unit/nsx_v/test_fwaas_driver.py
View File

@ -99,6 +99,8 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase):
self.assertEqual(router_count, update_fw.call_count)
bakend_rules = update_fw.call_args[0][1]['firewall_rule_list']
self.assertEqual(len(rule_list), len(bakend_rules))
allow_ext = update_fw.call_args[1]['allow_external']
self.assertEqual(False, allow_ext)
def test_create_firewall_no_rules(self):
apply_list = self._fake_apply_list()
@ -109,6 +111,8 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase):
self.assertEqual(1, update_fw.call_count)
bakend_rules = update_fw.call_args[0][1]['firewall_rule_list']
self.assertEqual(0, len(bakend_rules))
allow_ext = update_fw.call_args[1]['allow_external']
self.assertEqual(False, allow_ext)
def test_create_firewall_with_rules(self):
self._setup_firewall_with_rules(self.firewall.create_firewall)
@ -129,6 +133,8 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase):
self.assertEqual(1, update_fw.call_count)
bakend_rules = update_fw.call_args[0][1]['firewall_rule_list']
self.assertEqual(0, len(bakend_rules))
allow_ext = update_fw.call_args[1]['allow_external']
self.assertEqual(True, allow_ext)
def test_create_firewall_with_admin_down(self):
apply_list = self._fake_apply_list()
@ -140,3 +146,27 @@ class NsxvFwaasTestCase(test_v_plugin.NsxVPluginV2TestCase):
self.assertEqual(1, update_fw.call_count)
bakend_rules = update_fw.call_args[0][1]['firewall_rule_list']
self.assertEqual(0, len(bakend_rules))
allow_ext = update_fw.call_args[1]['allow_external']
self.assertEqual(False, allow_ext)
def test_should_apply_firewall_to_router(self):
router = {'id': 'fake_id',
'external_gateway_info': 'fake_data',
'router_type': 'exclusive',
'distributed': False}
self.assertTrue(self.firewall.should_apply_firewall_to_router(router))
# no external gateway:
router['external_gateway_info'] = None
self.assertFalse(self.firewall.should_apply_firewall_to_router(router))
router['external_gateway_info'] = 'Dummy'
# not for shared router:
router['router_type'] = 'shared'
router['distributed'] = False
self.assertFalse(self.firewall.should_apply_firewall_to_router(router))
# should work for distributed router
router['router_type'] = 'exclusive'
router['distributed'] = True
self.assertTrue(self.firewall.should_apply_firewall_to_router(router))

9
vmware_nsx/tests/unit/nsx_v/test_plugin.py
View File

@ -2294,10 +2294,12 @@ class L3NatTestCaseBase(test_l3_plugin.L3NatTestCaseMixin):
def test_router_set_gateway_with_nosnat(self, mock):
expected_fw = [{'action': 'allow',
'enabled': True,
'name': 'Subnet Rule',
'source_ip_address': [],
'destination_ip_address': []}]
nosnat_fw = [{'action': 'allow',
'enabled': True,
'name': 'No SNAT Rule',
'source_vnic_groups': ["external"],
'destination_ip_address': []}]
@ -2995,6 +2997,7 @@ class TestExclusiveRouterTestCase(L3NatTest, L3NatTestCaseBase,
expected_cidrs = [s1_cidr, s2_cidr]
expected_fw = [{'action': 'allow',
'enabled': True,
'name': 'Subnet Rule',
'source_ip_address': expected_cidrs,
'destination_ip_address': expected_cidrs}]
fw_rules = mock.call_args[0][3]['firewall_rule_list']
@ -3030,6 +3033,7 @@ class TestExclusiveRouterTestCase(L3NatTest, L3NatTestCaseBase,
expected_cidrs = [s1_cidr, s2_cidr]
fw_rule = {'action': 'allow',
'enabled': True,
'name': 'Subnet Rule',
'source_ip_address': expected_cidrs,
'destination_ip_address': expected_cidrs}
vse_rule = {'action': 'allow',
@ -3150,6 +3154,7 @@ class TestExclusiveRouterTestCase(L3NatTest, L3NatTestCaseBase,
expected_cidrs = [s1_cidr, s2_cidr]
expected_fw = [{'action': 'allow',
'enabled': True,
'name': 'Subnet Rule',
'source_ip_address': expected_cidrs,
'destination_ip_address': expected_cidrs}]
fw_rules = mock.call_args[0][3]['firewall_rule_list']
@ -4495,18 +4500,22 @@ class TestSharedRouterTestCase(L3NatTest, L3NatTestCaseBase,
def test_routers_set_gateway_with_nosnat(self, mock):
expected_fw1 = [{'action': 'allow',
'enabled': True,
'name': 'Subnet Rule',
'source_ip_address': [],
'destination_ip_address': []}]
expected_fw2 = [{'action': 'allow',
'enabled': True,
'name': 'Subnet Rule',
'source_ip_address': [],
'destination_ip_address': []}]
nosnat_fw1 = [{'action': 'allow',
'enabled': True,
'name': 'No SNAT Rule',
'source_vnic_groups': ["external"],
'destination_ip_address': []}]
nosnat_fw2 = [{'action': 'allow',
'enabled': True,
'name': 'No SNAT Rule',
'source_vnic_groups': ["external"],
'destination_ip_address': []}]
with self.router() as r1, self.router() as r2,\