openstack/neutron
Code Issues Proposed changes

ovs fw: apply the NORMAL action on egress traffic in a single table

Browse Source 
This change is a follow-up to Ib6ced838a7ec6d5c459a8475318556001c31bdf,
reintroducing a single place for applying the NORMAL action to
egress traffic, which is necessary to fix a regression introduced
by Ib6ced838a7ec6d5c459a8475318556001c31bdf.

Change-Id: I60d299275effd9ef35c8007773d3c9fcabfa50fa
Partial-Bug: 1789878
This commit is contained in:
Thomas Morin 2018-08-31 12:34:39 +02:00 committed by Nguyen Phuong An
parent 381d1cc715
commit 9feb5db61c
6 changed files with 80 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
doc/source/contributor/internals/openvswitch_firewall.rst
View File

@ -207,25 +207,25 @@ solicitation and neighbour advertisement.
:: ::
table=71, priority=95,icmp6,reg5=0x1,in_port=1,icmp_type=130 actions=NORMAL table=71, priority=95,icmp6,reg5=0x1,in_port=1,icmp_type=130 actions=resubmit(,94)
table=71, priority=95,icmp6,reg5=0x1,in_port=1,icmp_type=131 actions=NORMAL table=71, priority=95,icmp6,reg5=0x1,in_port=1,icmp_type=131 actions=resubmit(,94)
table=71, priority=95,icmp6,reg5=0x1,in_port=1,icmp_type=132 actions=NORMAL table=71, priority=95,icmp6,reg5=0x1,in_port=1,icmp_type=132 actions=resubmit(,94)
table=71, priority=95,icmp6,reg5=0x1,in_port=1,icmp_type=135 actions=NORMAL table=71, priority=95,icmp6,reg5=0x1,in_port=1,icmp_type=135 actions=resubmit(,94)
table=71, priority=95,icmp6,reg5=0x1,in_port=1,icmp_type=136 actions=NORMAL table=71, priority=95,icmp6,reg5=0x1,in_port=1,icmp_type=136 actions=resubmit(,94)
table=71, priority=95,icmp6,reg5=0x2,in_port=2,icmp_type=130 actions=NORMAL table=71, priority=95,icmp6,reg5=0x2,in_port=2,icmp_type=130 actions=resubmit(,94)
table=71, priority=95,icmp6,reg5=0x2,in_port=2,icmp_type=131 actions=NORMAL table=71, priority=95,icmp6,reg5=0x2,in_port=2,icmp_type=131 actions=resubmit(,94)
table=71, priority=95,icmp6,reg5=0x2,in_port=2,icmp_type=132 actions=NORMAL table=71, priority=95,icmp6,reg5=0x2,in_port=2,icmp_type=132 actions=resubmit(,94)
table=71, priority=95,icmp6,reg5=0x2,in_port=2,icmp_type=135 actions=NORMAL table=71, priority=95,icmp6,reg5=0x2,in_port=2,icmp_type=135 actions=resubmit(,94)
table=71, priority=95,icmp6,reg5=0x2,in_port=2,icmp_type=136 actions=NORMAL table=71, priority=95,icmp6,reg5=0x2,in_port=2,icmp_type=136 actions=resubmit(,94)
Following rules implement arp spoofing protection Following rules implement arp spoofing protection
:: ::
table=71, priority=95,arp,reg5=0x1,in_port=1,dl_src=fa:16:3e:a4:22:10,arp_spa=192.168.0.1 actions=NORMAL table=71, priority=95,arp,reg5=0x1,in_port=1,dl_src=fa:16:3e:a4:22:10,arp_spa=192.168.0.1 actions=resubmit(,94)
table=71, priority=95,arp,reg5=0x1,in_port=1,dl_src=fa:16:3e:8c:84:13,arp_spa=10.0.0.1 actions=NORMAL table=71, priority=95,arp,reg5=0x1,in_port=1,dl_src=fa:16:3e:8c:84:13,arp_spa=10.0.0.1 actions=resubmit(,94)
table=71, priority=95,arp,reg5=0x2,in_port=2,dl_src=fa:16:3e:24:57:c7,arp_spa=192.168.0.2 actions=NORMAL table=71, priority=95,arp,reg5=0x2,in_port=2,dl_src=fa:16:3e:24:57:c7,arp_spa=192.168.0.2 actions=resubmit(,94)
table=71, priority=95,arp,reg5=0x2,in_port=2,dl_src=fa:16:3e:8c:84:14,arp_spa=10.1.0.0/24 actions=NORMAL table=71, priority=95,arp,reg5=0x2,in_port=2,dl_src=fa:16:3e:8c:84:14,arp_spa=10.1.0.0/24 actions=resubmit(,94)
DHCP and DHCPv6 traffic is allowed to instance but DHCP servers are blocked on DHCP and DHCPv6 traffic is allowed to instance but DHCP servers are blocked on
instances. instances.
@ -288,10 +288,10 @@ allowed.
:: ::
table=72, priority=50,ct_state=+est-rel+rpl,ct_zone=644,ct_mark=0,reg5=0x1 actions=NORMAL table=72, priority=50,ct_state=+est-rel+rpl,ct_zone=644,ct_mark=0,reg5=0x1 actions=resubmit(,94)
table=72, priority=50,ct_state=+est-rel+rpl,ct_zone=644,ct_mark=0,reg5=0x2 actions=NORMAL table=72, priority=50,ct_state=+est-rel+rpl,ct_zone=644,ct_mark=0,reg5=0x2 actions=resubmit(,94)
table=72, priority=50,ct_state=-new-est+rel-inv,ct_zone=644,ct_mark=0,reg5=0x1 actions=NORMAL table=72, priority=50,ct_state=-new-est+rel-inv,ct_zone=644,ct_mark=0,reg5=0x1 actions=resubmit(,94)
table=72, priority=50,ct_state=-new-est+rel-inv,ct_zone=644,ct_mark=0,reg5=0x2 actions=NORMAL table=72, priority=50,ct_state=-new-est+rel-inv,ct_zone=644,ct_mark=0,reg5=0x2 actions=resubmit(,94)
In the following flows are marked established connections that weren't matched In the following flows are marked established connections that weren't matched
in the previous flows, which means they don't have accepting security group in the previous flows, which means they don't have accepting security group
@ -307,7 +307,8 @@ rule anymore.
In following ``table 73`` are all detected ingress connections sent to ingress In following ``table 73`` are all detected ingress connections sent to ingress
pipeline. Since the connection was already accepted by egress pipeline, all pipeline. Since the connection was already accepted by egress pipeline, all
remaining egress connections are sent to normal switching. remaining egress connections are sent to normal flood'n'learn switching
(table 94).
:: ::
@ -317,8 +318,8 @@ remaining egress connections are sent to normal switching.
table=73, priority=100,reg6=0x284,dl_dst=fa:16:3e:8c:84:14 actions=load:0x2->NXM_NX_REG5[],resubmit(,81) table=73, priority=100,reg6=0x284,dl_dst=fa:16:3e:8c:84:14 actions=load:0x2->NXM_NX_REG5[],resubmit(,81)
table=73, priority=90,ct_state=+new-est,reg5=0x1 actions=ct(commit,zone=NXM_NX_REG6[0..15]),resubmit(,91) table=73, priority=90,ct_state=+new-est,reg5=0x1 actions=ct(commit,zone=NXM_NX_REG6[0..15]),resubmit(,91)
table=73, priority=90,ct_state=+new-est,reg5=0x2 actions=ct(commit,zone=NXM_NX_REG6[0..15]),resubmit(,91) table=73, priority=90,ct_state=+new-est,reg5=0x2 actions=ct(commit,zone=NXM_NX_REG6[0..15]),resubmit(,91)
table=73, priority=80,reg5=0x1 actions=NORMAL table=73, priority=80,reg5=0x1 actions=resubmit(,94)
table=73, priority=80,reg5=0x2 actions=NORMAL table=73, priority=80,reg5=0x2 actions=resubmit(,94)
table=73, priority=0 actions=drop table=73, priority=0 actions=drop
``table 81`` is similar to ``table 71``, allows basic ingress traffic for ``table 81`` is similar to ``table 71``, allows basic ingress traffic for
@ -460,16 +461,24 @@ Using OpenFlow in conjunction with OVS firewall
There are three tables where packets are sent once they get through the OVS There are three tables where packets are sent once they get through the OVS
firewall pipeline. The tables can be used by other mechanisms using OpenFlow firewall pipeline. The tables can be used by other mechanisms using OpenFlow
that are supposed to work with the OVS firewall. Packets sent to ``table 91`` that are supposed to work with the OVS firewall. Packets sent to ``table 91``
are considered accepted by the egress pipeline and won't be processed further. are considered accepted by the egress pipeline, and will be processed so that
``NORMAL`` action is used by default in this table. Packets sent to they are forwarded to their destination by being submitted to a NORMAL action
``table 92`` were processed by the ingress filtering pipeline. As packets from that results in Ethernet flood/learn processing. Note that ``table 91`` merely
the ingress filtering pipeline were injected to its destination, ``table 92`` resubmits to ``table 94``that contains the actual NORMAL action; this allows
receives copies of those packets and therefore default action is ``drop``. to have``table 91`` be a single places where the NORMAL action can be overriden
Finally, packets sent to ``table 93`` were filtered by the firewall and should by other components (currently used by ``networking-bagpipe`` driver for
be dropped. Default action is ``drop`` in this table. ``networking-bgpvpn``).
In regard to the performance perspective, please note that only the first accepted Packets sent to ``table 92`` were processed by the ingress filtering pipeline.
packet of each connection session will go to ``table 91`` and ``table 92``. As packets from the ingress filtering pipeline were injected to its
destination, ``table 92`` receives copies of those packets and therefore
default action is ``drop``. Finally, packets sent to ``table 93`` were filtered
by the firewall and should be dropped. Default action is ``drop`` in this
table.
In regard to the performance perspective, please note that only the first
accepted packet of each connection session will go to ``table 91`` and
``table 92``.
Upgrade path from iptables hybrid driver Upgrade path from iptables hybrid driver

