From 116e73ba9b9e86d26e4f10be6cfd436b46390143 Mon Sep 17 00:00:00 2001 From: Thomas Morin Date: Fri, 31 Aug 2018 12:34:39 +0200 Subject: [PATCH] ovs fw: apply the NORMAL action on egress traffic in a single table 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 --- .../internals/openvswitch_firewall.rst | 69 +++++++++++-------- .../linux/openvswitch_firewall/firewall.py | 23 +++++-- .../openvswitch/agent/common/constants.py | 2 + .../drivers/openvswitch/ovs_firewall_log.py | 3 +- .../services/logapi/test_logging.py | 25 ++++--- .../openvswitch/test_ovs_firewall_log.py | 6 +- 6 files changed, 80 insertions(+), 48 deletions(-) diff --git a/doc/source/contributor/internals/openvswitch_firewall.rst b/doc/source/contributor/internals/openvswitch_firewall.rst index 3b49c08f733..d27d9f3570e 100644 --- a/doc/source/contributor/internals/openvswitch_firewall.rst +++ b/doc/source/contributor/internals/openvswitch_firewall.rst @@ -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=131 actions=NORMAL - 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=135 actions=NORMAL - table=71, priority=95,icmp6,reg5=0x1,in_port=1,icmp_type=136 actions=NORMAL - 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=131 actions=NORMAL - 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=135 actions=NORMAL - table=71, priority=95,icmp6,reg5=0x2,in_port=2,icmp_type=136 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=resubmit(,94) + 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=resubmit(,94) + 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=resubmit(,94) + 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=resubmit(,94) + 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=resubmit(,94) 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:8c:84:13,arp_spa=10.0.0.1 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=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=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=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=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=resubmit(,94) DHCP and DHCPv6 traffic is allowed to instance but DHCP servers are blocked on 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=0x2 actions=NORMAL - 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=0x2 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=resubmit(,94) + 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=resubmit(,94) 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 @@ -307,7 +307,8 @@ rule anymore. In following ``table 73`` are all detected ingress connections sent to ingress 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=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=80,reg5=0x1 actions=NORMAL - table=73, priority=80,reg5=0x2 actions=NORMAL + table=73, priority=80,reg5=0x1 actions=resubmit(,94) + table=73, priority=80,reg5=0x2 actions=resubmit(,94) table=73, priority=0 actions=drop ``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 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`` -are considered accepted by the egress pipeline and won't be processed further. -``NORMAL`` action is used by default in this table. Packets sent to -``table 92`` were processed by the ingress filtering pipeline. 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. +are considered accepted by the egress pipeline, and will be processed so that +they are forwarded to their destination by being submitted to a NORMAL action +that results in Ethernet flood/learn processing. Note that ``table 91`` merely +resubmits to ``table 94``that contains the actual NORMAL action; this allows +to have``table 91`` be a single places where the NORMAL action can be overriden +by other components (currently used by ``networking-bagpipe`` driver for +``networking-bgpvpn``). -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``. +Packets sent to ``table 92`` were processed by the ingress filtering pipeline. +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``. Future work ----------- diff --git a/neutron/agent/linux/openvswitch_firewall/firewall.py b/neutron/agent/linux/openvswitch_firewall/firewall.py index 41456604918..a3c65db79c9 100644 --- a/neutron/agent/linux/openvswitch_firewall/firewall.py +++ b/neutron/agent/linux/openvswitch_firewall/firewall.py @@ -473,9 +473,15 @@ class OVSFirewallDriver(firewall.FirewallDriver): def _initialize_third_party_tables(self): self.int_br.br.add_flow( - table=ovs_consts.ACCEPTED_EGRESS_TRAFFIC_TABLE, + table=ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE, priority=1, 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, ovs_consts.DROPPED_TRAFFIC_TABLE): self.int_br.br.add_flow( @@ -709,7 +715,8 @@ class OVSFirewallDriver(firewall.FirewallDriver): dl_type=constants.ETHERTYPE_IPV6, nw_proto=lib_const.PROTO_NUM_IPV6_ICMP, 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): @@ -743,7 +750,8 @@ class OVSFirewallDriver(firewall.FirewallDriver): table=ovs_consts.ACCEPT_OR_INGRESS_TABLE, priority=80, 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): @@ -778,7 +786,8 @@ class OVSFirewallDriver(firewall.FirewallDriver): dl_src=mac_addr, dl_type=constants.ETHERTYPE_ARP, arp_spa=ip_addr, - actions='normal' + actions='resubmit(,%d)' % ( + ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE) ) self._add_flow( table=ovs_consts.BASE_EGRESS_TABLE, @@ -893,7 +902,8 @@ class OVSFirewallDriver(firewall.FirewallDriver): table=ovs_consts.ACCEPT_OR_INGRESS_TABLE, priority=80, reg_port=port.ofport, - actions='normal' + actions='resubmit(,%d)' % ( + ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE) ) def _initialize_tracked_egress(self, port): @@ -924,7 +934,8 @@ class OVSFirewallDriver(firewall.FirewallDriver): ct_mark=ovsfw_consts.CT_MARK_NORMAL, reg_port=port.ofport, ct_zone=port.vlan_tag, - actions='normal' + actions='resubmit(,%d)' % ( + ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE) ) self._add_flow( table=ovs_consts.RULES_EGRESS_TABLE, diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py b/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py index dced2ccefee..f7b83757d3c 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py @@ -76,6 +76,8 @@ ACCEPTED_EGRESS_TRAFFIC_TABLE = 91 ACCEPTED_INGRESS_TRAFFIC_TABLE = 92 DROPPED_TRAFFIC_TABLE = 93 +ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE = 94 + # --- Tunnel bridge (tun_br) # Various tables for tunneling flows diff --git a/neutron/services/logapi/drivers/openvswitch/ovs_firewall_log.py b/neutron/services/logapi/drivers/openvswitch/ovs_firewall_log.py index 430ac8ae6c7..b89f7e293a0 100644 --- a/neutron/services/logapi/drivers/openvswitch/ovs_firewall_log.py +++ b/neutron/services/logapi/drivers/openvswitch/ovs_firewall_log.py @@ -344,7 +344,8 @@ class OVSFirewallLoggingDriver(log_ext.LoggingDriver): flow['actions'] = 'controller' # forward egress accepted packet and log 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) def _add_flow(self, **kwargs): diff --git a/neutron/tests/functional/services/logapi/test_logging.py b/neutron/tests/functional/services/logapi/test_logging.py index b51d2ef7a0d..438adca36f7 100644 --- a/neutron/tests/functional/services/logapi/test_logging.py +++ b/neutron/tests/functional/services/logapi/test_logging.py @@ -100,11 +100,10 @@ class TestLoggingExtension(LoggingExtensionTestFramework): 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) pattern = re.compile( - r"^.* table=%s.* " - r"actions=(NORMAL,CONTROLLER:65535|CONTROLLER:65535)" % table + r"^.* table=%s.* actions=%s" % (table, actions) ) for flow in flows.splitlines(): if pattern.match(flow.strip()): @@ -113,19 +112,27 @@ class TestLoggingExtension(LoggingExtensionTestFramework): def _assert_logging_flows_set(self): 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( - table=ovs_consts.ACCEPTED_INGRESS_TRAFFIC_TABLE)) + table=ovs_consts.ACCEPTED_INGRESS_TRAFFIC_TABLE, + actions="CONTROLLER:65535")) 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): 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( - table=ovs_consts.ACCEPTED_INGRESS_TRAFFIC_TABLE)) + table=ovs_consts.ACCEPTED_INGRESS_TRAFFIC_TABLE, + actions="CONTROLLER:65535")) 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): sg_rules = [{'ethertype': constants.IPv4, diff --git a/neutron/tests/unit/services/logapi/drivers/openvswitch/test_ovs_firewall_log.py b/neutron/tests/unit/services/logapi/drivers/openvswitch/test_ovs_firewall_log.py index 84e3cfb01dd..406674f15d9 100644 --- a/neutron/tests/unit/services/logapi/drivers/openvswitch/test_ovs_firewall_log.py +++ b/neutron/tests/unit/services/logapi/drivers/openvswitch/test_ovs_firewall_log.py @@ -181,7 +181,8 @@ class TestOVSFirewallLoggingDriver(base.BaseTestCase): tcp_dst='0x007b'), # log egress tcp6 mock.call( - actions='normal,controller', + actions='resubmit(,%d),controller' % ( + ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE), cookie=accept_cookie.id, reg5=self.port_ofport, dl_type="0x{:04x}".format(n_const.ETHERTYPE_IPV6), @@ -190,7 +191,8 @@ class TestOVSFirewallLoggingDriver(base.BaseTestCase): table=ovs_consts.ACCEPTED_EGRESS_TRAFFIC_TABLE), # log egress udp mock.call( - actions='normal,controller', + actions='resubmit(,%d),controller' % ( + ovs_consts.ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE), cookie=accept_cookie.id, reg5=self.port_ofport, dl_type="0x{:04x}".format(n_const.ETHERTYPE_IP),