From d5c07fe512502342cfde7c49e6ed75686608cc65 Mon Sep 17 00:00:00 2001 From: Jakub Libosvar Date: Thu, 24 Nov 2016 12:32:55 -0500 Subject: [PATCH] ovsfw: Support protocol numbers instead of just tcp and udp Neutron API accepts also protocol numbers as protocols for security groups. This patch makes support for it in OVS firewall driver. iptables driver already supports it. Fullstack test covering SCTP connection was added and it requires ip_conntrack_proto_sctp kernel module in order to make conntrack work with SCTP. Change-Id: I6c5665a994c4a50ddbb95cd1360be0de0a6c7e40 Closes-bug: 1625516 --- .../linux/openvswitch_firewall/constants.py | 6 ---- .../agent/linux/openvswitch_firewall/rules.py | 11 +++--- neutron/tests/common/net_helpers.py | 11 ++++-- neutron/tests/fullstack/test_securitygroup.py | 35 ++++++++++++++----- tools/configure_for_func_testing.sh | 1 + 5 files changed, 42 insertions(+), 22 deletions(-) diff --git a/neutron/agent/linux/openvswitch_firewall/constants.py b/neutron/agent/linux/openvswitch_firewall/constants.py index c03fdc3ba8d..00d142b4079 100644 --- a/neutron/agent/linux/openvswitch_firewall/constants.py +++ b/neutron/agent/linux/openvswitch_firewall/constants.py @@ -35,12 +35,6 @@ CT_MARK_INVALID = '0x1' REG_PORT = 5 REG_NET = 6 -protocol_to_nw_proto = { - constants.PROTO_NAME_ICMP: constants.PROTO_NUM_ICMP, - constants.PROTO_NAME_TCP: constants.PROTO_NUM_TCP, - constants.PROTO_NAME_UDP: constants.PROTO_NUM_UDP, -} - PROTOCOLS_WITH_PORTS = (constants.PROTO_NAME_TCP, constants.PROTO_NAME_UDP) ethertype_to_dl_type_map = { diff --git a/neutron/agent/linux/openvswitch_firewall/rules.py b/neutron/agent/linux/openvswitch_firewall/rules.py index 1c7c5f7d471..233972ed91f 100644 --- a/neutron/agent/linux/openvswitch_firewall/rules.py +++ b/neutron/agent/linux/openvswitch_firewall/rules.py @@ -79,12 +79,13 @@ def create_protocol_flows(direction, flow_template, port, rule): flow_template['actions'] = 'resubmit(,{:d})'.format( ovs_consts.ACCEPT_OR_INGRESS_TABLE) protocol = rule.get('protocol') - try: - flow_template['nw_proto'] = ovsfw_consts.protocol_to_nw_proto[protocol] - if rule['ethertype'] == n_consts.IPv6 and protocol == 'icmp': + if protocol: + if (rule.get('ethertype') == n_consts.IPv6 and + protocol == n_consts.PROTO_NAME_ICMP): flow_template['nw_proto'] = n_consts.PROTO_NUM_IPV6_ICMP - except KeyError: - pass + else: + flow_template['nw_proto'] = n_consts.IP_PROTOCOL_MAP.get( + protocol, protocol) flows = create_port_range_flows(flow_template, rule) return flows or [flow_template] diff --git a/neutron/tests/common/net_helpers.py b/neutron/tests/common/net_helpers.py index 470a27122a1..3917d2ef89f 100644 --- a/neutron/tests/common/net_helpers.py +++ b/neutron/tests/common/net_helpers.py @@ -72,7 +72,8 @@ READ_TIMEOUT = os.environ.get('OS_TEST_READ_TIMEOUT', 5) CHILD_PROCESS_TIMEOUT = os.environ.get('OS_TEST_CHILD_PROCESS_TIMEOUT', 20) CHILD_PROCESS_SLEEP = os.environ.get('OS_TEST_CHILD_PROCESS_SLEEP', 0.5) -TRANSPORT_PROTOCOLS = (n_const.PROTO_NAME_TCP, n_const.PROTO_NAME_UDP) +TRANSPORT_PROTOCOLS = (n_const.PROTO_NAME_TCP, n_const.PROTO_NAME_UDP, + n_const.PROTO_NAME_SCTP) OVS_MANAGER_TEST_PORT_FIRST = 6610 OVS_MANAGER_TEST_PORT_LAST = 6639 @@ -403,6 +404,7 @@ class Pinger(object): class NetcatTester(object): TCP = n_const.PROTO_NAME_TCP UDP = n_const.PROTO_NAME_UDP + SCTP = n_const.PROTO_NAME_SCTP VERSION_TO_ALL_ADDRESS = { 4: '0.0.0.0', 6: '::', @@ -423,7 +425,7 @@ class NetcatTester(object): will be spawned :param address: Server address from client point of view :param dst_port: Port on which netcat listens - :param protocol: Transport protocol, either 'tcp' or 'udp' + :param protocol: Transport protocol, either 'tcp', 'udp' or 'sctp' :param server_address: Address in server namespace on which netcat should listen :param src_port: Source port of netcat process spawned in client @@ -515,9 +517,12 @@ class NetcatTester(object): cmd = ['ncat', address, self.dst_port] if self.protocol == self.UDP: cmd.append('-u') + elif self.protocol == self.SCTP: + cmd.append('--sctp') + if listen: cmd.append('-l') - if self.protocol == self.TCP: + if self.protocol in (self.TCP, self.SCTP): cmd.append('-k') else: cmd.extend(['-w', '20']) diff --git a/neutron/tests/fullstack/test_securitygroup.py b/neutron/tests/fullstack/test_securitygroup.py index 0444e32436d..1817e4bd6c3 100644 --- a/neutron/tests/fullstack/test_securitygroup.py +++ b/neutron/tests/fullstack/test_securitygroup.py @@ -61,15 +61,17 @@ class BaseSecurityGroupsSameNetworkTest(base.BaseFullStackTestCase): def assert_connection(self, *args, **kwargs): netcat = net_helpers.NetcatTester(*args, **kwargs) - self.addCleanup(netcat.stop_processes) - self.assertTrue(netcat.test_connectivity()) - netcat.stop_processes() + try: + self.assertTrue(netcat.test_connectivity()) + finally: + netcat.stop_processes() def assert_no_connection(self, *args, **kwargs): netcat = net_helpers.NetcatTester(*args, **kwargs) - self.addCleanup(netcat.stop_processes) - self.assertRaises(RuntimeError, netcat.test_connectivity) - netcat.stop_processes() + try: + self.assertRaises(RuntimeError, netcat.test_connectivity) + finally: + netcat.stop_processes() class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest): @@ -98,14 +100,15 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest): # NOTE(toshii): As a firewall_driver can interfere with others, # the recommended way to add test is to expand this method, not # adding another. - def test_tcp_securitygroup(self): - """Tests if a TCP security group rule is working, by confirming + def test_securitygroup(self): + """Tests if a security group rules are working, by confirming that 1. connection from allowed security group is allowed, 2. connection from elsewhere is blocked, 3. traffic not explicitly allowed (eg. ICMP) is blocked, 4. a security group update takes effect, 5. a remote security group member addition works, and 6. an established connection stops by deleting a SG rule. + 7. test other protocol functionality by using SCTP protocol """ index_to_sg = [0, 0, 1] if self.firewall_driver == 'iptables_hybrid': @@ -228,3 +231,19 @@ class TestSecurityGroupsSameNetwork(BaseSecurityGroupsSameNetworkTest): common_utils.wait_until_true(lambda: netcat.test_no_connectivity(), sleep=8) netcat.stop_processes() + + # 7. check SCTP is supported by security group + self.assert_no_connection( + vms[1].namespace, vms[0].namespace, vms[0].ip, 3366, + net_helpers.NetcatTester.SCTP) + + self.safe_client.create_security_group_rule( + tenant_uuid, sgs[0]['id'], + remote_group_id=sgs[0]['id'], direction='ingress', + ethertype=constants.IPv4, + protocol=constants.PROTO_NUM_SCTP, + port_range_min=3366, port_range_max=3366) + + self.assert_connection( + vms[1].namespace, vms[0].namespace, vms[0].ip, 3366, + net_helpers.NetcatTester.SCTP) diff --git a/tools/configure_for_func_testing.sh b/tools/configure_for_func_testing.sh index 5b841bbd284..44ea366cbe4 100755 --- a/tools/configure_for_func_testing.sh +++ b/tools/configure_for_func_testing.sh @@ -283,4 +283,5 @@ fi if [[ "$VENV" =~ "dsvm-fullstack" ]]; then _configure_iptables_rules + sudo modprobe ip_conntrack_proto_sctp fi