diff --git a/doc/source/devref/openvswitch_firewall.rst b/doc/source/devref/openvswitch_firewall.rst index 2d49c4f11e8..2ebccd99239 100644 --- a/doc/source/devref/openvswitch_firewall.rst +++ b/doc/source/devref/openvswitch_firewall.rst @@ -30,6 +30,10 @@ Class ``SGPortMap`` was created to keep state consistent, and maps from ports to security groups and vice-versa. Every port and security group is represented by its own object encapsulating the necessary information. +Note: Open vSwitch firewall driver uses register 5 for marking flow +related to port and register 6 which defines network and is used for conntrack +zones. + Firewall API calls ------------------ @@ -86,53 +90,217 @@ then expanded into several OpenFlow rules by the method Rules example with explanation: ------------------------------- -TODO: Rules below will be awesomly explained +The following example presents two ports on the same host. They have different +security groups and there is icmp traffic allowed from first security group to +the second security group. Ports have following attributes: :: - table=0, priority=100,in_port=2 actions=load:0x2->NXM_NX_REG5[],resubmit(,71) - table=0, priority=100,in_port=1 actions=load:0x1->NXM_NX_REG5[],resubmit(,71) - table=0, priority=90,dl_dst=fa:16:3e:9b:67:b2 actions=load:0x2->NXM_NX_REG5[],resubmit(,81) - table=0, priority=90,dl_dst=fa:16:3e:44:de:7a actions=load:0x1->NXM_NX_REG5[],resubmit(,81) - table=0, priority=0 actions=NORMAL - table=0, priority=1 actions=NORMAL - table=71, priority=95,arp,in_port=2,dl_src=fa:16:3e:9b:67:b2,arp_spa=192.168.0.2 actions=NORMAL - table=71, priority=95,arp,in_port=1,dl_src=fa:16:3e:44:de:7a,arp_spa=192.168.0.1 actions=NORMAL - table=71, priority=90,ct_state=-trk,in_port=2,dl_src=fa:16:3e:9b:67:b2 actions=ct(table=72,zone=NXM_NX_REG5[0..15]) - table=71, priority=90,ct_state=-trk,in_port=1,dl_src=fa:16:3e:44:de:7a actions=ct(table=72,zone=NXM_NX_REG5[0..15]) - table=71, priority=70,udp,in_port=2,tp_src=68,tp_dst=67 actions=NORMAL - table=71, priority=70,udp6,in_port=2,tp_src=546,tp_dst=547 actions=NORMAL - table=71, priority=60,udp,in_port=2,tp_src=67,tp_dst=68 actions=drop - table=71, priority=60,udp6,in_port=2,tp_src=547,tp_dst=546 actions=drop - table=71, priority=70,udp,in_port=1,tp_src=68,tp_dst=67 actions=NORMAL - table=71, priority=70,udp6,in_port=1,tp_src=546,tp_dst=547 actions=NORMAL - table=71, priority=60,udp,in_port=1,tp_src=67,tp_dst=68 actions=drop - table=71, priority=60,udp6,in_port=1,tp_src=547,tp_dst=546 actions=drop - table=71, priority=10,ct_state=-trk,in_port=2 actions=drop - table=71, priority=10,ct_state=-trk,in_port=1 actions=drop + Port 1 + - plugged to the port 1 in OVS bridge + - ip address: 192.168.0.1 + - mac address: fa:16:3e:a4:22:10 + - security group 1: can send icmp packets out + + Port 2 + - plugged to the port 2 in OVS bridge + - ip address: 192.168.0.2 + - mac address: fa:16:3e:24:57:c7 + - security group 2: can receive icmp packets from security group 1 + +The first ``table 0`` distinguishes the traffic to ingress or egress and loads +to ``register 5`` value identifying port traffic. +Ingress flow is determined by switch port number and egress flow is determined +by destination mac address. ``register 6`` contains + +:: + + table=0, priority=100,in_port=1 actions=load:0x1->NXM_NX_REG5[],load:0x284->NXM_NX_REG6[],resubmit(,71) + table=0, priority=100,in_port=2 actions=load:0x2->NXM_NX_REG5[],load:0x284->NXM_NX_REG6[],resubmit(,71) + table=0, priority=90,dl_dst=fa:16:3e:a4:22:10 actions=load:0x1->NXM_NX_REG5[],load:0x284->NXM_NX_REG6[],resubmit(,81) + table=0, priority=90,dl_dst=fa:16:3e:24:57:c7 actions=load:0x2->NXM_NX_REG5[],load:0x284->NXM_NX_REG6[],resubmit(,81) + table=0, priority=0 actions=NORMAL + +:: + +Following ``table 71`` implements arp spoofing protection, ip spoofing +protection, allows traffic for obtaining ip addresses (dhcp, dhcpv6, slaac, +ndp) for egress traffic and allows arp replies. Also identifies not tracked +connections which are processed later with information obtained from +conntrack. Notice the ``zone=NXM_NX_REG6[0..15]`` in ``actions`` when obtaining +information from conntrack. It says every port has its own conntrack zone +defined by value in ``register 6``. It's there to avoid accepting established +traffic that belongs to different port with same conntrack parameters. + +Rules below allow ICMPv6 traffic for multicast listeners, neighbour +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 + +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=0x2,in_port=2,dl_src=fa:16:3e:24:57:c7,arp_spa=192.168.0.2 actions=NORMAL + +DHCP and DHCPv6 traffic is allowed to instance but DHCP servers are blocked on +instances. + +:: + + table=71, priority=80,udp,reg5=0x1,in_port=1,tp_src=68,tp_dst=67 actions=resubmit(,73) + table=71, priority=80,udp6,reg5=0x1,in_port=1,tp_src=546,tp_dst=547 actions=resubmit(,73) + table=71, priority=70,udp,reg5=0x1,in_port=1,tp_src=67,tp_dst=68 actions=drop + table=71, priority=70,udp6,reg5=0x1,in_port=1,tp_src=547,tp_dst=546 actions=drop + table=71, priority=80,udp,reg5=0x2,in_port=2,tp_src=68,tp_dst=67 actions=resubmit(,73) + table=71, priority=80,udp6,reg5=0x2,in_port=2,tp_src=546,tp_dst=547 actions=resubmit(,73) + table=71, priority=70,udp,reg5=0x2,in_port=2,tp_src=67,tp_dst=68 actions=drop + table=71, priority=70,udp6,reg5=0x2,in_port=2,tp_src=547,tp_dst=546 actions=drop + +Flowing rules obtain conntrack information for valid ip and mac address +combinations. All other packets are dropped. + +:: + + table=71, priority=65,ct_state=-trk,ip,reg5=0x1,in_port=1,dl_src=fa:16:3e:a4:22:10,nw_src=192.168.0.1 actions=ct(table=72,zone=NXM_NX_REG6[0..15]) + table=71, priority=65,ct_state=-trk,ip,reg5=0x2,in_port=2,dl_src=fa:16:3e:24:57:c7,nw_src=192.168.0.2 actions=ct(table=72,zone=NXM_NX_REG6[0..15]) + table=71, priority=65,ct_state=-trk,ipv6,reg5=0x1,in_port=1,dl_src=fa:16:3e:a4:22:10,ipv6_src=fe80::f816:3eff:fea4:2210 actions=ct(table=72,zone=NXM_NX_REG6[0..15]) + table=71, priority=65,ct_state=-trk,ipv6,reg5=0x2,in_port=2,dl_src=fa:16:3e:24:57:c7,ipv6_src=fe80::f816:3eff:fe24:57c7 actions=ct(table=72,zone=NXM_NX_REG6[0..15]) + table=71, priority=10,ct_state=-trk,reg5=0x1,in_port=1 actions=drop + table=71, priority=10,ct_state=-trk,reg5=0x2,in_port=2 actions=drop table=71, priority=0 actions=drop - table=72, priority=90,ct_state=+inv+trk actions=drop - table=72, priority=80,ct_state=+est-rel-inv+trk actions=NORMAL - table=72, priority=80,ct_state=-est+rel-inv+trk actions=NORMAL - table=72, priority=70,icmp,dl_src=fa:16:3e:44:de:7a,nw_src=192.168.0.1 actions=resubmit(,73) + + +``table 72`` accepts only established or related connections, and implements +rules defined by the security group. As this egress connection might also be an +ingress connection for some other port, it's not switched yet but eventually +processed by ingress pipeline. + +All established or new connections defined by security group rule are +``accepted``, which will be explained later. All invalid packets are dropped. +In case below we allow all icmp egress traffic. + +:: + + table=72, priority=70,ct_state=+est-rel-rpl,icmp,reg5=0x1,dl_src=fa:16:3e:a4:22:10 actions=resubmit(,73) + table=72, priority=70,ct_state=+new-est,icmp,reg5=0x1,dl_src=fa:16:3e:a4:22:10 actions=resubmit(,73) + table=72, priority=50,ct_state=+inv+trk actions=drop + + +Important on the flows below is the ``ct_mark=0x1``. Such value have flows that +were marked as not existing anymore by rule introduced later. Those are +typically connections that were allowed by some security group rule and the +rule was removed. + +:: + + table=72, priority=50,ct_mark=0x1,reg5=0x1 actions=drop + table=72, priority=50,ct_mark=0x1,reg5=0x2 actions=drop + +All other connections that are not marked and are established or related are +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 + +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 +rule anymore. + +:: + + table=72, priority=40,ct_state=-est,reg5=0x1 actions=drop + table=72, priority=40,ct_state=+est,reg5=0x1 actions=ct(commit,zone=NXM_NX_REG6[0..15],exec(load:0x1->NXM_NX_CT_MARK[])) + table=72, priority=40,ct_state=-est,reg5=0x2 actions=drop + table=72, priority=40,ct_state=+est,reg5=0x2 actions=ct(commit,zone=NXM_NX_REG6[0..15],exec(load:0x1->NXM_NX_CT_MARK[])) table=72, priority=0 actions=drop - table=73, priority=100,dl_dst=fa:16:3e:9b:67:b2 actions=resubmit(,81) - table=73, priority=100,dl_dst=fa:16:3e:44:de:7a actions=resubmit(,81) - table=73, priority=90,in_port=2 actions=ct(commit,zone=NXM_NX_REG5[0..15]) - table=73, priority=90,in_port=1 actions=ct(commit,zone=NXM_NX_REG5[0..15]) - table=81, priority=100,arp,dl_dst=fa:16:3e:9b:67:b2 actions=output:2 - table=81, priority=100,arp,dl_dst=fa:16:3e:44:de:7a actions=output:1 - table=81, priority=95,ct_state=-trk,ip actions=ct(table=82,zone=NXM_NX_REG5[0..15]) - table=81, priority=95,ct_state=-trk,ipv6 actions=ct(table=82,zone=NXM_NX_REG5[0..15]) - table=81, priority=80,dl_dst=fa:16:3e:9b:67:b2 actions=resubmit(,82) - table=81, priority=80,dl_dst=fa:16:3e:44:de:7a actions=resubmit(,82) + +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. + +:: + + table=73, priority=100,dl_dst=fa:16:3e:a4:22:10 actions=load:0x1->NXM_NX_REG5[],resubmit(,81) + table=73, priority=100,dl_dst=fa:16:3e:24:57:c7 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]),NORMAL + table=73, priority=90,ct_state=+new-est,reg5=0x2 actions=ct(commit,zone=NXM_NX_REG6[0..15]),NORMAL + table=73, priority=80,reg5=0x1 actions=NORMAL + table=73, priority=80,reg5=0x2 actions=NORMAL + table=73, priority=0 actions=drop + +``table 81`` is similar to ``table 71``, allows basic ingress traffic for +obtaining ip address and arp queries. Not tracked packets are sent to obtain +conntrack information. + +:: + + table=81, priority=100,arp,reg5=0x1,dl_dst=fa:16:3e:a4:22:10 actions=output:1 + table=81, priority=100,arp,reg5=0x2,dl_dst=fa:16:3e:24:57:c7 actions=output:2 + table=81, priority=100,icmp6,reg5=0x1,dl_dst=fa:16:3e:a4:22:10,icmp_type=130 actions=output:1 + table=81, priority=100,icmp6,reg5=0x1,dl_dst=fa:16:3e:a4:22:10,icmp_type=131 actions=output:1 + table=81, priority=100,icmp6,reg5=0x1,dl_dst=fa:16:3e:a4:22:10,icmp_type=132 actions=output:1 + table=81, priority=100,icmp6,reg5=0x1,dl_dst=fa:16:3e:a4:22:10,icmp_type=135 actions=output:1 + table=81, priority=100,icmp6,reg5=0x1,dl_dst=fa:16:3e:a4:22:10,icmp_type=136 actions=output:1 + table=81, priority=100,icmp6,reg5=0x2,dl_dst=fa:16:3e:24:57:c7,icmp_type=130 actions=output:2 + table=81, priority=100,icmp6,reg5=0x2,dl_dst=fa:16:3e:24:57:c7,icmp_type=131 actions=output:2 + table=81, priority=100,icmp6,reg5=0x2,dl_dst=fa:16:3e:24:57:c7,icmp_type=132 actions=output:2 + table=81, priority=100,icmp6,reg5=0x2,dl_dst=fa:16:3e:24:57:c7,icmp_type=135 actions=output:2 + table=81, priority=100,icmp6,reg5=0x2,dl_dst=fa:16:3e:24:57:c7,icmp_type=136 actions=output:2 + table=81, priority=95,udp,reg5=0x1,tp_src=67,tp_dst=68 actions=output:1 + table=81, priority=95,udp6,reg5=0x1,tp_src=547,tp_dst=546 actions=output:1 + table=81, priority=95,udp,reg5=0x2,tp_src=67,tp_dst=68 actions=output:2 + table=81, priority=95,udp6,reg5=0x2,tp_src=547,tp_dst=546 actions=output:2 + table=81, priority=90,ct_state=-trk,ip,reg5=0x1 actions=ct(table=82,zone=NXM_NX_REG6[0..15]) + table=81, priority=90,ct_state=-trk,ipv6,reg5=0x1 actions=ct(table=82,zone=NXM_NX_REG6[0..15]) + table=81, priority=90,ct_state=-trk,ip,reg5=0x2 actions=ct(table=82,zone=NXM_NX_REG6[0..15]) + table=81, priority=90,ct_state=-trk,ipv6,reg5=0x2 actions=ct(table=82,zone=NXM_NX_REG6[0..15]) + table=81, priority=80,ct_state=+trk,reg5=0x1,dl_dst=fa:16:3e:a4:22:10 actions=resubmit(,82) + table=81, priority=80,ct_state=+trk,reg5=0x2,dl_dst=fa:16:3e:24:57:c7 actions=resubmit(,82) table=81, priority=0 actions=drop - table=82, priority=100,ct_state=+inv+trk actions=drop - table=82, priority=80,ct_state=+est-rel-inv+trk,dl_dst=fa:16:3e:44:de:7a actions=output:1 - table=82, priority=80,ct_state=-est+rel-inv+trk,dl_dst=fa:16:3e:44:de:7a actions=output:1 - table=82, priority=80,ct_state=+est-rel-inv+trk,dl_dst=fa:16:3e:9b:67:b2 actions=output:2 - table=82, priority=80,ct_state=-est+rel-inv+trk,dl_dst=fa:16:3e:9b:67:b2 actions=output:2 - table=82, priority=70,icmp,dl_dst=fa:16:3e:9b:67:b2,nw_src=192.168.0.1,nw_dst=192.168.0.2 actions=ct(commit,zone=NXM_NX_REG5[0..15]),output:2 + +Similarly to ``table 72``, ``table 82`` accepts established and related +connections. In this case we allow all icmp traffic coming from +``security group 1`` which is in this case only ``port 1`` with ip address +``192.168.0.1``. + +:: + + table=82, priority=70,ct_state=+est-rel-rpl,icmp,reg5=0x2,dl_dst=fa:16:3e:24:57:c7,nw_src=192.168.0.1 actions=output:2 + table=82, priority=70,ct_state=+new-est,icmp,reg5=0x2,dl_dst=fa:16:3e:24:57:c7,nw_src=192.168.0.1 actions=output:2,ct(commit,zone=NXM_NX_REG6[0..15]) + table=82, priority=50,ct_state=+inv+trk actions=drop + +The mechanism for dropping connections that are not allowed anymore is the +same as in ``table 72``. + +:: + + table=82, priority=50,ct_mark=0x1,reg5=0x1 actions=drop + table=82, priority=50,ct_mark=0x1,reg5=0x2 actions=drop + table=82, priority=50,ct_state=+est-rel+rpl,ct_zone=644,ct_mark=0,reg5=0x1,dl_dst=fa:16:3e:a4:22:10 actions=output:1 + table=82, priority=50,ct_state=+est-rel+rpl,ct_zone=644,ct_mark=0,reg5=0x2,dl_dst=fa:16:3e:24:57:c7 actions=output:2 + table=82, priority=50,ct_state=-new-est+rel-inv,ct_zone=644,ct_mark=0,reg5=0x1,dl_dst=fa:16:3e:a4:22:10 actions=output:1 + table=82, priority=50,ct_state=-new-est+rel-inv,ct_zone=644,ct_mark=0,reg5=0x2,dl_dst=fa:16:3e:24:57:c7 actions=output:2 + table=82, priority=40,ct_state=-est,reg5=0x1 actions=drop + table=82, priority=40,ct_state=+est,reg5=0x1 actions=ct(commit,zone=NXM_NX_REG6[0..15],exec(load:0x1->NXM_NX_CT_MARK[])) + table=82, priority=40,ct_state=-est,reg5=0x2 actions=drop + table=82, priority=40,ct_state=+est,reg5=0x2 actions=ct(commit,zone=NXM_NX_REG6[0..15],exec(load:0x1->NXM_NX_CT_MARK[])) table=82, priority=0 actions=drop diff --git a/neutron/agent/linux/openvswitch_firewall/constants.py b/neutron/agent/linux/openvswitch_firewall/constants.py index 347db037a5f..f34578ec455 100644 --- a/neutron/agent/linux/openvswitch_firewall/constants.py +++ b/neutron/agent/linux/openvswitch_firewall/constants.py @@ -16,9 +16,22 @@ from neutron.common import constants OF_STATE_NOT_TRACKED = "-trk" -OF_STATE_ESTABLISHED = "+trk+est-rel-inv" -OF_STATE_RELATED = "+trk+rel-est-inv" +OF_STATE_TRACKED = "+trk" +OF_STATE_NEW_NOT_ESTABLISHED = "+new-est" +OF_STATE_NOT_ESTABLISHED = "-est" +OF_STATE_ESTABLISHED = "+est" +OF_STATE_ESTABLISHED_NOT_REPLY = "+est-rel-rpl" +OF_STATE_ESTABLISHED_REPLY = "+est-rel+rpl" +OF_STATE_RELATED = "-new-est+rel-inv" OF_STATE_INVALID = "+trk+inv" +OF_STATE_NEW = "+new" +OF_STATE_NOT_REPLY_NOT_NEW = "-new-rpl" + +CT_MARK_NORMAL = '0x0' +CT_MARK_INVALID = '0x1' + +REG_PORT = 5 +REG_NET = 6 protocol_to_nw_proto = { constants.PROTO_NAME_ICMP: constants.PROTO_NUM_ICMP, diff --git a/neutron/agent/linux/openvswitch_firewall/firewall.py b/neutron/agent/linux/openvswitch_firewall/firewall.py index 03f33d21d2b..b3ff55c8ae6 100644 --- a/neutron/agent/linux/openvswitch_firewall/firewall.py +++ b/neutron/agent/linux/openvswitch_firewall/firewall.py @@ -29,6 +29,31 @@ from neutron.plugins.ml2.drivers.openvswitch.agent.common import constants \ LOG = logging.getLogger(__name__) +def _replace_register(flow_params, register_number, register_value): + """Replace value from flows to given register number + + 'register_value' key in dictionary will be replaced by register number + given by 'register_number' + + :param flow_params: Dictionary containing defined flows + :param register_number: The number of register where value will be stored + :param register_value: Key to be replaced by register number + + """ + try: + reg_port = flow_params[register_value] + del flow_params[register_value] + flow_params['reg{:d}'.format(register_number)] = reg_port + except KeyError: + pass + + +def create_reg_numbers(flow_params): + """Replace reg_(port|net) values with defined register numbers""" + _replace_register(flow_params, ovsfw_consts.REG_PORT, 'reg_port') + _replace_register(flow_params, ovsfw_consts.REG_NET, 'reg_net') + + class OVSFWPortNotFound(exceptions.NeutronException): message = _("Port %(port_id)s is not managed by this agent. ") @@ -56,8 +81,9 @@ class SecurityGroup(object): class OFPort(object): - def __init__(self, port_dict, ovs_port): + def __init__(self, port_dict, ovs_port, vlan_tag): self.id = port_dict['device'] + self.vlan_tag = vlan_tag self.mac = ovs_port.vif_mac self.lla_address = str(ipv6_utils.get_ipv6_addr_by_EUI64( constants.IPV6_LLA_PREFIX, self.mac)) @@ -173,8 +199,18 @@ class OVSFirewallDriver(firewall.FirewallDriver): agent. This method is never called from that place. """ + def _accept_flow(self, **flow): + flow['ct_state'] = ovsfw_consts.OF_STATE_ESTABLISHED_NOT_REPLY + self._add_flow(**flow) + flow['ct_state'] = ovsfw_consts.OF_STATE_NEW_NOT_ESTABLISHED + if flow['table'] == ovs_consts.RULES_INGRESS_TABLE: + flow['actions'] += ',ct(commit,zone=NXM_NX_REG{:d}[0..15])'.format( + ovsfw_consts.REG_NET) + self._add_flow(**flow) + def _add_flow(self, **kwargs): dl_type = kwargs.get('dl_type') + create_reg_numbers(kwargs) if isinstance(dl_type, int): kwargs['dl_type'] = "0x{:04x}".format(dl_type) if self._deferred: @@ -183,6 +219,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): self.int_br.br.add_flow(**kwargs) def _delete_flows(self, **kwargs): + create_reg_numbers(kwargs) if self._deferred: self.int_br.delete_flows(**kwargs) else: @@ -205,7 +242,13 @@ class OVSFirewallDriver(firewall.FirewallDriver): ovs_port = self.int_br.br.get_vif_port_by_id(port_id) if not ovs_port: raise OVSFWPortNotFound(port_id=port_id) - of_port = OFPort(port, ovs_port) + + try: + port_vlan_id = int(self.int_br.br.db_get_val( + 'Port', ovs_port.port_name, 'tag')) + except TypeError: + port_vlan_id = ovs_consts.DEAD_VLAN_TAG + of_port = OFPort(port, ovs_port, port_vlan_id) self.sg_port_map.create_port(of_port, port) else: self.sg_port_map.update_port(of_port, port) @@ -289,8 +332,14 @@ class OVSFirewallDriver(firewall.FirewallDriver): table=ovs_consts.LOCAL_SWITCHING, priority=100, in_port=port.ofport, - actions='set_field:{:d}->reg5,resubmit(,{:d})'.format( - port.ofport, ovs_consts.BASE_EGRESS_TABLE) + actions='set_field:{:d}->reg{:d},' + 'set_field:{:d}->reg{:d},' + 'resubmit(,{:d})'.format( + port.ofport, + ovsfw_consts.REG_PORT, + port.vlan_tag, + ovsfw_consts.REG_NET, + ovs_consts.BASE_EGRESS_TABLE) ) # Identify ingress flows after egress filtering @@ -298,8 +347,14 @@ class OVSFirewallDriver(firewall.FirewallDriver): table=ovs_consts.LOCAL_SWITCHING, priority=90, dl_dst=port.mac, - actions='set_field:{:d}->reg5,resubmit(,{:d})'.format( - port.ofport, ovs_consts.BASE_INGRESS_TABLE), + actions='set_field:{:d}->reg{:d},' + 'set_field:{:d}->reg{:d},' + 'resubmit(,{:d})'.format( + port.ofport, + ovsfw_consts.REG_PORT, + port.vlan_tag, + ovsfw_consts.REG_NET, + ovs_consts.BASE_INGRESS_TABLE), ) self._initialize_egress(port) @@ -311,7 +366,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): table=ovs_consts.BASE_EGRESS_TABLE, priority=95, in_port=port.ofport, - reg5=port.ofport, + reg_port=port.ofport, dl_type=constants.ETHERTYPE_IPV6, nw_proto=constants.PROTO_NUM_IPV6_ICMP, icmp_type=icmp_type, @@ -330,7 +385,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): table=ovs_consts.BASE_EGRESS_TABLE, priority=95, in_port=port.ofport, - reg5=port.ofport, + reg_port=port.ofport, dl_src=mac_addr, dl_type=constants.ETHERTYPE_ARP, arp_spa=ip_addr, @@ -339,14 +394,15 @@ class OVSFirewallDriver(firewall.FirewallDriver): self._add_flow( table=ovs_consts.BASE_EGRESS_TABLE, priority=65, - reg5=port.ofport, + reg_port=port.ofport, ct_state=ovsfw_consts.OF_STATE_NOT_TRACKED, dl_type=constants.ETHERTYPE_IP, in_port=port.ofport, dl_src=mac_addr, nw_src=ip_addr, - actions='ct(table={:d},zone=NXM_NX_REG5[0..15])'.format( - ovs_consts.RULES_EGRESS_TABLE) + actions='ct(table={:d},zone=NXM_NX_REG{:d}[0..15])'.format( + ovs_consts.RULES_EGRESS_TABLE, + ovsfw_consts.REG_NET) ) # Apply mac/ip pairs for IPv6 @@ -356,14 +412,15 @@ class OVSFirewallDriver(firewall.FirewallDriver): self._add_flow( table=ovs_consts.BASE_EGRESS_TABLE, priority=65, - reg5=port.ofport, + reg_port=port.ofport, in_port=port.ofport, ct_state=ovsfw_consts.OF_STATE_NOT_TRACKED, dl_type=constants.ETHERTYPE_IPV6, dl_src=mac_addr, ipv6_src=ip_addr, - actions='ct(table={:d},zone=NXM_NX_REG5[0..15])'.format( - ovs_consts.RULES_EGRESS_TABLE) + actions='ct(table={:d},zone=NXM_NX_REG{:d}[0..15])'.format( + ovs_consts.RULES_EGRESS_TABLE, + ovsfw_consts.REG_NET) ) # DHCP discovery @@ -373,7 +430,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): self._add_flow( table=ovs_consts.BASE_EGRESS_TABLE, priority=80, - reg5=port.ofport, + reg_port=port.ofport, in_port=port.ofport, dl_type=dl_type, nw_proto=constants.PROTO_NUM_UDP, @@ -390,7 +447,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): table=ovs_consts.BASE_EGRESS_TABLE, priority=70, in_port=port.ofport, - reg5=port.ofport, + reg_port=port.ofport, dl_type=dl_type, nw_proto=constants.PROTO_NUM_UDP, tp_src=src_port, @@ -404,7 +461,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): priority=10, ct_state=ovsfw_consts.OF_STATE_NOT_TRACKED, in_port=port.ofport, - reg5=port.ofport, + reg_port=port.ofport, actions='drop' ) @@ -414,43 +471,80 @@ class OVSFirewallDriver(firewall.FirewallDriver): table=ovs_consts.ACCEPT_OR_INGRESS_TABLE, priority=100, dl_dst=port.mac, - actions='set_field:{:d}->reg5,resubmit(,{:d})'.format( - port.ofport, ovs_consts.BASE_INGRESS_TABLE), + actions='set_field:{:d}->reg{:d},resubmit(,{:d})'.format( + port.ofport, + ovsfw_consts.REG_PORT, + ovs_consts.BASE_INGRESS_TABLE), ) self._add_flow( table=ovs_consts.ACCEPT_OR_INGRESS_TABLE, priority=90, - reg5=port.ofport, - in_port=port.ofport, - actions='ct(commit,zone=NXM_NX_REG5[0..15]),normal' + reg_port=port.ofport, + ct_state=ovsfw_consts.OF_STATE_NEW_NOT_ESTABLISHED, + actions='ct(commit,zone=NXM_NX_REG{:d}[0..15]),normal'.format( + ovsfw_consts.REG_NET) + ) + self._add_flow( + table=ovs_consts.ACCEPT_OR_INGRESS_TABLE, + priority=80, + reg_port=port.ofport, + actions='normal' ) def _initialize_tracked_egress(self, port): + # Drop invalid packets self._add_flow( table=ovs_consts.RULES_EGRESS_TABLE, - priority=90, + priority=50, ct_state=ovsfw_consts.OF_STATE_INVALID, - actions='drop', + actions='drop' ) + # Drop traffic for removed sg rules + self._add_flow( + table=ovs_consts.RULES_EGRESS_TABLE, + priority=50, + reg_port=port.ofport, + ct_mark=ovsfw_consts.CT_MARK_INVALID, + actions='drop' + ) + for state in ( - ovsfw_consts.OF_STATE_ESTABLISHED, + ovsfw_consts.OF_STATE_ESTABLISHED_REPLY, ovsfw_consts.OF_STATE_RELATED, ): self._add_flow( table=ovs_consts.RULES_EGRESS_TABLE, - priority=80, + priority=50, ct_state=state, - reg5=port.ofport, - ct_zone=port.ofport, + ct_mark=ovsfw_consts.CT_MARK_NORMAL, + reg_port=port.ofport, + ct_zone=port.vlan_tag, actions='normal' ) + self._add_flow( + table=ovs_consts.RULES_EGRESS_TABLE, + priority=40, + reg_port=port.ofport, + ct_state=ovsfw_consts.OF_STATE_NOT_ESTABLISHED, + actions='drop' + ) + self._add_flow( + table=ovs_consts.RULES_EGRESS_TABLE, + priority=40, + reg_port=port.ofport, + ct_state=ovsfw_consts.OF_STATE_ESTABLISHED, + actions="ct(commit,zone=NXM_NX_REG{:d}[0..15]," + "exec(set_field:{:s}->ct_mark))".format( + ovsfw_consts.REG_NET, + ovsfw_consts.CT_MARK_INVALID) + ) def _initialize_ingress_ipv6_icmp(self, port): for icmp_type in constants.ICMPV6_ALLOWED_TYPES: self._add_flow( table=ovs_consts.BASE_INGRESS_TABLE, priority=100, - reg5=port.ofport, + reg_port=port.ofport, dl_dst=port.mac, dl_type=constants.ETHERTYPE_IPV6, nw_proto=constants.PROTO_NUM_IPV6_ICMP, @@ -464,7 +558,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): table=ovs_consts.BASE_INGRESS_TABLE, priority=100, dl_type=constants.ETHERTYPE_ARP, - reg5=port.ofport, + reg_port=port.ofport, dl_dst=port.mac, actions='output:{:d}'.format(port.ofport), ) @@ -477,7 +571,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): self._add_flow( table=ovs_consts.BASE_INGRESS_TABLE, priority=95, - reg5=port.ofport, + reg_port=port.ofport, dl_type=dl_type, nw_proto=constants.PROTO_NUM_UDP, tp_src=src_port, @@ -490,16 +584,18 @@ class OVSFirewallDriver(firewall.FirewallDriver): self._add_flow( table=ovs_consts.BASE_INGRESS_TABLE, priority=90, - reg5=port.ofport, + reg_port=port.ofport, dl_type=dl_type, ct_state=ovsfw_consts.OF_STATE_NOT_TRACKED, - actions='ct(table={:d},zone=NXM_NX_REG5[0..15])'.format( - ovs_consts.RULES_INGRESS_TABLE) + actions='ct(table={:d},zone=NXM_NX_REG{:d}[0..15])'.format( + ovs_consts.RULES_INGRESS_TABLE, + ovsfw_consts.REG_NET) ) self._add_flow( table=ovs_consts.BASE_INGRESS_TABLE, + ct_state=ovsfw_consts.OF_STATE_TRACKED, priority=80, - reg5=port.ofport, + reg_port=port.ofport, dl_dst=port.mac, actions='resubmit(,{:d})'.format(ovs_consts.RULES_INGRESS_TABLE) ) @@ -508,22 +604,49 @@ class OVSFirewallDriver(firewall.FirewallDriver): # Drop invalid packets self._add_flow( table=ovs_consts.RULES_INGRESS_TABLE, - priority=100, + priority=50, ct_state=ovsfw_consts.OF_STATE_INVALID, actions='drop' ) + # Drop traffic for removed sg rules + self._add_flow( + table=ovs_consts.RULES_INGRESS_TABLE, + priority=50, + reg_port=port.ofport, + ct_mark=ovsfw_consts.CT_MARK_INVALID, + actions='drop' + ) + # Allow established and related connections - for state in (ovsfw_consts.OF_STATE_ESTABLISHED, + for state in (ovsfw_consts.OF_STATE_ESTABLISHED_REPLY, ovsfw_consts.OF_STATE_RELATED): self._add_flow( table=ovs_consts.RULES_INGRESS_TABLE, - priority=80, + priority=50, dl_dst=port.mac, - reg5=port.ofport, + reg_port=port.ofport, ct_state=state, - ct_zone=port.ofport, + ct_mark=ovsfw_consts.CT_MARK_NORMAL, + ct_zone=port.vlan_tag, actions='output:{:d}'.format(port.ofport) ) + self._add_flow( + table=ovs_consts.RULES_INGRESS_TABLE, + priority=40, + reg_port=port.ofport, + ct_state=ovsfw_consts.OF_STATE_NOT_ESTABLISHED, + actions='drop' + ) + self._add_flow( + table=ovs_consts.RULES_INGRESS_TABLE, + priority=40, + reg_port=port.ofport, + ct_state=ovsfw_consts.OF_STATE_ESTABLISHED, + actions="ct(commit,zone=NXM_NX_REG{:d}[0..15]," + "exec(set_field:{:s}->ct_mark))".format( + ovsfw_consts.REG_NET, + ovsfw_consts.CT_MARK_INVALID) + ) def add_flows_from_rules(self, port): self._initialize_tracked_ingress(port) @@ -536,7 +659,7 @@ class OVSFirewallDriver(firewall.FirewallDriver): LOG.debug("RULGEN: Rules generated for flow %s are %s", rule, flows) for flow in flows: - self._add_flow(**flow) + self._accept_flow(**flow) def create_rules_generator_for_port(self, port): for sec_group in port.sec_groups: @@ -554,6 +677,6 @@ class OVSFirewallDriver(firewall.FirewallDriver): self._delete_flows(table=ovs_consts.LOCAL_SWITCHING, dl_dst=port.mac) self._delete_flows(table=ovs_consts.LOCAL_SWITCHING, in_port=port.ofport) - self._delete_flows(reg5=port.ofport) + self._delete_flows(reg_port=port.ofport) self._delete_flows(table=ovs_consts.ACCEPT_OR_INGRESS_TABLE, dl_dst=port.mac) diff --git a/neutron/agent/linux/openvswitch_firewall/rules.py b/neutron/agent/linux/openvswitch_firewall/rules.py index d5c376286df..996bd8ead2d 100644 --- a/neutron/agent/linux/openvswitch_firewall/rules.py +++ b/neutron/agent/linux/openvswitch_firewall/rules.py @@ -46,7 +46,7 @@ def create_flows_from_rule_and_port(rule, port): flow_template = { 'priority': 70, 'dl_type': ovsfw_consts.ethertype_to_dl_type_map[ethertype], - 'reg5': port.ofport, + 'reg_port': port.ofport, } if is_valid_prefix(dst_ip_prefix): @@ -71,8 +71,7 @@ def create_protocol_flows(direction, flow_template, port, rule): if direction == firewall.INGRESS_DIRECTION: flow_template['table'] = ovs_consts.RULES_INGRESS_TABLE flow_template['dl_dst'] = port.mac - flow_template['actions'] = ('ct(commit,zone=NXM_NX_REG5[0..15]),' - 'output:{:d}'.format(port.ofport)) + flow_template['actions'] = "output:{:d}".format(port.ofport) elif direction == firewall.EGRESS_DIRECTION: flow_template['table'] = ovs_consts.RULES_EGRESS_TABLE flow_template['dl_src'] = port.mac diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py b/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py index 650397027a2..72ba2e2bbe2 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py @@ -136,3 +136,6 @@ OPENFLOW11 = "OpenFlow11" OPENFLOW12 = "OpenFlow12" OPENFLOW13 = "OpenFlow13" OPENFLOW14 = "OpenFlow14" + +# A placeholder for dead vlans. +DEAD_VLAN_TAG = p_const.MAX_VLAN_TAG + 1 diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index a08d3d3ed1e..4b19934cbc6 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -61,9 +61,6 @@ cfg.CONF.import_group('AGENT', 'neutron.plugins.ml2.drivers.openvswitch.' cfg.CONF.import_group('OVS', 'neutron.plugins.ml2.drivers.openvswitch.agent.' 'common.config') -# A placeholder for dead vlans. -DEAD_VLAN_TAG = p_const.MAX_VLAN_TAG + 1 - class _mac_mydialect(netaddr.mac_unix): word_fmt = '%.2x' @@ -346,7 +343,7 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, continue net_uuid = local_vlan_map.get('net_uuid') if (net_uuid and net_uuid not in self._local_vlan_hints - and local_vlan != DEAD_VLAN_TAG): + and local_vlan != constants.DEAD_VLAN_TAG): self.available_local_vlans.remove(local_vlan) self._local_vlan_hints[local_vlan_map['net_uuid']] = \ local_vlan @@ -966,9 +963,10 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, # Don't kill a port if it's already dead cur_tag = self.int_br.db_get_val("Port", port.port_name, "tag", log_errors=log_errors) - if cur_tag and cur_tag != DEAD_VLAN_TAG: + if cur_tag and cur_tag != constants.DEAD_VLAN_TAG: self.int_br.set_db_attribute("Port", port.port_name, "tag", - DEAD_VLAN_TAG, log_errors=log_errors) + constants.DEAD_VLAN_TAG, + log_errors=log_errors) self.int_br.drop_port(in_port=port.ofport) def setup_integration_br(self): diff --git a/neutron/tests/common/conn_testers.py b/neutron/tests/common/conn_testers.py index 78460c01d6a..189ee604c5c 100644 --- a/neutron/tests/common/conn_testers.py +++ b/neutron/tests/common/conn_testers.py @@ -124,6 +124,9 @@ class ConnectionTester(fixtures.Fixture): def peer_ip_address(self): return self._peer.ip + def set_vm_default_gateway(self, default_gw): + self._vm.set_default_gateway(default_gw) + def flush_arp_tables(self): """Flush arptables in all used namespaces""" for machine in (self._peer, self._vm): @@ -202,7 +205,13 @@ class ConnectionTester(fixtures.Fixture): nc_tester = self._nc_testers.get(nc_params) if nc_tester: if nc_tester.is_established: - nc_tester.test_connectivity() + try: + nc_tester.test_connectivity() + except RuntimeError: + raise ConnectionTesterException( + "Established %s connection with protocol %s, source " + "port %s and destination port %s can no longer " + "communicate") else: nc_tester.stop_processes() raise ConnectionTesterException( @@ -296,6 +305,17 @@ class ConnectionTester(fixtures.Fixture): pinger = self._get_pinger(direction) return pinger.received + def assert_net_unreachable(self, direction, destination): + src_namespace, dst_address = self._get_namespace_and_address( + direction) + pinger = net_helpers.Pinger(src_namespace, destination, count=5) + pinger.start() + pinger.wait() + if not pinger.destination_unreachable: + raise ConnectionTesterException( + 'No Host Destination Unreachable packets were received when ' + 'sending icmp packets to %s' % destination) + class OVSConnectionTester(ConnectionTester): """Tester with OVS bridge in the middle diff --git a/neutron/tests/common/machine_fixtures.py b/neutron/tests/common/machine_fixtures.py index c812e5e5d85..e1293715a32 100644 --- a/neutron/tests/common/machine_fixtures.py +++ b/neutron/tests/common/machine_fixtures.py @@ -119,6 +119,9 @@ class FakeMachine(FakeMachineBase): self.port.link.set_address(mac_address) self.port.link.set_up() + def set_default_gateway(self, default_gw): + self.port.route.add_gateway(default_gw) + class PeerMachines(fixtures.Fixture): """Create 'amount' peered machines on an ip_cidr. diff --git a/neutron/tests/common/net_helpers.py b/neutron/tests/common/net_helpers.py index 8cd37f97597..695414aa17e 100644 --- a/neutron/tests/common/net_helpers.py +++ b/neutron/tests/common/net_helpers.py @@ -288,6 +288,8 @@ class Pinger(object): stats_pattern = re.compile( r'^(?P\d+) packets transmitted,.*(?P\d+) received.*$') + unreachable_pattern = re.compile( + r'.* Destination .* Unreachable') TIMEOUT = 15 def __init__(self, namespace, address, count=None, timeout=1): @@ -296,6 +298,7 @@ class Pinger(object): self.address = address self.count = count self.timeout = timeout + self.destination_unreachable = False self.sent = 0 self.received = 0 @@ -307,6 +310,10 @@ class Pinger(object): def _parse_stats(self): for line in self.proc.stdout: + if (not self.destination_unreachable and + self.unreachable_pattern.match(line)): + self.destination_unreachable = True + continue result = self.stats_pattern.match(line) if result: self.sent = int(result.group('trans')) @@ -331,6 +338,14 @@ class Pinger(object): self._wait_for_death() self._parse_stats() + def wait(self): + if self.count: + self._wait_for_death() + self._parse_stats() + else: + raise RuntimeError("Pinger is running infinitelly, use stop() " + "first") + class NetcatTester(object): TCP = n_const.PROTO_NAME_TCP diff --git a/neutron/tests/functional/agent/test_firewall.py b/neutron/tests/functional/agent/test_firewall.py index 791190b0ad6..1104e286931 100644 --- a/neutron/tests/functional/agent/test_firewall.py +++ b/neutron/tests/functional/agent/test_firewall.py @@ -19,9 +19,11 @@ import copy import functools +import random import netaddr from oslo_config import cfg +from oslo_log import log as logging import testscenarios from neutron.agent import firewall @@ -33,10 +35,10 @@ from neutron.common import constants from neutron.tests.common import conn_testers from neutron.tests.functional import base +LOG = logging.getLogger(__name__) load_tests = testscenarios.load_tests_apply_scenarios - reverse_direction = { conn_testers.ConnectionTester.INGRESS: conn_testers.ConnectionTester.EGRESS, @@ -46,17 +48,20 @@ reverse_transport_protocol = { conn_testers.ConnectionTester.TCP: conn_testers.ConnectionTester.UDP, conn_testers.ConnectionTester.UDP: conn_testers.ConnectionTester.TCP} - DEVICE_OWNER_COMPUTE = constants.DEVICE_OWNER_COMPUTE_PREFIX + 'fake' +VLAN_COUNT = 4096 -def skip_if_not_iptables(f): - @functools.wraps(f) - def wrap(self, *args, **kwargs): - if not hasattr(self, 'enable_ipset'): - self.skipTest("This test doesn't use iptables") - return f(self, *args, **kwargs) - return wrap +def skip_if_firewall(firewall_name): + def outter(f): + @functools.wraps(f) + def wrap(self, *args, **kwargs): + if self.firewall_name == firewall_name: + self.skipTest("This test doesn't use %s firewall" % + firewall_name) + return f(self, *args, **kwargs) + return wrap + return outter def _add_rule(sg_rules, base, port_range_min=None, port_range_max=None): @@ -73,18 +78,24 @@ class BaseFirewallTestCase(base.BaseSudoTestCase): MAC_SPOOFED = "fa:16:3e:9a:2f:48" scenarios = [('IptablesFirewallDriver without ipset', {'enable_ipset': False, - 'initialize': 'initialize_iptables'}), + 'initialize': 'initialize_iptables', + 'firewall_name': 'iptables'}), ('IptablesFirewallDriver with ipset', {'enable_ipset': True, - 'initialize': 'initialize_iptables'}), + 'initialize': 'initialize_iptables', + 'firewall_name': 'iptables'}), ('OVS Firewall Driver', - {'initialize': 'initialize_ovs'})] + {'initialize': 'initialize_ovs', + 'firewall_name': 'openvswitch'})] ip_cidr = None + vlan_range = set(range(VLAN_COUNT)) def setUp(self): cfg.CONF.register_opts(sg_cfg.security_group_opts, 'SECURITYGROUP') super(BaseFirewallTestCase, self).setUp() self.tester, self.firewall = getattr(self, self.initialize)() + if self.firewall_name == "openvswitch": + self.assign_vlan_to_peers() self.src_port_desc = self._create_port_description( self.tester.vm_port_id, [self.tester.vm_ip_address], @@ -117,6 +128,19 @@ class BaseFirewallTestCase(base.BaseSudoTestCase): firewall_drv = openvswitch_firewall.OVSFirewallDriver(tester.bridge) return tester, firewall_drv + def assign_vlan_to_peers(self): + vlan = self.get_not_used_vlan() + LOG.debug("Using %d vlan tag for this test", vlan) + self.tester.set_vm_tag(vlan) + self.tester.set_peer_tag(vlan) + + def get_not_used_vlan(self): + port_vlans = self.firewall.int_br.br.ovsdb.db_find( + 'Port', ('tag', '!=', '[]'), columns=['tag']).execute() + used_vlan_tags = {val['tag'] for val in port_vlans} + available_vlans = self.vlan_range - used_vlan_tags + return random.choice(list(available_vlans)) + @staticmethod def _create_port_description(port_id, ip_addresses, mac_address, sg_ids): return {'admin_state_up': True, @@ -142,7 +166,7 @@ class BaseFirewallTestCase(base.BaseSudoTestCase): class FirewallTestCase(BaseFirewallTestCase): ip_cidr = '192.168.0.1/24' - @skip_if_not_iptables + @skip_if_firewall('openvswitch') def test_rule_application_converges(self): sg_rules = [{'ethertype': 'IPv4', 'direction': 'egress'}, {'ethertype': 'IPv6', 'direction': 'egress'}, @@ -207,7 +231,7 @@ class FirewallTestCase(BaseFirewallTestCase): # and the new one was inserted in the correct position self.assertEqual([], self.firewall.iptables._apply()) - @skip_if_not_iptables + @skip_if_firewall('openvswitch') def test_rule_ordering_correct(self): sg_rules = [ {'ethertype': 'IPv4', 'direction': 'egress', 'protocol': 'tcp', @@ -280,7 +304,7 @@ class FirewallTestCase(BaseFirewallTestCase): self.tester.assert_no_connection(protocol=self.tester.ICMP, direction=self.tester.EGRESS) - @skip_if_not_iptables + @skip_if_firewall('openvswitch') def test_mac_spoofing_works_without_port_security_enabled(self): self.src_port_desc['port_security_enabled'] = False self.firewall.update_port_filter(self.src_port_desc) @@ -332,7 +356,7 @@ class FirewallTestCase(BaseFirewallTestCase): self.tester.assert_no_connection(protocol=self.tester.ICMP, direction=self.tester.EGRESS) - @skip_if_not_iptables + @skip_if_firewall('openvswitch') def test_ip_spoofing_works_without_port_security_enabled(self): self.src_port_desc['port_security_enabled'] = False self.firewall.update_port_filter(self.src_port_desc) @@ -456,7 +480,8 @@ class FirewallTestCase(BaseFirewallTestCase): direction=self.tester.EGRESS, src_port=source_port_max + 1) - def test_established_connection_is_not_cut(self): + @skip_if_firewall('iptables') + def test_established_connection_is_cut(self): port = 12345 sg_rules = [{'ethertype': constants.IPv4, 'direction': firewall.INGRESS_DIRECTION, @@ -470,7 +495,7 @@ class FirewallTestCase(BaseFirewallTestCase): self.tester.establish_connection(**connection) self._apply_security_group_rules(self.FAKE_SECURITY_GROUP_ID, list()) - self.tester.assert_established_connection(**connection) + self.tester.assert_no_established_connection(**connection) def test_preventing_firewall_blink(self): direction = self.tester.INGRESS @@ -514,9 +539,35 @@ class FirewallTestCase(BaseFirewallTestCase): self.tester.assert_no_connection(protocol=self.tester.ICMP, direction=self.tester.EGRESS) + def test_related_connection(self): + """Test ICMP net unreachable packets get back + + When destination address of ip traffic is not reachable, ICMP packets + are returned. This packets are marked as RELATED traffic by conntrack + and this test case validates such packets are not dropped by the + firewall as ingress ICMP packets are not allowed in this test case. The + used address below 1.2.3.4 is outside of subnet that is used in tester + object. + + """ + # Enable ip forwarding on the interface in order to reply with + # destionation net unreachable + self.tester._peer.execute([ + 'sysctl', '-w', 'net.ipv4.conf.%s.forwarding=1' % + self.tester._peer.port.name]) + self.tester.set_vm_default_gateway(self.tester.peer_ip_address) + vm_sg_rules = [{'ethertype': 'IPv4', 'direction': 'egress', + 'protocol': 'icmp'}] + self._apply_security_group_rules(self.FAKE_SECURITY_GROUP_ID, + vm_sg_rules) + + self.tester.assert_net_unreachable(self.tester.EGRESS, '1.2.3.4') + class FirewallTestCaseIPv6(BaseFirewallTestCase): - scenarios = [('OVS Firewall Driver', {'initialize': 'initialize_ovs'})] + scenarios = [('OVS Firewall Driver', + {'initialize': 'initialize_ovs', + 'firewall_name': 'openvswitch'})] ip_cidr = '2001:db8:aaaa::1/64' def test_icmp_from_specific_address(self): diff --git a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py index bc31f48fd88..dd283e23b86 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py @@ -17,15 +17,34 @@ import testtools from neutron.agent.common import ovs_lib from neutron.agent import firewall +from neutron.agent.linux.openvswitch_firewall import constants as ovsfw_consts from neutron.agent.linux.openvswitch_firewall import firewall as ovsfw from neutron.common import constants from neutron.plugins.ml2.drivers.openvswitch.agent.common import constants \ as ovs_consts from neutron.tests import base +TESTING_VLAN_TAG = 1 + def create_ofport(port_dict): - return ovsfw.OFPort(port_dict, mock.Mock(vif_mac='00:00:00:00:00:00')) + ovs_port = mock.Mock(vif_mac='00:00:00:00:00:00', port_name="port-name") + return ovsfw.OFPort(port_dict, ovs_port, vlan_tag=TESTING_VLAN_TAG) + + +class TestCreateRegNumbers(base.BaseTestCase): + def test_no_registers_defined(self): + flow = {'foo': 'bar'} + ovsfw.create_reg_numbers(flow) + self.assertEqual({'foo': 'bar'}, flow) + + def test_both_registers_defined(self): + flow = {'foo': 'bar', 'reg_port': 1, 'reg_net': 2} + expected_flow = {'foo': 'bar', + 'reg{:d}'.format(ovsfw_consts.REG_PORT): 1, + 'reg{:d}'.format(ovsfw_consts.REG_NET): 2} + ovsfw.create_reg_numbers(flow) + self.assertEqual(expected_flow, flow) class TestSecurityGroup(base.BaseTestCase): @@ -250,7 +269,14 @@ class TestOVSFirewallDriver(base.BaseTestCase): def test__add_flow_dl_type_formatted_to_string(self): dl_type = 0x0800 self.firewall._add_flow(dl_type=dl_type) - self.mock_bridge.br.add_flow.assert_called_once_with(dl_type="0x0800") + + def test__add_flow_registers_are_replaced(self): + self.firewall._add_flow(in_port=1, reg_port=1, reg_net=2) + expected_calls = {'in_port': 1, + 'reg{:d}'.format(ovsfw_consts.REG_PORT): 1, + 'reg{:d}'.format(ovsfw_consts.REG_NET): 2} + self.mock_bridge.br.add_flow.assert_called_once_with( + **expected_calls) def test__drop_all_unmatched_flows(self): self.firewall._drop_all_unmatched_flows() @@ -307,6 +333,14 @@ class TestOVSFirewallDriver(base.BaseTestCase): with testtools.ExpectedException(ovsfw.OVSFWPortNotFound): self.firewall.get_or_create_ofport(port_dict) + def test_get_or_create_ofport_not_tagged(self): + port_dict = { + 'device': 'port-id', + 'security_groups': [123, 456]} + self.mock_bridge.br.db_get_val.return_value = None + port = self.firewall.get_or_create_ofport(port_dict) + self.assertEqual(ovs_consts.DEAD_VLAN_TAG, port.vlan_tag) + def test_is_port_managed_managed_port(self): port_dict = {'device': 'port-id'} self.firewall.sg_port_map.ports[port_dict['device']] = object() @@ -324,25 +358,30 @@ class TestOVSFirewallDriver(base.BaseTestCase): self._prepare_security_group() self.firewall.prepare_port_filter(port_dict) exp_ingress_classifier = mock.call( - actions='set_field:{:d}->reg5,resubmit(,{:d})'.format( - self.port_ofport, ovs_consts.BASE_EGRESS_TABLE), + actions='set_field:{:d}->reg5,set_field:{:d}->reg6,' + 'resubmit(,{:d})'.format( + self.port_ofport, TESTING_VLAN_TAG, + ovs_consts.BASE_EGRESS_TABLE), in_port=self.port_ofport, priority=100, table=ovs_consts.LOCAL_SWITCHING) exp_egress_classifier = mock.call( - actions='set_field:{:d}->reg5,resubmit(,{:d})'.format( - self.port_ofport, ovs_consts.BASE_INGRESS_TABLE), + actions='set_field:{:d}->reg5,set_field:{:d}->reg6,' + 'resubmit(,{:d})'.format( + self.port_ofport, TESTING_VLAN_TAG, + ovs_consts.BASE_INGRESS_TABLE), dl_dst=self.port_mac, priority=90, table=ovs_consts.LOCAL_SWITCHING) filter_rule = mock.call( - actions='ct(commit,zone=NXM_NX_REG5[0..15]),output:{:d}'.format( + actions='output:{:d},ct(commit,zone=NXM_NX_REG6[0..15])'.format( self.port_ofport), dl_dst=self.port_mac, dl_type="0x{:04x}".format(constants.ETHERTYPE_IP), nw_proto=constants.PROTO_NUM_TCP, priority=70, reg5=self.port_ofport, + ct_state=ovsfw_consts.OF_STATE_NEW_NOT_ESTABLISHED, table=ovs_consts.RULES_INGRESS_TABLE, tcp_dst='0x007b') calls = self.mock_bridge.br.add_flow.call_args_list @@ -384,6 +423,7 @@ class TestOVSFirewallDriver(base.BaseTestCase): dl_type="0x{:04x}".format(constants.ETHERTYPE_IP), nw_proto=constants.PROTO_NUM_UDP, priority=70, + ct_state=ovsfw_consts.OF_STATE_NEW_NOT_ESTABLISHED, reg5=self.port_ofport, table=ovs_consts.RULES_EGRESS_TABLE) self.assertIn(filter_rule, add_calls) diff --git a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py index 333ab4b2fac..89fe26c4ce6 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_rules.py @@ -22,6 +22,8 @@ from neutron.plugins.ml2.drivers.openvswitch.agent.common import constants \ as ovs_consts from neutron.tests import base +TESTING_VLAN_TAG = 1 + class TestIsValidPrefix(base.BaseTestCase): def test_valid_prefix_ipv4(self): @@ -51,7 +53,8 @@ class TestCreateFlowsFromRuleAndPort(base.BaseTestCase): ovs_port = mock.Mock(vif_mac='00:00:00:00:00:00') ovs_port.ofport = 1 port_dict = {'device': 'port_id'} - self.port = ovsfw.OFPort(port_dict, ovs_port) + self.port = ovsfw.OFPort( + port_dict, ovs_port, vlan_tag=TESTING_VLAN_TAG) self.create_flows_mock = mock.patch.object( rules, 'create_protocol_flows').start() @@ -74,7 +77,7 @@ class TestCreateFlowsFromRuleAndPort(base.BaseTestCase): expected_template = { 'priority': 70, 'dl_type': constants.ETHERTYPE_IP, - 'reg5': self.port.ofport, + 'reg_port': self.port.ofport, } self._test_create_flows_from_rule_and_port_helper(rule, expected_template) @@ -89,7 +92,7 @@ class TestCreateFlowsFromRuleAndPort(base.BaseTestCase): expected_template = { 'priority': 70, 'dl_type': constants.ETHERTYPE_IP, - 'reg5': self.port.ofport, + 'reg_port': self.port.ofport, 'nw_src': '192.168.0.0/24', 'nw_dst': '10.0.0.1/32', } @@ -106,7 +109,7 @@ class TestCreateFlowsFromRuleAndPort(base.BaseTestCase): expected_template = { 'priority': 70, 'dl_type': constants.ETHERTYPE_IP, - 'reg5': self.port.ofport, + 'reg_port': self.port.ofport, 'nw_src': '192.168.0.0/24', } self._test_create_flows_from_rule_and_port_helper(rule, @@ -120,7 +123,7 @@ class TestCreateFlowsFromRuleAndPort(base.BaseTestCase): expected_template = { 'priority': 70, 'dl_type': constants.ETHERTYPE_IPV6, - 'reg5': self.port.ofport, + 'reg_port': self.port.ofport, } self._test_create_flows_from_rule_and_port_helper(rule, expected_template) @@ -135,7 +138,7 @@ class TestCreateFlowsFromRuleAndPort(base.BaseTestCase): expected_template = { 'priority': 70, 'dl_type': constants.ETHERTYPE_IPV6, - 'reg5': self.port.ofport, + 'reg_port': self.port.ofport, 'ipv6_src': '2001:db8:bbbb::1/64', 'ipv6_dst': '2001:db8:aaaa::1/64', } @@ -152,7 +155,7 @@ class TestCreateFlowsFromRuleAndPort(base.BaseTestCase): expected_template = { 'priority': 70, 'dl_type': constants.ETHERTYPE_IPV6, - 'reg5': self.port.ofport, + 'reg_port': self.port.ofport, 'ipv6_src': '2001:db8:bbbb::1/64', } self._test_create_flows_from_rule_and_port_helper(rule, @@ -165,7 +168,8 @@ class TestCreateProtocolFlows(base.BaseTestCase): ovs_port = mock.Mock(vif_mac='00:00:00:00:00:00') ovs_port.ofport = 1 port_dict = {'device': 'port_id'} - self.port = ovsfw.OFPort(port_dict, ovs_port) + self.port = ovsfw.OFPort( + port_dict, ovs_port, vlan_tag=TESTING_VLAN_TAG) def _test_create_protocol_flows_helper(self, direction, rule, expected_flows): @@ -181,7 +185,7 @@ class TestCreateProtocolFlows(base.BaseTestCase): expected_flows = [{ 'table': ovs_consts.RULES_INGRESS_TABLE, 'dl_dst': self.port.mac, - 'actions': 'ct(commit,zone=NXM_NX_REG5[0..15]),output:1', + 'actions': 'output:1', 'nw_proto': constants.PROTO_NUM_TCP, }] self._test_create_protocol_flows_helper( diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index a94dd5a8f00..7c649246b90 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -297,13 +297,13 @@ class TestOvsNeutronAgent(object): with mock.patch.object(self.agent, 'int_br') as int_br: int_br.db_get_val.return_value = cur_tag self.agent.port_dead(port) - if cur_tag is None or cur_tag == self.mod_agent.DEAD_VLAN_TAG: + if cur_tag is None or cur_tag == constants.DEAD_VLAN_TAG: self.assertFalse(int_br.set_db_attribute.called) self.assertFalse(int_br.drop_port.called) else: int_br.assert_has_calls([ mock.call.set_db_attribute("Port", mock.ANY, "tag", - self.mod_agent.DEAD_VLAN_TAG, + constants.DEAD_VLAN_TAG, log_errors=True), mock.call.drop_port(in_port=port.ofport), ]) @@ -312,7 +312,7 @@ class TestOvsNeutronAgent(object): self._test_port_dead() def test_port_dead_with_port_already_dead(self): - self._test_port_dead(self.mod_agent.DEAD_VLAN_TAG) + self._test_port_dead(constants.DEAD_VLAN_TAG) def test_port_dead_with_valid_tag(self): self._test_port_dead(cur_tag=1) @@ -1046,7 +1046,7 @@ class TestOvsNeutronAgent(object): # the main things we care about are that it gets put in the # dead vlan and gets blocked int_br.set_db_attribute.assert_any_call( - 'Port', vif.port_name, 'tag', self.mod_agent.DEAD_VLAN_TAG, + 'Port', vif.port_name, 'tag', constants.DEAD_VLAN_TAG, log_errors=False) int_br.drop_port.assert_called_once_with(in_port=vif.ofport) diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py index c50f6778fb6..82a7b958688 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py @@ -473,7 +473,7 @@ class TunnelTest(object): log_errors=True), mock.call.set_db_attribute( 'Port', VIF_PORT.port_name, - 'tag', self.mod_agent.DEAD_VLAN_TAG, + 'tag', constants.DEAD_VLAN_TAG, log_errors=True), mock.call.drop_port(in_port=VIF_PORT.ofport), ]