23
neutron/agent/linux/openvswitch_firewall/firewall.py
View File

@ -473,9 +473,15 @@ class OVSFirewallDriver(firewall.FirewallDriver):
def _initialize_third_party_tables(self): def _initialize_third_party_tables(self):
self.int_br.br.add_flow( self.int_br.br.add_flow(
table=ovs_consts.ACCEPTED_EGRESS_TRAFFIC_TABLE, table=ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE,
priority=1, priority=1,
actions='normal') actions='normal')
self.int_br.br.add_flow(
table=ovs_consts.ACCEPTED_EGRESS_TRAFFIC_TABLE,
priority=1,
actions='resubmit(,%d)' % (
ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE)
)
for table in (ovs_consts.ACCEPTED_INGRESS_TRAFFIC_TABLE, for table in (ovs_consts.ACCEPTED_INGRESS_TRAFFIC_TABLE,
ovs_consts.DROPPED_TRAFFIC_TABLE): ovs_consts.DROPPED_TRAFFIC_TABLE):
self.int_br.br.add_flow( self.int_br.br.add_flow(
@ -709,7 +715,8 @@ class OVSFirewallDriver(firewall.FirewallDriver):
dl_type=constants.ETHERTYPE_IPV6, dl_type=constants.ETHERTYPE_IPV6,
nw_proto=lib_const.PROTO_NUM_IPV6_ICMP, nw_proto=lib_const.PROTO_NUM_IPV6_ICMP,
icmp_type=icmp_type, icmp_type=icmp_type,
actions='normal' actions='resubmit(,%d)' % (
ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE)
) )
def _initialize_egress_no_port_security(self, port_id): def _initialize_egress_no_port_security(self, port_id):
@ -743,7 +750,8 @@ class OVSFirewallDriver(firewall.FirewallDriver):
table=ovs_consts.ACCEPT_OR_INGRESS_TABLE, table=ovs_consts.ACCEPT_OR_INGRESS_TABLE,
priority=80, priority=80,
reg_port=ovs_port.ofport, reg_port=ovs_port.ofport,
actions='normal' actions='resubmit(,%d)' % (
ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE)
) )
def _remove_egress_no_port_security(self, port_id): def _remove_egress_no_port_security(self, port_id):
@ -778,7 +786,8 @@ class OVSFirewallDriver(firewall.FirewallDriver):
dl_src=mac_addr, dl_src=mac_addr,
dl_type=constants.ETHERTYPE_ARP, dl_type=constants.ETHERTYPE_ARP,
arp_spa=ip_addr, arp_spa=ip_addr,
actions='normal' actions='resubmit(,%d)' % (
ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE)
) )
self._add_flow( self._add_flow(
table=ovs_consts.BASE_EGRESS_TABLE, table=ovs_consts.BASE_EGRESS_TABLE,
@ -893,7 +902,8 @@ class OVSFirewallDriver(firewall.FirewallDriver):
table=ovs_consts.ACCEPT_OR_INGRESS_TABLE, table=ovs_consts.ACCEPT_OR_INGRESS_TABLE,
priority=80, priority=80,
reg_port=port.ofport, reg_port=port.ofport,
actions='normal' actions='resubmit(,%d)' % (
ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE)
) )
def _initialize_tracked_egress(self, port): def _initialize_tracked_egress(self, port):
@ -924,7 +934,8 @@ class OVSFirewallDriver(firewall.FirewallDriver):
ct_mark=ovsfw_consts.CT_MARK_NORMAL, ct_mark=ovsfw_consts.CT_MARK_NORMAL,
reg_port=port.ofport, reg_port=port.ofport,
ct_zone=port.vlan_tag, ct_zone=port.vlan_tag,
actions='normal' actions='resubmit(,%d)' % (
ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE)
) )
self._add_flow( self._add_flow(
table=ovs_consts.RULES_EGRESS_TABLE, table=ovs_consts.RULES_EGRESS_TABLE,

2
neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py
View File

@ -76,6 +76,8 @@ ACCEPTED_EGRESS_TRAFFIC_TABLE = 91
ACCEPTED_INGRESS_TRAFFIC_TABLE = 92 ACCEPTED_INGRESS_TRAFFIC_TABLE = 92
DROPPED_TRAFFIC_TABLE = 93 DROPPED_TRAFFIC_TABLE = 93
ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE = 94
# --- Tunnel bridge (tun_br) # --- Tunnel bridge (tun_br)
# Various tables for tunneling flows # Various tables for tunneling flows

3
neutron/services/logapi/drivers/openvswitch/ovs_firewall_log.py
View File

@ -344,7 +344,8 @@ class OVSFirewallLoggingDriver(log_ext.LoggingDriver):
flow['actions'] = 'controller' flow['actions'] = 'controller'
# forward egress accepted packet and log # forward egress accepted packet and log
if flow['table'] == ovs_consts.ACCEPTED_EGRESS_TRAFFIC_TABLE: if flow['table'] == ovs_consts.ACCEPTED_EGRESS_TRAFFIC_TABLE:
flow['actions'] = 'normal,controller' flow['actions'] = 'resubmit(,%d),controller' % (
ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE)
self._add_flow(**flow) self._add_flow(**flow)
def _add_flow(self, **kwargs): def _add_flow(self, **kwargs):

25
neutron/tests/functional/services/logapi/test_logging.py
View File

@ -100,11 +100,10 @@ class TestLoggingExtension(LoggingExtensionTestFramework):
ip_cidr = '192.168.0.1/24' ip_cidr = '192.168.0.1/24'
def _is_log_flow_set(self, table): def _is_log_flow_set(self, table, actions):
flows = self.log_driver.int_br.br.dump_flows_for_table(table) flows = self.log_driver.int_br.br.dump_flows_for_table(table)
pattern = re.compile( pattern = re.compile(
r"^.* table=%s.* " r"^.* table=%s.* actions=%s" % (table, actions)
r"actions=(NORMAL,CONTROLLER:65535|CONTROLLER:65535)" % table
) )
for flow in flows.splitlines(): for flow in flows.splitlines():
if pattern.match(flow.strip()): if pattern.match(flow.strip()):
@ -113,19 +112,27 @@ class TestLoggingExtension(LoggingExtensionTestFramework):
def _assert_logging_flows_set(self): def _assert_logging_flows_set(self):
self.assertTrue(self._is_log_flow_set( self.assertTrue(self._is_log_flow_set(
table=ovs_consts.ACCEPTED_EGRESS_TRAFFIC_TABLE)) table=ovs_consts.ACCEPTED_EGRESS_TRAFFIC_TABLE,
actions=r"resubmit\(,%d\),CONTROLLER:65535" % (
ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE)))
self.assertTrue(self._is_log_flow_set( self.assertTrue(self._is_log_flow_set(
table=ovs_consts.ACCEPTED_INGRESS_TRAFFIC_TABLE)) table=ovs_consts.ACCEPTED_INGRESS_TRAFFIC_TABLE,
actions="CONTROLLER:65535"))
self.assertTrue(self._is_log_flow_set( self.assertTrue(self._is_log_flow_set(
table=ovs_consts.DROPPED_TRAFFIC_TABLE)) table=ovs_consts.DROPPED_TRAFFIC_TABLE,
actions="CONTROLLER:65535"))
def _assert_logging_flows_not_set(self): def _assert_logging_flows_not_set(self):
self.assertFalse(self._is_log_flow_set( self.assertFalse(self._is_log_flow_set(
table=ovs_consts.ACCEPTED_EGRESS_TRAFFIC_TABLE)) table=ovs_consts.ACCEPTED_EGRESS_TRAFFIC_TABLE,
actions=r"resubmit\(,%d\),CONTROLLER:65535" % (
ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE)))
self.assertFalse(self._is_log_flow_set( self.assertFalse(self._is_log_flow_set(
table=ovs_consts.ACCEPTED_INGRESS_TRAFFIC_TABLE)) table=ovs_consts.ACCEPTED_INGRESS_TRAFFIC_TABLE,
actions="CONTROLLER:65535"))
self.assertFalse(self._is_log_flow_set( self.assertFalse(self._is_log_flow_set(
table=ovs_consts.DROPPED_TRAFFIC_TABLE)) table=ovs_consts.DROPPED_TRAFFIC_TABLE,
actions="CONTROLLER:65535"))
def test_log_lifecycle(self): def test_log_lifecycle(self):
sg_rules = [{'ethertype': constants.IPv4, sg_rules = [{'ethertype': constants.IPv4,

6
neutron/tests/unit/services/logapi/drivers/openvswitch/test_ovs_firewall_log.py
View File

@ -181,7 +181,8 @@ class TestOVSFirewallLoggingDriver(base.BaseTestCase):
tcp_dst='0x007b'), tcp_dst='0x007b'),
# log egress tcp6 # log egress tcp6
mock.call( mock.call(
actions='normal,controller', actions='resubmit(,%d),controller' % (
ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE),
cookie=accept_cookie.id, cookie=accept_cookie.id,
reg5=self.port_ofport, reg5=self.port_ofport,
dl_type="0x{:04x}".format(n_const.ETHERTYPE_IPV6), dl_type="0x{:04x}".format(n_const.ETHERTYPE_IPV6),
@ -190,7 +191,8 @@ class TestOVSFirewallLoggingDriver(base.BaseTestCase):
table=ovs_consts.ACCEPTED_EGRESS_TRAFFIC_TABLE), table=ovs_consts.ACCEPTED_EGRESS_TRAFFIC_TABLE),
# log egress udp # log egress udp
mock.call( mock.call(
actions='normal,controller', actions='resubmit(,%d),controller' % (
ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE),
cookie=accept_cookie.id, cookie=accept_cookie.id,
reg5=self.port_ofport, reg5=self.port_ofport,
dl_type="0x{:04x}".format(n_const.ETHERTYPE_IP), dl_type="0x{:04x}".format(n_const.ETHERTYPE_IP),