openstack
/
neutron
Code Issues Proposed changes

Fix error when using protocol number in security groups 

When the support of protocol numbers in security groups
was fixed in [1], it introduced two deficiencies in the
iptables code:

- it was missing some protocols, for example, 'icmp', 'tcp'
  and 'udp', so when rules were added by number we did not
  use their name as iptables expects
- it used a dictionary to map numbers to names, but protocol
  numbers are stored as strings (i.e. '1' != 1)

Updated the iptables number mapping dict to have all
currently-known values, even those that are already well-known
and should have been using a string instead of a number.

Also changed the iptables number mapping dict to use
strings as the keys instead of numbers, since that's
what will be passed from the security group code.

Removed IPTABLES_PROTOCOL_MAP as it lives in neutron-lib,
and accidentally snuck-in in [1].

[1] I5895250b47ddf664d214cf085be693c3897e0c87

Change-Id: I6b7575eb531b4f35579960c3feb47000cd259b86
Closes-Bug: 1719711
This commit is contained in:
Jens Harbott 2017-11-28 07:39:04 +00:00 committed by Brian Haley
parent 618ee8658f
commit 37bd42e4f5
3 changed files with 124 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
neutron/agent/linux/iptables_firewall.py
View File

@ -634,18 +634,17 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
return iptables_rules
def _protocol_arg(self, protocol, is_port):
if not protocol:
return []
rule_protocol = protocol
if (protocol in n_const.IPTABLES_PROTOCOL_NAME_MAP):
rule_protocol = n_const.IPTABLES_PROTOCOL_NAME_MAP[protocol]
iptables_rule = []
rule_protocol = n_const.IPTABLES_PROTOCOL_NAME_MAP.get(protocol,
protocol)
# protocol zero is a special case and requires no '-p'
if rule_protocol:
iptables_rule = ['-p', rule_protocol]
if (is_port and protocol in constants.IPTABLES_PROTOCOL_MAP):
# iptables adds '-m protocol' when the port number is specified
iptables_rule += ['-m', constants.IPTABLES_PROTOCOL_MAP[protocol]]
if (is_port and rule_protocol in constants.IPTABLES_PROTOCOL_MAP):
# iptables adds '-m protocol' when the port number is specified
iptables_rule += ['-m',
constants.IPTABLES_PROTOCOL_MAP[rule_protocol]]
return iptables_rule
def _port_arg(self, direction, protocol, port_range_min, port_range_max):

123
neutron/common/constants.py
View File

@ -50,64 +50,79 @@ IP_PROTOCOL_NUM_TO_NAME_MAP = {
# When using iptables-save we specify '-p {proto}',
# but sometimes those values are not identical. This is a map
# of known protocol names or numbers that require a name change.
# This legacy mapping can go away once neutron-lib is updated.
IPTABLES_PROTOCOL_LEGACY_NUM_MAP = {3: 'ggp',
4: 'ipencap',
5: 'st',
9: 'igp',
12: 'pup',
20: 'hmp',
22: 'xns-idp',
27: 'rdp',
29: 'iso-tp4',
36: 'xtp',
37: 'ddp',
38: 'idpr-cmtp',
45: 'idrp',
57: 'skip',
73: 'rspf',
81: 'vmtp',
88: 'eigrp',
93: 'ax.25',
94: 'ipip',
97: 'etherip',
98: 'encap',
103: 'pim',
108: 'ipcomp',
115: 'lt2p',
124: 'isis',
133: 'fc',
135: 'mobility-header',
137: 'mpls-in-ip',
138: 'manet',
139: 'hip',
140: 'shim6',
141: 'wesp',
142: 'rohc'}
# - protocol 0 uses no -p argument
# of known protocol numbers that require a name to be used and
# protocol names that require a different name to be used,
# because that is how iptables-save will display them.
#
# This is how the list was created, so there is a possibility
# it will need to be updated in the future:
#
# $ for num in {0..255}; do iptables -A INPUT -p $num; done
# $ iptables-save
#
# These cases are special, and were found by inspection:
# - 'ipv6-encap' uses 'ipv6'
# - 'icmpv6' uses 'ipv6-icmp'
# - 'pgm' uses number 113 instead of its name
IPTABLES_PROTOCOL_NAME_MAP = {0: None,
lib_constants.PROTO_NAME_IPV6_ENCAP:
'ipv6',
# - 'pgm' uses '113' instead of its name
# - protocol '0' uses no -p argument
IPTABLES_PROTOCOL_NAME_MAP = {lib_constants.PROTO_NAME_IPV6_ENCAP: 'ipv6',
lib_constants.PROTO_NAME_IPV6_ICMP_LEGACY:
'ipv6-icmp',
lib_constants.PROTO_NAME_PGM: '113'}
IPTABLES_PROTOCOL_NAME_MAP.update(IPTABLES_PROTOCOL_LEGACY_NUM_MAP)
# When using iptables-save we specify '-p {proto} -m {module}',
# but sometimes those values are not identical. This is a map
# of known protocols that require a '-m {module}', along with
# the module name that should be used.
IPTABLES_PROTOCOL_MAP = {lib_constants.PROTO_NAME_DCCP: 'dccp',
lib_constants.PROTO_NAME_ICMP: 'icmp',
lib_constants.PROTO_NAME_IPV6_ICMP: 'icmp6',
lib_constants.PROTO_NAME_SCTP: 'sctp',
lib_constants.PROTO_NAME_TCP: 'tcp',
lib_constants.PROTO_NAME_UDP: 'udp'}
lib_constants.PROTO_NAME_PGM: '113',
'0': None,
'1': 'icmp',
'2': 'igmp',
'3': 'ggp',
'4': 'ipencap',
'5': 'st',
'6': 'tcp',
'8': 'egp',
'9': 'igp',
'12': 'pup',
'17': 'udp',
'20': 'hmp',
'22': 'xns-idp',
'27': 'rdp',
'29': 'iso-tp4',
'33': 'dccp',
'36': 'xtp',
'37': 'ddp',
'38': 'idpr-cmtp',
'41': 'ipv6',
'43': 'ipv6-route',
'44': 'ipv6-frag',
'45': 'idrp',
'46': 'rsvp',
'47': 'gre',
'50': 'esp',
'51': 'ah',
'57': 'skip',
'58': 'ipv6-icmp',
'59': 'ipv6-nonxt',
'60': 'ipv6-opts',
'73': 'rspf',
'81': 'vmtp',
'88': 'eigrp',
'89': 'ospf',
'93': 'ax.25',
'94': 'ipip',
'97': 'etherip',
'98': 'encap',
'103': 'pim',
'108': 'ipcomp',
'112': 'vrrp',
'115': 'l2tp',
'124': 'isis',
'132': 'sctp',
'133': 'fc',
'135': 'mobility-header',
'136': 'udplite',
'137': 'mpls-in-ip',
'138': 'manet',
'139': 'hip',
'140': 'shim6',
'141': 'wesp',
'142': 'rohc'}
# Timeout in seconds for getting an IPv6 LLA
LLA_TASK_TIMEOUT = 40

48
neutron/tests/unit/agent/linux/test_iptables_firewall.py
View File

@ -276,6 +276,18 @@ class IptablesFirewallTestCase(BaseIptablesFirewallTestCase):
egress = None
self._test_prepare_port_filter(rule, ingress, egress)
def test_filter_ipv4_ingress_tcp_port_by_num(self):
rule = {'ethertype': 'IPv4',
'direction': 'ingress',
'protocol': '6',
'port_range_min': 10,
'port_range_max': 10}
ingress = mock.call.add_rule('ifake_dev',
'-p tcp -m tcp --dport 10 -j RETURN',
comment=None)
egress = None
self._test_prepare_port_filter(rule, ingress, egress)
def test_filter_ipv4_ingress_tcp_mport(self):
rule = {'ethertype': 'IPv4',
'direction': 'ingress',
@ -389,6 +401,42 @@ class IptablesFirewallTestCase(BaseIptablesFirewallTestCase):
egress = None
self._test_prepare_port_filter(rule, ingress, egress)
def test_filter_ipv4_ingress_protocol_blank(self):
rule = {'ethertype': 'IPv4',
'direction': 'ingress',
'protocol': ''}
ingress = mock.call.add_rule('ifake_dev', '-j RETURN', comment=None)
egress = None
self._test_prepare_port_filter(rule, ingress, egress)
def test_filter_ipv4_ingress_protocol_zero(self):
rule = {'ethertype': 'IPv4',
'direction': 'ingress',
'protocol': '0'}
ingress = mock.call.add_rule('ifake_dev', '-j RETURN', comment=None)
egress = None
self._test_prepare_port_filter(rule, ingress, egress)
def test_filter_ipv4_ingress_protocol_encap(self):
rule = {'ethertype': 'IPv4',
'direction': 'ingress',
'protocol': 'encap'}
ingress = mock.call.add_rule('ifake_dev',
'-p encap -j RETURN',
comment=None)
egress = None
self._test_prepare_port_filter(rule, ingress, egress)
def test_filter_ipv4_ingress_protocol_encap_by_num(self):
rule = {'ethertype': 'IPv4',
'direction': 'ingress',
'protocol': '98'}
ingress = mock.call.add_rule('ifake_dev',
'-p encap -j RETURN',
comment=None)
egress = None
self._test_prepare_port_filter(rule, ingress, egress)
def test_filter_ipv4_egress(self):
rule = {'ethertype': 'IPv4',
'direction': 'egress'}